Merge lp:~jcsackett/launchpad/private-bugs-404 into lp:launchpad

[j.c.sackett]

[incr] [r=danilo][bug=434733] Fixes links to private bugs so that they're only text if the user can't see the bug.

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

[Robert Collins (lifeless)]

This is good, but possibly incomplete.

If the master bug isn't viewable, perhaps we shouldn't list the bug
number *either* - the whole point being that said bug is invisible to
the submitter. We've a cluster of bugs around the behaviour and UI of
duplicates-against-private-bugs, and similarly in the planning for bug
relationships and better privacy, the idea was put forward to simply
hide relationships (of which duplicate is one that we already have)
where the other side was invisible to the viewing user.

Hmm, in writing this I realise that actually the thing is way to
complex to shoe-horn in here.

But I do think we should do something to help with the support load -
folk will be confused when its not a link and they can't see it.
Perhaps 'duplicate of bug 42 which is embargoed (you cannot view it)'.

Or something.

[j.c.sackett (jcsackett)]

I was actually thinking something similar, but I wanted to keep consistent with the portlet, which I don't think has room for the expanded text.

I'll work on some phrasing that works both above the comment box and in the portlet, and make those changes.

[Данило Шеган (danilo)]

Looks good.

review: Approve

=== 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-17 18:14:37 +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/bug-portlet-actions.pt'
--- lib/lp/bugs/templates/bug-portlet-actions.pt	2010-09-21 09:37:06 +0000
+++ lib/lp/bugs/templates/bug-portlet-actions.pt	2011-04-17 18:14:37 +0000
@@ -42,7 +42,7 @@
       >bug #42</a>
         <span
           tal:condition="not:duplicateof/required:launchpad.View"
-          tal:replace="string:bug #${duplicateof/id}" />
+          tal:replace="string:a private bug" />
     </span>
     </tal:block>
   </div>

=== 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-17 18:14:37 +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-warning-link;"
+        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: a private bug." />
                   Comment here only if you think the duplicate status is wrong.
               </div>
               <h2>Add comment</h2>