launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/launchpad/structural-subscriptions-with-filters-6 into lp:launchpad with lp:~allenap/launchpad/structural-subscriptions-with-filters-5 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
  #650991 Add getSubscriptionsForBug to IStructuralSubscriptionTarget
  https://bugs.launchpad.net/bugs/650991


getSubscriptionsForBug() now filters by the presence or absence of tags. It can't yet deal with finding specific tags, but I need to get on with some UI work now so this will do until next cycle.
-- 
https://code.launchpad.net/~allenap/launchpad/structural-subscriptions-with-filters-6/+merge/37653
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/structural-subscriptions-with-filters-6 into lp:launchpad.

=== modified file 'lib/lp/registry/model/structuralsubscription.py'
--- lib/lp/registry/model/structuralsubscription.py	2010-10-05 19:27:48 +0000
+++ lib/lp/registry/model/structuralsubscription.py	2010-10-05 19:27:49 +0000
@@ -515,23 +515,34 @@
                     BugSubscriptionFilter.id)),
             ]
 
+        if len(bug.tags) == 0:
+            tag_conditions = [
+                BugSubscriptionFilter.include_any_tags == False,
+                ]
+        else:
+            tag_conditions = [
+                BugSubscriptionFilter.exclude_any_tags == False,
+                ]
+
         conditions = [
             StructuralSubscription.bug_notification_level >= level,
             Or(
                 # There's no filter or ...
                 BugSubscriptionFilter.id == None,
-                # there is a filter and ...
+                # There is a filter and ...
                 And(
-                    # there's no status filter, or there is a status filter
+                    # There's no status filter, or there is a status filter
                     # and and it matches.
                     Or(BugSubscriptionFilterStatus.id == None,
                        BugSubscriptionFilterStatus.status.is_in(
                             bugtask.status for bugtask in bugtasks)),
-                    # there's no importance filter, or there is an importance
+                    # There's no importance filter, or there is an importance
                     # filter and it matches.
                     Or(BugSubscriptionFilterImportance.id == None,
                        BugSubscriptionFilterImportance.importance.is_in(
-                            bugtask.importance for bugtask in bugtasks)))),
+                            bugtask.importance for bugtask in bugtasks)),
+                    # Any number of conditions relating to tags.
+                    *tag_conditions)),
             ]
 
         return Store.of(self.__helper.pillar).using(*origin).find(

=== modified file 'lib/lp/registry/tests/test_structuralsubscriptiontarget.py'
--- lib/lp/registry/tests/test_structuralsubscriptiontarget.py	2010-10-05 19:27:48 +0000
+++ lib/lp/registry/tests/test_structuralsubscriptiontarget.py	2010-10-05 19:27:49 +0000
@@ -271,6 +271,54 @@
             bugtask.bug, BugNotificationLevel.COMMENTS)
         self.assertEqual([], list(subscriptions_for_bug))
 
+    def test_getSubscriptionsForBug_with_filter_include_any_tags(self):
+        # If a subscription filter has include_any_tags, a bug with one or
+        # more tags is matched.
+        bugtask = self.makeBugTask()
+
+        # Create a new subscription on self.target.
+        login_person(self.ordinary_subscriber)
+        subscription = self.target.addSubscription(
+            self.ordinary_subscriber, self.ordinary_subscriber)
+        subscription.bug_notification_level = BugNotificationLevel.COMMENTS
+        subscription_filter = subscription.newBugFilter()
+        subscription_filter.include_any_tags = True
+
+        # Without any tags the subscription is not found.
+        subscriptions_for_bug = self.target.getSubscriptionsForBug(
+            bugtask.bug, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bug))
+
+        # With any tag the subscription is found.
+        bugtask.bug.tags = ["foo"]
+        subscriptions_for_bug = self.target.getSubscriptionsForBug(
+            bugtask.bug, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bug))
+
+    def test_getSubscriptionsForBug_with_filter_exclude_any_tags(self):
+        # If a subscription filter has exclude_any_tags, only bugs with no
+        # tags are matched.
+        bugtask = self.makeBugTask()
+
+        # Create a new subscription on self.target.
+        login_person(self.ordinary_subscriber)
+        subscription = self.target.addSubscription(
+            self.ordinary_subscriber, self.ordinary_subscriber)
+        subscription.bug_notification_level = BugNotificationLevel.COMMENTS
+        subscription_filter = subscription.newBugFilter()
+        subscription_filter.exclude_any_tags = True
+
+        # Without any tags the subscription is found.
+        subscriptions_for_bug = self.target.getSubscriptionsForBug(
+            bugtask.bug, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bug))
+
+        # With any tag the subscription is not found.
+        bugtask.bug.tags = ["foo"]
+        subscriptions_for_bug = self.target.getSubscriptionsForBug(
+            bugtask.bug, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bug))
+
     def test_getSubscriptionsForBug_with_multiple_filters(self):
         # If multiple filters exist for a subscription, all filters must
         # match.