launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01346
[Merge] lp:~allenap/launchpad/structural-subscriptions-with-filters-4 into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/structural-subscriptions-with-filters-4 into lp:launchpad with lp:~allenap/launchpad/structural-subscriptions-with-filters-3 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#650991 Add getSubscriptionsForBug to IStructuralSubscriptionTarget
https://bugs.launchpad.net/bugs/650991
This branch:
- Adds an IBugSubscriptionFilter interface,
- A security adapter for it,
- A bug_filters property and a newBugFilter() method to IStructuralSubscription,
- Uses the convenience API introduced in the prerequisite branch to simplify testing.
--
https://code.launchpad.net/~allenap/launchpad/structural-subscriptions-with-filters-4/+merge/37467
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/structural-subscriptions-with-filters-4 into lp:launchpad.
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2010-09-30 05:47:44 +0000
+++ lib/lp/bugs/configure.zcml 2010-10-04 13:42:53 +0000
@@ -609,6 +609,18 @@
permission="zope.Public"
set_schema="lp.bugs.interfaces.bugsubscription.IBugSubscription"/>
</class>
+
+ <!-- BugSubscriptionFilter -->
+ <class
+ class=".model.bugsubscriptionfilter.BugSubscriptionFilter">
+ <allow
+ interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterAttributes"/>
+ <require
+ permission="launchpad.Edit"
+ interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterMethods"
+ set_schema=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterAttributes" />
+ </class>
+
<facet
facet="bugs">
=== added file 'lib/lp/bugs/interfaces/bugsubscriptionfilter.py'
--- lib/lp/bugs/interfaces/bugsubscriptionfilter.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/interfaces/bugsubscriptionfilter.py 2010-10-04 13:42:53 +0000
@@ -0,0 +1,81 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Bug subscription filter interfaces."""
+
+__metaclass__ = type
+__all__ = [
+ "IBugSubscriptionFilter",
+ ]
+
+
+from lazr.restful.fields import Reference
+from zope.interface import Interface
+from zope.schema import (
+ Bool,
+ Choice,
+ FrozenSet,
+ Text,
+ )
+
+from canonical.launchpad import _
+from lp.bugs.interfaces.bugtask import (
+ BugTaskImportance,
+ BugTaskStatus,
+ )
+from lp.registry.interfaces.structuralsubscription import (
+ IStructuralSubscription,
+ )
+from lp.services.fields import SearchTag
+
+
+class IBugSubscriptionFilterAttributes(Interface):
+ """Attributes of `IBugSubscriptionFilter`."""
+
+ structural_subscription = Reference(
+ IStructuralSubscription,
+ title=_("Structural subscription"),
+ required=True, readonly=True)
+
+ find_all_tags = Bool(
+ title=_("All given tags must be found, or any."),
+ required=True, default=False)
+ include_any_tags = Bool(
+ title=_("Include any tags."),
+ required=True, default=False)
+ exclude_any_tags = Bool(
+ title=_("Exclude all tags."),
+ required=True, default=False)
+
+ description = Text(
+ title=_("Description of this filter."),
+ required=False)
+
+ statuses = FrozenSet(
+ title=_("The statuses to filter on."),
+ required=True, default=frozenset(),
+ value_type=Choice(
+ title=_('Status'), vocabulary=BugTaskStatus))
+
+ importances = FrozenSet(
+ title=_("The importances to filter on."),
+ required=True, default=frozenset(),
+ value_type=Choice(
+ title=_('Importance'), vocabulary=BugTaskImportance))
+
+ tags = FrozenSet(
+ title=_("The tags to filter on."),
+ required=True, default=frozenset(),
+ value_type=SearchTag())
+
+
+class IBugSubscriptionFilterMethods(Interface):
+ """Methods of `IBugSubscriptionFilter`."""
+
+ def delete():
+ """Delete this bug subscription filter."""
+
+
+class IBugSubscriptionFilter(
+ IBugSubscriptionFilterAttributes, IBugSubscriptionFilterMethods):
+ """A bug subscription filter."""
=== modified file 'lib/lp/bugs/model/bugsubscriptionfilter.py'
--- lib/lp/bugs/model/bugsubscriptionfilter.py 2010-10-04 13:42:50 +0000
+++ lib/lp/bugs/model/bugsubscriptionfilter.py 2010-10-04 13:42:53 +0000
@@ -13,11 +13,14 @@
Bool,
Int,
Reference,
+ Store,
Unicode,
)
+from zope.interface import implements
from canonical.launchpad import searchbuilder
from canonical.launchpad.interfaces.lpstorm import IStore
+from lp.bugs.interfaces.bugsubscriptionfilter import IBugSubscriptionFilter
from lp.bugs.model.bugsubscriptionfilterimportance import (
BugSubscriptionFilterImportance,
)
@@ -30,6 +33,8 @@
class BugSubscriptionFilter(Storm):
"""A filter to specialize a *structural* subscription."""
+ implements(IBugSubscriptionFilter)
+
__storm_table__ = "BugSubscriptionFilter"
id = Int(primary=True)
@@ -178,3 +183,8 @@
tags = property(
_get_tags, _set_tags, doc=(
"A frozenset of tags filtered on."))
+
+ def delete(self):
+ """See `IBugSubscriptionFilter`."""
+ self.importances = self.statuses = self.tags = ()
+ Store.of(self).remove(self)
=== modified file 'lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py'
--- lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py 2010-10-04 13:42:50 +0000
+++ lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py 2010-10-04 13:42:53 +0000
@@ -5,6 +5,10 @@
__metaclass__ = type
+from storm.store import Store
+from zope.security.interfaces import Unauthorized
+from zope.security.proxy import ProxyFactory
+
from canonical.launchpad import searchbuilder
from canonical.launchpad.interfaces.lpstorm import IStore
from canonical.testing import DatabaseFunctionalLayer
@@ -14,7 +18,9 @@
)
from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
from lp.testing import (
+ anonymous_logged_in,
login_person,
+ person_logged_in,
TestCaseWithFactory,
)
@@ -70,6 +76,24 @@
self.assertIs(None, bug_subscription_filter.other_parameters)
self.assertIs(None, bug_subscription_filter.description)
+ def test_delete(self):
+ """`BugSubscriptionFilter` objects can be deleted.
+
+ Child objects - like `BugSubscriptionFilterTags` - will also be
+ deleted.
+ """
+ bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter.structural_subscription = self.subscription
+ bug_subscription_filter.importances = [BugTaskImportance.LOW]
+ bug_subscription_filter.statuses = [BugTaskStatus.NEW]
+ bug_subscription_filter.tags = [u"foo"]
+ IStore(bug_subscription_filter).flush()
+ self.assertIsNot(None, Store.of(bug_subscription_filter))
+ # Delete.
+ bug_subscription_filter.delete()
+ IStore(bug_subscription_filter).flush()
+ self.assertIs(None, Store.of(bug_subscription_filter))
+
def test_statuses(self):
# The statuses property is a frozenset of the statuses that are
# filtered upon.
@@ -196,3 +220,83 @@
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)
+
+
+class TestBugSubscriptionFilterPermissions(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestBugSubscriptionFilterPermissions, self).setUp()
+ self.target = self.factory.makeProduct()
+ self.subscriber = self.target.owner
+ with person_logged_in(self.subscriber):
+ self.subscription = self.target.addBugSubscription(
+ self.subscriber, self.subscriber)
+
+ def test_read_to_all(self):
+ """`BugSubscriptionFilter`s can be read by anyone."""
+ bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter.structural_subscription = self.subscription
+ bug_subscription_filter = ProxyFactory(bug_subscription_filter)
+ with person_logged_in(self.subscriber):
+ bug_subscription_filter.find_all_tags
+ with person_logged_in(self.factory.makePerson()):
+ bug_subscription_filter.find_all_tags
+ with anonymous_logged_in():
+ bug_subscription_filter.find_all_tags
+
+ def test_write_to_subscribers(self):
+ """`BugSubscriptionFilter`s can only be modifed by subscribers."""
+ bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter.structural_subscription = self.subscription
+ bug_subscription_filter = ProxyFactory(bug_subscription_filter)
+ # The subscriber can edit the filter.
+ with person_logged_in(self.subscriber):
+ bug_subscription_filter.find_all_tags = True
+ # Any other person is denied rights to edit the filter.
+ with person_logged_in(self.factory.makePerson()):
+ self.assertRaises(
+ Unauthorized, setattr, bug_subscription_filter,
+ "find_all_tags", True)
+ # Anonymous users are also denied.
+ with anonymous_logged_in():
+ self.assertRaises(
+ Unauthorized, setattr, bug_subscription_filter,
+ "find_all_tags", True)
+
+ def test_delete_by_subscribers(self):
+ """`BugSubscriptionFilter`s can only be deleted by subscribers."""
+ bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter.structural_subscription = self.subscription
+ bug_subscription_filter = ProxyFactory(bug_subscription_filter)
+ # Anonymous users are denied rights to delete the filter.
+ with anonymous_logged_in():
+ self.assertRaises(
+ Unauthorized, getattr, bug_subscription_filter, "delete")
+ # Any other person is also denied.
+ with person_logged_in(self.factory.makePerson()):
+ self.assertRaises(
+ Unauthorized, getattr, bug_subscription_filter, "delete")
+ # The subscriber can delete the filter.
+ with person_logged_in(self.subscriber):
+ bug_subscription_filter.delete()
+
+ def test_write_to_any_user_when_no_subscription(self):
+ """
+ `BugSubscriptionFilter`s can be modifed by any logged-in user when
+ there is no related subscription.
+ """
+ bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = ProxyFactory(bug_subscription_filter)
+ # The subscriber can edit the filter.
+ with person_logged_in(self.subscriber):
+ bug_subscription_filter.find_all_tags = True
+ # Any other person can edit the filter.
+ with person_logged_in(self.factory.makePerson()):
+ bug_subscription_filter.find_all_tags = True
+ # Anonymous users are denied rights to edit the filter.
+ with anonymous_logged_in():
+ self.assertRaises(
+ Unauthorized, setattr, bug_subscription_filter,
+ "find_all_tags", True)
=== modified file 'lib/lp/bugs/security.py'
--- lib/lp/bugs/security.py 2010-09-10 16:21:21 +0000
+++ lib/lp/bugs/security.py 2010-10-04 13:42:53 +0000
@@ -17,6 +17,7 @@
from lp.bugs.interfaces.bugbranch import IBugBranch
from lp.bugs.interfaces.bugnomination import IBugNomination
from lp.bugs.interfaces.bugsubscription import IBugSubscription
+from lp.bugs.interfaces.bugsubscriptionfilter import IBugSubscriptionFilter
from lp.bugs.interfaces.bugtracker import IBugTracker
from lp.bugs.interfaces.bugwatch import IBugWatch
@@ -171,3 +172,14 @@
return (
user.in_admin or
user.in_launchpad_developers)
+
+
+class EditBugSubscriptionFilter(AuthorizationBase):
+ """Bug subscription filters may only be modified by the subscriber."""
+ permission = 'launchpad.Edit'
+ usedfor = IBugSubscriptionFilter
+
+ def checkAuthenticated(self, user):
+ return (
+ self.obj.structural_subscription is None or
+ user.inTeam(self.obj.structural_subscription.subscriber))
=== modified file 'lib/lp/registry/interfaces/structuralsubscription.py'
--- lib/lp/registry/interfaces/structuralsubscription.py 2010-10-04 13:42:50 +0000
+++ lib/lp/registry/interfaces/structuralsubscription.py 2010-10-04 13:42:53 +0000
@@ -35,7 +35,10 @@
REQUEST_USER,
webservice_error,
)
-from lazr.restful.fields import Reference
+from lazr.restful.fields import (
+ CollectionField,
+ Reference,
+ )
from zope.interface import (
Attribute,
Interface,
@@ -129,6 +132,14 @@
required=True, readonly=True,
title=_("The structure to which this subscription belongs.")))
+ bug_filters = CollectionField(
+ title=_('List of bug filters that narrow this subscription.'),
+ readonly=True, required=False,
+ value_type=Reference(schema=Interface))
+
+ def newBugFilter():
+ """Returns a new `BugSubscriptionFilter` for this subscription."""
+
class IStructuralSubscriptionTargetRead(Interface):
"""A Launchpad Structure allowing users to subscribe to it.
=== modified file 'lib/lp/registry/model/structuralsubscription.py'
--- lib/lp/registry/model/structuralsubscription.py 2010-10-04 13:42:50 +0000
+++ lib/lp/registry/model/structuralsubscription.py 2010-10-04 13:42:53 +0000
@@ -26,6 +26,7 @@
SQLBase,
)
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from canonical.launchpad.interfaces.lpstorm import IStore
from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
from lp.bugs.model.bugsubscriptionfilterimportance import (
BugSubscriptionFilterImportance,
@@ -133,6 +134,19 @@
else:
raise AssertionError('StructuralSubscription has no target.')
+ @property
+ def bug_filters(self):
+ """See `IStructuralSubscription`."""
+ return IStore(BugSubscriptionFilter).find(
+ BugSubscriptionFilter,
+ BugSubscriptionFilter.structural_subscription == self)
+
+ def newBugFilter(self):
+ """See `IStructuralSubscription`."""
+ bug_filter = BugSubscriptionFilter()
+ bug_filter.structural_subscription = self
+ return bug_filter
+
class DistroSeriesTargetHelper:
"""A helper for `IDistroSeries`s."""
=== added file 'lib/lp/registry/tests/test_structuralsubscription.py'
--- lib/lp/registry/tests/test_structuralsubscription.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/tests/test_structuralsubscription.py 2010-10-04 13:42:53 +0000
@@ -0,0 +1,41 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for `StructuralSubscription`."""
+
+__metaclass__ = type
+
+from canonical.testing.layers import LaunchpadZopelessLayer
+from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
+from lp.testing import TestCaseWithFactory
+
+
+class TestStructuralSubscription(TestCaseWithFactory):
+
+ layer = LaunchpadZopelessLayer
+
+ def test_bug_filters_empty(self):
+ # StructuralSubscription.filters returns the BugSubscriptionFilter
+ # records associated with this subscription. It's empty to begin with.
+ product = self.factory.makeProduct()
+ subscription = product.addSubscription(product.owner, product.owner)
+ self.assertEqual([], list(subscription.bug_filters))
+
+ def test_bug_filters(self):
+ # StructuralSubscription.filters returns the BugSubscriptionFilter
+ # records associated with this subscription.
+ product = self.factory.makeProduct()
+ subscription = product.addSubscription(product.owner, product.owner)
+ subscription_filter = BugSubscriptionFilter()
+ subscription_filter.structural_subscription = subscription
+ self.assertEqual([subscription_filter], list(subscription.bug_filters))
+
+ def test_newBugFilter(self):
+ # Structural_Subscription.newBugFilter() creates a new subscription
+ # filter linked to the subscription.
+ product = self.factory.makeProduct()
+ subscription = product.addSubscription(product.owner, product.owner)
+ subscription_filter = subscription.newBugFilter()
+ self.assertEqual(
+ subscription, subscription_filter.structural_subscription)
+ self.assertEqual([subscription_filter], list(subscription.bug_filters))
=== modified file 'lib/lp/registry/tests/test_structuralsubscriptiontarget.py'
--- lib/lp/registry/tests/test_structuralsubscriptiontarget.py 2010-10-04 13:42:50 +0000
+++ lib/lp/registry/tests/test_structuralsubscriptiontarget.py 2010-10-04 13:42:53 +0000
@@ -33,12 +33,6 @@
BugTaskStatus,
)
from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
-from lp.bugs.model.bugsubscriptionfilterimportance import (
- BugSubscriptionFilterImportance,
- )
-from lp.bugs.model.bugsubscriptionfilterstatus import (
- BugSubscriptionFilterStatus,
- )
from lp.bugs.tests.test_bugtarget import bugtarget_filebug
from lp.registry.enum import BugNotificationLevel
from lp.registry.interfaces.distribution import IDistributionSet
@@ -206,9 +200,7 @@
# Filter the subscription to bugs in the CONFIRMED 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.statuses = [BugTaskStatus.CONFIRMED]
# With the filter the subscription is not found.
subscriptions_for_bug = self.target.getSubscriptionsForBug(
@@ -216,7 +208,7 @@
self.assertEqual([], list(subscriptions_for_bug))
# If the filter is adjusted, the subscription is found again.
- subscription_filter_status.status = bugtask.status
+ subscription_filter.statuses = [bugtask.status]
subscriptions_for_bug = self.target.getSubscriptionsForBug(
bugtask.bug, BugNotificationLevel.NOTHING)
self.assertEqual([subscription], list(subscriptions_for_bug))
@@ -240,9 +232,7 @@
# 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
+ subscription_filter.importances = [BugTaskImportance.CRITICAL]
# With the filter the subscription is not found.
subscriptions_for_bug = self.target.getSubscriptionsForBug(
@@ -250,7 +240,7 @@
self.assertEqual([], list(subscriptions_for_bug))
# If the filter is adjusted, the subscription is found again.
- subscription_filter_importance.importance = bugtask.importance
+ subscription_filter.importances = [bugtask.importance]
subscriptions_for_bug = self.target.getSubscriptionsForBug(
bugtask.bug, BugNotificationLevel.NOTHING)
self.assertEqual([subscription], list(subscriptions_for_bug))
@@ -269,12 +259,8 @@
# 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
+ subscription_filter.statuses = [BugTaskStatus.CONFIRMED]
+ subscription_filter.importances = [BugTaskImportance.CRITICAL]
# With the filter the subscription is not found.
subscriptions_for_bug = self.target.getSubscriptionsForBug(
@@ -283,14 +269,14 @@
# If the filter is adjusted to match status but not importance, the
# subscription is still not found.
- subscription_filter_status.status = bugtask.status
+ subscription_filter.statuses = [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
+ subscription_filter.importances = [bugtask.importance]
subscriptions_for_bug = self.target.getSubscriptionsForBug(
bugtask.bug, BugNotificationLevel.NOTHING)
self.assertEqual([subscription], list(subscriptions_for_bug))
