launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01332
[Merge] lp:~allenap/launchpad/structural-subscriptions-with-filters-3 into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/structural-subscriptions-with-filters-3 into lp:launchpad with lp:~allenap/launchpad/structural-subscriptions-with-filters-2 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
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.
--
https://code.launchpad.net/~allenap/launchpad/structural-subscriptions-with-filters-3/+merge/37270
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/structural-subscriptions-with-filters-3 into lp:launchpad.
=== 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-01 15:05:00 +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-01 15:05:00 +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-01 15:05:00 +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-01 15:05:00 +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)