launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/bugsubscriptionfilter-itype into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1026757 in Launchpad itself: "cannot filter structural subscriptions by information type"
  https://bugs.launchpad.net/launchpad/+bug/1026757

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/bugsubscriptionfilter-itype/+merge/117825

Add model code for support filtering structural subscriptions by information types. Since this is incredibly similar to the current filtering that is supported like status or importance, it looks very similar. It can not land on devel until db-devel r11803 has been deployed to production and merged into devel.
-- 
https://code.launchpad.net/~stevenk/launchpad/bugsubscriptionfilter-itype/+merge/117825
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/bugsubscriptionfilter-itype into lp:launchpad.

=== modified file 'lib/lp/bugs/interfaces/bugsubscriptionfilter.py'
--- lib/lp/bugs/interfaces/bugsubscriptionfilter.py	2012-03-13 00:45:33 +0000
+++ lib/lp/bugs/interfaces/bugsubscriptionfilter.py	2012-08-02 02:36:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Bug subscription filter interfaces."""
@@ -37,6 +37,7 @@
     BugTaskStatus,
     )
 from lp.bugs.interfaces.structuralsubscription import IStructuralSubscription
+from lp.registry.enums import InformationType
 from lp.services.fields import (
     PersonChoice,
     SearchTag,
@@ -100,6 +101,13 @@
             required=True, default=frozenset(),
             value_type=SearchTag()))
 
+    information_types = exported(
+        FrozenSet(
+            title=_("The information types interested in (empty for all)"),
+            required=True, default=frozenset(),
+            value_type=Choice(
+                title=_('Information Type'), vocabulary=InformationType)))
+
 
 class IBugSubscriptionFilterMethodsPublic(Interface):
     """Methods on `IBugSubscriptionFilter` that can be called by anyone."""

=== modified file 'lib/lp/bugs/model/bugsubscriptionfilter.py'
--- lib/lp/bugs/model/bugsubscriptionfilter.py	2012-07-31 23:03:51 +0000
+++ lib/lp/bugs/model/bugsubscriptionfilter.py	2012-08-02 02:36:20 +0000
@@ -4,6 +4,7 @@
 __metaclass__ = type
 __all__ = [
     'BugSubscriptionFilter',
+    'BugSubscriptionFilterInformationType',
     'BugSubscriptionFilterImportance',
     'BugSubscriptionFilterMute',
     'BugSubscriptionFilterStatus',
@@ -33,6 +34,7 @@
     BugTaskImportance,
     BugTaskStatus,
     )
+from lp.registry.enums import InformationType
 from lp.registry.interfaces.person import validate_person
 from lp.services import searchbuilder
 from lp.services.database.constants import UTC_NOW
@@ -218,6 +220,47 @@
         _get_tags, _set_tags, doc=(
             "A frozenset of tags filtered on."))
 
+    def _get_information_types(self):
+        """Return a frozenset of information_types to filter on."""
+        return frozenset(
+            IStore(BugSubscriptionFilterInformationType).find(
+                BugSubscriptionFilterInformationType,
+                BugSubscriptionFilterInformationType.filter == self).values(
+                BugSubscriptionFilterInformationType.information_type))
+
+    def _set_information_types(self, information_types):
+        """Update the information_types to filter on.
+
+        The information types must be from the `InformationType` enum, but
+        can be bundled in any iterable.
+
+        Setting all information types is equivalent to setting no statuses,
+        and is normalized that way.
+        """
+        itypes = frozenset(information_types)
+        if itypes == frozenset(InformationType.items):
+            # Setting all is the same as setting none, and setting none is
+            # cheaper for reading and storage.
+            itypes = frozenset()
+        current_itypes = self.information_types
+        store = IStore(BugSubscriptionFilterInformationType)
+        # Add additional information_types.
+        for information_type in itypes.difference(current_itypes):
+            itype_filter = BugSubscriptionFilterInformationType()
+            itype_filter.filter = self
+            itype_filter.information_type = information_type
+            store.add(itype_filter)
+        # Delete unused ones.
+        store.find(
+            BugSubscriptionFilterInformationType,
+            BugSubscriptionFilterInformationType.filter == self,
+            BugSubscriptionFilterInformationType.information_type.is_in(
+                current_itypes.difference(itypes))).remove()
+
+    information_types = property(
+        _get_information_types, _set_information_types, doc=(
+            "A frozenset of information_types filtered on."))
+
     def _has_other_filters(self):
         """Are there other filters for parent `StructuralSubscription`?"""
         store = Store.of(self)
@@ -238,6 +281,7 @@
         # This clears up all of the linked sub-records in the associated
         # tables.
         self.importances = self.statuses = self.tags = ()
+        self.information_types = ()
 
         if self._has_other_filters():
             Store.of(self).remove(self)
@@ -364,3 +408,15 @@
             return self.tag
         else:
             return u"-" + self.tag
+
+
+class BugSubscriptionFilterInformationType(StormBase):
+    """Information types to filter."""
+
+    __storm_table__ = "BugSubscriptionFilterInformationType"
+    __storm_primary__ = ('filter_id', 'information_type')
+
+    filter_id = Int("filter", allow_none=False)
+    filter = Reference(filter_id, "BugSubscriptionFilter.id")
+
+    information_type = DBEnum(enum=InformationType, allow_none=False)

=== modified file 'lib/lp/bugs/model/structuralsubscription.py'
--- lib/lp/bugs/model/structuralsubscription.py	2012-07-31 23:03:51 +0000
+++ lib/lp/bugs/model/structuralsubscription.py	2012-08-02 02:36:20 +0000
@@ -56,6 +56,7 @@
 from lp.bugs.model.bugsubscriptionfilter import (
     BugSubscriptionFilter,
     BugSubscriptionFilterImportance,
+    BugSubscriptionFilterInformationType,
     BugSubscriptionFilterStatus,
     BugSubscriptionFilterTag,
     )
@@ -760,6 +761,11 @@
             BugSubscriptionFilter.bug_notification_level >= level)
     # This handles the bugtask-specific attributes of status and importance.
     conditions.append(_calculate_bugtask_condition(query_arguments))
+    # Handle filtering by information type.
+    conditions.append(Or(
+        BugSubscriptionFilterInformationType.information_type == 
+            bug.information_type,
+        BugSubscriptionFilterStatus.status == None))
     # Now we handle tags.  This actually assembles the query, because it
     # may have to union two queries together.
     # Note that casting bug.tags to a list subtly removes the security
@@ -845,6 +851,9 @@
                  BugSubscriptionFilter.id),
         LeftJoin(BugSubscriptionFilterImportance,
                  BugSubscriptionFilterImportance.filter_id ==
+                 BugSubscriptionFilter.id),
+        LeftJoin(BugSubscriptionFilterInformationType,
+                 BugSubscriptionFilterInformationType.filter_id ==
                  BugSubscriptionFilter.id)]
     tag_join = LeftJoin(
         BugSubscriptionFilterTag,

=== modified file 'lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py'
--- lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py	2012-07-31 23:03:51 +0000
+++ lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py	2012-08-02 02:36:20 +0000
@@ -20,9 +20,11 @@
 from lp.bugs.model.bugsubscriptionfilter import (
     BugSubscriptionFilter,
     BugSubscriptionFilterImportance,
+    BugSubscriptionFilterInformationType,
     BugSubscriptionFilterStatus,
     BugSubscriptionFilterTag,
     )
+from lp.registry.enums import InformationType
 from lp.services import searchbuilder
 from lp.services.database.lpstorm import IStore
 from lp.testing import (
@@ -184,7 +186,7 @@
             bug_subscription_filter.statuses)
 
     def test_statuses_set_all(self):
-        # Setting all importances is normalized into setting no importances.
+        # Setting all statuses is normalized into setting no statuses.
         bug_subscription_filter = BugSubscriptionFilter()
         bug_subscription_filter.statuses = list(BugTaskStatus.items)
         self.assertEqual(frozenset(), bug_subscription_filter.statuses)
@@ -228,6 +230,46 @@
         bug_subscription_filter.importances = []
         self.assertEqual(frozenset(), bug_subscription_filter.importances)
 
+    def test_information_types(self):
+        # The information_types property is a frozenset of the
+        # information_types that are filtered upon.
+        bug_subscription_filter = BugSubscriptionFilter()
+        self.assertEqual(
+            frozenset(), bug_subscription_filter.information_types)
+
+    def test_information_types_set(self):
+        # Assigning any iterable to information_types updates the database.
+        bug_subscription_filter = BugSubscriptionFilter()
+        bug_subscription_filter.information_types = [
+            InformationType.PRIVATESECURITY, InformationType.USERDATA]
+        self.assertEqual(
+            frozenset((InformationType.PRIVATESECURITY,
+                InformationType.USERDATA)),
+            bug_subscription_filter.information_types)
+        # Assigning a subset causes the other status filters to be removed.
+        bug_subscription_filter.information_types = [
+            InformationType.USERDATA]
+        self.assertEqual(
+            frozenset((InformationType.USERDATA,)),
+            bug_subscription_filter.information_types)
+
+    def test_information_types_set_all(self):
+        # Setting all information_types is normalized into setting no
+        # information_types.
+        bug_subscription_filter = BugSubscriptionFilter()
+        bug_subscription_filter.information_types = list(
+            InformationType.items)
+        self.assertEqual(
+            frozenset(), bug_subscription_filter.information_types)
+
+    def test_information_types_set_empty(self):
+        # Assigning an empty iterable to information_types updates the
+        # database.
+        bug_subscription_filter = BugSubscriptionFilter()
+        bug_subscription_filter.information_types = []
+        self.assertEqual(
+            frozenset(), bug_subscription_filter.information_types)
+
     def test_tags(self):
         # The tags property is a frozenset of the tags that are filtered upon.
         bug_subscription_filter = BugSubscriptionFilter()
@@ -492,3 +534,35 @@
         self.assertEqual(u"foo", bug_sub_filter_tag.qualified_tag)
         bug_sub_filter_tag.include = False
         self.assertEqual(u"-foo", bug_sub_filter_tag.qualified_tag)
+
+
+class TestBugSubscriptionFilterInformationType(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestBugSubscriptionFilterInformationType, self).setUp()
+        self.target = self.factory.makeProduct()
+        self.subscriber = self.target.owner
+        login_person(self.subscriber)
+        self.subscription = self.target.addBugSubscription(
+            self.subscriber, self.subscriber)
+        self.subscription_filter = BugSubscriptionFilter()
+        self.subscription_filter.structural_subscription = self.subscription
+
+    def test_basics(self):
+        # Test the basics of `BugSubscriptionFilterInformationType` objects.
+        # Create.
+        bug_sub_filter_itype = BugSubscriptionFilterInformationType()
+        bug_sub_filter_itype.filter = self.subscription_filter
+        bug_sub_filter_itype.information_type = InformationType.USERDATA
+        # Flush and reload.
+        IStore(bug_sub_filter_itype).flush()
+        IStore(bug_sub_filter_itype).reload(bug_sub_filter_itype)
+        # Check.
+        self.assertEqual(
+            self.subscription_filter.id, bug_sub_filter_itype.filter_id)
+        self.assertEqual(
+            self.subscription_filter, bug_sub_filter_itype.filter)
+        self.assertEqual(
+            InformationType.USERDATA, bug_sub_filter_itype.information_type)

=== modified file 'lib/lp/bugs/tests/test_structuralsubscription.py'
--- lib/lp/bugs/tests/test_structuralsubscription.py	2012-03-13 00:45:33 +0000
+++ lib/lp/bugs/tests/test_structuralsubscription.py	2012-08-02 02:36:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for `StructuralSubscription`."""
@@ -30,6 +30,7 @@
     get_structural_subscriptions,
     get_structural_subscriptions_for_bug,
     )
+from lp.registry.enums import InformationType
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.testing import (
     anonymous_logged_in,
@@ -205,6 +206,14 @@
         self.initial_filter.importances = [self.bugtask.importance]
         self.assertSubscribers([self.ordinary_subscriber])
 
+    def test_getStructuralSubscribers_with_filter_on_information_type(self):
+        self.assertSubscribers([self.ordinary_subscriber])
+        self.initial_filter.information_types = [InformationType.USERDATA]
+        self.assertSubscribers([])
+        self.initial_filter.information_types = [
+            self.bugtask.bug.information_type]
+        self.assertSubscribers([self.ordinary_subscriber])
+
     def test_getStructuralSubscribers_with_filter_on_level(self):
         # All structural subscriptions have a level for bug notifications
         # which get_structural_subscribers() observes.