Merge lp:~allenap/launchpad/structural-subscriptions-with-filters-3 into lp:launchpad/db-devel

[Gavin Panella]

This adds three helper properties to BugSubscriptionFilter - statuses, importances, tags - that will make its use much simpler. They abstract away a lot of the database jiggery pokery that needs to go on when creating or manipulating subscription filters.

[Abel Deuring (adeuring)]

review: Approve (code)

=== modified file 'lib/lp/bugs/model/bugsubscriptionfilter.py'
--- lib/lp/bugs/model/bugsubscriptionfilter.py	2010-09-17 11:14:25 +0000
+++ lib/lp/bugs/model/bugsubscriptionfilter.py	2010-10-05 09:28:22 +0000
@@ -6,6 +6,8 @@
 __metaclass__ = type
 __all__ = ['BugSubscriptionFilter']
 
+from itertools import chain
+
 from storm.base import Storm
 from storm.locals import (
     Bool,
@@ -14,6 +16,16 @@
     Unicode,
     )
 
+from canonical.launchpad import searchbuilder
+from canonical.launchpad.interfaces.lpstorm import IStore
+from lp.bugs.model.bugsubscriptionfilterimportance import (
+    BugSubscriptionFilterImportance,
+    )
+from lp.bugs.model.bugsubscriptionfilterstatus import (
+    BugSubscriptionFilterStatus,
+    )
+from lp.bugs.model.bugsubscriptionfiltertag import BugSubscriptionFilterTag
+
 
 class BugSubscriptionFilter(Storm):
     """A filter to specialize a *structural* subscription."""
@@ -34,3 +46,135 @@
     other_parameters = Unicode()
 
     description = Unicode()
+
+    def _get_statuses(self):
+        """Return a frozenset of statuses to filter on."""
+        return frozenset(
+            IStore(BugSubscriptionFilterStatus).find(
+                BugSubscriptionFilterStatus,
+                BugSubscriptionFilterStatus.filter == self).values(
+                BugSubscriptionFilterStatus.status))
+
+    def _set_statuses(self, statuses):
+        """Update the statuses to filter on.
+
+        The statuses must be from the `BugTaskStatus` enum, but can be
+        bundled in any iterable.
+        """
+        statuses = frozenset(statuses)
+        current_statuses = self.statuses
+        store = IStore(BugSubscriptionFilterStatus)
+        # Add additional statuses.
+        for status in statuses.difference(current_statuses):
+            status_filter = BugSubscriptionFilterStatus()
+            status_filter.filter = self
+            status_filter.status = status
+            store.add(status_filter)
+        # Delete unused ones.
+        store.find(
+            BugSubscriptionFilterStatus,
+            BugSubscriptionFilterStatus.filter == self,
+            BugSubscriptionFilterStatus.status.is_in(
+                current_statuses.difference(statuses))).remove()
+
+    statuses = property(
+        _get_statuses, _set_statuses, doc=(
+            "A frozenset of statuses filtered on."))
+
+    def _get_importances(self):
+        """Return a frozenset of importances to filter on."""
+        return frozenset(
+            IStore(BugSubscriptionFilterImportance).find(
+                BugSubscriptionFilterImportance,
+                BugSubscriptionFilterImportance.filter == self).values(
+                BugSubscriptionFilterImportance.importance))
+
+    def _set_importances(self, importances):
+        """Update the importances to filter on.
+
+        The importances must be from the `BugTaskImportance` enum, but can be
+        bundled in any iterable.
+        """
+        importances = frozenset(importances)
+        current_importances = self.importances
+        store = IStore(BugSubscriptionFilterImportance)
+        # Add additional importances.
+        for importance in importances.difference(current_importances):
+            importance_filter = BugSubscriptionFilterImportance()
+            importance_filter.filter = self
+            importance_filter.importance = importance
+            store.add(importance_filter)
+        # Delete unused ones.
+        store.find(
+            BugSubscriptionFilterImportance,
+            BugSubscriptionFilterImportance.filter == self,
+            BugSubscriptionFilterImportance.importance.is_in(
+                current_importances.difference(importances))).remove()
+
+    importances = property(
+        _get_importances, _set_importances, doc=(
+            "A frozenset of importances filtered on."))
+
+    def _get_tags(self):
+        """Return a frozenset of tags to filter on."""
+        wildcards = []
+        if self.include_any_tags:
+            wildcards.append(u"*")
+        if self.exclude_any_tags:
+            wildcards.append(u"-*")
+        tags = (
+            tag_filter.qualified_tag
+            for tag_filter in IStore(BugSubscriptionFilterTag).find(
+                BugSubscriptionFilterTag,
+                BugSubscriptionFilterTag.filter == self))
+        return frozenset(chain(wildcards, tags))
+
+    def _set_tags(self, tags):
+        """Update the tags to filter on.
+
+        The tags can be qualified with a leading hyphen, and can be bundled in
+        any iterable.
+
+        If they are passed within a `searchbuilder.any` or `searchbuilder.all`
+        object, the `find_all_tags` attribute will be updated to match.
+
+        Wildcard tags - `*` and `-*` - can be given too, and will update
+        `include_any_tags` and `exclude_any_tags`.
+        """
+        # Deal with searchbuilder terms.
+        if isinstance(tags, searchbuilder.all):
+            self.find_all_tags = True
+            tags = frozenset(tags.query_values)
+        elif isinstance(tags, searchbuilder.any):
+            self.find_all_tags = False
+            tags = frozenset(tags.query_values)
+        else:
+            # Leave find_all_tags unchanged.
+            tags = frozenset(tags)
+        wildcards = frozenset((u"*", u"-*")).intersection(tags)
+        # Set wildcards.
+        self.include_any_tags = "*" in wildcards
+        self.exclude_any_tags = "-*" in wildcards
+        # Deal with other tags.
+        tags = tags - wildcards
+        store = IStore(BugSubscriptionFilterTag)
+        current_tag_filters = dict(
+            (tag_filter.qualified_tag, tag_filter)
+            for tag_filter in store.find(
+                BugSubscriptionFilterTag,
+                BugSubscriptionFilterTag.filter == self))
+        # Remove unused tags.
+        for tag in set(current_tag_filters).difference(tags):
+            tag_filter = current_tag_filters.pop(tag)
+            store.remove(tag_filter)
+        # Add additional tags.
+        for tag in tags.difference(current_tag_filters):
+            tag_filter = BugSubscriptionFilterTag()
+            tag_filter.filter = self
+            tag_filter.include = not tag.startswith("-")
+            tag_filter.tag = tag.lstrip("-")
+            store.add(tag_filter)
+
+    tags = property(
+        _get_tags, _set_tags, doc=(
+            "A frozenset of tags filtered on."))

=== modified file 'lib/lp/bugs/model/bugsubscriptionfiltertag.py'
--- lib/lp/bugs/model/bugsubscriptionfiltertag.py	2010-09-17 11:14:25 +0000
+++ lib/lp/bugs/model/bugsubscriptionfiltertag.py	2010-10-05 09:28:22 +0000
@@ -27,3 +27,11 @@
 
     include = Bool(allow_none=False)
     tag = Unicode(allow_none=False)
+
+    @property
+    def qualified_tag(self):
+        """The tag qualified with a hyphen if it is to be omitted."""
+        if self.include:
+            return self.tag
+        else:
+            return u"-" + self.tag

=== modified file 'lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py'
--- lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py	2010-09-17 12:32:14 +0000
+++ lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py	2010-10-05 09:28:22 +0000
@@ -5,8 +5,13 @@
 
 __metaclass__ = type
 
+from canonical.launchpad import searchbuilder
 from canonical.launchpad.interfaces.lpstorm import IStore
 from canonical.testing import DatabaseFunctionalLayer
+from lp.bugs.interfaces.bugtask import (
+    BugTaskImportance,
+    BugTaskStatus,
+    )
 from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
 from lp.testing import (
     login_person,
@@ -64,3 +69,130 @@
         self.assertIs(False, bug_subscription_filter.exclude_any_tags)
         self.assertIs(None, bug_subscription_filter.other_parameters)
         self.assertIs(None, bug_subscription_filter.description)
+
+    def test_statuses(self):
+        # The statuses property is a frozenset of the statuses that are
+        # filtered upon.
+        bug_subscription_filter = BugSubscriptionFilter()
+        self.assertEqual(frozenset(), bug_subscription_filter.statuses)
+
+    def test_statuses_set(self):
+        # Assigning any iterable to statuses updates the database.
+        bug_subscription_filter = BugSubscriptionFilter()
+        bug_subscription_filter.statuses = [
+            BugTaskStatus.NEW, BugTaskStatus.INCOMPLETE]
+        self.assertEqual(
+            frozenset((BugTaskStatus.NEW, BugTaskStatus.INCOMPLETE)),
+            bug_subscription_filter.statuses)
+        # Assigning a subset causes the other status filters to be removed.
+        bug_subscription_filter.statuses = [BugTaskStatus.NEW]
+        self.assertEqual(
+            frozenset((BugTaskStatus.NEW,)),
+            bug_subscription_filter.statuses)
+
+    def test_statuses_set_empty(self):
+        # Assigning an empty iterable to statuses updates the database.
+        bug_subscription_filter = BugSubscriptionFilter()
+        bug_subscription_filter.statuses = []
+        self.assertEqual(frozenset(), bug_subscription_filter.statuses)
+
+    def test_importances(self):
+        # The importances property is a frozenset of the importances that are
+        # filtered upon.
+        bug_subscription_filter = BugSubscriptionFilter()
+        self.assertEqual(frozenset(), bug_subscription_filter.importances)
+
+    def test_importances_set(self):
+        # Assigning any iterable to importances updates the database.
+        bug_subscription_filter = BugSubscriptionFilter()
+        bug_subscription_filter.importances = [
+            BugTaskImportance.HIGH, BugTaskImportance.LOW]
+        self.assertEqual(
+            frozenset((BugTaskImportance.HIGH, BugTaskImportance.LOW)),
+            bug_subscription_filter.importances)
+        # Assigning a subset causes the other importance filters to be
+        # removed.
+        bug_subscription_filter.importances = [BugTaskImportance.HIGH]
+        self.assertEqual(
+            frozenset((BugTaskImportance.HIGH,)),
+            bug_subscription_filter.importances)
+
+    def test_importances_set_empty(self):
+        # Assigning an empty iterable to importances updates the database.
+        bug_subscription_filter = BugSubscriptionFilter()
+        bug_subscription_filter.importances = []
+        self.assertEqual(frozenset(), bug_subscription_filter.importances)
+
+    def test_tags(self):
+        # The tags property is a frozenset of the tags that are filtered upon.
+        bug_subscription_filter = BugSubscriptionFilter()
+        self.assertEqual(frozenset(), bug_subscription_filter.tags)
+
+    def test_tags_set(self):
+        # Assigning any iterable to tags updates the database.
+        bug_subscription_filter = BugSubscriptionFilter()
+        bug_subscription_filter.tags = [u"foo", u"-bar"]
+        self.assertEqual(
+            frozenset((u"foo", u"-bar")),
+            bug_subscription_filter.tags)
+        # Assigning a subset causes the other tag filters to be removed.
+        bug_subscription_filter.tags = [u"foo"]
+        self.assertEqual(
+            frozenset((u"foo",)),
+            bug_subscription_filter.tags)
+
+    def test_tags_set_empty(self):
+        # Assigning an empty iterable to tags updates the database.
+        bug_subscription_filter = BugSubscriptionFilter()
+        bug_subscription_filter.tags = []
+        self.assertEqual(frozenset(), bug_subscription_filter.tags)
+
+    def test_tags_set_wildcard(self):
+        # Setting one or more wildcard tags may update include_any_tags or
+        # exclude_any_tags.
+        bug_subscription_filter = BugSubscriptionFilter()
+        self.assertEqual(frozenset(), bug_subscription_filter.tags)
+        self.assertFalse(bug_subscription_filter.include_any_tags)
+        self.assertFalse(bug_subscription_filter.exclude_any_tags)
+
+        bug_subscription_filter.tags = [u"*"]
+        self.assertEqual(frozenset((u"*",)), bug_subscription_filter.tags)
+        self.assertTrue(bug_subscription_filter.include_any_tags)
+        self.assertFalse(bug_subscription_filter.exclude_any_tags)
+
+        bug_subscription_filter.tags = [u"-*"]
+        self.assertEqual(frozenset((u"-*",)), bug_subscription_filter.tags)
+        self.assertFalse(bug_subscription_filter.include_any_tags)
+        self.assertTrue(bug_subscription_filter.exclude_any_tags)
+
+        bug_subscription_filter.tags = [u"*", u"-*"]
+        self.assertEqual(
+            frozenset((u"*", u"-*")), bug_subscription_filter.tags)
+        self.assertTrue(bug_subscription_filter.include_any_tags)
+        self.assertTrue(bug_subscription_filter.exclude_any_tags)
+
+        bug_subscription_filter.tags = []
+        self.assertEqual(frozenset(), bug_subscription_filter.tags)
+        self.assertFalse(bug_subscription_filter.include_any_tags)
+        self.assertFalse(bug_subscription_filter.exclude_any_tags)
+
+    def test_tags_with_any_and_all(self):
+        # If the tags are bundled in a c.l.searchbuilder.any or .all, the
+        # find_any_tags attribute will also be updated.
+        bug_subscription_filter = BugSubscriptionFilter()
+        self.assertEqual(frozenset(), bug_subscription_filter.tags)
+        self.assertFalse(bug_subscription_filter.find_all_tags)
+
+        bug_subscription_filter.tags = searchbuilder.all(u"foo")
+        self.assertEqual(frozenset((u"foo",)), bug_subscription_filter.tags)
+        self.assertTrue(bug_subscription_filter.find_all_tags)
+
+        # Not using `searchbuilder.any` or `.all` leaves find_all_tags
+        # unchanged.
+        bug_subscription_filter.tags = [u"-bar"]
+        self.assertEqual(frozenset((u"-bar",)), bug_subscription_filter.tags)
+        self.assertTrue(bug_subscription_filter.find_all_tags)
+
+        bug_subscription_filter.tags = searchbuilder.any(u"baz")
+        self.assertEqual(frozenset((u"baz",)), bug_subscription_filter.tags)
+        self.assertFalse(bug_subscription_filter.find_all_tags)

=== modified file 'lib/lp/bugs/model/tests/test_bugsubscriptionfiltertag.py'
--- lib/lp/bugs/model/tests/test_bugsubscriptionfiltertag.py	2010-09-17 12:32:14 +0000
+++ lib/lp/bugs/model/tests/test_bugsubscriptionfiltertag.py	2010-10-05 09:28:22 +0000
@@ -49,3 +49,15 @@
             bug_sub_filter_tag.filter)
         self.assertIs(True, bug_sub_filter_tag.include)
         self.assertEqual(u"foo", bug_sub_filter_tag.tag)
+
+    def test_qualified_tag(self):
+        """
+        `BugSubscriptionFilterTag.qualified_tag` returns a tag with a
+        preceeding hyphen if `include` is `False`.
+        """
+        bug_sub_filter_tag = BugSubscriptionFilterTag()
+        bug_sub_filter_tag.tag = u"foo"
+        bug_sub_filter_tag.include = True
+        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)