launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01308
[Merge] lp:~allenap/launchpad/structural-subscriptions-with-filters-2 into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/structural-subscriptions-with-filters-2 into lp:launchpad with lp:~allenap/launchpad/structural-subscriptions-with-filters as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
#650991 Add getSubscriptionsForBug to IStructuralSubscriptionTarget
https://bugs.launchpad.net/bugs/650991
Adds filtering by importance to StructuralSubscriptionTargetMixin.getSubscriptionsForBug().
The tests are getting heavy on set-up, so my next branch is going to be to create some simple helper methods to do that lifting instead.
This is an intermediate branch and probably won't be landed as-is.
--
https://code.launchpad.net/~allenap/launchpad/structural-subscriptions-with-filters-2/+merge/37051
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/structural-subscriptions-with-filters-2 into lp:launchpad.
=== modified file 'lib/lp/bugs/stories/structural-subscriptions/xx-bug-subscriptions.txt'
--- lib/lp/bugs/stories/structural-subscriptions/xx-bug-subscriptions.txt 2010-09-14 19:16:47 +0000
+++ lib/lp/bugs/stories/structural-subscriptions/xx-bug-subscriptions.txt 2010-09-29 19:53:06 +0000
@@ -1,4 +1,5 @@
-= Subscriptions to bug notifications =
+Subscriptions to bug notifications
+----------------------------------
Any logged in user can subscribe themselves to bug notifications for any
Launchpad structure, as well as subscribe any of the teams to which
@@ -64,7 +65,8 @@
Landscape Developers
-== Additional options for distribution drivers ==
+Additional options for distribution drivers
+===========================================
When editing the subscriptions for a package, a distribution driver
can subscribe any Launchpad user.
@@ -97,7 +99,8 @@
>>> for message in find_tags_by_class(browser.contents, 'message'):
... print message.renderContents()
- No Privileges Person will now receive an e-mail each time someone reports or changes a public bug in "mozilla-firefox in ubuntu".
+ No Privileges Person will now receive an e-mail each time someone reports
+ or changes a public bug in "mozilla-firefox in ubuntu".
>>> print extract_text(find_portlet(browser.contents, 'Bug subscriptions'))
Bug subscriptions
@@ -117,7 +120,8 @@
>>> browser.getControl('Save these changes').click()
>>> print find_tags_by_class(
... browser.contents, 'informational message')[0].contents[0]
- No Privileges Person will no longer automatically receive e-mail about public bugs in "mozilla-firefox in ubuntu".
+ No Privileges Person will no longer automatically receive e-mail about
+ public bugs in "mozilla-firefox in ubuntu".
>>> print extract_text(find_portlet(browser.contents, 'Bug subscriptions'))
Bug subscriptions
For...mozilla-firefox...package in Ubuntu:
@@ -179,7 +183,8 @@
LookupError: label '\xa0Foo Bar'
-== Bug #462742 ==
+Bug #462742
+===========
Loading pages with the subscription menu works fine for anonymous users, even
when there are existing subscriptions.
@@ -195,7 +200,8 @@
>>> print anon_browser.title
Bazaar Version Control System in Launchpad
-== Distribution with a bug supervisor ==
+Distribution with a bug supervisor
+==================================
If a distribution has a bug supervisor only that team or members of it can
subscribe to all of the distribution's bugs.
=== modified file 'lib/lp/registry/doc/structural-subscriptions.txt'
--- lib/lp/registry/doc/structural-subscriptions.txt 2010-08-16 13:50:28 +0000
+++ lib/lp/registry/doc/structural-subscriptions.txt 2010-09-29 19:53:06 +0000
@@ -1,4 +1,5 @@
-= Structural Subscriptions =
+Structural Subscriptions
+------------------------
Structural subscriptions allow a user to subscribe to a launchpad
structure like a product, project, productseries, distribution,
@@ -44,7 +45,8 @@
3
-== Parent subscription targets ==
+Parent subscription targets
+===========================
Some subscription targets relate to other targets hierarchically. An
IDistribution, for example, can be said to be a parent of all
@@ -68,17 +70,17 @@
>>> print ff_milestone.parent_subscription_target.displayname
Mozilla Firefox
- >>> ff_trunk = firefox.getSeries('trunk')
- >>> ff_trunk.parent_subscription_target == firefox
- True
- >>> print ff_trunk.parent_subscription_target.displayname
- Mozilla Firefox
+ >>> ff_trunk = firefox.getSeries('trunk')
+ >>> ff_trunk.parent_subscription_target == firefox
+ True
+ >>> print ff_trunk.parent_subscription_target.displayname
+ Mozilla Firefox
- >>> warty = ubuntu.getSeries('warty')
- >>> warty.parent_subscription_target == ubuntu
- True
- >>> print warty.parent_subscription_target.displayname
- Ubuntu
+ >>> warty = ubuntu.getSeries('warty')
+ >>> warty.parent_subscription_target == ubuntu
+ True
+ >>> print warty.parent_subscription_target.displayname
+ Ubuntu
When notifying subscribers of bug activity, both subscribers to the
target and to the target's parent are notified.
@@ -128,8 +130,6 @@
>>> print_bug_recipients(recipients)
name12 "Subscriber (evolution in ubuntu)"
-
-
Foo Bar subscribes to Ubuntu.
>>> login('foo.bar@xxxxxxxxxxxxx')
@@ -198,7 +198,8 @@
name16
-== Retrieving structural subscribers of targets ==
+Retrieving structural subscribers of targets
+============================================
Subscribers of one or more targets are retrieved by
PersonSet.getSubscribersForTargets.
@@ -252,7 +253,8 @@
name16
-== Target type display ==
+Target type display
+===================
Structural subscription targets have a `target_type_display` attribute, which
can be used to refer to them in display.
@@ -263,4 +265,3 @@
package
>>> print ff_milestone.target_type_display
milestone
-
=== modified file 'lib/lp/registry/model/structuralsubscription.py'
--- lib/lp/registry/model/structuralsubscription.py 2010-09-29 19:53:01 +0000
+++ lib/lp/registry/model/structuralsubscription.py 2010-09-29 19:53:06 +0000
@@ -27,6 +27,9 @@
)
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
+from lp.bugs.model.bugsubscriptionfilterimportance import (
+ BugSubscriptionFilterImportance,
+ )
from lp.bugs.model.bugsubscriptionfilterstatus import (
BugSubscriptionFilterStatus,
)
@@ -490,12 +493,27 @@
BugSubscriptionFilterStatus,
BugSubscriptionFilterStatus.filter_id == (
BugSubscriptionFilter.id)),
+ LeftJoin(
+ BugSubscriptionFilterImportance,
+ BugSubscriptionFilterImportance.filter_id == (
+ BugSubscriptionFilter.id)),
]
conditions = [
+ # There's no filter or ...
Or(BugSubscriptionFilter.id == None,
- BugSubscriptionFilterStatus.status.is_in(
- bugtask.status for bugtask in bugtasks)),
+ # there is a filter ...
+ And(
+ # 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
+ # filter and it matches.
+ Or(BugSubscriptionFilterImportance.id == None,
+ BugSubscriptionFilterImportance.importance.is_in(
+ bugtask.importance for bugtask in bugtasks)))),
]
return Store.of(self.__helper.pillar).using(*origin).find(
=== modified file 'lib/lp/registry/tests/structural-subscription-target.txt'
--- lib/lp/registry/tests/structural-subscription-target.txt 2010-08-18 15:52:41 +0000
+++ lib/lp/registry/tests/structural-subscription-target.txt 2010-09-29 19:53:06 +0000
@@ -1,5 +1,5 @@
-= IStructuralSubscriptionTarget =
-
+IStructuralSubscriptionTarget
+-----------------------------
Let's subscribe ubuntu-team.
@@ -61,7 +61,8 @@
ubuntu-team COMMENTS NOTHING
-=== Structural subscriptions and indirect bug subscriptions ===
+Structural subscriptions and indirect bug subscriptions
+=======================================================
>>> bug = filebug(target, 'test bug one')
>>> indirect_subscribers = set(
=== modified file 'lib/lp/registry/tests/test_structuralsubscriptiontarget.py'
--- lib/lp/registry/tests/test_structuralsubscriptiontarget.py 2010-09-29 19:53:01 +0000
+++ lib/lp/registry/tests/test_structuralsubscriptiontarget.py 2010-09-29 19:53:06 +0000
@@ -28,8 +28,14 @@
LaunchpadZopelessLayer,
)
from lp.bugs.interfaces.bug import CreateBugParams
-from lp.bugs.interfaces.bugtask import BugTaskStatus
+from lp.bugs.interfaces.bugtask import (
+ BugTaskImportance,
+ BugTaskStatus,
+ )
from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
+from lp.bugs.model.bugsubscriptionfilterimportance import (
+ BugSubscriptionFilterImportance,
+ )
from lp.bugs.model.bugsubscriptionfilterstatus import (
BugSubscriptionFilterStatus,
)
@@ -215,6 +221,80 @@
bugtask.bug, BugNotificationLevel.NOTHING)
self.assertEqual([subscription], list(subscriptions_for_bug))
+ def test_getSubscriptionsForBug_with_filter_on_importance(self):
+ # If an importance filter exists for a subscription, the result of
+ # getSubscriptionsForBug() may be a subset of getSubscriptions().
+ 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
+
+ # Without any filters the subscription is found.
+ subscriptions_for_bug = self.target.getSubscriptionsForBug(
+ bugtask.bug, BugNotificationLevel.NOTHING)
+ self.assertEqual([subscription], list(subscriptions_for_bug))
+
+ # Filter the subscription to bugs in the CRITICAL state.
+ subscription_filter = BugSubscriptionFilter()
+ subscription_filter.structural_subscription = subscription
+ subscription_filter_importance = BugSubscriptionFilterImportance()
+ subscription_filter_importance.filter = subscription_filter
+ subscription_filter_importance.importance = BugTaskImportance.CRITICAL
+
+ # With the filter the subscription is not found.
+ subscriptions_for_bug = self.target.getSubscriptionsForBug(
+ bugtask.bug, BugNotificationLevel.NOTHING)
+ self.assertEqual([], list(subscriptions_for_bug))
+
+ # If the filter is adjusted, the subscription is found again.
+ subscription_filter_importance.importance = bugtask.importance
+ subscriptions_for_bug = self.target.getSubscriptionsForBug(
+ bugtask.bug, BugNotificationLevel.NOTHING)
+ self.assertEqual([subscription], list(subscriptions_for_bug))
+
+ def test_getSubscriptionsForBug_with_multiple_filters(self):
+ # If multiple filters exist for a subscription, all filters must
+ # match.
+ 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
+
+ # Filter the subscription to bugs in the CRITICAL state.
+ subscription_filter = BugSubscriptionFilter()
+ subscription_filter.structural_subscription = subscription
+ subscription_filter_status = BugSubscriptionFilterStatus()
+ subscription_filter_status.filter = subscription_filter
+ subscription_filter_status.status = BugTaskStatus.CONFIRMED
+ subscription_filter_importance = BugSubscriptionFilterImportance()
+ subscription_filter_importance.filter = subscription_filter
+ subscription_filter_importance.importance = BugTaskImportance.CRITICAL
+
+ # With the filter the subscription is not found.
+ subscriptions_for_bug = self.target.getSubscriptionsForBug(
+ bugtask.bug, BugNotificationLevel.NOTHING)
+ self.assertEqual([], list(subscriptions_for_bug))
+
+ # If the filter is adjusted to match status but not importance, the
+ # subscription is still not found.
+ subscription_filter_status.status = bugtask.status
+ subscriptions_for_bug = self.target.getSubscriptionsForBug(
+ bugtask.bug, BugNotificationLevel.NOTHING)
+ self.assertEqual([], list(subscriptions_for_bug))
+
+ # If the filter is adjusted to also match importance, the subscription
+ # is found again.
+ subscription_filter_importance.importance = bugtask.importance
+ subscriptions_for_bug = self.target.getSubscriptionsForBug(
+ bugtask.bug, BugNotificationLevel.NOTHING)
+ self.assertEqual([subscription], list(subscriptions_for_bug))
+
class TestStructuralSubscriptionForDistro(
FilteredStructuralSubscriptionTestBase, TestCaseWithFactory):
Follow ups
