launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/dupe-privacy into lp:launchpad.

Commit message:
BugTask:+index's duplicates portlet will no longer link to invisible private bugs and now uses the correct sprite for each bug.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1443418 in Launchpad itself: "Public duplicate of private bug report has padlock icon"
  https://bugs.launchpad.net/launchpad/+bug/1443418
  Bug #1465880 in Launchpad itself: "Bug duplicates portlet lists inaccessible bugs"
  https://bugs.launchpad.net/launchpad/+bug/1465880

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/dupe-privacy/+merge/270791

BugTask:+index's duplicates portlet will no longer link to invisible private bugs and now uses the correct sprite for each bug.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/dupe-privacy into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2015-07-08 16:05:11 +0000
+++ lib/lp/bugs/browser/bug.py	2015-09-11 10:09:11 +0000
@@ -572,12 +572,15 @@
                 BugTaskSearchParams(self.user, bug=any(*duplicate_bugs))))
         dupes = []
         for bug in duplicate_bugs:
-            dupe = {}
-            try:
-                dupe['title'] = bug.title
-            except Unauthorized:
-                dupe['title'] = 'Private Bug'
-            dupe['id'] = bug.id
+            # Don't disclose even the ID of a private bug. The link will
+            # just 404.
+            if not check_permission('launchpad.View', bug):
+                continue
+            dupe = {
+                'title': bug.title,
+                'id': bug.id,
+                'bug': bug,
+                }
             # If the dupe has the same context as the one we're in, link
             # to that bug task directly.
             if bug in dupes_in_current_context:

=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py	2014-04-29 00:44:32 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py	2015-09-11 10:09:11 +0000
@@ -9,6 +9,7 @@
     datetime,
     timedelta,
     )
+import re
 
 from BeautifulSoup import BeautifulSoup
 import pytz
@@ -16,6 +17,7 @@
 from soupmatchers import (
     HTMLContains,
     Tag,
+    Within,
     )
 from storm.store import Store
 from testtools.matchers import (
@@ -40,6 +42,7 @@
 from lp.services.webapp.publisher import canonical_url
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.testing import (
+    admin_logged_in,
     BrowserTestCase,
     login_person,
     person_logged_in,
@@ -62,24 +65,60 @@
 
     layer = DatabaseFunctionalLayer
 
-    def makeDupeOfPrivateBug(self):
-        bug = self.factory.makeBug()
+    def test_private_master_not_linked_without_permission(self):
+        bug = self.factory.makeBug(
+            information_type=InformationType.PRIVATESECURITY)
         dupe = self.factory.makeBug()
-        with person_logged_in(bug.owner):
-            bug.setPrivate(private=True, who=bug.owner)
+        with admin_logged_in():
             dupe.markAsDuplicate(bug)
-        return dupe
-
-    def test_private_bugs_are_not_linked_without_permission(self):
-        bug = self.makeDupeOfPrivateBug()
-        url = canonical_url(bug, rootsite="bugs")
-        browser = self.getUserBrowser(url)
-        dupe_warning = find_tag_by_id(
-            browser.contents,
-            'warning-comment-on-duplicate')
+        with person_logged_in(dupe.owner):
+            getUtility(IOpenLaunchBag).add(dupe.default_bugtask)
+            html = create_initialized_view(
+                dupe.default_bugtask, "+index", principal=dupe.owner)()
+        dupe_warning = find_tag_by_id(html, 'warning-comment-on-duplicate')
         # There is no link in the dupe_warning.
         self.assertTrue('href' not in dupe_warning)
 
+    def test_private_dupes_not_linked_without_permission(self):
+        bug = self.factory.makeBug()
+        publidupe = self.factory.makeBug()
+        visidupe = self.factory.makeBug(
+            information_type=InformationType.PRIVATESECURITY)
+        invisidupe = self.factory.makeBug(
+            information_type=InformationType.PRIVATESECURITY)
+        with admin_logged_in():
+            publidupe.markAsDuplicate(bug)
+            visidupe.markAsDuplicate(bug)
+            invisidupe.markAsDuplicate(bug)
+            visidupe.subscribe(bug.owner, visidupe.owner)
+        with person_logged_in(bug.owner):
+            getUtility(IOpenLaunchBag).add(bug.default_bugtask)
+            html = create_initialized_view(
+                bug.default_bugtask, "+index", principal=bug.owner)()
+        # The public dupe and subscribed private dupe are listed, but
+        # the unsubscribed one is not.
+        dupes_portlet = Tag(
+            "dupes portlet", "div", attrs={"id": "portlet-duplicates"})
+        self.assertThat(
+            html,
+            MatchesAll(
+                HTMLContains(
+                    Within(
+                        dupes_portlet,
+                        Tag(
+                            "public dupe", "a",
+                            text=re.compile("Bug #%d" % publidupe.id),
+                            attrs={"class": "sprite bug"})),
+                    Within(
+                        dupes_portlet,
+                        Tag(
+                            "private dupe", "a",
+                            text=re.compile("Bug #%d" % visidupe.id),
+                            attrs={"class": "sprite bug private"}))),
+                Not(HTMLContains(Tag(
+                    "invisible dupe", "a",
+                    text=re.compile("Bug #%d" % invisidupe.id))))))
+
 
 class TestAlsoAffectsLinks(BrowserTestCase):
     """ Tests the rendering of the Also Affects links on the bug index view.

=== modified file 'lib/lp/bugs/templates/bug-portlet-duplicates.pt'
--- lib/lp/bugs/templates/bug-portlet-duplicates.pt	2009-09-10 16:13:39 +0000
+++ lib/lp/bugs/templates/bug-portlet-duplicates.pt	2015-09-11 10:09:11 +0000
@@ -9,7 +9,7 @@
     <li tal:repeat="dupe view/duplicates">
       <a tal:content="string: Bug #${dupe/id}"
          tal:attributes="href dupe/url; title dupe/title;
-                         class context/image:sprite_css" />
+                         class dupe/bug/image:sprite_css" />
     </li>
   </ul>
 </div>