launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snapshot-modifying-helper-use-answers into lp:launchpad.

Commit message:
Convert lp.answers to notify_modified.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snapshot-modifying-helper-use-answers/+merge/358962
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snapshot-modifying-helper-use-answers into lp:launchpad.

=== modified file 'lib/lp/answers/browser/question.py'
--- lib/lp/answers/browser/question.py	2018-08-09 15:08:14 +0000
+++ lib/lp/answers/browser/question.py	2018-11-18 18:00:36 +0000
@@ -29,13 +29,10 @@
 from operator import attrgetter
 import re
 
-from lazr.lifecycle.event import ObjectModifiedEvent
-from lazr.lifecycle.snapshot import Snapshot
 from lazr.restful.interface import copy_field
 from lazr.restful.utils import smartquote
 from z3c.ptcompat import ViewPageTemplateFile
 from zope.component import getUtility
-from zope.event import notify
 from zope.formlib import form
 from zope.formlib.interfaces import IWidgetFactory
 from zope.formlib.widget import (
@@ -49,7 +46,6 @@
 from zope.interface import (
     alsoProvides,
     implementer,
-    providedBy,
     )
 from zope.schema import Choice
 from zope.schema.interfaces import IContextSourceBinder
@@ -118,6 +114,7 @@
 from lp.services.webapp.breadcrumb import Breadcrumb
 from lp.services.webapp.escaping import structured
 from lp.services.webapp.interfaces import IAlwaysSubmittedWidget
+from lp.services.webapp.snapshot import notify_modified
 from lp.services.worlddata.helpers import (
     is_english_variant,
     preferred_or_request_languages,
@@ -364,27 +361,23 @@
             # No post, nothing to do
             return
 
-        question_unmodified = Snapshot(
-            self.context, providing=providedBy(self.context))
         modified_fields = set()
-
-        response = self.request.response
-        # Establish if a subscription form was posted.
-        newsub = self.request.form.get('subscribe', None)
-        if newsub is not None:
-            if newsub == 'Subscribe':
-                self.context.subscribe(self.user)
-                response.addNotification(
-                    _("You have subscribed to this question."))
-                modified_fields.add('subscribers')
-            elif newsub == 'Unsubscribe':
-                self.context.unsubscribe(self.user, self.user)
-                response.addNotification(
-                    _("You have unsubscribed from this question."))
-                modified_fields.add('subscribers')
-            response.redirect(canonical_url(self.context))
-        notify(ObjectModifiedEvent(
-            self.context, question_unmodified, list(modified_fields)))
+        with notify_modified(self.context):
+            response = self.request.response
+            # Establish if a subscription form was posted.
+            newsub = self.request.form.get('subscribe', None)
+            if newsub is not None:
+                if newsub == 'Subscribe':
+                    self.context.subscribe(self.user)
+                    response.addNotification(
+                        _("You have subscribed to this question."))
+                    modified_fields.add('subscribers')
+                elif newsub == 'Unsubscribe':
+                    self.context.unsubscribe(self.user, self.user)
+                    response.addNotification(
+                        _("You have unsubscribed from this question."))
+                    modified_fields.add('subscribers')
+                response.redirect(canonical_url(self.context))
 
     @property
     def page_title(self):

=== modified file 'lib/lp/answers/doc/faq.txt'
--- lib/lp/answers/doc/faq.txt	2018-06-01 23:12:25 +0000
+++ lib/lp/answers/doc/faq.txt	2018-11-18 18:00:36 +0000
@@ -154,14 +154,9 @@
 
 When the FAQ is modified, the attributes are automatically updated.
 
-    >>> from zope.event import notify
-    >>> from zope.interface import providedBy
-    >>> from lazr.lifecycle.event import ObjectModifiedEvent
-    >>> from lazr.lifecycle.snapshot import Snapshot
-    >>> old_faq = Snapshot(firefox_faq, providing=providedBy(firefox_faq))
-    >>> firefox_faq.keywords = 'extension'
-    >>> notify(ObjectModifiedEvent(
-    ...     firefox_faq, old_faq, ['keywords'], user=sample_person))
+    >>> from lp.services.webapp.snapshot import notify_modified
+    >>> with notify_modified(firefox_faq, ['keywords'], user=sample_person):
+    ...     firefox_faq.keywords = 'extension'
 
     >>> print(firefox_faq.last_updated_by.displayname)
     Sample Person

=== modified file 'lib/lp/answers/doc/karma.txt'
--- lib/lp/answers/doc/karma.txt	2016-01-26 15:47:37 +0000
+++ lib/lp/answers/doc/karma.txt	2018-11-18 18:00:36 +0000
@@ -61,7 +61,6 @@
 ...................
 
     >>> login('test@xxxxxxxxxxxxx')
-    >>> from zope.event import notify
     >>> firefox = getUtility(IProductSet)['firefox']
     >>> firefox_question = firefox.newQuestion(
     ...     title='New question', description='Question description.',
@@ -163,28 +162,23 @@
 Changing the title of a question
 ................................
 
-    >>> from zope.interface import providedBy
-    >>> from lazr.lifecycle.event import ObjectModifiedEvent
-    >>> from lazr.lifecycle.snapshot import Snapshot
-    >>> old_question = Snapshot(
-    ...     firefox_question, providing=providedBy(firefox_question))
+    >>> from lp.services.webapp.snapshot import notify_modified
     >>> login('test@xxxxxxxxxxxxx')
-    >>> firefox_question.title = ('Firefox 1.5.0.5 does not have any '
-    ...                         '"Quick Searches" installed by default')
-    >>> notify(ObjectModifiedEvent(firefox_question, old_question, ['title']))
+    >>> with notify_modified(firefox_question, ['title']):
+    ...     firefox_question.title = (
+    ...         'Firefox 1.5.0.5 does not have any "Quick Searches" '
+    ...         'installed by default')
     Karma added: action=questiontitlechanged, product=firefox, person=name12
 
 
 Changing the description of a question
 ......................................
 
-    >>> old_question = Snapshot(
-    ...     firefox_question, providing=providedBy(firefox_question))
-    >>> firefox_question.description = (
-    ...     'Firefox 1.5.0.5 does not have any "Quick Searches" installed '
-    ...     'in the bookmarks by default, like the official ones do.')
-    >>> notify(ObjectModifiedEvent(
-    ...     firefox_question, old_question, ['description']))
+    >>> with notify_modified(firefox_question, ['description']):
+    ...     firefox_question.description = (
+    ...         'Firefox 1.5.0.5 does not have any "Quick Searches" '
+    ...         'installed in the bookmarks by default, like the official '
+    ...         'ones do.')
     Karma added: action=questiondescriptionchanged, product=firefox,
         person=name12
 
@@ -225,11 +219,9 @@
 Modifying a FAQ
 ...............
 
-    >>> old_faq = Snapshot(firefox_faq, providing=providedBy(firefox_faq))
-    >>> firefox_faq.title = 'How can I make the Fnord appears?'
-    >>> firefox_faq.content = 'Install the Fnords highlighter extensions.'
-    >>> notify(ObjectModifiedEvent(
-    ...     firefox_faq, old_faq, ['title', 'content']))
+    >>> with notify_modified(firefox_faq, ['title', 'content']):
+    ...     firefox_faq.title = 'How can I make the Fnord appears?'
+    ...     firefox_faq.content = 'Install the Fnords highlighter extensions.'
     Karma added: action=faqedited, product=firefox, person=name12
 
 

=== modified file 'lib/lp/answers/doc/notifications.txt'
--- lib/lp/answers/doc/notifications.txt	2018-06-01 23:12:25 +0000
+++ lib/lp/answers/doc/notifications.txt	2018-11-18 18:00:36 +0000
@@ -6,7 +6,6 @@
 Let's start with creating a question, and see what the resulting
 notification looks like:
 
-    >>> from zope.event import notify
     >>> from lp.answers.tests.test_question_notifications import (
     ...     pop_questionemailjobs)
     >>> from lp.registry.interfaces.distribution import IDistributionSet
@@ -84,23 +83,18 @@
 If we edit the title and description of the question, a notification
 will be sent.
 
-    >>> from zope.interface import providedBy
-    >>> from lazr.lifecycle.event import ObjectModifiedEvent
-    >>> from lazr.lifecycle.snapshot import Snapshot
+    >>> from lp.services.webapp.snapshot import notify_modified
 
     >>> login('test@xxxxxxxxxxxxx')
-    >>> unmodified_question = Snapshot(
-    ...     ubuntu_question, providing=providedBy(ubuntu_question))
-    >>> ubuntu_question.title = "Installer doesn't work on a Mac"
-    >>> ubuntu_question.description = """I insert the install CD in the CD-ROM
-    ... drive, but it won't boot.
-    ...
-    ... It boots straight into MacOS 9."""
-    >>> libstdcplusplus_sourcepackage = ubuntu.getSourcePackage('libstdc++')
-    >>> ubuntu_question.target = libstdcplusplus_sourcepackage
-    >>> notify(ObjectModifiedEvent(
-    ...     ubuntu_question, unmodified_question,
-    ...     ['title', 'description', 'target']))
+    >>> with notify_modified(
+    ...         ubuntu_question, ['title', 'description', 'target']):
+    ...     ubuntu_question.title = "Installer doesn't work on a Mac"
+    ...     ubuntu_question.description = (
+    ...         "I insert the install CD in the CD-ROM\n"
+    ...         "drive, but it won't boot.\n"
+    ...         "\n"
+    ...         "It boots straight into MacOS 9.")
+    ...     ubuntu_question.target = ubuntu.getSourcePackage('libstdc++')
 
 Three copies of the notification got sent, one to Sample Person, one to
 Foo Bar, and one to Ubuntu Team:
@@ -130,11 +124,8 @@
 to another QuestionTarget and priority is changed, # the notification
 does not include priority.
 
-    >>> unmodified_question = Snapshot(
-    ...     ubuntu_question, providing=providedBy(ubuntu_question))
-    >>> ubuntu_question.target = ubuntu
-    >>> notify(ObjectModifiedEvent(
-    ...     ubuntu_question, unmodified_question, ['target']))
+    >>> with notify_modified(ubuntu_question, ['target']):
+    ...     ubuntu_question.target = ubuntu
     >>> notifications = pop_questionemailjobs()
     >>> edit_notification = notifications[1]
     >>> print(edit_notification.body)
@@ -147,11 +138,8 @@
 
     >>> login('foo.bar@xxxxxxxxxxxxx')
     >>> no_priv = getUtility(IPersonSet).getByName('no-priv')
-    >>> unmodified_question = Snapshot(
-    ...     ubuntu_question, providing=providedBy(ubuntu_question))
-    >>> ubuntu_question.assignee = no_priv
-    >>> notify(ObjectModifiedEvent(
-    ...     ubuntu_question, unmodified_question, ['assignee']))
+    >>> with notify_modified(ubuntu_question, ['assignee']):
+    ...     ubuntu_question.assignee = no_priv
     >>> notifications = pop_questionemailjobs()
     >>> edit_notification = notifications[1]
     >>> print(edit_notification.body)
@@ -163,10 +151,8 @@
 If we trigger a modification event when no changes worth notifying about
 was made, no notification is sent:
 
-    >>> unmodified_question = Snapshot(
-    ...     ubuntu_question, providing=providedBy(ubuntu_question))
-    >>> notify(ObjectModifiedEvent(
-    ...     ubuntu_question, unmodified_question, ['status']))
+    >>> with notify_modified(ubuntu_question, ['status']):
+    ...     pass
 
     >>> notifications = pop_questionemailjobs()
     >>> len(notifications)
@@ -190,18 +176,14 @@
     >>> from lp.bugs.interfaces.bug import CreateBugParams
 
     >>> login('no-priv@xxxxxxxxxxxxx')
-    >>> unmodified_question = Snapshot(
-    ...     ubuntu_question, providing=providedBy(ubuntu_question))
-    >>> params = CreateBugParams(
-    ...     owner=no_priv, title="Installer fails on a Mac PPC",
-    ...     comment=ubuntu_question.description)
-    >>> bug = ubuntu_question.target.createBug(params)
-    >>> ubuntu_question.linkBug(bug)
+    >>> with notify_modified(ubuntu_question, ['bugs']):
+    ...     params = CreateBugParams(
+    ...         owner=no_priv, title="Installer fails on a Mac PPC",
+    ...         comment=ubuntu_question.description)
+    ...     bug = ubuntu_question.target.createBug(params)
+    ...     ubuntu_question.linkBug(bug)
     True
 
-    >>> notify(ObjectModifiedEvent(
-    ...     ubuntu_question, unmodified_question, ['bugs']))
-
     >>> notifications = pop_questionemailjobs()
     >>> len(notifications)
     2
@@ -221,14 +203,10 @@
 
 A notification is also sent when a bug is unlinked from the question:
 
-    >>> unmodified_question = Snapshot(ubuntu_question,
-    ...     providing=providedBy(ubuntu_question))
-    >>> ubuntu_question.unlinkBug(bug)
+    >>> with notify_modified(ubuntu_question, ['bugs']):
+    ...     ubuntu_question.unlinkBug(bug)
     True
 
-    >>> notify(ObjectModifiedEvent(
-    ...     ubuntu_question, unmodified_question, ['bugs']))
-
     >>> notifications = pop_questionemailjobs()
     >>> len(notifications)
     2
@@ -788,11 +766,8 @@
 is modified. Only the owner will receive a modification notification
 with a warning appended to it.
 
-    >>> unmodified_question = Snapshot(
-    ...     french_question, providing=providedBy(french_question))
-    >>> french_question.title = u"CD d'Ubuntu ne d\xe9marre pas"
-    >>> notify(ObjectModifiedEvent(
-    ...     french_question, unmodified_question, ['title']))
+    >>> with notify_modified(french_question, ['title']):
+    ...     french_question.title = u"CD d'Ubuntu ne d\xe9marre pas"
     >>> notifications = pop_questionemailjobs()
 
     >>> notification_body = recode_text(notifications[0])

=== modified file 'lib/lp/answers/tests/test_faq_webservice.py'
--- lib/lp/answers/tests/test_faq_webservice.py	2017-10-25 10:02:12 +0000
+++ lib/lp/answers/tests/test_faq_webservice.py	2018-11-18 18:00:36 +0000
@@ -1,11 +1,10 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 
-from lazr.lifecycle.event import ObjectModifiedEvent
 from testtools.matchers import (
     Contains,
     ContainsDict,
@@ -13,10 +12,10 @@
     MatchesRegex,
     )
 from zope.component import getUtility
-from zope.event import notify
 
 from lp.registry.interfaces.person import IPersonSet
 from lp.services.webapp.interfaces import OAuthPermission
+from lp.services.webapp.snapshot import notify_modified
 from lp.testing import (
     admin_logged_in,
     api_url,
@@ -33,10 +32,9 @@
     def test_representation(self):
         with admin_logged_in():
             faq = self.factory.makeFAQ(title="Nothing works")
-            faq.keywords = "foo bar"
-            faq.content = "It is all broken."
-            notify(ObjectModifiedEvent(
-                faq, faq, ['keywords', 'content'], user=faq.owner))
+            with notify_modified(faq, ['keywords', 'content'], user=faq.owner):
+                faq.keywords = "foo bar"
+                faq.content = "It is all broken."
             faq_url = api_url(faq)
         webservice = webservice_for_person(self.factory.makePerson())
         repr = webservice.get(faq_url, api_version='devel').jsonBody()