launchpad-reviewers team mailing list archive

[Graham Binns <graham@xxxxxxxxxxxxx>]

Graham Binns has proposed merging lp:~gmb/launchpad/no-mute-for-non-subscribers-bug-741821 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #741821 in Launchpad itself: ""Mute bug mail" in new bug reports is odd"
  https://bugs.launchpad.net/launchpad/+bug/741821

For more details, see:
https://code.launchpad.net/~gmb/launchpad/no-mute-for-non-subscribers-bug-741821/+merge/55512

This branch fixes bug 741821, which suggests that the mute link should only appear if:

 - The user is directly subscribed
 - The user is subscribed via a duplicate
 - The user is structurally subscribed
 - The user is "also notified" (i.e. is BugTarget owner or bug supervisor)
 - The user is already muted (in which case it will read "Unmute...")

The fix is quite simple, and I've added a simple unit test to cover the change.
-- 
https://code.launchpad.net/~gmb/launchpad/no-mute-for-non-subscribers-bug-741821/+merge/55512
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/no-mute-for-non-subscribers-bug-741821 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2011-03-23 16:28:51 +0000
+++ lib/lp/bugs/browser/bug.py	2011-03-30 11:08:03 +0000
@@ -522,6 +522,20 @@
             return 'muted-false %s' % subscription_class
 
     @cachedproperty
+    def user_should_see_mute_link(self):
+        """Return True if the user should see the Mute link."""
+        if features.getFeatureFlag('malone.advanced-subscriptions.enabled'):
+            user_is_subscribed = (
+                # Note that we don't have to check for isMuted(), since
+                # it's a subset of isSubscribed().
+                self.context.isSubscribed(self.user) or
+                self.context.isSubscribedToDupes(self.user) or
+                self.context.personIsAlsoNotifiedSubscriber(self.user))
+            return user_is_subscribed
+        else:
+            return False
+
+    @cachedproperty
     def _bug_attachments(self):
         """Get a dict of attachment type -> attachments list."""
         # Note that this is duplicated with get_comments_for_bugtask

=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml	2011-03-23 16:28:51 +0000
+++ lib/lp/bugs/browser/configure.zcml	2011-03-30 11:08:03 +0000
@@ -526,6 +526,12 @@
                 template="../templates/bugtarget-portlet-search.pt"/>
         </browser:pages>
         <browser:page
+            for="lp.bugs.interfaces.bug.IBug"
+            class="lp.bugs.browser.bug.BugView"
+            permission="launchpad.View"
+            name="+portlet-subscribers"
+            template="../templates/bug-portlet-subscribers.pt"/>
+        <browser:page
             for="lp.bugs.interfaces.bugtask.IBugTask"
             class="lp.bugs.browser.bug.BugView"
             permission="launchpad.View"
@@ -1050,9 +1056,6 @@
                 name="+portlet-attachments"
                 template="../templates/bug-portlet-attachments.pt"/>
             <browser:page
-                name="+portlet-subscribers"
-                template="../templates/bug-portlet-subscribers.pt"/>
-            <browser:page
                 name="+portlet-notified"
                 template="../templates/bug-portlet-notified.pt"/>
             <browser:page

=== added file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py	2011-03-30 11:08:03 +0000
@@ -0,0 +1,56 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for Bug Views."""
+
+__metaclass__ = type
+
+from zope.component import getUtility
+
+from canonical.launchpad.webapp.interfaces import IOpenLaunchBag
+from canonical.testing.layers import DatabaseFunctionalLayer
+
+from lp.testing import (
+    feature_flags,
+    person_logged_in,
+    set_feature_flag,
+    TestCaseWithFactory,
+    )
+from lp.testing.views import create_initialized_view
+
+
+class TestBugPortletSubscribers(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestBugPortletSubscribers, self).setUp()
+        self.bug = self.factory.makeBug()
+        # We need to put the Bug and default BugTask into the LaunchBag
+        # because BugContextMenu relies on the LaunchBag to populate its
+        # context property
+        launchbag = getUtility(IOpenLaunchBag)
+        launchbag.add(self.bug)
+        launchbag.add(self.bug.default_bugtask)
+        with feature_flags():
+            set_feature_flag(u'malone.advanced-subscriptions.enabled', u'on')
+
+    def test_mute_subscription_link_not_shown_for_non_subscribers(self):
+        # If a person is not already subscribed to a bug in some way,
+        # the mute link will not be displayed to them.
+        person = self.factory.makePerson()
+        with person_logged_in(person):
+            with feature_flags():
+                # The user isn't subscribed or muted already.
+                self.assertFalse(self.bug.isSubscribed(person))
+                self.assertFalse(self.bug.isMuted(person))
+                self.assertFalse(
+                    self.bug.personIsAlsoNotifiedSubscriber(
+                        person))
+                view = create_initialized_view(
+                    self.bug, name="+portlet-subscribers")
+                self.assertFalse(view.user_should_see_mute_link)
+                # The template uses user_should_see_mute_link to decide
+                # whether or not to display the mute link.
+                html = view.render()
+                self.assertFalse('mute_subscription' in html)

=== modified file 'lib/lp/bugs/templates/bug-portlet-subscribers.pt'
--- lib/lp/bugs/templates/bug-portlet-subscribers.pt	2011-03-23 16:28:51 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscribers.pt	2011-03-30 11:08:03 +0000
@@ -13,8 +13,7 @@
       tal:content="structure context_menu/subscription/render" />
     <div id="sub-unsub-spinner">Subscribing...</div>
     <div tal:content="structure context_menu/addsubscriber/render" />
-    <tal:show-mute
-        condition="request/features/malone.advanced-subscriptions.enabled">
+    <tal:show-mute condition="view/user_should_see_mute_link">
       <div
         tal:attributes="class view/current_user_mute_class"
         tal:content="structure context_menu/mute_subscription/render" />