launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03176
[Merge] lp:~gmb/launchpad/team-subscription-opt-out-apis into lp:launchpad/db-devel
Graham Binns has proposed merging lp:~gmb/launchpad/team-subscription-opt-out-apis into lp:launchpad/db-devel with lp:~gmb/launchpad/team-subscription-opt-out as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~gmb/launchpad/team-subscription-opt-out-apis/+merge/55982
This branch adds APIs around the BugSubscriptionFilterMute model. The APIs added are:
- BugSubscriptionFilter.isMuteAllowed(person):
Returns True if `person` can add a mute for the current filter.
- BugSubscriptionFilter.mute(person):
Add a mute for `person` and return it. If called for a user who is already
muted, return the existing mute. Raise an error if isMuteAllowed() would
return False for `person`.
- BugSubscriptionFilter.unmute(person):
Remove any mutes for `person` on the current Filter.
I've added tests to cover these APIs.
--
https://code.launchpad.net/~gmb/launchpad/team-subscription-opt-out-apis/+merge/55982
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/team-subscription-opt-out-apis into lp:launchpad/db-devel.
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2011-04-01 17:37:24 +0000
+++ lib/lp/bugs/configure.zcml 2011-04-01 17:37:29 +0000
@@ -636,9 +636,11 @@
class=".model.bugsubscriptionfilter.BugSubscriptionFilter">
<allow
interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterAttributes"/>
+ <allow
+ interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterMethodsPublic"/>
<require
permission="launchpad.Edit"
- interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterMethods"
+ interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterMethodsProtected"
set_schema=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterAttributes" />
</class>
=== modified file 'lib/lp/bugs/interfaces/bugsubscriptionfilter.py'
--- lib/lp/bugs/interfaces/bugsubscriptionfilter.py 2011-04-01 17:37:24 +0000
+++ lib/lp/bugs/interfaces/bugsubscriptionfilter.py 2011-04-01 17:37:29 +0000
@@ -11,9 +11,15 @@
from lazr.restful.declarations import (
+ call_with,
export_as_webservice_entry,
export_destructor_operation,
+ export_read_operation,
+ export_write_operation,
exported,
+ operation_for_version,
+ operation_parameters,
+ REQUEST_USER,
)
from lazr.restful.fields import Reference
from zope.interface import Interface
@@ -35,6 +41,7 @@
from lp.bugs.interfaces.structuralsubscription import (
IStructuralSubscription,
)
+from lp.registry.interfaces.person import IPerson
from lp.services.fields import (
PersonChoice,
SearchTag,
@@ -99,8 +106,36 @@
value_type=SearchTag()))
-class IBugSubscriptionFilterMethods(Interface):
- """Methods of `IBugSubscriptionFilter`."""
+class IBugSubscriptionFilterMethodsPublic(Interface):
+ """Methods on `IBugSubscriptionFilter` that can be called by anyone."""
+
+ @call_with(person=REQUEST_USER)
+ @operation_parameters(
+ person=Reference(IPerson, title=_('Person'), required=True))
+ @export_read_operation()
+ @operation_for_version('devel')
+ def isMuteAllowed(person):
+ """Return True if this filter can be muted for `person`."""
+
+ @call_with(person=REQUEST_USER)
+ @operation_parameters(
+ person=Reference(IPerson, title=_('Person'), required=True))
+ @export_write_operation()
+ @operation_for_version('devel')
+ def mute(person):
+ """Add a mute for `person` to this filter."""
+
+ @call_with(person=REQUEST_USER)
+ @operation_parameters(
+ person=Reference(IPerson, title=_('Person'), required=True))
+ @export_write_operation()
+ @operation_for_version('devel')
+ def unmute(person):
+ """Remove any mute for `person` to this filter."""
+
+
+class IBugSubscriptionFilterMethodsProtected(Interface):
+ """Methods of `IBugSubscriptionFilter` that require launchpad.Edit."""
@export_destructor_operation()
def delete():
@@ -111,7 +146,8 @@
class IBugSubscriptionFilter(
- IBugSubscriptionFilterAttributes, IBugSubscriptionFilterMethods):
+ IBugSubscriptionFilterAttributes, IBugSubscriptionFilterMethodsProtected,
+ IBugSubscriptionFilterMethodsPublic):
"""A bug subscription filter."""
export_as_webservice_entry()
@@ -119,6 +155,8 @@
class IBugSubscriptionFilterMute(Interface):
"""A mute on an IBugSubscriptionFilter."""
+ export_as_webservice_entry()
+
person = PersonChoice(
title=_('Person'), required=True, vocabulary='ValidPersonOrTeam',
readonly=True, description=_("The person subscribed."))
=== modified file 'lib/lp/bugs/model/bugsubscriptionfilter.py'
--- lib/lp/bugs/model/bugsubscriptionfilter.py 2011-04-01 17:37:24 +0000
+++ lib/lp/bugs/model/bugsubscriptionfilter.py 2011-04-01 17:37:29 +0000
@@ -50,6 +50,10 @@
from lp.services.database.stormbase import StormBase
+class MuteNotAllowed(Exception):
+ """Raised when someone tries to mute a filter that can't be muted."""
+
+
class BugSubscriptionFilter(StormBase):
"""A filter to specialize a *structural* subscription."""
@@ -265,6 +269,41 @@
# subscription.
self.structural_subscription.delete()
+ def isMuteAllowed(self, person):
+ """See `IBugSubscriptionFilter`."""
+ return (
+ self.structural_subscription.subscriber.isTeam() and
+ person.inTeam(self.structural_subscription.subscriber))
+
+ def mute(self, person):
+ """See `IBugSubscriptionFilter`."""
+ if not self.isMuteAllowed(person):
+ raise MuteNotAllowed(
+ "This subscription cannot be muted for %s" % person.name)
+
+ store = Store.of(self)
+ existing_mutes = store.find(
+ BugSubscriptionFilterMute,
+ BugSubscriptionFilterMute.filter_id == self.id,
+ BugSubscriptionFilterMute.person_id == person.id)
+ if not existing_mutes.is_empty():
+ return existing_mutes.one()
+ else:
+ mute = BugSubscriptionFilterMute()
+ mute.person = person
+ mute.filter = self.id
+ store.add(mute)
+ return mute
+
+ def unmute(self, person):
+ """See `IBugSubscriptionFilter`."""
+ store = Store.of(self)
+ existing_mutes = store.find(
+ BugSubscriptionFilterMute,
+ BugSubscriptionFilterMute.filter_id == self.id,
+ BugSubscriptionFilterMute.person_id == person.id)
+ existing_mutes.remove()
+
class BugSubscriptionFilterMute(StormBase):
"""A filter to specialize a *structural* subscription."""
=== modified file 'lib/lp/bugs/tests/test_structuralsubscription.py'
--- lib/lp/bugs/tests/test_structuralsubscription.py 2011-03-21 18:23:31 +0000
+++ lib/lp/bugs/tests/test_structuralsubscription.py 2011-04-01 17:37:29 +0000
@@ -23,7 +23,11 @@
BugTaskStatus,
)
from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients
-from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
+from lp.bugs.model.bugsubscriptionfilter import (
+ BugSubscriptionFilter,
+ BugSubscriptionFilterMute,
+ MuteNotAllowed,
+ )
from lp.bugs.model.structuralsubscription import (
get_structural_subscriptions_for_bug,
get_structural_subscribers,
@@ -680,3 +684,108 @@
[], list(
get_structural_subscribers(
bug, None, BugNotificationLevel.COMMENTS, None)))
+
+
+class TestBugSubscriptionFilterMute(TestCaseWithFactory):
+ """Tests for the BugSubscriptionFilterMute class."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestBugSubscriptionFilterMute, self).setUp()
+ self.target = self.factory.makeProduct()
+ self.team = self.factory.makeTeam()
+ self.team_member = self.factory.makePerson()
+ with person_logged_in(self.team.teamowner):
+ self.team.addMember(self.team_member, self.team.teamowner)
+ self.team_subscription = self.target.addBugSubscription(
+ self.team, self.team.teamowner)
+ self.filter = self.team_subscription.bug_filters.one()
+
+ def test_isMuteAllowed_returns_true_for_team_subscriptions(self):
+ # BugSubscriptionFilter.isMuteAllowed() will return True for
+ # subscriptions where the owner of the subscription is a team.
+ self.assertTrue(self.filter.isMuteAllowed(self.team_member))
+
+ def test_isMuteAllowed_returns_false_for_non_team_subscriptions(self):
+ # BugSubscriptionFilter.isMuteAllowed() will return False for
+ # subscriptions where the owner of the subscription is not a team.
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ non_team_subscription = self.target.addBugSubscription(
+ person, person)
+ filter = non_team_subscription.bug_filters.one()
+ self.assertFalse(filter.isMuteAllowed(person))
+
+ def test_isMuteAllowed_returns_false_for_non_team_members(self):
+ # BugSubscriptionFilter.isMuteAllowed() will return False if the
+ # user passed to it is not a member of the subscribing team.
+ non_team_person = self.factory.makePerson()
+ self.assertFalse(self.filter.isMuteAllowed(non_team_person))
+
+ def test_mute_adds_mute(self):
+ # BugSubscriptionFilter.mute() adds a mute for the filter.
+ filter_id = self.filter.id
+ person_id = self.team_member.id
+ store = Store.of(self.filter)
+ mutes = store.find(
+ BugSubscriptionFilterMute,
+ BugSubscriptionFilterMute.filter == filter_id,
+ BugSubscriptionFilterMute.person == person_id)
+ self.assertTrue(mutes.is_empty())
+ self.filter.mute(self.team_member)
+ store.flush()
+
+ def test_unmute_removes_mute(self):
+ # BugSubscriptionFilter.unmute() removes any mute for a given
+ # person on that filter.
+ filter_id = self.filter.id
+ person_id = self.team_member.id
+ store = Store.of(self.filter)
+ self.filter.mute(self.team_member)
+ store.flush()
+ mutes = store.find(
+ BugSubscriptionFilterMute,
+ BugSubscriptionFilterMute.filter == filter_id,
+ BugSubscriptionFilterMute.person == person_id)
+ self.assertFalse(mutes.is_empty())
+ self.filter.unmute(self.team_member)
+ store.flush()
+ self.assertTrue(mutes.is_empty())
+
+ def test_mute_is_idempotent(self):
+ # Muting works even if the user is already muted.
+ store = Store.of(self.filter)
+ mute = self.filter.mute(self.team_member)
+ store.flush()
+ second_mute = self.filter.mute(self.team_member)
+ self.assertEqual(mute, second_mute)
+
+ def test_unmute_is_idempotent(self):
+ # Unmuting works even if the user is not muted
+ store = Store.of(self.filter)
+ mutes = store.find(
+ BugSubscriptionFilterMute,
+ BugSubscriptionFilterMute.filter == self.filter.id,
+ BugSubscriptionFilterMute.person == self.team_member.id)
+ self.assertTrue(mutes.is_empty())
+ self.filter.unmute(self.team_member)
+ self.assertTrue(mutes.is_empty())
+
+ def test_mute_raises_error_for_non_team_subscriptions(self):
+ # BugSubscriptionFilter.mute() will raise an error if called on
+ # a non-team subscription.
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ non_team_subscription = self.target.addBugSubscription(
+ person, person)
+ filter = non_team_subscription.bug_filters.one()
+ self.assertFalse(filter.isMuteAllowed(person))
+ self.assertRaises(MuteNotAllowed, filter.mute, person)
+
+ def test_mute_raises_error_for_non_team_members(self):
+ # BugSubscriptionFilter.mute() will raise an error if called on
+ # a subscription of which the calling person is not a member.
+ non_team_person = self.factory.makePerson()
+ self.assertFalse(self.filter.isMuteAllowed(non_team_person))
+ self.assertRaises(MuteNotAllowed, self.filter.mute, non_team_person)
