Merge lp:~cjwatson/launchpad/snapshot-modifying-helper-use-answers into lp:launchpad

Proposed by Colin Watson on 2018-11-18
Status: Merged
Merged at revision: 18848
Proposed branch: lp:~cjwatson/launchpad/snapshot-modifying-helper-use-answers
Merge into: lp:launchpad
Diff against target: 402 lines (+83/-111)
6 files modified
lib/lp/answers/browser/question.py (+20/-24)
lib/lp/answers/browser/tests/views.txt (+16/-0)
lib/lp/answers/doc/faq.txt (+3/-8)
lib/lp/answers/doc/karma.txt (+13/-21)
lib/lp/answers/doc/notifications.txt (+26/-51)
lib/lp/answers/tests/test_faq_webservice.py (+5/-7)
To merge this branch: bzr merge lp:~cjwatson/launchpad/snapshot-modifying-helper-use-answers
Reviewer Review Type Date Requested Status
William Grant code 2018-11-18 Approve on 2018-12-07
Review via email: mp+358962@code.launchpad.net

Commit message

Convert lp.answers to notify_modified.

To post a comment you must log in.
William Grant (wgrant) :
review: Needs Fixing
18825. By Colin Watson on 2018-12-07

Restore edited_fields to the ObjectModifiedEvent emitted by QuestionSubscriptionView.

Colin Watson (cjwatson) :
William Grant (wgrant) :
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/answers/browser/question.py'
2--- lib/lp/answers/browser/question.py 2018-08-09 15:08:14 +0000
3+++ lib/lp/answers/browser/question.py 2018-12-07 13:25:36 +0000
4@@ -29,13 +29,10 @@
5 from operator import attrgetter
6 import re
7
8-from lazr.lifecycle.event import ObjectModifiedEvent
9-from lazr.lifecycle.snapshot import Snapshot
10 from lazr.restful.interface import copy_field
11 from lazr.restful.utils import smartquote
12 from z3c.ptcompat import ViewPageTemplateFile
13 from zope.component import getUtility
14-from zope.event import notify
15 from zope.formlib import form
16 from zope.formlib.interfaces import IWidgetFactory
17 from zope.formlib.widget import (
18@@ -49,7 +46,6 @@
19 from zope.interface import (
20 alsoProvides,
21 implementer,
22- providedBy,
23 )
24 from zope.schema import Choice
25 from zope.schema.interfaces import IContextSourceBinder
26@@ -118,6 +114,7 @@
27 from lp.services.webapp.breadcrumb import Breadcrumb
28 from lp.services.webapp.escaping import structured
29 from lp.services.webapp.interfaces import IAlwaysSubmittedWidget
30+from lp.services.webapp.snapshot import notify_modified
31 from lp.services.worlddata.helpers import (
32 is_english_variant,
33 preferred_or_request_languages,
34@@ -364,27 +361,26 @@
35 # No post, nothing to do
36 return
37
38- question_unmodified = Snapshot(
39- self.context, providing=providedBy(self.context))
40+ # XXX cjwatson 2018-12-07: None of the (IQuestion,
41+ # IObjectModifiedEvent) subscribers seem to care about the
42+ # 'subscribers' field. Do we need this notification?
43 modified_fields = set()
44-
45- response = self.request.response
46- # Establish if a subscription form was posted.
47- newsub = self.request.form.get('subscribe', None)
48- if newsub is not None:
49- if newsub == 'Subscribe':
50- self.context.subscribe(self.user)
51- response.addNotification(
52- _("You have subscribed to this question."))
53- modified_fields.add('subscribers')
54- elif newsub == 'Unsubscribe':
55- self.context.unsubscribe(self.user, self.user)
56- response.addNotification(
57- _("You have unsubscribed from this question."))
58- modified_fields.add('subscribers')
59- response.redirect(canonical_url(self.context))
60- notify(ObjectModifiedEvent(
61- self.context, question_unmodified, list(modified_fields)))
62+ with notify_modified(self.context, modified_fields):
63+ response = self.request.response
64+ # Establish if a subscription form was posted.
65+ newsub = self.request.form.get('subscribe', None)
66+ if newsub is not None:
67+ if newsub == 'Subscribe':
68+ self.context.subscribe(self.user)
69+ response.addNotification(
70+ _("You have subscribed to this question."))
71+ modified_fields.add('subscribers')
72+ elif newsub == 'Unsubscribe':
73+ self.context.unsubscribe(self.user, self.user)
74+ response.addNotification(
75+ _("You have unsubscribed from this question."))
76+ modified_fields.add('subscribers')
77+ response.redirect(canonical_url(self.context))
78
79 @property
80 def page_title(self):
81
82=== modified file 'lib/lp/answers/browser/tests/views.txt'
83--- lib/lp/answers/browser/tests/views.txt 2018-06-02 19:27:16 +0000
84+++ lib/lp/answers/browser/tests/views.txt 2018-12-07 13:25:36 +0000
85@@ -26,6 +26,18 @@
86 This view is used to subscribe and unsubscribe from a question.
87 Subscription is done when the user click on the 'Subscribe' button.
88
89+Register an event listener that will print events it receives.
90+
91+ >>> from lazr.lifecycle.interfaces import IObjectModifiedEvent
92+ >>> from lp.answers.interfaces.question import IQuestion
93+ >>> from lp.testing.event import TestEventListener
94+
95+ >>> def print_modified_event(object, event):
96+ ... print("Received ObjectModifiedEvent: %s" % (
97+ ... ", ".join(sorted(event.edited_fields))))
98+ >>> question_event_listener = TestEventListener(
99+ ... IQuestion, IObjectModifiedEvent, print_modified_event)
100+
101 >>> view = create_initialized_view(question_three, name='+subscribe')
102 >>> print(view.label)
103 Subscribe to question
104@@ -36,6 +48,7 @@
105 >>> form = {'subscribe': 'Subscribe'}
106 >>> view = create_initialized_view(
107 ... question_three, name='+subscribe', form=form)
108+ Received ObjectModifiedEvent: subscribers
109 >>> question_three.isSubscribed(getUtility(ILaunchBag).user)
110 True
111
112@@ -58,6 +71,7 @@
113 >>> form = {'subscribe': 'Unsubscribe'}
114 >>> view = create_initialized_view(
115 ... question_three, name='+subscribe', form=form)
116+ Received ObjectModifiedEvent: subscribers
117 >>> question_three.isSubscribed(getUtility(ILaunchBag).user)
118 False
119
120@@ -68,6 +82,8 @@
121 >>> view.request.response.getHeader('Location')
122 '.../+question/3'
123
124+ >>> question_event_listener.unregister()
125+
126
127 QuestionWorkflowView
128 --------------------
129
130=== modified file 'lib/lp/answers/doc/faq.txt'
131--- lib/lp/answers/doc/faq.txt 2018-06-01 23:12:25 +0000
132+++ lib/lp/answers/doc/faq.txt 2018-12-07 13:25:36 +0000
133@@ -154,14 +154,9 @@
134
135 When the FAQ is modified, the attributes are automatically updated.
136
137- >>> from zope.event import notify
138- >>> from zope.interface import providedBy
139- >>> from lazr.lifecycle.event import ObjectModifiedEvent
140- >>> from lazr.lifecycle.snapshot import Snapshot
141- >>> old_faq = Snapshot(firefox_faq, providing=providedBy(firefox_faq))
142- >>> firefox_faq.keywords = 'extension'
143- >>> notify(ObjectModifiedEvent(
144- ... firefox_faq, old_faq, ['keywords'], user=sample_person))
145+ >>> from lp.services.webapp.snapshot import notify_modified
146+ >>> with notify_modified(firefox_faq, ['keywords'], user=sample_person):
147+ ... firefox_faq.keywords = 'extension'
148
149 >>> print(firefox_faq.last_updated_by.displayname)
150 Sample Person
151
152=== modified file 'lib/lp/answers/doc/karma.txt'
153--- lib/lp/answers/doc/karma.txt 2016-01-26 15:47:37 +0000
154+++ lib/lp/answers/doc/karma.txt 2018-12-07 13:25:36 +0000
155@@ -61,7 +61,6 @@
156 ...................
157
158 >>> login('test@canonical.com')
159- >>> from zope.event import notify
160 >>> firefox = getUtility(IProductSet)['firefox']
161 >>> firefox_question = firefox.newQuestion(
162 ... title='New question', description='Question description.',
163@@ -163,28 +162,23 @@
164 Changing the title of a question
165 ................................
166
167- >>> from zope.interface import providedBy
168- >>> from lazr.lifecycle.event import ObjectModifiedEvent
169- >>> from lazr.lifecycle.snapshot import Snapshot
170- >>> old_question = Snapshot(
171- ... firefox_question, providing=providedBy(firefox_question))
172+ >>> from lp.services.webapp.snapshot import notify_modified
173 >>> login('test@canonical.com')
174- >>> firefox_question.title = ('Firefox 1.5.0.5 does not have any '
175- ... '"Quick Searches" installed by default')
176- >>> notify(ObjectModifiedEvent(firefox_question, old_question, ['title']))
177+ >>> with notify_modified(firefox_question, ['title']):
178+ ... firefox_question.title = (
179+ ... 'Firefox 1.5.0.5 does not have any "Quick Searches" '
180+ ... 'installed by default')
181 Karma added: action=questiontitlechanged, product=firefox, person=name12
182
183
184 Changing the description of a question
185 ......................................
186
187- >>> old_question = Snapshot(
188- ... firefox_question, providing=providedBy(firefox_question))
189- >>> firefox_question.description = (
190- ... 'Firefox 1.5.0.5 does not have any "Quick Searches" installed '
191- ... 'in the bookmarks by default, like the official ones do.')
192- >>> notify(ObjectModifiedEvent(
193- ... firefox_question, old_question, ['description']))
194+ >>> with notify_modified(firefox_question, ['description']):
195+ ... firefox_question.description = (
196+ ... 'Firefox 1.5.0.5 does not have any "Quick Searches" '
197+ ... 'installed in the bookmarks by default, like the official '
198+ ... 'ones do.')
199 Karma added: action=questiondescriptionchanged, product=firefox,
200 person=name12
201
202@@ -225,11 +219,9 @@
203 Modifying a FAQ
204 ...............
205
206- >>> old_faq = Snapshot(firefox_faq, providing=providedBy(firefox_faq))
207- >>> firefox_faq.title = 'How can I make the Fnord appears?'
208- >>> firefox_faq.content = 'Install the Fnords highlighter extensions.'
209- >>> notify(ObjectModifiedEvent(
210- ... firefox_faq, old_faq, ['title', 'content']))
211+ >>> with notify_modified(firefox_faq, ['title', 'content']):
212+ ... firefox_faq.title = 'How can I make the Fnord appears?'
213+ ... firefox_faq.content = 'Install the Fnords highlighter extensions.'
214 Karma added: action=faqedited, product=firefox, person=name12
215
216
217
218=== modified file 'lib/lp/answers/doc/notifications.txt'
219--- lib/lp/answers/doc/notifications.txt 2018-06-01 23:12:25 +0000
220+++ lib/lp/answers/doc/notifications.txt 2018-12-07 13:25:36 +0000
221@@ -6,7 +6,6 @@
222 Let's start with creating a question, and see what the resulting
223 notification looks like:
224
225- >>> from zope.event import notify
226 >>> from lp.answers.tests.test_question_notifications import (
227 ... pop_questionemailjobs)
228 >>> from lp.registry.interfaces.distribution import IDistributionSet
229@@ -84,23 +83,18 @@
230 If we edit the title and description of the question, a notification
231 will be sent.
232
233- >>> from zope.interface import providedBy
234- >>> from lazr.lifecycle.event import ObjectModifiedEvent
235- >>> from lazr.lifecycle.snapshot import Snapshot
236+ >>> from lp.services.webapp.snapshot import notify_modified
237
238 >>> login('test@canonical.com')
239- >>> unmodified_question = Snapshot(
240- ... ubuntu_question, providing=providedBy(ubuntu_question))
241- >>> ubuntu_question.title = "Installer doesn't work on a Mac"
242- >>> ubuntu_question.description = """I insert the install CD in the CD-ROM
243- ... drive, but it won't boot.
244- ...
245- ... It boots straight into MacOS 9."""
246- >>> libstdcplusplus_sourcepackage = ubuntu.getSourcePackage('libstdc++')
247- >>> ubuntu_question.target = libstdcplusplus_sourcepackage
248- >>> notify(ObjectModifiedEvent(
249- ... ubuntu_question, unmodified_question,
250- ... ['title', 'description', 'target']))
251+ >>> with notify_modified(
252+ ... ubuntu_question, ['title', 'description', 'target']):
253+ ... ubuntu_question.title = "Installer doesn't work on a Mac"
254+ ... ubuntu_question.description = (
255+ ... "I insert the install CD in the CD-ROM\n"
256+ ... "drive, but it won't boot.\n"
257+ ... "\n"
258+ ... "It boots straight into MacOS 9.")
259+ ... ubuntu_question.target = ubuntu.getSourcePackage('libstdc++')
260
261 Three copies of the notification got sent, one to Sample Person, one to
262 Foo Bar, and one to Ubuntu Team:
263@@ -130,11 +124,8 @@
264 to another QuestionTarget and priority is changed, # the notification
265 does not include priority.
266
267- >>> unmodified_question = Snapshot(
268- ... ubuntu_question, providing=providedBy(ubuntu_question))
269- >>> ubuntu_question.target = ubuntu
270- >>> notify(ObjectModifiedEvent(
271- ... ubuntu_question, unmodified_question, ['target']))
272+ >>> with notify_modified(ubuntu_question, ['target']):
273+ ... ubuntu_question.target = ubuntu
274 >>> notifications = pop_questionemailjobs()
275 >>> edit_notification = notifications[1]
276 >>> print(edit_notification.body)
277@@ -147,11 +138,8 @@
278
279 >>> login('foo.bar@canonical.com')
280 >>> no_priv = getUtility(IPersonSet).getByName('no-priv')
281- >>> unmodified_question = Snapshot(
282- ... ubuntu_question, providing=providedBy(ubuntu_question))
283- >>> ubuntu_question.assignee = no_priv
284- >>> notify(ObjectModifiedEvent(
285- ... ubuntu_question, unmodified_question, ['assignee']))
286+ >>> with notify_modified(ubuntu_question, ['assignee']):
287+ ... ubuntu_question.assignee = no_priv
288 >>> notifications = pop_questionemailjobs()
289 >>> edit_notification = notifications[1]
290 >>> print(edit_notification.body)
291@@ -163,10 +151,8 @@
292 If we trigger a modification event when no changes worth notifying about
293 was made, no notification is sent:
294
295- >>> unmodified_question = Snapshot(
296- ... ubuntu_question, providing=providedBy(ubuntu_question))
297- >>> notify(ObjectModifiedEvent(
298- ... ubuntu_question, unmodified_question, ['status']))
299+ >>> with notify_modified(ubuntu_question, ['status']):
300+ ... pass
301
302 >>> notifications = pop_questionemailjobs()
303 >>> len(notifications)
304@@ -190,18 +176,14 @@
305 >>> from lp.bugs.interfaces.bug import CreateBugParams
306
307 >>> login('no-priv@canonical.com')
308- >>> unmodified_question = Snapshot(
309- ... ubuntu_question, providing=providedBy(ubuntu_question))
310- >>> params = CreateBugParams(
311- ... owner=no_priv, title="Installer fails on a Mac PPC",
312- ... comment=ubuntu_question.description)
313- >>> bug = ubuntu_question.target.createBug(params)
314- >>> ubuntu_question.linkBug(bug)
315+ >>> with notify_modified(ubuntu_question, ['bugs']):
316+ ... params = CreateBugParams(
317+ ... owner=no_priv, title="Installer fails on a Mac PPC",
318+ ... comment=ubuntu_question.description)
319+ ... bug = ubuntu_question.target.createBug(params)
320+ ... ubuntu_question.linkBug(bug)
321 True
322
323- >>> notify(ObjectModifiedEvent(
324- ... ubuntu_question, unmodified_question, ['bugs']))
325-
326 >>> notifications = pop_questionemailjobs()
327 >>> len(notifications)
328 2
329@@ -221,14 +203,10 @@
330
331 A notification is also sent when a bug is unlinked from the question:
332
333- >>> unmodified_question = Snapshot(ubuntu_question,
334- ... providing=providedBy(ubuntu_question))
335- >>> ubuntu_question.unlinkBug(bug)
336+ >>> with notify_modified(ubuntu_question, ['bugs']):
337+ ... ubuntu_question.unlinkBug(bug)
338 True
339
340- >>> notify(ObjectModifiedEvent(
341- ... ubuntu_question, unmodified_question, ['bugs']))
342-
343 >>> notifications = pop_questionemailjobs()
344 >>> len(notifications)
345 2
346@@ -788,11 +766,8 @@
347 is modified. Only the owner will receive a modification notification
348 with a warning appended to it.
349
350- >>> unmodified_question = Snapshot(
351- ... french_question, providing=providedBy(french_question))
352- >>> french_question.title = u"CD d'Ubuntu ne d\xe9marre pas"
353- >>> notify(ObjectModifiedEvent(
354- ... french_question, unmodified_question, ['title']))
355+ >>> with notify_modified(french_question, ['title']):
356+ ... french_question.title = u"CD d'Ubuntu ne d\xe9marre pas"
357 >>> notifications = pop_questionemailjobs()
358
359 >>> notification_body = recode_text(notifications[0])
360
361=== modified file 'lib/lp/answers/tests/test_faq_webservice.py'
362--- lib/lp/answers/tests/test_faq_webservice.py 2017-10-25 10:02:12 +0000
363+++ lib/lp/answers/tests/test_faq_webservice.py 2018-12-07 13:25:36 +0000
364@@ -1,11 +1,10 @@
365-# Copyright 2015 Canonical Ltd. This software is licensed under the
366+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
367 # GNU Affero General Public License version 3 (see the file LICENSE).
368
369 from __future__ import absolute_import, print_function, unicode_literals
370
371 __metaclass__ = type
372
373-from lazr.lifecycle.event import ObjectModifiedEvent
374 from testtools.matchers import (
375 Contains,
376 ContainsDict,
377@@ -13,10 +12,10 @@
378 MatchesRegex,
379 )
380 from zope.component import getUtility
381-from zope.event import notify
382
383 from lp.registry.interfaces.person import IPersonSet
384 from lp.services.webapp.interfaces import OAuthPermission
385+from lp.services.webapp.snapshot import notify_modified
386 from lp.testing import (
387 admin_logged_in,
388 api_url,
389@@ -33,10 +32,9 @@
390 def test_representation(self):
391 with admin_logged_in():
392 faq = self.factory.makeFAQ(title="Nothing works")
393- faq.keywords = "foo bar"
394- faq.content = "It is all broken."
395- notify(ObjectModifiedEvent(
396- faq, faq, ['keywords', 'content'], user=faq.owner))
397+ with notify_modified(faq, ['keywords', 'content'], user=faq.owner):
398+ faq.keywords = "foo bar"
399+ faq.content = "It is all broken."
400 faq_url = api_url(faq)
401 webservice = webservice_for_person(self.factory.makePerson())
402 repr = webservice.get(faq_url, api_version='devel').jsonBody()