launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/bug-770248 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #770248 in Launchpad itself: "I can't mute on the main bug page when I only have a structural subscription"
  https://bugs.launchpad.net/launchpad/+bug/770248

For more details, see:
https://code.launchpad.net/~bac/launchpad/bug-770248/+merge/59047

= Summary =

If a user's team has a structural subscription to a pillar then the mute
link should be shown.

== Proposed fix ==

Fix Bug. personIsAlsoNotifiedSubscriber to take team membership into
account.

== Pre-implementation notes ==

Chat with Gary.

== Implementation details ==

As above.

== Tests ==

bin/test -vvm -t test_bug_views

== Demo and Q/A ==

Create a structural subscription on a product for a team you administer.
 Go to a bug where you have no other subscriptions.  See mute link.
Dance a little jig.

= 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/model/bug.py
  lib/lp/bugs/browser/tests/test_bugsubscription_views.py
-- 
https://code.launchpad.net/~bac/launchpad/bug-770248/+merge/59047
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-770248 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-11 21:52:02 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py	2011-04-26 12:06:01 +0000
@@ -17,7 +17,6 @@
 from lp.services.features import get_relevant_feature_controller
 from lp.testing import (
     BrowserTestCase,
-    feature_flags,
     person_logged_in,
     TestCaseWithFactory,
     )
@@ -50,10 +49,14 @@
 class TestBugPortletSubscribers(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
+    feature_flag_1 = 'malone.advanced-subscriptions.enabled'
+    feature_flag_2 = 'malone.advanced-structural-subscriptions.enabled'
 
     def setUp(self):
         super(TestBugPortletSubscribers, self).setUp()
-        self.bug = self.factory.makeBug()
+        self.target = self.factory.makeProduct()
+        bug_owner = self.factory.makePerson(name="bug-owner")
+        self.bug = self.factory.makeBug(owner=bug_owner, product=self.target)
         # We need to put the Bug and default BugTask into the LaunchBag
         # because BugContextMenu relies on the LaunchBag to populate its
         # context property
@@ -66,7 +69,7 @@
         # the mute link will not be displayed to them.
         person = self.factory.makePerson()
         with person_logged_in(person):
-            with feature_flags():
+            with FeatureFixture({self.feature_flag_2: None}):
                 # The user isn't subscribed or muted already.
                 self.assertFalse(self.bug.isSubscribed(person))
                 self.assertFalse(self.bug.isMuted(person))
@@ -82,8 +85,7 @@
                 self.assertFalse('mute_subscription' in html)
 
     def test_edit_subscriptions_link_shown_when_feature_enabled(self):
-        flag = 'malone.advanced-structural-subscriptions.enabled'
-        with FeatureFixture({flag: 'on'}):
+        with FeatureFixture({self.feature_flag_2: 'on'}):
             request = LaunchpadTestRequest()
             request.features = get_relevant_feature_controller()
             view = create_initialized_view(
@@ -98,3 +100,48 @@
         html = view.render()
         self.assertTrue('menu-link-editsubscriptions' not in html)
         self.assertTrue('/+subscriptions' not in html)
+
+    def test_bug_mute_for_individual_structural_subscription(self):
+        person = self.factory.makePerson(name="a-person")
+        with FeatureFixture({self.feature_flag_1: 'on'}):
+            with person_logged_in(person):
+                self.target.addBugSubscription(person, person)
+                self.assertFalse(self.bug.isMuted(person))
+                view = create_initialized_view(
+                    self.bug, name="+portlet-subscribers")
+                self.assertTrue(view.user_should_see_mute_link,
+                                "User should see mute link.")
+                contents = view.render()
+                self.assertTrue('mute_subscription' in contents,
+                                "'mute_subscription' not in contents.")
+                create_initialized_view(
+                    self.bug.default_bugtask, name="+mute",
+                    form={'field.actions.mute': 'Mute bug mail'})
+                self.assertTrue(self.bug.isMuted(person))
+
+    def test_mute_subscription_link_shown_for_team_subscription(self):
+        # If the person belongs to a team with a structural subscription,
+        # then the mute link will not displayed to them.
+        person = self.factory.makePerson(name="a-person")
+        team_owner = self.factory.makePerson(name="team-owner")
+        team = self.factory.makeTeam(owner=team_owner, name="subscribed-team")
+        with FeatureFixture({self.feature_flag_1: 'on'}):
+            with person_logged_in(team_owner):
+                team.addMember(person, team_owner)
+                self.target.addBugSubscription(team, team_owner)
+            with person_logged_in(person):
+                self.assertFalse(self.bug.isMuted(person))
+                self.assertTrue(
+                    self.bug.personIsAlsoNotifiedSubscriber(
+                        person), "Person should be a notified subscriber")
+                view = create_initialized_view(
+                    self.bug, name="+portlet-subscribers")
+                self.assertTrue(view.user_should_see_mute_link,
+                                "User should see mute link.")
+                contents = view.render()
+                self.assertTrue('mute_subscription' in contents,
+                                "'mute_subscription' not in contents.")
+                create_initialized_view(
+                    self.bug.default_bugtask, name="+mute",
+                    form={'field.actions.mute': 'Mute bug mail'})
+                self.assertTrue(self.bug.isMuted(person))

=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2011-04-21 18:29:18 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2011-04-26 12:06:01 +0000
@@ -36,7 +36,6 @@
         super(BugSubscriptionAdvancedFeaturesTestCase, self).setUp()
         self.bug = self.factory.makeBug()
         self.person = self.factory.makePerson()
-        self.team = self.factory.makeTeam()
 
     def test_subscribe_uses_bug_notification_level(self):
         # When a user subscribes to a bug using the advanced features on

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2011-04-19 18:30:12 +0000
+++ lib/lp/bugs/model/bug.py	2011-04-26 12:06:01 +0000
@@ -1894,8 +1894,14 @@
         # and assignees. As such, it's not possible to get them all with
         # one query.
         also_notified_subscribers = self.getAlsoNotifiedSubscribers()
-
-        return person in also_notified_subscribers
+        if person in also_notified_subscribers:
+            return True
+        # Otherwise check to see if the person is a member of any of the
+        # subscribed teams.
+        for subscriber in also_notified_subscribers:
+            if subscriber.is_team and person.inTeam(subscriber):
+                return True
+        return False
 
     def personIsSubscribedToDuplicate(self, person):
         """See `IBug`."""