launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02491
[Merge] lp:~allenap/launchpad/remove-addChangeNotification into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/remove-addChangeNotification into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#712012 Remove Bug.addChangeNotification()
https://bugs.launchpad.net/bugs/712012
For more details, see:
https://code.launchpad.net/~allenap/launchpad/remove-addChangeNotification/+merge/48351
Bug.addChangeNotification() is obsolete and no longer used except in tests. I've updated the tests that refer to it and removed addChangeNotification() altogether.
--
https://code.launchpad.net/~allenap/launchpad/remove-addChangeNotification/+merge/48351
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/remove-addChangeNotification into lp:launchpad.
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2011-01-31 14:07:47 +0000
+++ lib/lp/bugs/configure.zcml 2011-02-02 17:37:57 +0000
@@ -747,7 +747,6 @@
attributes="
addAttachment
addChange
- addChangeNotification
addCommentNotification
addNomination
addTask
=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
--- lib/lp/bugs/doc/bugnotification-sending.txt 2011-01-06 14:44:56 +0000
+++ lib/lp/bugs/doc/bugnotification-sending.txt 2011-02-02 17:37:57 +0000
@@ -178,7 +178,6 @@
... notification.date_emailed = utc_now
... flush_database_updates()
-
To every message that gets sent out, [Bug $bugid] is prefixed to the
subject. It gets prefixed only if it's not already present in the
subject, though, which is often the case when someone replies via email.
@@ -219,12 +218,17 @@
Let's add a few changes and see how it looks like:
- >>> bug_one.addChangeNotification(
- ... '** Summary changed to: New summary.', sample_person,
- ... when=ten_minutes_ago)
- >>> bug_one.addChangeNotification(
- ... '** Visibility changed to: Private.', sample_person,
- ... when=ten_minutes_ago)
+ >>> from lp.bugs.adapters.bugchange import (
+ ... BugTitleChange, BugVisibilityChange)
+
+ >>> bug_one.addChange(
+ ... BugTitleChange(
+ ... ten_minutes_ago, sample_person, "title",
+ ... "Old summary", "New summary"))
+ >>> bug_one.addChange(
+ ... BugVisibilityChange(
+ ... ten_minutes_ago, sample_person, "private",
+ ... False, True))
>>> pending_notifications = getUtility(
... IBugNotificationSet).getNotificationsToSend()
>>> len(pending_notifications)
@@ -243,9 +247,11 @@
Subject: [Bug 1] Re: Firefox does not support SVG
X-Launchpad-Message-Rationale: Registrant (Ubuntu) @ubuntu-team
<BLANKLINE>
- ** Summary changed to: New summary.
+ ** Summary changed:
+ - Old summary
+ + New summary
<BLANKLINE>
- ** Visibility changed to: Private.
+ ** Visibility changed to: Private
<BLANKLINE>
...
You received this bug notification because you are a member of Ubuntu
@@ -266,12 +272,14 @@
... 'subject', 'a new comment.', sample_person,
... datecreated=ten_minutes_ago)
>>> bug_one.addCommentNotification(comment)
- >>> bug_one.addChangeNotification(
- ... '** Summary changed to: Another summary.', sample_person,
- ... when=ten_minutes_ago)
- >>> bug_one.addChangeNotification(
- ... '** Visibility changed to: Public.', sample_person,
- ... when=ten_minutes_ago)
+ >>> bug_one.addChange(
+ ... BugTitleChange(
+ ... ten_minutes_ago, sample_person, "title",
+ ... "New summary", "Another summary"))
+ >>> bug_one.addChange(
+ ... BugVisibilityChange(
+ ... ten_minutes_ago, sample_person, "private",
+ ... True, False))
>>> pending_notifications = getUtility(
... IBugNotificationSet).getNotificationsToSend()
>>> len(pending_notifications)
@@ -295,13 +303,17 @@
<BLANKLINE>
a new comment.
<BLANKLINE>
- ** Summary changed to: New summary.
- <BLANKLINE>
- ** Visibility changed to: Private.
- <BLANKLINE>
- ** Summary changed to: Another summary.
- <BLANKLINE>
- ** Visibility changed to: Public.
+ ** Summary changed:
+ - Old summary
+ + New summary
+ <BLANKLINE>
+ ** Visibility changed to: Private
+ <BLANKLINE>
+ ** Summary changed:
+ - New summary
+ + Another summary
+ <BLANKLINE>
+ ** Visibility changed to: Public
<BLANKLINE>
--
You received this bug notification because you are a member of Ubuntu
@@ -319,9 +331,10 @@
>>> now = datetime.now(pytz.timezone('UTC'))
>>> for minutes_ago in reversed(range(10)):
- ... bug_one.addChangeNotification(
- ... '** Visibility changed to: Private.', sample_person,
- ... when=now - timedelta(minutes=minutes_ago))
+ ... bug_one.addChange(
+ ... BugVisibilityChange(
+ ... now - timedelta(minutes=minutes_ago), sample_person,
+ ... "private", False, True))
>>> pending_notifications = getUtility(
... IBugNotificationSet).getNotificationsToSend()
>>> len(pending_notifications)
@@ -538,25 +551,28 @@
... 'subject', 'a comment.', sample_person,
... datecreated=ten_minutes_ago)
>>> bug_one.addCommentNotification(comment)
- >>> bug_one.addChangeNotification(
- ... '** Summary changed to: New summary.', sample_person,
- ... when=ten_minutes_ago)
+ >>> bug_one.addChange(
+ ... BugTitleChange(
+ ... ten_minutes_ago, sample_person, "title",
+ ... "Another summary", "New summary"))
>>> comment = getUtility(IMessageSet).fromText(
... 'subject', 'another comment.', sample_person,
... datecreated=ten_minutes_ago)
>>> bug_one.addCommentNotification(comment)
- >>> bug_one.addChangeNotification(
- ... '** Summary changed to: Old summary.', sample_person,
- ... when=ten_minutes_ago)
+ >>> bug_one.addChange(
+ ... BugTitleChange(
+ ... ten_minutes_ago, sample_person, "title",
+ ... "Summary #431", "Summary bleugh I'm going mad"))
>>> bug_two = getUtility(IBugSet).get(2)
>>> comment = getUtility(IMessageSet).fromText(
... 'subject', 'a comment.', sample_person,
... datecreated=ten_minutes_ago)
>>> bug_two.addCommentNotification(comment)
- >>> bug_two.addChangeNotification(
- ... '** Summary changed to: New summary.', sample_person,
- ... when=ten_minutes_ago)
+ >>> bug_two.addChange(
+ ... BugTitleChange(
+ ... ten_minutes_ago, sample_person, "title",
+ ... "Old summary", "New summary"))
>>> notifications = getUtility(
... IBugNotificationSet).getNotificationsToSend()
@@ -628,7 +644,9 @@
<BLANKLINE>
another comment.
<BLANKLINE>
- ** Summary changed to: Old summary.
+ ** Summary changed:
+ - Summary #431
+ + Summary bleugh I'm going mad
<BLANKLINE>
-- =
<BLANKLINE>
@@ -1231,13 +1249,15 @@
...
----------------------------------------------------------------------
-The notifications generated by addChangeNotification() are sent only to
-structural subscribers with the notification level METADATA or higher.
-The notification level of Sample Person is currently METADATA, hence he
+The notifications generated by addChange() are sent only to structural
+subscribers with the notification level METADATA or higher. The
+notification level of Sample Person is currently METADATA, hence he
receives these notifications.
- >>> bug_one.addChangeNotification('** Summary changed to: whatever.',
- ... sample_person, when=ten_minutes_ago)
+ >>> bug_one.addChange(
+ ... BugTitleChange(
+ ... ten_minutes_ago, sample_person, "title",
+ ... "New summary", "Whatever"))
>>> pending_notifications = getUtility(
... IBugNotificationSet).getNotificationsToSend()
>>> email_notifications = get_email_notifications(pending_notifications)
@@ -1269,7 +1289,9 @@
<BLANKLINE>
no comment for no-priv.
<BLANKLINE>
- ** Summary changed to: whatever.
+ ** Summary changed:
+ - New summary
+ + Whatever
<BLANKLINE>
--
You received this bug notification because you are subscribed to Mozilla
@@ -1290,7 +1312,7 @@
----------------------------------------------------------------------
If Sample Person sets his notification level to LIFECYCLE, he receives
-no notifications created by addChangeNotification().
+no notifications created by addChange().
>>> flush_notifications()
>>> switch_db_to_launchpad()
@@ -1298,8 +1320,10 @@
... BugNotificationLevel.LIFECYCLE)
>>> switch_db_to_bugnotification()
- >>> bug_one.addChangeNotification('** Summary changed to: something.',
- ... sample_person, when=ten_minutes_ago)
+ >>> bug_one.addChange(
+ ... BugTitleChange(
+ ... ten_minutes_ago, sample_person, "title",
+ ... "Whatever", "Whatever else"))
>>> pending_notifications = getUtility(
... IBugNotificationSet).getNotificationsToSend()
>>> email_notifications = get_email_notifications(pending_notifications)
@@ -1318,7 +1342,9 @@
Subject: [Bug 1] Re: Firefox does not support SVG
X-Launchpad-Message-Rationale: Subscriber @shipit-admins
<BLANKLINE>
- ** Summary changed to: something.
+ ** Summary changed:
+ - Whatever
+ + Whatever else
<BLANKLINE>
--
You received this bug notification because you are a member of ShipIt
=== modified file 'lib/lp/bugs/doc/bugnotification-threading.txt'
--- lib/lp/bugs/doc/bugnotification-threading.txt 2010-11-06 12:50:22 +0000
+++ lib/lp/bugs/doc/bugnotification-threading.txt 2011-02-02 17:37:57 +0000
@@ -18,17 +18,22 @@
>>> from datetime import datetime, timedelta
>>> from canonical.launchpad.interfaces.message import IMessageSet
>>> from lp.bugs.interfaces.bug import IBugSet
+ >>> from lp.bugs.adapters.bugchange import BugVisibilityChange
+
>>> ten_minutes_ago = (
... datetime.now(pytz.timezone('UTC')) - timedelta(minutes=10))
>>> sample_person = getUtility(ILaunchBag).user
>>> bug_one = getUtility(IBugSet).get(1)
- >>> bug_one.addChangeNotification(
- ... '** Some change.', sample_person, when=ten_minutes_ago)
+ >>> bug_one.addChange(
+ ... BugVisibilityChange(
+ ... ten_minutes_ago, sample_person, "private",
+ ... False, True))
>>> from lp.bugs.interfaces.bugnotification import IBugNotificationSet
>>> from lp.bugs.scripts.bugnotification import (
... get_email_notifications)
- >>> notifications = getUtility(IBugNotificationSet).getNotificationsToSend()
+ >>> notifications = getUtility(
+ ... IBugNotificationSet).getNotificationsToSend()
>>> messages = [emails for dummy, emails in
... get_email_notifications(notifications)]
@@ -62,9 +67,12 @@
>>> bug_one.addCommentNotification(comment)
>>> bug_one.linkMessage(comment)
<...>
- >>> bug_one.addChangeNotification(
- ... '** Some other change.', sample_person, when=ten_minutes_ago)
- >>> notifications = getUtility(IBugNotificationSet).getNotificationsToSend()
+ >>> bug_one.addChange(
+ ... BugVisibilityChange(
+ ... ten_minutes_ago, sample_person, "private",
+ ... True, False))
+ >>> notifications = getUtility(
+ ... IBugNotificationSet).getNotificationsToSend()
>>> messages = [emails for dummy, emails in
... get_email_notifications(notifications)]
>>> len(messages)
@@ -87,7 +95,8 @@
>>> flush_database_updates()
>>> reply = MessageSet().fromText(
- ... 'Re: subject', 'reply', sample_person, datecreated=ten_minutes_ago)
+ ... 'Re: subject', 'reply', sample_person,
+ ... datecreated=ten_minutes_ago)
>>> reply.parent = comment
>>> bug_one.addCommentNotification(reply)
>>> bug_one.linkMessage(reply)
@@ -95,7 +104,8 @@
Grab the notifications:
- >>> notifications = getUtility(IBugNotificationSet).getNotificationsToSend()
+ >>> notifications = getUtility(
+ ... IBugNotificationSet).getNotificationsToSend()
>>> messages = [emails for dummy, emails in
... get_email_notifications(notifications)]
>>> len(messages)
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2011-01-31 14:07:47 +0000
+++ lib/lp/bugs/interfaces/bug.py 2011-02-02 17:37:57 +0000
@@ -430,8 +430,6 @@
def newMessage(owner, subject, content):
"""Create a new message, and link it to this object."""
- # subscription-related methods
-
@operation_parameters(
person=Reference(IPerson, title=_('Person'), required=True),
# level actually uses BugNotificationLevel as its vocabulary,
@@ -543,9 +541,6 @@
True to include the master bug's subscribers as recipients.
"""
- def addChangeNotification(text, person, recipients=None, when=None):
- """Add a bug change notification."""
-
def addCommentNotification(message, recipients=None):
"""Add a bug comment notification."""
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-01-31 09:24:13 +0000
+++ lib/lp/bugs/model/bug.py 2011-02-02 17:37:57 +0000
@@ -436,7 +436,7 @@
"""See `IBug`."""
indexed_messages = self._indexed_messages(include_bugmessage=True)
for indexed_message, bugmessage in indexed_messages:
- if bugmessage.index != indexed_message.index:
+ if bugmessage.index != indexed_message.index:
bugmessage.index = indexed_message.index
@property
@@ -1043,19 +1043,6 @@
# original `Bug`.
return recipients
- def addChangeNotification(self, text, person, recipients=None, when=None):
- """See `IBug`."""
- if recipients is None:
- recipients = self.getBugNotificationRecipients(
- level=BugNotificationLevel.METADATA)
- if when is None:
- when = UTC_NOW
- message = MessageSet().fromText(
- self.followup_subject(), text, owner=person, datecreated=when)
- getUtility(IBugNotificationSet).addNotification(
- bug=self, is_comment=False,
- message=message, recipients=recipients)
-
def addCommentNotification(self, message, recipients=None):
"""See `IBug`."""
if recipients is None:
@@ -1071,8 +1058,6 @@
if when is None:
when = UTC_NOW
- # Only try to add something to the activity log if we have some
- # data.
activity_data = change.getBugActivity()
if activity_data is not None:
getUtility(IBugActivitySet).new(
@@ -1086,10 +1071,18 @@
if notification_data is not None:
assert notification_data.get('text') is not None, (
"notification_data must include a `text` value.")
-
- self.addChangeNotification(
- notification_data['text'], change.person, recipients,
- when)
+ message = MessageSet().fromText(
+ self.followup_subject(), notification_data['text'],
+ owner=change.person, datecreated=when)
+ if recipients is None:
+ getUtility(IBugNotificationSet).addNotification(
+ bug=self, is_comment=False, message=message,
+ recipients=self.getBugNotificationRecipients(
+ level=BugNotificationLevel.METADATA))
+ else:
+ getUtility(IBugNotificationSet).addNotification(
+ bug=self, is_comment=False, message=message,
+ recipients=recipients)
self.updateHeat()
=== modified file 'lib/lp/bugs/subscribers/bug.py'
--- lib/lp/bugs/subscribers/bug.py 2010-12-24 11:02:19 +0000
+++ lib/lp/bugs/subscribers/bug.py 2011-02-02 17:37:57 +0000
@@ -35,7 +35,6 @@
get_bug_changes,
)
from lp.bugs.adapters.bugdelta import BugDelta
-from lp.bugs.interfaces.bugchange import IBugChange
from lp.bugs.interfaces.bugtask import IBugTaskSet
from lp.bugs.mail.bugnotificationbuilder import BugNotificationBuilder
from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients
@@ -211,30 +210,23 @@
level=BugNotificationLevel.METADATA)
recipients.update(old_bugtask_recipients)
for change in changes:
- # XXX 2009-03-17 gmb [bug=344125]
- # This if..else should be removed once the new BugChange API
- # is complete and ubiquitous.
- if IBugChange.providedBy(change):
- if isinstance(change, BugDuplicateChange):
- no_dupe_master_recipients = (
- bug_delta.bug.getBugNotificationRecipients(
- old_bug=bug_delta.bug_before_modification,
- level=BugNotificationLevel.METADATA,
- include_master_dupe_subscribers=False))
- bug_delta.bug.addChange(
- change, recipients=no_dupe_master_recipients)
- elif (isinstance(change, BugTaskAssigneeChange) and
- new_subscribers is not None):
- for person in new_subscribers:
- reason, rationale = recipients.getReason(person)
- if 'Assignee' in rationale:
- recipients.remove(person)
- bug_delta.bug.addChange(change, recipients=recipients)
- else:
- bug_delta.bug.addChange(change, recipients=recipients)
+ if isinstance(change, BugDuplicateChange):
+ no_dupe_master_recipients = (
+ bug_delta.bug.getBugNotificationRecipients(
+ old_bug=bug_delta.bug_before_modification,
+ level=BugNotificationLevel.METADATA,
+ include_master_dupe_subscribers=False))
+ bug_delta.bug.addChange(
+ change, recipients=no_dupe_master_recipients)
+ elif (isinstance(change, BugTaskAssigneeChange) and
+ new_subscribers is not None):
+ for person in new_subscribers:
+ reason, rationale = recipients.getReason(person)
+ if 'Assignee' in rationale:
+ recipients.remove(person)
+ bug_delta.bug.addChange(change, recipients=recipients)
else:
- bug_delta.bug.addChangeNotification(
- change, person=bug_delta.user, recipients=recipients)
+ bug_delta.bug.addChange(change, recipients=recipients)
def send_bug_details_to_new_bug_subscribers(
