launchpad-reviewers team mailing list archive

[Guruprasad <mp+452490@xxxxxxxxxxxxxxxxxx>]

Guruprasad has proposed merging ~lgp171188/launchpad:allow-distribution-bug-supervisor-to-create-structural-subscriptions into launchpad:master.

Commit message:
Allow the distribution bug supervisor to add structural subscriptions                                                                                                                        

LP: #2037777

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #2037777 in Launchpad itself: "The bug supervisor of a distribution is unable to create a new structural subscription for the bugs in that distribution"
  https://bugs.launchpad.net/launchpad/+bug/2037777

For more details, see:
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/452490
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:allow-distribution-bug-supervisor-to-create-structural-subscriptions into launchpad:master.

diff --git a/lib/lp/bugs/model/structuralsubscription.py b/lib/lp/bugs/model/structuralsubscription.py
index 1694f09..37e8be2 100644
--- a/lib/lp/bugs/model/structuralsubscription.py
+++ b/lib/lp/bugs/model/structuralsubscription.py
@@ -430,8 +430,8 @@ class StructuralSubscriptionTargetMixin:
             if subscriber is None or subscribed_by is None:
                 return False
             elif (
-                subscriber != self.bug_supervisor
-                and not subscriber.inTeam(self.bug_supervisor)
+                subscribed_by != self.bug_supervisor
+                and not subscribed_by.inTeam(self.bug_supervisor)
                 and not subscribed_by.inTeam(admins)
             ):
                 return False
diff --git a/lib/lp/bugs/tests/test_structuralsubscription.py b/lib/lp/bugs/tests/test_structuralsubscription.py
index ce1f712..2911d2e 100644
--- a/lib/lp/bugs/tests/test_structuralsubscription.py
+++ b/lib/lp/bugs/tests/test_structuralsubscription.py
@@ -5,9 +5,11 @@
 
 from storm.store import EmptyResultSet, ResultSet, Store
 from testtools.matchers import StartsWith
+from zope.component import getUtility
 from zope.security.interfaces import Unauthorized
 
 from lp.app.enums import InformationType
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.bugs.enums import BugNotificationLevel
 from lp.bugs.interfaces.bugtask import BugTaskImportance, BugTaskStatus
 from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients
@@ -135,6 +137,73 @@ class TestStructuralSubscription(TestCaseWithFactory):
                 Unauthorized, lambda: self.subscription.newBugFilter
             )
 
+    def test_distribution_userCanAlterBugSubscription_bug_supervisor_unset(
+        self,
+    ):
+        # When the bug supervisor for a distribution is unset, any person
+        # can alter the bug subscription of any other person in that
+        # distribution.
+        distribution = self.factory.makeDistribution()
+        self.assertIsNone(distribution.bug_supervisor)
+        person1 = self.factory.makePerson()
+        person2 = self.factory.makePerson()
+        self.assertTrue(
+            distribution.userCanAlterBugSubscription(person2, person1)
+        )
+
+    def test_distribution_userCanAlterBugSubscription_subscriber_None(self):
+        distribution = self.factory.makeDistribution(
+            bug_supervisor=self.factory.makeTeam()
+        )
+        subscribed_by = self.factory.makePerson()
+        self.assertFalse(
+            distribution.userCanAlterBugSubscription(None, subscribed_by)
+        )
+
+    def test_distribution_userCanAlterBugSubscription_subscribed_by_None(self):
+        distribution = self.factory.makeDistribution(
+            bug_supervisor=self.factory.makeTeam()
+        )
+        subscriber = self.factory.makePerson()
+        self.assertFalse(
+            distribution.userCanAlterBugSubscription(subscriber, None)
+        )
+
+    def test_distribution_userCanAlterBugSubscription_bug_supervisor(self):
+        bug_supervisor = self.factory.makePerson()
+        distribution = self.factory.makeDistribution(
+            bug_supervisor=bug_supervisor
+        )
+        subscriber = self.factory.makePerson()
+        self.assertTrue(
+            distribution.userCanAlterBugSubscription(
+                subscriber, bug_supervisor
+            )
+        )
+
+    def test_distribution_userCanAlterBugSubscription_bug_supervisor_member(
+        self,
+    ):
+        member = self.factory.makePerson()
+        bug_supervisor_team = self.factory.makeTeam(members=[member])
+        distribution = self.factory.makeDistribution(
+            bug_supervisor=bug_supervisor_team
+        )
+        subscriber = self.factory.makePerson()
+        self.assertTrue(
+            distribution.userCanAlterBugSubscription(subscriber, member)
+        )
+
+    def test_distribution_userCanAlterBugSubscription_admin(self):
+        distribution = self.factory.makeDistribution(
+            bug_supervisor=self.factory.makeTeam()
+        )
+        subscriber = self.factory.makePerson()
+        admin = getUtility(ILaunchpadCelebrities).admin.teamowner
+        self.assertTrue(
+            distribution.userCanAlterBugSubscription(subscriber, admin)
+        )
+
 
 class FilteredStructuralSubscriptionTestBase:
     """Tests for filtered structural subscriptions."""