launchpad-reviewers team mailing list archive

[Graham Binns <graham@xxxxxxxxxxxxx>]

Graham Binns has proposed merging lp:~gmb/launchpad/make-sub-link-nothing-aware-bug-721410 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #721410 The Subscribe link on the bug page should still read "Subscribe" if the user has a subscription with a bug_notification_level of NOTHING
  https://bugs.launchpad.net/bugs/721410

For more details, see:
https://code.launchpad.net/~gmb/launchpad/make-sub-link-nothing-aware-bug-721410/+merge/52409

This branch makes the Subscribe link in the bug subscription portlet
aware of BugNotificationLevel.NOTHING as part of the story for adding a
mute button to bugs.

I've added a function, isMuted(), to IBug. This can be called for any
user and is used to determine whether that user has a muted subscription
for the bug.

I've updated BugContextMenu.subscription() to make use of isMuted().
I've also added tests for isMuted() and BugContextMenu.subscription() in
the context of muted bug subscriptions (I'll file a bug about the fact
that it's not otherwise tested as far as I can tell).
-- 
https://code.launchpad.net/~gmb/launchpad/make-sub-link-nothing-aware-bug-721410/+merge/52409
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/make-sub-link-nothing-aware-bug-721410 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2011-03-02 21:44:33 +0000
+++ lib/lp/bugs/browser/bug.py	2011-03-07 13:11:32 +0000
@@ -248,8 +248,12 @@
             self.context.bug.isSubscribed(user) or
             self.context.bug.isSubscribedToDupes(user)):
             if self._use_advanced_features:
-                text = 'Edit subscription'
-                icon = 'edit'
+                if self.context.bug.isMuted(user):
+                    text = 'Subscribe'
+                    icon = 'add'
+                else:
+                    text = 'Edit subscription'
+                    icon = 'edit'
             else:
                 text = 'Unsubscribe'
                 icon = 'remove'

=== added file 'lib/lp/bugs/browser/tests/test_bug_context_menu.py'
--- lib/lp/bugs/browser/tests/test_bug_context_menu.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/tests/test_bug_context_menu.py	2011-03-07 13:11:32 +0000
@@ -0,0 +1,49 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the `BugContextMenu`."""
+
+__metaclass__ = type
+
+from zope.component import getUtility
+
+from canonical.launchpad.webapp.interfaces import IOpenLaunchBag
+from canonical.testing.layers import DatabaseFunctionalLayer
+
+from lp.bugs.browser.bug import BugContextMenu
+from lp.bugs.enum import BugNotificationLevel
+from lp.testing import (
+    feature_flags,
+    person_logged_in,
+    set_feature_flag,
+    TestCaseWithFactory,
+    )
+
+class TestBugContextMenu(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestBugContextMenu, 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)
+        self.context_menu = BugContextMenu(self.bug)
+        with feature_flags():
+            set_feature_flag(u'malone.advanced-subscriptions.enabled', u'on')
+
+    def test_text_for_muted_subscriptions(self):
+        # If a user has a mute on a bug it's recorded internally as a
+        # type of subscription. However, the subscription text of the
+        # BugContextMenu will still read 'Subscribe'.
+        person = self.factory.makePerson()
+        with feature_flags():
+            with person_logged_in(person):
+                self.bug.subscribe(
+                    person, person, level=BugNotificationLevel.NOTHING)
+                link = self.context_menu.subscription()
+                self.assertEqual('Subscribe', link.text)

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2011-03-02 23:08:54 +0000
+++ lib/lp/bugs/configure.zcml	2011-03-07 13:11:32 +0000
@@ -771,7 +771,9 @@
                     bug_messages
                     isUserAffected
                     getHWSubmissions
-                    isExpirable"/>
+                    isExpirable
+                    isMuted
+                    "/>
             <require
                 permission="launchpad.Edit"
                 attributes="

=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2011-03-02 05:09:52 +0000
+++ lib/lp/bugs/interfaces/bug.py	2011-03-07 13:11:32 +0000
@@ -482,6 +482,13 @@
         duplicate of this bug, otherwise False.
         """
 
+    def isMuted(person):
+        """Does person have a muted on this bug?
+
+        :returns: True if the user has a direct subscription to this bug
+            with a BugNotificationLevel of NOTHING.
+        """
+
     def getDirectSubscriptions():
         """A sequence of IBugSubscriptions directly linked to this bug."""
 

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2011-03-05 00:06:26 +0000
+++ lib/lp/bugs/model/bug.py	2011-03-07 13:11:32 +0000
@@ -820,6 +820,17 @@
         """See `IBug`."""
         return self.personIsSubscribedToDuplicate(person)
 
+    def isMuted(self, person):
+        """See `IBug`."""
+        store = Store.of(self)
+        subscriptions = store.find(
+            BugSubscription,
+            BugSubscription.bug == self,
+            BugSubscription.person == person,
+            BugSubscription.bug_notification_level ==
+                BugNotificationLevel.NOTHING)
+        return not subscriptions.is_empty()
+
     @property
     def subscriptions(self):
         """The set of `BugSubscriptions` for this bug."""

=== added file 'lib/lp/bugs/tests/test_bug.py'
--- lib/lp/bugs/tests/test_bug.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/tests/test_bug.py	2011-03-07 13:11:32 +0000
@@ -0,0 +1,46 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for lp.bugs.model.Bug."""
+
+__metaclass__ = type
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+
+from lp.bugs.enum import BugNotificationLevel
+from lp.testing import (
+    feature_flags,
+    person_logged_in,
+    set_feature_flag,
+    TestCaseWithFactory,
+    )
+
+class TestBugSubscriptionMethods(TestCaseWithFactory):
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestBugSubscriptionMethods, self).setUp()
+        self.bug = self.factory.makeBug()
+        self.person = self.factory.makePerson()
+
+    def test_is_muted_returns_true_for_muted_users(self):
+        # Bug.isMuted() will return True if the passed to it has a
+        # BugSubscription with a BugNotificationLevel of NOTHING.
+        with person_logged_in(self.person):
+            subscription = self.bug.subscribe(
+                self.person, self.person, level=BugNotificationLevel.NOTHING)
+            self.assertEqual(True, self.bug.isMuted(self.person))
+
+    def test_is_muted_returns_false_for_direct_subscribers(self):
+        # Bug.isMuted() will return False if the user has a subscription
+        # with BugNotificationLevel that's not NOTHING.
+        with person_logged_in(self.person):
+            subscription = self.bug.subscribe(
+                self.person, self.person, level=BugNotificationLevel.METADATA)
+            self.assertEqual(False, self.bug.isMuted(self.person))
+
+    def test_is_muted_returns_false_for_non_subscribers(self):
+        # Bug.isMuted() will return False if the user has no
+        # subscription.
+        with person_logged_in(self.person):
+            self.assertEqual(False, self.bug.isMuted(self.person))