launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/nomination-investigation-0 into lp:launchpad.

Commit message:
Fix title, breadcrumbs, and heading on old nomination edit page.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #163490 in Launchpad itself: "Nomination management page doesn't explicitly say which series is being managed"
  https://bugs.launchpad.net/launchpad/+bug/163490

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/nomination-investigation-0/+merge/129223

This branch fixes some small issues observed while looking at
BugNomination:+editstatus timeout.

--------------------------------------------------------------------

RULES

    Pre-implementation: no one
    https://bugs.launchpad.net/launchpad/+bug/163490
    * The breadcrumb is always truncated because the text is too long.
      It contains redundant information that comes from the other
      crumbs.
      * Only state the action and the series that is being targeted too.
    * The title property on the view is obsolete. Code and templates
      use label only.


QA

    * Visit https://bugs.qastaging.launchpad.net/gdp/+bug/459328/nominations/55934/+editstatus
    * Verify the page title and breadcrumbs are not truncated.
    * Verify the bread crumbs are
      Review nomination for Gedit Developer Plugins packaging


LINT

    lib/lp/bugs/browser/bugnomination.py
    lib/lp/bugs/browser/tests/test_bugnomination.py
    lib/lp/bugs/model/bugnomination.py


LoC

    I've got a 4.5K credit as of Monday.


TEST

    ./bin/test -vvc lp.bugs.browser.tests.test_bugnomination


IMPLEMENTATION

Replaced the title property with the label property. Provided a sensible
page_title.
    lib/lp/bugs/browser/bugnomination.py
    lib/lp/bugs/browser/tests/test_bugnomination.py

Wrapped a long line.
    lib/lp/bugs/model/bugnomination.py

-- 
https://code.launchpad.net/~sinzui/launchpad/nomination-investigation-0/+merge/129223
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/nomination-investigation-0 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugnomination.py'
--- lib/lp/bugs/browser/bugnomination.py	2012-01-24 05:10:08 +0000
+++ lib/lp/bugs/browser/bugnomination.py	2012-10-11 15:43:23 +0000
@@ -218,11 +218,14 @@
     field_names = []
 
     @property
-    def title(self):
+    def label(self):
         return 'Approve or decline nomination for bug #%d in %s' % (
             self.context.bug.id, self.context.target.bugtargetdisplayname)
-    label = title
-    page_title = title
+
+    @property
+    def page_title(self):
+        text = 'Review nomination for %s'
+        return text % self.context.target.bugtargetdisplayname
 
     def initialize(self):
         self.current_bugtask = getUtility(ILaunchBag).bugtask

=== modified file 'lib/lp/bugs/browser/tests/test_bugnomination.py'
--- lib/lp/bugs/browser/tests/test_bugnomination.py	2012-08-16 05:18:54 +0000
+++ lib/lp/bugs/browser/tests/test_bugnomination.py	2012-10-11 15:43:23 +0000
@@ -240,6 +240,28 @@
                 ).request.response.getStatus())
         self.assertTrue(nomination.isApproved())
 
+    def test_label(self):
+        nomination = self.getNomination()
+        target = nomination.target
+        view = self.getNominationEditView(nomination, {})
+        self.assertEqual(
+            'Approve or decline nomination for bug #%d in %s' % (
+                nomination.bug.id, target.bugtargetdisplayname),
+            view.label)
+
+    def test_page_title(self):
+        nomination = self.getNomination()
+        target = nomination.target
+        view = self.getNominationEditView(nomination, {})
+        self.assertEqual(
+            'Review nomination for %s' % target.bugtargetdisplayname,
+            view.page_title)
+
+    def test_next_url(self):
+        nomination = self.getNomination()
+        view = self.getNominationEditView(nomination, {})
+        self.assertEqual(canonical_url(view.current_bugtask), view.next_url)
+
     def test_approving_twice_is_noop(self):
         nomination = self.getNomination()
         self.assertApproves(nomination)

=== modified file 'lib/lp/bugs/model/bugnomination.py'
--- lib/lp/bugs/model/bugnomination.py	2012-05-11 02:39:08 +0000
+++ lib/lp/bugs/model/bugnomination.py	2012-10-11 15:43:23 +0000
@@ -93,7 +93,8 @@
                     targets.append(distroseries)
         else:
             targets.append(self.productseries)
-        bugtasks = getUtility(IBugTaskSet).createManyTasks(self.bug, approver, targets)
+        bugtasks = getUtility(IBugTaskSet).createManyTasks(
+            self.bug, approver, targets)
         for bug_task in bugtasks:
             self.bug.addChange(BugTaskAdded(UTC_NOW, approver, bug_task))