Merge lp:~danilo/launchpad/ss-storm-rewrite-methods into lp:launchpad

Proposed by Данило Шеган on 2011-01-19
Status: Merged
Approved by: Graham Binns on 2011-01-19
Approved revision: no longer in the source branch.
Merged at revision: 12254
Proposed branch: lp:~danilo/launchpad/ss-storm-rewrite-methods
Merge into: lp:launchpad
Prerequisite: lp:~danilo/launchpad/ss-storm-rewrite
Diff against target: 569 lines (+102/-92)
12 files modified
lib/lp/bugs/doc/bugsubscription.txt (+33/-28)
lib/lp/bugs/doc/bugtask-search.txt (+9/-9)
lib/lp/bugs/doc/initial-bug-contacts.txt (+7/-7)
lib/lp/bugs/stories/patches-view/patches-view.txt (+1/-1)
lib/lp/bugs/tests/bugs-emailinterface.txt (+1/-1)
lib/lp/registry/browser/tests/milestone-views.txt (+1/-1)
lib/lp/registry/doc/milestone.txt (+2/-2)
lib/lp/registry/doc/person.txt (+2/-2)
lib/lp/registry/model/person.py (+4/-2)
lib/lp/registry/model/structuralsubscription.py (+38/-35)
lib/lp/registry/templates/person-structural-subscriptions.pt (+2/-2)
lib/lp/registry/tests/structural-subscription-target.txt (+2/-2)
To merge this branch: bzr merge lp:~danilo/launchpad/ss-storm-rewrite-methods
Reviewer Review Type Date Requested Status
Graham Binns (community) code 2011-01-19 Approve on 2011-01-19
Review via email: mp+46843@code.launchpad.net

Commit message

[r=gmb][ui=none][no-qa] Convert StructuralSubscription to native Storm class.

Description of the change

= Convert StructuralSubscription to Storm =

This branch completes the migration of StructuralSubscription to Storm. The first step in lp:~danilo/launchpad/bugs-subscriptions was to replace column definitions with Storm versions.

This branch continues on that and moves all .select() and .selectBy() calls on StructuralSubscription to .find calls, uses store.remove() instead of object.destroySelf(), provides a constructor similar to one provided by SQLBase and then finally switches the class to inherit from Storm instead of SQLBase.

Followed https://dev.launchpad.net/StormMigrationGuide (constructor switch was not documented there, so I'll be adding it shortly).

== Implementation details ==

There are not enough unit tests for the functionality I am changing, but the full test run over all the layers of implementation (doc tests, unit tests, API tests and browser tests) did catch a few bugs (or incompatible changes that I did). While I'd love to add new tests for all the methods in StructuralSubscriptionTargetMixin, with the existing test coverage and our plan (where we'd allow more than one StructuralSubscription per target and person), most of the code I'd write tests for would go away.

It also includes slight optimizations in two places where we go over all the subscriptions to find one for a particular subscriber. It now executes a query which returns just one result instead.

To achieve this, subscriber parameter is added to getSubscriptions(), but it's not exposed in the interface because we are only using it from inside the module. That looks dirty even to me, so I'd be happy to change it if a reviewer thinks I should do it.

I am ignoring doctest lint errors since it might make diff harder to read. I'd be happy to update it after the review is done.

== Tests ==

bin/test -cvvt bugsubscription.txt -t bugnotification-sending.txt -t structural-subscriptions.txt -t test_structuralsubscription

== Demo and Q/A ==

no-qa

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/doc/bugsubscription.txt
  lib/lp/registry/model/structuralsubscription.py
  lib/lp/registry/model/person.py

./lib/lp/bugs/doc/bugsubscription.txt
       1: narrative uses a moin header.
       6: narrative uses a moin header.
      25: narrative uses a moin header.
     438: source has bad indentation.
     442: source has bad indentation.
     459: source has bad indentation.
     474: source has bad indentation.
     487: source has bad indentation.
     494: want exceeds 78 characters.
     528: narrative uses a moin header.
     589: narrative uses a moin header.
     644: narrative uses a moin header.
     852: narrative uses a moin header.

To post a comment you must log in.
Graham Binns (gmb) :
review: Approve (code)
Robert Collins (lifeless) wrote :

We can't use the Storm base class directly, we need a launchpad
specific one with CachedProperty __invalidate__ hooked in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/doc/bugsubscription.txt'
2--- lib/lp/bugs/doc/bugsubscription.txt 2011-01-18 22:16:03 +0000
3+++ lib/lp/bugs/doc/bugsubscription.txt 2011-01-20 17:50:31 +0000
4@@ -1,9 +1,11 @@
5-= BugSubscription =
6+BugSubscription
7+===============
8
9 Users can get email notifications of changes to bugs by subscribing to
10 them.
11
12-== Bug Subscriber APIs ==
13+Bug Subscriber APIs
14+-------------------
15
16 First, let's login:
17
18@@ -22,7 +24,8 @@
19 This list returns only *direct* subscribers. Bugs can also have
20 indirect subscribers.
21
22-=== Direct vs. Indirect Subscriptions ===
23+Direct vs. Indirect Subscriptions
24+.................................
25
26 A user is directly subscribed to a bug if they or someone else has
27 subscribed them to the bug.
28@@ -147,6 +150,7 @@
29 >>> subscription_no_priv = linux_source.addBugSubscription(
30 ... mr_no_privs, mr_no_privs)
31
32+ >>> transaction.commit()
33 >>> print_displayname(
34 ... sub.subscriber for sub in linux_source.bug_subscriptions)
35 No Privileges Person
36@@ -434,12 +438,12 @@
37 a bug, and why, IBug.getBugNotificationRecipients() assembles an
38 INotificationRecipientSet instance for us:
39
40- >>> recipients = linux_source_bug.getBugNotificationRecipients()
41+ >>> recipients = linux_source_bug.getBugNotificationRecipients()
42
43 You can query for the addresses and reasons:
44
45- >>> addresses = recipients.getEmails()
46- >>> [(address, recipients.getReason(address)[1]) for address in addresses]
47+ >>> addresses = recipients.getEmails()
48+ >>> [(address, recipients.getReason(address)[1]) for address in addresses]
49 [('foo.bar@canonical.com', 'Subscriber'),
50 ('mark@example.com', 'Subscriber'),
51 ('no-priv@canonical.com', 'Subscriber'),
52@@ -455,10 +459,10 @@
53 getBugNotificationRecipients() even if the parameter level is larger than
54 his structural subscription setting.
55
56- >>> recipients = linux_source_bug.getBugNotificationRecipients(
57- ... level=BugNotificationLevel.COMMENTS)
58- >>> addresses = recipients.getEmails()
59- >>> [(address, recipients.getReason(address)[1]) for address in addresses]
60+ >>> recipients = linux_source_bug.getBugNotificationRecipients(
61+ ... level=BugNotificationLevel.COMMENTS)
62+ >>> addresses = recipients.getEmails()
63+ >>> [(address, recipients.getReason(address)[1]) for address in addresses]
64 [('foo.bar@canonical.com', 'Subscriber'),
65 ('mark@example.com', 'Subscriber'),
66 ('no-priv@canonical.com', 'Subscriber'),
67@@ -470,11 +474,11 @@
68 longer included in the result of getBugNotificationRecipients() for
69 the COMMENTS level...
70
71- >>> linux_source_bug.unsubscribe(mr_no_privs, mr_no_privs)
72- >>> recipients = linux_source_bug.getBugNotificationRecipients(
73- ... level=BugNotificationLevel.COMMENTS)
74- >>> addresses = recipients.getEmails()
75- >>> [(address, recipients.getReason(address)[1]) for address in addresses]
76+ >>> linux_source_bug.unsubscribe(mr_no_privs, mr_no_privs)
77+ >>> recipients = linux_source_bug.getBugNotificationRecipients(
78+ ... level=BugNotificationLevel.COMMENTS)
79+ >>> addresses = recipients.getEmails()
80+ >>> [(address, recipients.getReason(address)[1]) for address in addresses]
81 [('foo.bar@canonical.com', 'Subscriber'),
82 ('mark@example.com', 'Subscriber'),
83 ('robertc@robertcollins.net', 'Subscriber'),
84@@ -483,11 +487,11 @@
85
86 ...but remains included for the level LIFECYCLE.
87
88- >>> linux_source_bug.unsubscribe(mr_no_privs, mr_no_privs)
89- >>> recipients = linux_source_bug.getBugNotificationRecipients(
90- ... level=BugNotificationLevel.LIFECYCLE)
91- >>> addresses = recipients.getEmails()
92- >>> [(address, recipients.getReason(address)[1]) for address in addresses]
93+ >>> linux_source_bug.unsubscribe(mr_no_privs, mr_no_privs)
94+ >>> recipients = linux_source_bug.getBugNotificationRecipients(
95+ ... level=BugNotificationLevel.LIFECYCLE)
96+ >>> addresses = recipients.getEmails()
97+ >>> [(address, recipients.getReason(address)[1]) for address in addresses]
98 [('foo.bar@canonical.com', 'Subscriber'),
99 ('mark@example.com', 'Subscriber'),
100 ('no-priv@canonical.com', u'Subscriber (linux-source-2.6.15 in ubuntu)'),
101@@ -524,7 +528,8 @@
102 >>> bug_five.isSubscribedToDupes(sample_person)
103 True
104
105-== Subscribing and Unsubscribing ==
106+Subscribing and Unsubscribing
107+-----------------------------
108
109 To subscribe people to and unsubscribe people from a bug, use
110 IBug.subscribe and IBug.unsubscribe:
111@@ -585,7 +590,8 @@
112 False
113
114
115-=== Determining whether a user can unsubscribe someone ===
116+Determining whether a user can unsubscribe someone
117+..................................................
118
119 As user can't unsubscribe just anyone from a bug. To check whether
120 someone can be unusubscribed, the canBeUnsubscribedByUser() method on
121@@ -640,14 +646,12 @@
122 UserCannotUnsubscribePerson: ...
123
124
125-== Automatic Subscriptions on Bug Creation ==
126+Automatic Subscriptions on Bug Creation
127+---------------------------------------
128
129 When a new bug is opened, only the bug reporter is automatically, explicitly
130 subscribed to the bug:
131
132-XXX: Brad Bollenbach, 2005-11-25: These bits need real sample data. See
133-https://launchpad.net/bugs/5484.
134-
135 Define a function that get subscriber email addresses back conveniently:
136
137 >>> def getSubscribers(bug):
138@@ -804,7 +808,7 @@
139 So, if the Ubuntu team is added as a bug contact to evolution:
140
141 >>> evolution.addBugSubscription(ubuntu_team, ubuntu_team)
142- <StructuralSubscription at ...>
143+ <...StructuralSubscription object at ...>
144
145 The team will be implicitly subscribed to the previous bug we
146 created. (Remember that Sample Person is also implicitly subscribed
147@@ -848,7 +852,8 @@
148 ['foo.bar@canonical.com', 'support@ubuntu.com']
149
150
151-== Subscribed by ==
152+Subscribed by
153+-------------
154
155 Each `BugSubscription` records who created it, and provides a handy
156 utility method for formatting this information. The methods
157
158=== modified file 'lib/lp/bugs/doc/bugtask-search.txt'
159--- lib/lp/bugs/doc/bugtask-search.txt 2010-10-25 15:03:38 +0000
160+++ lib/lp/bugs/doc/bugtask-search.txt 2011-01-20 17:50:31 +0000
161@@ -1230,7 +1230,7 @@
162 ... name='product-struct-subber')
163 >>> firefox.addBugSubscription(product_struct_subber,
164 ... product_struct_subber)
165- <StructuralSubscription at ...>
166+ <...StructuralSubscription object at ...>
167 >>> product_struct_sub_search = BugTaskSearchParams(
168 ... user=None, structural_subscriber=product_struct_subber)
169 >>> found_bugtasks = bugtask_set.search(product_struct_sub_search)
170@@ -1248,7 +1248,7 @@
171 ... name='series-struct-subber')
172 >>> product_series.addBugSubscription(series_struct_subber,
173 ... series_struct_subber)
174- <StructuralSubscription at ...>
175+ <...StructuralSubscription object at ...>
176 >>> series_struct_sub_search = BugTaskSearchParams(
177 ... user=None, structural_subscriber=series_struct_subber)
178 >>> found_bugtasks = bugtask_set.search(series_struct_sub_search)
179@@ -1269,7 +1269,7 @@
180 ... name='project-struct-subber')
181 >>> project.addBugSubscription(project_struct_subber,
182 ... project_struct_subber)
183- <StructuralSubscription at ...>
184+ <...StructuralSubscription object at ...>
185 >>> project_struct_sub_search = BugTaskSearchParams(
186 ... user=None, structural_subscriber=project_struct_subber)
187 >>> found_bugtasks = bugtask_set.search(project_struct_sub_search)
188@@ -1285,7 +1285,7 @@
189 >>> product2.project = project
190 >>> product2.addBugSubscription(project_struct_subber,
191 ... project_struct_subber)
192- <StructuralSubscription at ...>
193+ <...StructuralSubscription object at ...>
194 >>> project_struct_sub_search = BugTaskSearchParams(
195 ... user=None, structural_subscriber=project_struct_subber)
196 >>> found_bugtasks = bugtask_set.search(project_struct_sub_search)
197@@ -1300,7 +1300,7 @@
198 ... name='milestone-struct-subber')
199 >>> product_milestone.addBugSubscription(milestone_struct_subber,
200 ... milestone_struct_subber)
201- <StructuralSubscription at ...>
202+ <...StructuralSubscription object at ...>
203 >>> milestone_struct_sub_search = BugTaskSearchParams(
204 ... user=None, structural_subscriber=milestone_struct_subber)
205 >>> found_bugtasks = bugtask_set.search(milestone_struct_sub_search)
206@@ -1315,7 +1315,7 @@
207 ... name='distro-struct-subber')
208 >>> ubuntu.addBugSubscription(distro_struct_subber,
209 ... distro_struct_subber)
210- <StructuralSubscription at ...>
211+ <...StructuralSubscription object at ...>
212 >>> distro_struct_sub_search = BugTaskSearchParams(
213 ... user=None, structural_subscriber=distro_struct_subber)
214 >>> found_bugtasks = bugtask_set.search(distro_struct_sub_search)
215@@ -1332,7 +1332,7 @@
216 ... name='distro-series-struct-subber')
217 >>> hoary.addBugSubscription(distro_series_struct_subber,
218 ... distro_series_struct_subber)
219- <StructuralSubscription at ...>
220+ <...StructuralSubscription object at ...>
221 >>> distro_series_struct_sub_search = BugTaskSearchParams(
222 ... user=None, structural_subscriber=distro_series_struct_subber)
223 >>> found_bugtasks = bugtask_set.search(distro_series_struct_sub_search)
224@@ -1349,7 +1349,7 @@
225 ... name='package-struct-subber')
226 >>> ubuntu_firefox.addBugSubscription(package_struct_subber,
227 ... package_struct_subber)
228- <StructuralSubscription at ...>
229+ <...StructuralSubscription object at ...>
230 >>> package_struct_sub_search = BugTaskSearchParams(
231 ... user=None, structural_subscriber=package_struct_subber)
232 >>> found_bugtasks = bugtask_set.search(package_struct_sub_search)
233@@ -1363,7 +1363,7 @@
234
235 >>> product_series.addBugSubscription(package_struct_subber,
236 ... package_struct_subber)
237- <StructuralSubscription at ...>
238+ <...StructuralSubscription object at ...>
239 >>> package_struct_sub_search = BugTaskSearchParams(
240 ... user=None, structural_subscriber=package_struct_subber)
241 >>> found_bugtasks = bugtask_set.search(package_struct_sub_search)
242
243=== modified file 'lib/lp/bugs/doc/initial-bug-contacts.txt'
244--- lib/lp/bugs/doc/initial-bug-contacts.txt 2010-12-24 11:02:19 +0000
245+++ lib/lp/bugs/doc/initial-bug-contacts.txt 2011-01-20 17:50:31 +0000
246@@ -38,7 +38,7 @@
247 >>> login("foo.bar@canonical.com")
248
249 >>> debian_firefox.addBugSubscription(sample_person, sample_person)
250- <StructuralSubscription ...>
251+ <...StructuralSubscription object at ...>
252 >>> [pbc.subscriber.name for pbc in debian_firefox.bug_subscriptions]
253 [u'name12']
254
255@@ -46,13 +46,13 @@
256 already subscribe to that package will return the existing subscription.
257
258 >>> debian_firefox.addBugSubscription(sample_person, sample_person)
259- <StructuralSubscription ...>
260+ <...StructuralSubscription object at ...>
261
262 Let's add an ITeam as one of the subscribers:
263
264 >>> ubuntu_team = personset.getByName("ubuntu-team")
265 >>> debian_firefox.addBugSubscription(ubuntu_team, ubuntu_team)
266- <StructuralSubscription ...>
267+ <...StructuralSubscription object at ...>
268
269 >>> sorted([sub.subscriber.name for sub in debian_firefox.bug_subscriptions])
270 [u'name12', u'ubuntu-team']
271@@ -125,9 +125,9 @@
272
273 >>> daf = personset.getByName("daf")
274 >>> ubuntu_pmount.addBugSubscription(daf, daf)
275- <StructuralSubscription ...>
276+ <...StructuralSubscription object at ...>
277 >>> ubuntu_pmount.addBugSubscription(sample_person, sample_person)
278- <StructuralSubscription ...>
279+ <...StructuralSubscription object at ...>
280
281 >>> old_state = Snapshot(
282 ... bug_one_in_ubuntu_firefox, providing=IDistroBugTask)
283@@ -247,7 +247,7 @@
284 >>> ubuntu_gnome.preferredemail is None
285 True
286 >>> ubuntu_pmount.addBugSubscription(ubuntu_gnome, ubuntu_gnome)
287- <StructuralSubscription ...>
288+ <...StructuralSubscription object at ...>
289
290 >>> old_state = Snapshot(
291 ... bug_one_in_ubuntu_firefox, providing=IDistroBugTask)
292@@ -279,7 +279,7 @@
293
294 >>> pitti = personset.getByName("martin-pitt")
295 >>> ubuntu_firefox.addBugSubscription(pitti, pitti)
296- <StructuralSubscription ...>
297+ <...StructuralSubscription object at ...>
298
299 and then the bug gets reassigned to mozilla firefox:
300
301
302=== modified file 'lib/lp/bugs/stories/patches-view/patches-view.txt'
303--- lib/lp/bugs/stories/patches-view/patches-view.txt 2010-07-17 21:02:11 +0000
304+++ lib/lp/bugs/stories/patches-view/patches-view.txt 2011-01-20 17:50:31 +0000
305@@ -420,7 +420,7 @@
306 ... name="project-subscriber", displayname="Project Subscriber")
307 >>> add_bug_sub = lambda *args: patchy_product.addBugSubscription(*args)
308 >>> with_anybody(add_bug_sub)(project_subscriber, project_subscriber)
309- <StructuralSubscription at ...>
310+ <...StructuralSubscription object at ...>
311 >>> from zope.security.proxy import removeSecurityProxy
312 >>> subscriber_name = removeSecurityProxy(project_subscriber).name
313 >>> anon_browser.open(
314
315=== modified file 'lib/lp/bugs/tests/bugs-emailinterface.txt'
316--- lib/lp/bugs/tests/bugs-emailinterface.txt 2010-12-13 13:40:38 +0000
317+++ lib/lp/bugs/tests/bugs-emailinterface.txt 2011-01-20 17:50:31 +0000
318@@ -1679,7 +1679,7 @@
319 >>> helge = getUtility(IPersonSet).getByName('kreutzm')
320 >>> mozilla_package = ubuntu.getSourcePackage(mozilla_name)
321 >>> mozilla_package.addBugSubscription(helge, helge)
322- <StructuralSubscription ...>
323+ <...StructuralSubscription ...>
324 >>> commit()
325 >>> LaunchpadZopelessLayer.switchDbUser(config.processmail.dbuser)
326
327
328=== modified file 'lib/lp/registry/browser/tests/milestone-views.txt'
329--- lib/lp/registry/browser/tests/milestone-views.txt 2010-11-02 20:10:56 +0000
330+++ lib/lp/registry/browser/tests/milestone-views.txt 2011-01-20 17:50:31 +0000
331@@ -682,7 +682,7 @@
332 >>> bugtask.milestone = milestone
333 >>> subscription = milestone.addSubscription(owner, owner)
334 >>> [subscription for subscription in owner.structural_subscriptions]
335- [<StructuralSubscription ...>]
336+ [<...StructuralSubscription ...>]
337
338 >>> view = create_initialized_view(milestone, '+delete')
339 >>> [bugtask.milestone.name for bugtask in view.bugtasks]
340
341=== modified file 'lib/lp/registry/doc/milestone.txt'
342--- lib/lp/registry/doc/milestone.txt 2010-12-22 00:07:36 +0000
343+++ lib/lp/registry/doc/milestone.txt 2011-01-20 17:50:31 +0000
344@@ -348,10 +348,10 @@
345 >>> cprov = getUtility(IPersonSet).getByName('cprov')
346 >>> ddaa = getUtility(IPersonSet).getByName('ddaa')
347 >>> milestone_one.addBugSubscription(cprov, cprov)
348- <StructuralSubscription at ...>
349+ <...StructuralSubscription object at ...>
350
351 >>> milestone_two.addBugSubscription(ddaa, ddaa)
352- <StructuralSubscription at ...>
353+ <...StructuralSubscription object at ...>
354
355 We change the milestone for the task from 1.0 to 2.0, and fire the
356 change event.
357
358=== modified file 'lib/lp/registry/doc/person.txt'
359--- lib/lp/registry/doc/person.txt 2010-12-07 00:58:46 +0000
360+++ lib/lp/registry/doc/person.txt 2011-01-20 17:50:31 +0000
361@@ -1027,11 +1027,11 @@
362 >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
363 >>> pmount = ubuntu.getSourcePackage('pmount')
364 >>> pmount.addBugSubscription(no_priv, no_priv)
365- <StructuralSubscription at ...>
366+ <...StructuralSubscription object at ...>
367
368 >>> mozilla_firefox = ubuntu.getSourcePackage('mozilla-firefox')
369 >>> mozilla_firefox.addBugSubscription(no_priv, no_priv)
370- <StructuralSubscription at ...>
371+ <...StructuralSubscription object at ...>
372
373 >>> [package.name for package in no_priv.getBugSubscriberPackages()]
374 [u'mozilla-firefox', u'pmount']
375
376=== modified file 'lib/lp/registry/model/person.py'
377--- lib/lp/registry/model/person.py 2011-01-18 04:40:41 +0000
378+++ lib/lp/registry/model/person.py 2011-01-20 17:50:31 +0000
379@@ -2713,8 +2713,10 @@
380 @property
381 def structural_subscriptions(self):
382 """See `IPerson`."""
383- return StructuralSubscription.selectBy(
384- subscriber=self, orderBy=['-date_created'])
385+ return IStore(self).find(
386+ StructuralSubscription,
387+ StructuralSubscription.subscriberID==self.id).order_by(
388+ Desc(StructuralSubscription.date_created))
389
390 def autoSubscribeToMailingList(self, mailinglist, requester=None):
391 """See `IPerson`."""
392
393=== modified file 'lib/lp/registry/model/structuralsubscription.py'
394--- lib/lp/registry/model/structuralsubscription.py 2011-01-19 18:12:07 +0000
395+++ lib/lp/registry/model/structuralsubscription.py 2011-01-20 17:50:31 +0000
396@@ -15,6 +15,7 @@
397 Reference,
398 )
399
400+from storm.base import Storm
401 from storm.expr import (
402 Alias,
403 And,
404@@ -39,10 +40,7 @@
405
406 from canonical.database.constants import UTC_NOW
407 from canonical.database.enumcol import DBEnum
408-from canonical.database.sqlbase import (
409- quote,
410- SQLBase,
411- )
412+from canonical.database.sqlbase import quote
413 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
414 from canonical.launchpad.interfaces.lpstorm import IStore
415 from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
416@@ -80,13 +78,11 @@
417 from lp.services.propertycache import cachedproperty
418
419
420-class StructuralSubscription(SQLBase):
421+class StructuralSubscription(Storm):
422 """A subscription to a Launchpad structure."""
423
424 implements(IStructuralSubscription)
425
426- _table = 'StructuralSubscription'
427-
428 __storm_table__ = 'StructuralSubscription'
429
430 id = Int(primary=True)
431@@ -135,6 +131,12 @@
432 "date_last_updated", allow_none=False, default=UTC_NOW,
433 tzinfo=pytz.UTC)
434
435+ def __init__(self, subscriber, subscribed_by, **kwargs):
436+ self.subscriber = subscriber
437+ self.subscribed_by = subscribed_by
438+ for arg, value in kwargs.iteritems():
439+ setattr(self, arg, value)
440+
441 @property
442 def target(self):
443 """See `IStructuralSubscription`."""
444@@ -427,13 +429,9 @@
445 '%s does not have permission to unsubscribe %s.' % (
446 unsubscribed_by.name, subscriber.name))
447
448- subscription_to_remove = None
449- for subscription in self.getSubscriptions(
450- min_bug_notification_level=BugNotificationLevel.METADATA):
451- # Only search for bug subscriptions
452- if subscription.subscriber == subscriber:
453- subscription_to_remove = subscription
454- break
455+ subscription_to_remove = self.getSubscriptions(
456+ min_bug_notification_level=BugNotificationLevel.METADATA,
457+ subscriber=subscriber).one()
458
459 if subscription_to_remove is None:
460 raise DeleteSubscriptionError(
461@@ -447,39 +445,44 @@
462 subscription_to_remove.bug_notification_level = (
463 BugNotificationLevel.NOTHING)
464 else:
465- subscription_to_remove.destroySelf()
466+ store = Store.of(subscription_to_remove)
467+ store.remove(subscription_to_remove)
468
469 def getSubscription(self, person):
470 """See `IStructuralSubscriptionTarget`."""
471- all_subscriptions = self.getSubscriptions()
472- for subscription in all_subscriptions:
473- if subscription.subscriber == person:
474- return subscription
475- return None
476+ # getSubscriptions returns all subscriptions regardless of
477+ # the person for person==None, so we special-case that.
478+ if person is None:
479+ return None
480+ all_subscriptions = self.getSubscriptions(subscriber=person)
481+ return all_subscriptions.one()
482
483 def getSubscriptions(self,
484 min_bug_notification_level=
485 BugNotificationLevel.NOTHING,
486 min_blueprint_notification_level=
487- BlueprintNotificationLevel.NOTHING):
488+ BlueprintNotificationLevel.NOTHING,
489+ subscriber=None):
490 """See `IStructuralSubscriptionTarget`."""
491+ from lp.registry.model.person import Person
492 clauses = [
493- "StructuralSubscription.subscriber = Person.id",
494- "StructuralSubscription.bug_notification_level "
495- ">= %s" % quote(min_bug_notification_level),
496- "StructuralSubscription.blueprint_notification_level "
497- ">= %s" % quote(min_blueprint_notification_level),
498+ StructuralSubscription.subscriberID==Person.id,
499+ (StructuralSubscription.bug_notification_level >=
500+ min_bug_notification_level),
501+ (StructuralSubscription.blueprint_notification_level >=
502+ min_blueprint_notification_level),
503 ]
504 for key, value in self._target_args.iteritems():
505- if value is None:
506- clauses.append(
507- "StructuralSubscription.%s IS NULL" % (key,))
508- else:
509- clauses.append(
510- "StructuralSubscription.%s = %s" % (key, quote(value)))
511- query = " AND ".join(clauses)
512- return StructuralSubscription.select(
513- query, orderBy='Person.displayname', clauseTables=['Person'])
514+ clauses.append(
515+ getattr(StructuralSubscription, key)==value)
516+
517+ if subscriber is not None:
518+ clauses.append(
519+ StructuralSubscription.subscriberID==subscriber.id)
520+
521+ store = Store.of(self.__helper.pillar)
522+ return store.find(
523+ StructuralSubscription, *clauses).order_by('Person.displayname')
524
525 @property
526 def bug_subscriptions(self):
527
528=== modified file 'lib/lp/registry/templates/person-structural-subscriptions.pt'
529--- lib/lp/registry/templates/person-structural-subscriptions.pt 2011-01-13 16:30:15 +0000
530+++ lib/lp/registry/templates/person-structural-subscriptions.pt 2011-01-20 17:50:31 +0000
531@@ -23,7 +23,7 @@
532 <div class="yui-g">
533 <div class="portlet" id="structural-subscriptions">
534
535- <ul tal:condition="structural_subscriptions">
536+ <ul tal:condition="python:not structural_subscriptions.is_empty()">
537 <li tal:repeat="subscription structural_subscriptions"
538 style="margin: 1em 0em; padding: 1em; border: 1px solid #ddd;">
539 <span tal:replace="structure subscription/target/fmt:link" />
540@@ -74,7 +74,7 @@
541 </li>
542 </ul>
543
544- <p tal:condition="not: structural_subscriptions">
545+ <p tal:condition="python: structural_subscriptions.is_empty()">
546 <tal:person content="context/fmt:displayname" /> does not
547 have any structural subscriptions.
548 </p>
549
550=== modified file 'lib/lp/registry/tests/structural-subscription-target.txt'
551--- lib/lp/registry/tests/structural-subscription-target.txt 2010-10-17 15:44:08 +0000
552+++ lib/lp/registry/tests/structural-subscription-target.txt 2011-01-20 17:50:31 +0000
553@@ -11,7 +11,7 @@
554 >>> from canonical.launchpad.ftests import login
555 >>> login("foo.bar@canonical.com")
556 >>> target.addBugSubscription(ubuntu_team, foobar)
557- <StructuralSubscription ...>
558+ <...StructuralSubscription ...>
559
560 Trying to remove a bug subscription when notification levels for other
561 applications are set, doesn't remove the subscription. Instead the
562@@ -41,7 +41,7 @@
563 IStructuralSubscriptionTarget.getSubscription.
564
565 >>> target.getSubscription(ubuntu_team)
566- <StructuralSubscription ...>
567+ <...StructuralSubscription ...>
568 >>> print target.getSubscription(no_priv)
569 None
570