launchpad-reviewers team mailing list archive

[Graham Binns <graham@xxxxxxxxxxxxx>]

Graham Binns has proposed merging lp:~gmb/launchpad/bug-777783 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #777783 in Launchpad itself: "Project +subscriptions doesn't show subscriptions to the project group"
  https://bugs.launchpad.net/launchpad/+bug/777783

For more details, see:
https://code.launchpad.net/~gmb/launchpad/bug-777783/+merge/60910

This branch fixes bug 777783, whereby structural subscriptions to a project group weren't shown when the user visited the +subscriptions page for each of its sub-products.

The fix itself was simple: I added a clause to the join property of the StructuralSubscriptionTargetHelper for Products which will cause structural subscriptions on a Product's Project to be included in the resultset returned by get_structural_subscriptions_for_target().

I also added three unit tests, one to cover my change (test_subscribed_to_project_group()) and two others to confirm the expected behaviour of get_structural_subscriptions_for_bug() and get_structural_subscription_targets().
-- 
https://code.launchpad.net/~gmb/launchpad/bug-777783/+merge/60910
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/bug-777783 into lp:launchpad.

=== modified file 'lib/lp/bugs/model/structuralsubscription.py'
--- lib/lp/bugs/model/structuralsubscription.py	2011-04-26 15:26:22 +0000
+++ lib/lp/bugs/model/structuralsubscription.py	2011-05-13 12:27:31 +0000
@@ -272,7 +272,9 @@
         self.target_parent = target.project
         self.target_arguments = {"product": target}
         self.pillar = target
-        self.join = (StructuralSubscription.product == target)
+        self.join = Or(
+            StructuralSubscription.product == target,
+            StructuralSubscription.project == target.project)
 
 
 class ProductSeriesTargetHelper:

=== modified file 'lib/lp/bugs/tests/test_structuralsubscription.py'
--- lib/lp/bugs/tests/test_structuralsubscription.py	2011-04-03 00:53:10 +0000
+++ lib/lp/bugs/tests/test_structuralsubscription.py	2011-05-13 12:27:31 +0000
@@ -516,6 +516,22 @@
              (dist_sourcepackage_bugtask, dist_sourcepackage),
              (dist_sourcepackage_bugtask, distribution))))
 
+    def test_product_with_project_group(self):
+        # get_structural_subscription_targets() will yield both a
+        # product and its parent project group if it has one.
+        project = self.factory.makeProject()
+        product = self.factory.makeProduct(
+            project=project, owner=project.owner)
+        subscriber = self.factory.makePerson()
+        with person_logged_in(subscriber):
+            self_sub = project.addBugSubscription(subscriber, subscriber)
+        # This is a sanity check.
+        self.assertEqual(project, product.parent_subscription_target)
+        bug = self.factory.makeBug(product=product)
+        result = get_structural_subscription_targets(bug.bugtasks)
+        self.assertEqual(
+            set([(bug.bugtasks[0], product), (bug.bugtasks[0], project)]),
+            set(result))
 
 class TestGetStructuralSubscriptionsForBug(TestCaseWithFactory):
 
@@ -600,6 +616,22 @@
         subscriptions = self.getSubscriptions(self.subscriber)
         self.assertEqual(set([self_sub, team_sub]), set(subscriptions))
 
+    def test_subscriptions_from_parent(self):
+        # get_structural_subscriptions_for_bug() will return any
+        # structural subscriptions from the parents of the targets of
+        # that bug.
+        project = self.factory.makeProject()
+        product = self.factory.makeProduct(
+            project=project, owner=project.owner)
+        subscriber = self.factory.makePerson()
+        self_sub = project.addBugSubscription(subscriber, subscriber)
+        # This is a sanity check.
+        self.assertEqual(project, product.parent_subscription_target)
+        bug = self.factory.makeBug(product=product)
+        subscriptions = get_structural_subscriptions_for_bug(
+            bug, subscriber)
+        self.assertEqual(set([self_sub]), set(subscriptions))
+
 
 class TestGetStructuralSubscribers(TestCaseWithFactory):
 

=== modified file 'lib/lp/bugs/tests/test_structuralsubscriptiontarget.py'
--- lib/lp/bugs/tests/test_structuralsubscriptiontarget.py	2011-03-29 05:38:15 +0000
+++ lib/lp/bugs/tests/test_structuralsubscriptiontarget.py	2011-05-13 12:27:31 +0000
@@ -476,6 +476,19 @@
         subscriptions = self.getSubscriptions()
         self.assertEqual(set([self_sub, team_sub]), set(subscriptions))
 
+    def test_subscribed_to_project_group(self):
+        # If a user is subscribed to a project group, calls to
+        # get_structural_subscriptions_for_target made against the
+        # products in that group will return the group-level
+        # subscription along with any subscriptions to the product.
+        project = self.factory.makeProject()
+        product = self.factory.makeProduct(project=project)
+        project_sub = project.addBugSubscription(
+            self.subscriber, self.subscriber)
+        subscriptions = get_structural_subscriptions_for_target(
+            product, self.subscriber)
+        self.assertEqual(set([project_sub]), set(subscriptions))
+
 
 def distributionSourcePackageSetUp(test):
     setUp(test)