launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/ghostly-series-tasks into lp:launchpad with lp:~wgrant/launchpad/iseriesbugtarget as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #817336 in Launchpad itself: "BugTaskView should handle series tasks without a parent"
  https://bugs.launchpad.net/launchpad/+bug/817336

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/ghostly-series-tasks/+merge/70101

BugTaskView only displays the series name on series tasks; it assumes they are always preceded by a row for their parent target. But we permit retargeting of the parent task, so it may be missing, resulting in a fairly broken UI (eg. <http://people.canonical.com/~wgrant/launchpad/2011/broken-tasks.png>).

This branch fixes the view to always show the parent context, even if there is no task. As seen in <http://people.canonical.com/~wgrant/launchpad/2011/fixed-tasks.png>, there is no Firefox task, but the Firefox 1.0 task causes a statusless faded Firefox row to show up.

I'm not sure if the fading is useful or not.
-- 
https://code.launchpad.net/~wgrant/launchpad/ghostly-series-tasks/+merge/70101
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/ghostly-series-tasks into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-07-28 17:34:34 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-08-02 01:43:33 +0000
@@ -220,6 +220,7 @@
     BugNominationStatus,
     IBugNominationSet,
     )
+from lp.bugs.interfaces.bugtarget import ISeriesBugTarget
 from lp.bugs.interfaces.bugtask import (
     BugBlueprintSearch,
     BugBranchSearch,
@@ -3246,7 +3247,25 @@
         # iteration.
         bugtasks_by_package = bug.getBugTasksByPackageName(all_bugtasks)
 
+        latest_parent = None
+
         for bugtask in all_bugtasks:
+            # Series bug targets only display the series name, so they
+            # must always be preceded by their parent context. Normally
+            # the parent will have a task, but if not we need to show a
+            # fake one.
+            if ISeriesBugTarget.providedBy(bugtask.target):
+                parent = bugtask.target.bugtarget_parent
+            else:
+                latest_parent = parent = bugtask.target
+
+            if parent != latest_parent:
+                latest_parent = parent
+                bugtask_and_nomination_views.append(
+                    getMultiAdapter(
+                        (parent, self.request),
+                        name='+bugtasks-and-nominations-table-row'))
+
             conjoined_master = bugtask.getConjoinedMaster(
                 bugtasks, bugtasks_by_package)
             view = self._getTableRowView(

=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml	2011-07-27 13:23:38 +0000
+++ lib/lp/bugs/browser/configure.zcml	2011-08-02 01:43:33 +0000
@@ -561,6 +561,11 @@
             name="+bugtasks-and-nominations-table-row"
             template="../templates/bugtask-tasks-and-nominations-table-row.pt"/>
         <browser:page
+            for="lp.bugs.interfaces.bugtarget.IBugTarget"
+            permission="zope.Public"
+            name="+bugtasks-and-nominations-table-row"
+            template="../templates/bugtarget-tasks-and-nominations-table-row.pt"/>
+        <browser:page
             for="lp.bugs.interfaces.bugtask.IBugTask"
             name="+bugtask-macros-listing"
             template="../templates/bugtask-macros-listing.pt"

=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2011-07-25 11:22:55 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2011-08-02 01:43:33 +0000
@@ -534,6 +534,30 @@
             foo_bugtasks_and_nominations_view.getBugTaskAndNominationViews())
         self.assertEqual([], task_and_nomination_views)
 
+    def test_bugtarget_parent_shown_for_orphaned_series_tasks(self):
+        # Test that a row is shown for the parent of a series task, even
+        # if the parent doesn't actually have a task.
+        series = self.factory.makeProductSeries()
+        bug = self.factory.makeBug(series=series)
+        self.assertEqual(2, len(bug.bugtasks))
+        new_prod = self.factory.makeProduct()
+        bug.getBugTask(series.product).transitionToTarget(new_prod)
+
+        view = create_initialized_view(bug, "+bugtasks-and-nominations-table")
+        subviews = view.getBugTaskAndNominationViews()
+        self.assertEqual([
+            (series.product, '+bugtasks-and-nominations-table-row'),
+            (bug.getBugTask(series), '+bugtasks-and-nominations-table-row'),
+            (bug.getBugTask(new_prod), '+bugtasks-and-nominations-table-row'),
+            ], [(v.context, v.__name__) for v in subviews])
+
+        content = subviews[0]()
+        self.assertIn(
+            'href="%s"' % canonical_url(
+                series.product, path_only_if_possible=True),
+            content)
+        self.assertIn(series.product.displayname, content)
+
 
 class TestBugTaskEditViewStatusField(TestCaseWithFactory):
     """We show only those options as possible value in the status

=== added file 'lib/lp/bugs/templates/bugtarget-tasks-and-nominations-table-row.pt'
--- lib/lp/bugs/templates/bugtarget-tasks-and-nominations-table-row.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/templates/bugtarget-tasks-and-nominations-table-row.pt	2011-08-02 01:43:33 +0000
@@ -0,0 +1,11 @@
+<tal:bugtarget xmlns:tal="http://xml.zope.org/namespaces/tal";>
+  <tr style="opacity: 0.5">
+    <td></td>
+    <td>
+      <a tal:attributes="href context/fmt:url;
+                         class context/image:sprite_css"
+         tal:content="context/bugtargetdisplayname" />
+    </td>
+    <td colspan="5"></td>
+  </tr>
+</tal:bugtarget>