Merge lp:~gmb/launchpad/team-subscription-opt-out-apis into lp:launchpad/db-devel

Proposed by Graham Binns on 2011-04-01
Status: Merged
Merged at revision: 10392
Proposed branch: lp:~gmb/launchpad/team-subscription-opt-out-apis
Merge into: lp:launchpad/db-devel
Prerequisite: lp:~gmb/launchpad/team-subscription-opt-out
Diff against target: 285 lines (+193/-5)
4 files modified
lib/lp/bugs/configure.zcml (+3/-1)
lib/lp/bugs/interfaces/bugsubscriptionfilter.py (+41/-3)
lib/lp/bugs/model/bugsubscriptionfilter.py (+39/-0)
lib/lp/bugs/tests/test_structuralsubscription.py (+110/-1)
To merge this branch: bzr merge lp:~gmb/launchpad/team-subscription-opt-out-apis
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code 2011-04-01 Approve on 2011-04-01
Review via email: mp+55982@code.launchpad.net

Commit message

[r=bac][ui=none][bug=751173] APIs have been added to IBugSubscriptionFilter to allow for muting and unmuting of subscriptions. These have also been exposed in the devel API.

Description of the change

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.

To post a comment you must log in.
Brad Crittenden (bac) wrote :

Graham this branch looks great and is quite thorough. I like your idempotent tests.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/configure.zcml'
2--- lib/lp/bugs/configure.zcml 2011-04-04 12:34:00 +0000
3+++ lib/lp/bugs/configure.zcml 2011-04-04 12:34:01 +0000
4@@ -636,9 +636,11 @@
5 class=".model.bugsubscriptionfilter.BugSubscriptionFilter">
6 <allow
7 interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterAttributes"/>
8+ <allow
9+ interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterMethodsPublic"/>
10 <require
11 permission="launchpad.Edit"
12- interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterMethods"
13+ interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterMethodsProtected"
14 set_schema=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterAttributes" />
15 </class>
16
17
18=== modified file 'lib/lp/bugs/interfaces/bugsubscriptionfilter.py'
19--- lib/lp/bugs/interfaces/bugsubscriptionfilter.py 2011-04-04 12:34:00 +0000
20+++ lib/lp/bugs/interfaces/bugsubscriptionfilter.py 2011-04-04 12:34:01 +0000
21@@ -11,9 +11,15 @@
22
23
24 from lazr.restful.declarations import (
25+ call_with,
26 export_as_webservice_entry,
27 export_destructor_operation,
28+ export_read_operation,
29+ export_write_operation,
30 exported,
31+ operation_for_version,
32+ operation_parameters,
33+ REQUEST_USER,
34 )
35 from lazr.restful.fields import Reference
36 from zope.interface import Interface
37@@ -35,6 +41,7 @@
38 from lp.bugs.interfaces.structuralsubscription import (
39 IStructuralSubscription,
40 )
41+from lp.registry.interfaces.person import IPerson
42 from lp.services.fields import (
43 PersonChoice,
44 SearchTag,
45@@ -99,8 +106,36 @@
46 value_type=SearchTag()))
47
48
49-class IBugSubscriptionFilterMethods(Interface):
50- """Methods of `IBugSubscriptionFilter`."""
51+class IBugSubscriptionFilterMethodsPublic(Interface):
52+ """Methods on `IBugSubscriptionFilter` that can be called by anyone."""
53+
54+ @call_with(person=REQUEST_USER)
55+ @operation_parameters(
56+ person=Reference(IPerson, title=_('Person'), required=True))
57+ @export_read_operation()
58+ @operation_for_version('devel')
59+ def isMuteAllowed(person):
60+ """Return True if this filter can be muted for `person`."""
61+
62+ @call_with(person=REQUEST_USER)
63+ @operation_parameters(
64+ person=Reference(IPerson, title=_('Person'), required=True))
65+ @export_write_operation()
66+ @operation_for_version('devel')
67+ def mute(person):
68+ """Add a mute for `person` to this filter."""
69+
70+ @call_with(person=REQUEST_USER)
71+ @operation_parameters(
72+ person=Reference(IPerson, title=_('Person'), required=True))
73+ @export_write_operation()
74+ @operation_for_version('devel')
75+ def unmute(person):
76+ """Remove any mute for `person` to this filter."""
77+
78+
79+class IBugSubscriptionFilterMethodsProtected(Interface):
80+ """Methods of `IBugSubscriptionFilter` that require launchpad.Edit."""
81
82 @export_destructor_operation()
83 def delete():
84@@ -111,7 +146,8 @@
85
86
87 class IBugSubscriptionFilter(
88- IBugSubscriptionFilterAttributes, IBugSubscriptionFilterMethods):
89+ IBugSubscriptionFilterAttributes, IBugSubscriptionFilterMethodsProtected,
90+ IBugSubscriptionFilterMethodsPublic):
91 """A bug subscription filter."""
92 export_as_webservice_entry()
93
94@@ -119,6 +155,8 @@
95 class IBugSubscriptionFilterMute(Interface):
96 """A mute on an IBugSubscriptionFilter."""
97
98+ export_as_webservice_entry()
99+
100 person = PersonChoice(
101 title=_('Person'), required=True, vocabulary='ValidPersonOrTeam',
102 readonly=True, description=_("The person subscribed."))
103
104=== modified file 'lib/lp/bugs/model/bugsubscriptionfilter.py'
105--- lib/lp/bugs/model/bugsubscriptionfilter.py 2011-04-04 12:34:00 +0000
106+++ lib/lp/bugs/model/bugsubscriptionfilter.py 2011-04-04 12:34:01 +0000
107@@ -49,6 +49,10 @@
108 from lp.services.database.stormbase import StormBase
109
110
111+class MuteNotAllowed(Exception):
112+ """Raised when someone tries to mute a filter that can't be muted."""
113+
114+
115 class BugSubscriptionFilter(StormBase):
116 """A filter to specialize a *structural* subscription."""
117
118@@ -249,6 +253,41 @@
119 # subscription.
120 self.structural_subscription.delete()
121
122+ def isMuteAllowed(self, person):
123+ """See `IBugSubscriptionFilter`."""
124+ return (
125+ self.structural_subscription.subscriber.isTeam() and
126+ person.inTeam(self.structural_subscription.subscriber))
127+
128+ def mute(self, person):
129+ """See `IBugSubscriptionFilter`."""
130+ if not self.isMuteAllowed(person):
131+ raise MuteNotAllowed(
132+ "This subscription cannot be muted for %s" % person.name)
133+
134+ store = Store.of(self)
135+ existing_mutes = store.find(
136+ BugSubscriptionFilterMute,
137+ BugSubscriptionFilterMute.filter_id == self.id,
138+ BugSubscriptionFilterMute.person_id == person.id)
139+ if not existing_mutes.is_empty():
140+ return existing_mutes.one()
141+ else:
142+ mute = BugSubscriptionFilterMute()
143+ mute.person = person
144+ mute.filter = self.id
145+ store.add(mute)
146+ return mute
147+
148+ def unmute(self, person):
149+ """See `IBugSubscriptionFilter`."""
150+ store = Store.of(self)
151+ existing_mutes = store.find(
152+ BugSubscriptionFilterMute,
153+ BugSubscriptionFilterMute.filter_id == self.id,
154+ BugSubscriptionFilterMute.person_id == person.id)
155+ existing_mutes.remove()
156+
157
158 class BugSubscriptionFilterMute(StormBase):
159 """A filter to specialize a *structural* subscription."""
160
161=== modified file 'lib/lp/bugs/tests/test_structuralsubscription.py'
162--- lib/lp/bugs/tests/test_structuralsubscription.py 2011-03-21 18:23:31 +0000
163+++ lib/lp/bugs/tests/test_structuralsubscription.py 2011-04-04 12:34:01 +0000
164@@ -23,7 +23,11 @@
165 BugTaskStatus,
166 )
167 from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients
168-from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
169+from lp.bugs.model.bugsubscriptionfilter import (
170+ BugSubscriptionFilter,
171+ BugSubscriptionFilterMute,
172+ MuteNotAllowed,
173+ )
174 from lp.bugs.model.structuralsubscription import (
175 get_structural_subscriptions_for_bug,
176 get_structural_subscribers,
177@@ -680,3 +684,108 @@
178 [], list(
179 get_structural_subscribers(
180 bug, None, BugNotificationLevel.COMMENTS, None)))
181+
182+
183+class TestBugSubscriptionFilterMute(TestCaseWithFactory):
184+ """Tests for the BugSubscriptionFilterMute class."""
185+
186+ layer = DatabaseFunctionalLayer
187+
188+ def setUp(self):
189+ super(TestBugSubscriptionFilterMute, self).setUp()
190+ self.target = self.factory.makeProduct()
191+ self.team = self.factory.makeTeam()
192+ self.team_member = self.factory.makePerson()
193+ with person_logged_in(self.team.teamowner):
194+ self.team.addMember(self.team_member, self.team.teamowner)
195+ self.team_subscription = self.target.addBugSubscription(
196+ self.team, self.team.teamowner)
197+ self.filter = self.team_subscription.bug_filters.one()
198+
199+ def test_isMuteAllowed_returns_true_for_team_subscriptions(self):
200+ # BugSubscriptionFilter.isMuteAllowed() will return True for
201+ # subscriptions where the owner of the subscription is a team.
202+ self.assertTrue(self.filter.isMuteAllowed(self.team_member))
203+
204+ def test_isMuteAllowed_returns_false_for_non_team_subscriptions(self):
205+ # BugSubscriptionFilter.isMuteAllowed() will return False for
206+ # subscriptions where the owner of the subscription is not a team.
207+ person = self.factory.makePerson()
208+ with person_logged_in(person):
209+ non_team_subscription = self.target.addBugSubscription(
210+ person, person)
211+ filter = non_team_subscription.bug_filters.one()
212+ self.assertFalse(filter.isMuteAllowed(person))
213+
214+ def test_isMuteAllowed_returns_false_for_non_team_members(self):
215+ # BugSubscriptionFilter.isMuteAllowed() will return False if the
216+ # user passed to it is not a member of the subscribing team.
217+ non_team_person = self.factory.makePerson()
218+ self.assertFalse(self.filter.isMuteAllowed(non_team_person))
219+
220+ def test_mute_adds_mute(self):
221+ # BugSubscriptionFilter.mute() adds a mute for the filter.
222+ filter_id = self.filter.id
223+ person_id = self.team_member.id
224+ store = Store.of(self.filter)
225+ mutes = store.find(
226+ BugSubscriptionFilterMute,
227+ BugSubscriptionFilterMute.filter == filter_id,
228+ BugSubscriptionFilterMute.person == person_id)
229+ self.assertTrue(mutes.is_empty())
230+ self.filter.mute(self.team_member)
231+ store.flush()
232+
233+ def test_unmute_removes_mute(self):
234+ # BugSubscriptionFilter.unmute() removes any mute for a given
235+ # person on that filter.
236+ filter_id = self.filter.id
237+ person_id = self.team_member.id
238+ store = Store.of(self.filter)
239+ self.filter.mute(self.team_member)
240+ store.flush()
241+ mutes = store.find(
242+ BugSubscriptionFilterMute,
243+ BugSubscriptionFilterMute.filter == filter_id,
244+ BugSubscriptionFilterMute.person == person_id)
245+ self.assertFalse(mutes.is_empty())
246+ self.filter.unmute(self.team_member)
247+ store.flush()
248+ self.assertTrue(mutes.is_empty())
249+
250+ def test_mute_is_idempotent(self):
251+ # Muting works even if the user is already muted.
252+ store = Store.of(self.filter)
253+ mute = self.filter.mute(self.team_member)
254+ store.flush()
255+ second_mute = self.filter.mute(self.team_member)
256+ self.assertEqual(mute, second_mute)
257+
258+ def test_unmute_is_idempotent(self):
259+ # Unmuting works even if the user is not muted
260+ store = Store.of(self.filter)
261+ mutes = store.find(
262+ BugSubscriptionFilterMute,
263+ BugSubscriptionFilterMute.filter == self.filter.id,
264+ BugSubscriptionFilterMute.person == self.team_member.id)
265+ self.assertTrue(mutes.is_empty())
266+ self.filter.unmute(self.team_member)
267+ self.assertTrue(mutes.is_empty())
268+
269+ def test_mute_raises_error_for_non_team_subscriptions(self):
270+ # BugSubscriptionFilter.mute() will raise an error if called on
271+ # a non-team subscription.
272+ person = self.factory.makePerson()
273+ with person_logged_in(person):
274+ non_team_subscription = self.target.addBugSubscription(
275+ person, person)
276+ filter = non_team_subscription.bug_filters.one()
277+ self.assertFalse(filter.isMuteAllowed(person))
278+ self.assertRaises(MuteNotAllowed, filter.mute, person)
279+
280+ def test_mute_raises_error_for_non_team_members(self):
281+ # BugSubscriptionFilter.mute() will raise an error if called on
282+ # a subscription of which the calling person is not a member.
283+ non_team_person = self.factory.makePerson()
284+ self.assertFalse(self.filter.isMuteAllowed(non_team_person))
285+ self.assertRaises(MuteNotAllowed, self.filter.mute, non_team_person)

Subscribers

People subscribed via source and target branches

to status/vote changes: