← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~sinzui/launchpad/upstream-downstream-bug-links-0 into lp:launchpad

 

Curtis Hovey has proposed merging lp:~sinzui/launchpad/upstream-downstream-bug-links-0 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #3152 Link between project and package bug listings, or combine them
  https://bugs.launchpad.net/bugs/3152


This is my branch to add reciprocal links between packages and upstream
projects.

    lp:~sinzui/launchpad/upstream-downstream-bug-links-0
    Diff size: 404
    Launchpad bug:
        https://bugs.launchpad.net/bugs/3152
    Test command: ./bin/test -vv \
        -t TestBugTaskSearchListingView -t bugtask-searches
    Pre-implementation: Edwin, Deryck
    Target release: 10.12


Add reciprocal links between packages and upstream projects
-----------------------------------------------------------

The bug search page should contain a notice that bugs are also tracked
in a downstream package or upstream project. Contributors can search the
alternate project or package when investigating bugs.

This link already appears on project pages when the project does not use
Lp to track bugs. The link should also appear when the project uses Lp.
The package page needs the reciprocating link.

This implementation meets the requirements from bridging the gap, but I
am dissatisfied with it. There is an open bug that could be fixed in
another branch to address the awkward experience I had using this feature
in dev: When my search fails, and I see that there is an upstream/downstream
location to search, I expect the link to perform my search. re-typing text
is not difficult, but setting up an advanced search is :( The work to fix the
bug and two related bugs are intrinsic to the search form--the solution is
not obvious.

Screenshots
    http://people.canonical.com/~curtis/downstream-bugs.png
    http://people.canonical.com/~curtis/upstream-bugs.png
    http://people.canonical.com/~curtis/upstream-bugs-searched.png


Rules
-----

    * Refactor the views and templates so that the also-in-ubuntu block
      is available on the project bug search page.
    * Add and also-in-upstream block for DSP and SP pages.


QA
--

    * Visit https://bugs.launchpad.net/jokosher
    * Verify their is a message that bugs are also tracked in Ubuntu.
    * Follow the link to Ubuntu Jokosher bug page.
    * Verify there is a message that bugs are also tracked in Jokosher
      and there is a link back.


Lint
----

Linting changed files:
  lib/canonical/launchpad/icing/style.css
  lib/lp/bugs/browser/bugtarget.py
  lib/lp/bugs/browser/bugtask.py
  lib/lp/bugs/browser/tests/test_buglisting.py
  lib/lp/bugs/templates/bugtarget-bugs.pt
  lib/lp/bugs/templates/bugtarget-macros-search.pt


Test
----

Added tests for the 3 moved/added properties, and verified the also-in-ubuntu
and also-in-upstream blocks in the markup.
    lib/lp/bugs/browser/tests/test_buglisting.py


Implementation
--------------

Extracted the external_bugtracker() method and also-in-ubuntu markup to a
base class and macro template so that they can be reused.
    lib/lp/bugs/browser/bugtarget.py
    lib/lp/bugs/templates/bugtarget-bugs.pt

Added has_bugtracker() and upstream_launchpad_project() to support the
cases in the template that need to show also-in-ubuntu and also-in-upstream.
The template markup needed revision to support presenting the additional
information with proper spacing. This change also removed the exceptional
CSS used by the search form when a search has not been performed.
    lib/lp/bugs/browser/bugtask.py
    lib/lp/bugs/templates/bugtarget-macros-search.pt

Removed obsolete style that changed the color of the text.
    lib/canonical/launchpad/icing/style.css

-- 
https://code.launchpad.net/~sinzui/launchpad/upstream-downstream-bug-links-0/+merge/41371
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/upstream-downstream-bug-links-0 into lp:launchpad.
=== modified file 'lib/canonical/launchpad/icing/style.css'
--- lib/canonical/launchpad/icing/style.css	2010-10-15 01:48:05 +0000
+++ lib/canonical/launchpad/icing/style.css	2010-11-19 21:59:34 +0000
@@ -655,21 +655,6 @@
   color: #3840be;
 }
 
-.search-box {
-  text-align: left;
-  float: left;
-  border:1px solid #d6d6d6;
-  margin: 0.5em 0 0.5em 0;
-  color: #717171;
-  padding: 8px;
-  -moz-border-radius: 5px;
-  -o-border-radius: 5px;
-  -webkit-border-radius: 5px;
-}
-
-.search-box form.primary.search {
-  margin: 0.5em;
-}
 
 /* ====== Content area styles ====== */
 

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2010-11-02 21:38:30 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2010-11-19 21:59:34 +0000
@@ -59,7 +59,6 @@
     )
 from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
 from canonical.launchpad.interfaces.launchpad import (
-    IHasExternalBugTracker,
     ILaunchpadCelebrities,
     )
 from canonical.launchpad.searchbuilder import any
@@ -1275,18 +1274,6 @@
         return service_usage.bug_tracking_usage
 
     @property
-    def external_bugtracker(self):
-        """External bug tracking system designated for the context.
-
-        :returns: `IBugTracker` or None
-        """
-        has_external_bugtracker = IHasExternalBugTracker(self.context, None)
-        if has_external_bugtracker is None:
-            return None
-        else:
-            return has_external_bugtracker.getExternalBugTracker()
-
-    @property
     def bugtracker(self):
         """Description of the context's bugtracker.
 

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2010-10-31 20:18:45 +0000
+++ lib/lp/bugs/browser/bugtask.py	2010-11-19 21:59:34 +0000
@@ -130,7 +130,10 @@
     BugTargetLatestBugsFeedLink,
     FeedsMixin,
     )
-from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from canonical.launchpad.interfaces.launchpad import (
+    IHasExternalBugTracker,
+    ILaunchpadCelebrities,
+    )
 from canonical.launchpad.interfaces.validation import (
     valid_upstreamtask,
     validate_distrotask,
@@ -2300,6 +2303,52 @@
         service_usage = IServiceUsage(self.context)
         return service_usage.bug_tracking_usage
 
+    @cachedproperty
+    def external_bugtracker(self):
+        """External bug tracking system designated for the context.
+
+        :returns: `IBugTracker` or None
+        """
+        has_external_bugtracker = IHasExternalBugTracker(self.context, None)
+        if has_external_bugtracker is None:
+            return None
+        else:
+            return has_external_bugtracker.getExternalBugTracker()
+
+    @property
+    def has_bugtracker(self):
+        """Does the `IBugTarget` have a bug tracker or use Launchpad?"""
+        usage = IServiceUsage(self.context)
+        uses_lp = usage.bug_tracking_usage == ServiceUsage.LAUNCHPAD
+        if self.external_bugtracker or uses_lp:
+            return True
+        return False
+
+    @property
+    def upstream_launchpad_project(self):
+        """The linked upstream `IProduct` for the package.
+
+        If this `IBugTarget` is a `IDistributionSourcePackage` or an
+        `ISourcePackage` and it is linked to an upstream project that uses
+        Launchpad to track bugs, return the `IProduct`. Otherwise,
+        return None
+
+        :returns: `IProduct` or None
+        """
+        if self._sourcePackageContext():
+            sp = self.context
+        elif self._distroSourcePackageContext():
+            sp = self.context.development_version
+        else:
+            sp = None
+        if sp is not None:
+            packaging = sp.packaging
+            if packaging is not None:
+                product = packaging.productseries.product
+                if product.bug_tracking_usage == ServiceUsage.LAUNCHPAD:
+                    return product
+        return None
+
     @property
     def page_title(self):
         return "Bugs in %s" % self.context.title

=== modified file 'lib/lp/bugs/browser/tests/test_buglisting.py'
--- lib/lp/bugs/browser/tests/test_buglisting.py	2010-10-06 22:05:15 +0000
+++ lib/lp/bugs/browser/tests/test_buglisting.py	2010-11-19 21:59:34 +0000
@@ -3,19 +3,28 @@
 
 __metaclass__ = type
 
+from zope.component import getUtility
+
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.testing.pages import (
     extract_text,
     find_tag_by_id,
     find_tags_by_class,
     )
-from canonical.launchpad.webapp import canonical_url
-from canonical.testing.layers import LaunchpadFunctionalLayer
-from lp.testing import BrowserTestCase
-
-
-class TestBugTaskSearchListingView(BrowserTestCase):
-
-    layer = LaunchpadFunctionalLayer
+from canonical.launchpad.webapp.publisher import canonical_url
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.testing import (
+    BrowserTestCase,
+    login_person,
+    person_logged_in,
+    TestCaseWithFactory,
+    )
+from lp.testing.views import create_initialized_view
+
+
+class TestBugTaskSearchListingPage(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
 
     def _makeDistributionSourcePackage(self):
         distro = self.factory.makeDistribution('test-distro')
@@ -120,3 +129,127 @@
         self.assertIs(None,
                       find_tag_by_id(browser.contents, 'portlet-tags'),
                       "portlet-tags should not be shown.")
+
+
+class BugTargetMixin:
+    """Test helpers for setting up `IBugTarget` tests."""
+
+    def _makeBugTargetProduct(self, bug_tracker=None, packaging=False):
+        """Return a product that may use Launchpad or an external bug tracker.
+
+        bug_tracker may be None, 'launchpad', or 'external'.
+        """
+        product = self.factory.makeProduct()
+        if bug_tracker is not None:
+            with person_logged_in(product.owner):
+                if bug_tracker == 'launchpad':
+                    product.official_malone = True
+                else:
+                    product.bugtracker = self.factory.makeBugTracker()
+        if packaging:
+            self.factory.makePackagingLink(
+                productseries=product.development_focus, in_ubuntu=True)
+        return product
+
+
+class TestBugTaskSearchListingViewProduct(TestCaseWithFactory,
+                                          BugTargetMixin):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_external_bugtracker_is_none(self):
+        bug_target = self._makeBugTargetProduct()
+        view = create_initialized_view(bug_target, '+bugs')
+        self.assertEqual(None, view.external_bugtracker)
+
+    def test_external_bugtracker(self):
+        bug_target = self._makeBugTargetProduct(bug_tracker='external')
+        view = create_initialized_view(bug_target, '+bugs')
+        self.assertEqual(bug_target.bugtracker, view.external_bugtracker)
+
+    def test_has_bugtracker_is_false(self):
+        bug_target = self.factory.makeProduct()
+        view = create_initialized_view(bug_target, '+bugs')
+        self.assertEqual(False, view.has_bugtracker)
+
+    def test_has_bugtracker_external_is_true(self):
+        bug_target = self._makeBugTargetProduct(bug_tracker='external')
+        view = create_initialized_view(bug_target, '+bugs')
+        self.assertEqual(True, view.has_bugtracker)
+
+    def test_has_bugtracker_launchpad_is_true(self):
+        bug_target = self._makeBugTargetProduct(bug_tracker='launchpad')
+        view = create_initialized_view(bug_target, '+bugs')
+        self.assertEqual(True, view.has_bugtracker)
+
+    def test_product_without_packaging_also_in_ubuntu_is_none(self):
+        bug_target = self._makeBugTargetProduct(bug_tracker='launchpad')
+        login_person(bug_target.owner)
+        view = create_initialized_view(
+            bug_target, '+bugs', principal=bug_target.owner)
+        self.assertEqual(None, find_tag_by_id(view(), 'also-in-ubuntu'))
+
+    def test_product_with_packaging_also_in_ubuntu(self):
+        bug_target = self._makeBugTargetProduct(
+            bug_tracker='launchpad', packaging=True)
+        login_person(bug_target.owner)
+        view = create_initialized_view(
+            bug_target, '+bugs', principal=bug_target.owner)
+        content = find_tag_by_id(view.render(), 'also-in-ubuntu')
+        link = canonical_url(
+            bug_target.ubuntu_packages[0], force_local_path=True)
+        self.assertEqual(link, content.a['href'])
+
+
+class TestBugTaskSearchListingViewDSP(TestCaseWithFactory, BugTargetMixin):
+
+    layer = DatabaseFunctionalLayer
+
+    def _getBugTarget(self, obj):
+        """Return the `IBugTarget` under test.
+
+        Return the object that was passed. Sub-classes can redefine
+        this method.
+        """
+        return obj
+
+    def test_package_with_upstream_launchpad_project(self):
+        upstream_project = self._makeBugTargetProduct(
+            bug_tracker='launchpad', packaging=True)
+        login_person(upstream_project.owner)
+        bug_target = self._getBugTarget(
+            upstream_project.distrosourcepackages[0])
+        view = create_initialized_view(
+            bug_target, '+bugs', principal=upstream_project.owner)
+        self.assertEqual(upstream_project, view.upstream_launchpad_project)
+        content = find_tag_by_id(view.render(), 'also-in-upstream')
+        link = canonical_url(upstream_project, rootsite='bugs')
+        self.assertEqual(link, content.a['href'])
+
+    def test_package_with_upstream_nonlaunchpad_project(self):
+        upstream_project = self._makeBugTargetProduct(packaging=True)
+        login_person(upstream_project.owner)
+        bug_target = self._getBugTarget(
+            upstream_project.distrosourcepackages[0])
+        view = create_initialized_view(
+            bug_target, '+bugs', principal=upstream_project.owner)
+        self.assertEqual(None, view.upstream_launchpad_project)
+        self.assertEqual(None, find_tag_by_id(view(), 'also-in-upstream'))
+
+    def test_package_without_upstream_project(self):
+        observer = self.factory.makePerson()
+        dsp = self.factory.makeDistributionSourcePackage(
+            'test-dsp', distribution=getUtility(ILaunchpadCelebrities).ubuntu)
+        bug_target = self._getBugTarget(dsp)
+        login_person(observer)
+        view = create_initialized_view(
+            bug_target, '+bugs', principal=observer)
+        self.assertEqual(None, view.upstream_launchpad_project)
+        self.assertEqual(None, find_tag_by_id(view(), 'also-in-upstream'))
+
+
+class TestBugTaskSearchListingViewSP(TestBugTaskSearchListingViewDSP):
+
+        def _getBugTarget(self, dsp):
+            """Return the current `ISourcePackage` for the dsp."""
+            return dsp.development_version

=== modified file 'lib/lp/bugs/templates/bugtarget-bugs.pt'
--- lib/lp/bugs/templates/bugtarget-bugs.pt	2010-09-13 21:25:14 +0000
+++ lib/lp/bugs/templates/bugtarget-bugs.pt	2010-11-19 21:59:34 +0000
@@ -101,7 +101,7 @@
       <tal:uses_launchpad_bugtracker
         condition="view/bug_tracking_usage/enumvalue:LAUNCHPAD">
       <tal:has_bugtasks condition="context/has_bugtasks">
-        <div class="search-box" style="margin-bottom:2em">
+        <div class="portlet-top" style="margin-bottom:2em">
           <metal:search
             use-macro="context/@@+bugtarget-macros-search/simple-search-form"
           />
@@ -111,7 +111,7 @@
         --></script>
       </tal:has_bugtasks>
 
-      <tal:has_hot_bugs condition="view/hot_bugs_info/bugtasks">
+      <div class="portlet" tal:condition="view/hot_bugs_info/bugtasks">
         <h2>Hot bugs</h2>
 
         <table class="listing" id="hot-bugs"
@@ -157,7 +157,7 @@
            tal:condition="view/hot_bugs_info/has_more_bugs">
           <a tal:attributes="href string:${context/fmt:url/+bugs}?orderby=-heat&field.status%3Alist=NEW&field.status%3Alist=INCOMPLETE_WITH_RESPONSE&field.status%3Alist=INCOMPLETE_WITHOUT_RESPONSE&field.status%3Alist=CONFIRMED&field.status%3Alist=TRIAGED&field.status%3Alist=INPROGRESS&field.status%3Alist=FIXCOMMITTED&field.omit_dupes=on">See all hot bugs</a>
         </p>
-      </tal:has_hot_bugs>
+      </div>
 
       <tal:no_hot_bugs condition="not: view/hot_bugs_info/bugtasks">
         <p id="no-bugs-filed"><strong>There are currently no
@@ -190,17 +190,8 @@
 
     <tal:also_in_ubuntu
       condition="not: view/bug_tracking_usage/enumvalue:LAUNCHPAD">
-      <p tal:define="packages context/ubuntu_packages | nothing"
-         tal:condition="packages"
-         id="also-in-ubuntu">
-        Ubuntu
-        <tal:also condition="view/external_bugtracker">also</tal:also>
-        tracks bugs for packages derived from this project:
-        <tal:packages repeat="package packages">
-          <span style="white-space: nowrap"
-                tal:content="structure package/fmt:link" /><tal:comma
-          condition="not:repeat/package/end">,</tal:comma></tal:packages>.
-      </p>
+      <metal:also-in-ubuntu
+        use-macro="context/@@+bugtarget-macros-search/also-in-ubuntu" />
     </tal:also_in_ubuntu>
 
     <div

=== modified file 'lib/lp/bugs/templates/bugtarget-macros-search.pt'
--- lib/lp/bugs/templates/bugtarget-macros-search.pt	2010-01-19 22:34:00 +0000
+++ lib/lp/bugs/templates/bugtarget-macros-search.pt	2010-11-19 21:59:34 +0000
@@ -3,6 +3,21 @@
   xmlns:metal="http://xml.zope.org/namespaces/metal";
   omit-tag="">
 
+<metal:block define-macro="also-in-ubuntu">
+  <p id="also-in-ubuntu"
+    tal:define="packages context/ubuntu_packages | nothing"
+    tal:condition="packages">
+    Ubuntu
+    <tal:also condition="view/has_bugtracker">also</tal:also>
+    tracks bugs for packages derived from this project:
+    <tal:packages repeat="package packages">
+      <span style="white-space: nowrap"
+            tal:content="structure package/fmt:link" /><tal:comma
+      condition="not:repeat/package/end">,</tal:comma></tal:packages>.
+  </p>
+</metal:block>
+
+
 <metal:block define-macro="sortwidget">
   <tal:comment condition="nothing">
     This macro expects that the callsite's view class has a shouldShowTargetName
@@ -65,6 +80,7 @@
 <metal:block define-macro="simple-search-form">
 <form method="get" name="search" class="primary search"
       tal:attributes="action search_url|string:">
+  <p>
   <tal:searchbox replace="structure view/widgets/searchtext" />
   <metal:widget use-macro="context/@@+bugtarget-macros-search/sortwidget" />
   <input type="submit" name="search" value="Search" />
@@ -77,9 +93,22 @@
   <tal:widget replace="structure view/widgets/has_patch/hidden" />
   <tal:widget replace="structure view/widgets/component/hidden" />
   <tal:widget replace="structure view/widgets/has_no_package/hidden" />
-  <br /><br />
+  </p>
+  <p>
   <a tal:attributes="href advanced_search_url|string:?advanced=1"
   >Advanced search</a>
+  </p>
+
+  <metal:also-in-ubuntu
+    use-macro="context/@@+bugtarget-macros-search/also-in-ubuntu" />
+
+  <p id="also-in-upstream"
+    tal:define="product view/upstream_launchpad_project | nothing"
+    tal:condition="product">
+    <a tal:replace="structure product/fmt:link:bugs" /> also
+    tracks bugs for this package.
+  </p>
+
   <div metal:define-slot="extra-search-widgets">
   </div>
 </form>


Follow ups