launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/private-bugs-404 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #434733 in Launchpad itself: "marking public bug as duplicate of private bug leads to confusing UI"
  https://bugs.launchpad.net/launchpad/+bug/434733

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/private-bugs-404/+merge/57244

Summary
=======

When a public bug is marked as a duplicate of a private bug, the duplicate listings in the portlet do not provide links, just text. However, the warning below the page links to the private bug, which then yields a 404. We shouldn't generate 404ing links if we can avoid it.

This just changes that link text to regular text when the user can't see the master bug, and adds a test to ensure that it's working.

Preimplementation
================

Spoke with Curtis Hovey

Implementation
==============
lib/lp/bugs/browser/tests/test_bug_views.py
-------------------------------------------
Test

lib/lp/bugs/templates/bugtask-index.pt
--------------------------------------
Altered dupe link in the warning above the comment box to only create a link if the master bug is viewable by the user, and just show the text otherwise.
Tests
=====

QA
==
Find (or create) a public duplicate of a private bug--the warning before the comment form should list the private bug, but should not link to it.

Lint
====
make lint output:

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/browser/tests/test_bug_views.py
  lib/lp/bugs/templates/bugtask-index.pt

-- 
https://code.launchpad.net/~jcsackett/launchpad/private-bugs-404/+merge/57244
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/private-bugs-404 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py	2011-04-05 18:33:58 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py	2011-04-11 22:20:18 +0000
@@ -7,13 +7,16 @@
 
 from zope.component import getUtility
 
+from canonical.launchpad.webapp.publisher import canonical_url
 from canonical.launchpad.webapp.interfaces import IOpenLaunchBag
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
+from canonical.launchpad.testing.pages import find_tag_by_id
 from canonical.testing.layers import DatabaseFunctionalLayer
 
 from lp.services.features.testing import FeatureFixture
 from lp.services.features import get_relevant_feature_controller
 from lp.testing import (
+    BrowserTestCase,
     feature_flags,
     person_logged_in,
     TestCaseWithFactory,
@@ -21,6 +24,29 @@
 from lp.testing.views import create_initialized_view
 
 
+class TestPrivateBugLinks(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def makeDupeOfPrivateBug(self):
+        bug = self.factory.makeBug()
+        dupe = self.factory.makeBug()
+        with person_logged_in(bug.owner):
+            bug.setPrivate(private=True, who=bug.owner)
+            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')
+        # There is no link in the dupe_warning.
+        self.assertTrue('href' not in dupe_warning)
+
+
 class TestBugPortletSubscribers(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer

=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt	2011-04-04 01:13:22 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt	2011-04-11 22:20:18 +0000
@@ -296,9 +296,18 @@
                   class="warning message"
                   id="warning-comment-on-duplicate">
                   Remember, this bug report is a duplicate of
-                    <a href="#" tal:attributes="href
-                      context/bug/duplicateof/fmt:url">bug #<span
-                      tal:replace="context/bug/duplicateof/id">42</span></a>.
+      <a
+        tal:define="duplicateof context/bug/duplicateof"
+        tal:condition="duplicateof/required:launchpad.View"
+        tal:attributes="href duplicateof/fmt:url; title
+           duplicateof/title; style string:margin-right: 4px;
+           id string:duplicate-of;"
+        tal:content="string:bug #${duplicateof/id}."
+      >bug #42</a>
+        <span
+          tal:define="duplicateof context/bug/duplicateof"
+          tal:condition="not:duplicateof/required:launchpad.View"
+          tal:replace="string:bug #${duplicateof/id}." />
                   Comment here only if you think the duplicate status is wrong.
               </div>
               <h2>Add comment</h2>