Merge lp:~brian-murray/launchpad/x-launchpad-bug-modifier-follow-on into lp:launchpad

[Brian Murray]

Add in X-Launchpad-Bug-Modifier header, identifying the event creator's display name and name, to bug mail generated by bugnotification.py.

This branch is a follows up a merge proposal, https://code.edge.launchpad.net/~brian-murray/launchpad/bug-605340/+merge/31221, to add an X-Launchpad-Bug-Modifier header to emails sent regarding bug changes. That branch was supposed to fix bug 605340, however it does this incompletely by only adding the header when bug details are sent to new subscribers. This branch adds person to BugNotificationBuilder call in lp/bugs/scripts/bugnotification.py so that all bug mails will have the X-Launchpad-Bug-Modifier header.

I also added in a test, test_bug_task_modification.py, to confirm that the header appears when a bug's status is changed.

== Tests ==

bin/test -cvv -t bugnotification-email -t test_bug_task_modification

[Māris Fogels (mars)]

Hi Brian,

These changes look good. I just had a few questions:

 * On line 28 of the diff, there are brackets around ObjectModifiedEvent. I assume that is a left-over from earlier imports?
 * The docstring for TestModificationNotification should either be made more general, or the class name should be made more specific to match the docstring.
 * Is the sanity check for the stub mailer on line 50 of the diff necessary? I would hope the TestCase object reliably cleans up the stub mailer for you.

I think the branch looks good. r=mars with the tweaks I listed above.

Maris

review: Approve

=== added file 'lib/lp/bugs/mail/tests/test_bug_task_modification.py'
--- lib/lp/bugs/mail/tests/test_bug_task_modification.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/mail/tests/test_bug_task_modification.py	2010-08-17 19:22:48 +0000
@@ -0,0 +1,60 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test for emails sent after bug task modification."""
+
+from unittest import TestLoader
+
+import transaction
+
+from zope.component import getUtility
+from zope.interface import providedBy
+from zope.event import notify
+
+from canonical.testing import DatabaseFunctionalLayer
+from canonical.launchpad.database import BugNotification
+from canonical.launchpad.webapp.interfaces import ILaunchBag
+
+
+from lp.bugs.interfaces.bugtask import BugTaskStatus
+from lp.bugs.scripts.bugnotification import construct_email_notifications
+from lp.testing import TestCaseWithFactory
+
+from lazr.lifecycle.event import ObjectModifiedEvent
+from lazr.lifecycle.snapshot import Snapshot
+
+
+class TestModificationNotification(TestCaseWithFactory):
+    """Test email notifications when a bug task is modified."""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        # Run the tests as a logged-in user.
+        super(TestModificationNotification, self).setUp(
+            user='[e-mail]')
+        self.user = getUtility(ILaunchBag).user
+        self.product = self.factory.makeProduct(owner=self.user)
+        self.bug = self.factory.makeBug(product=self.product)
+        self.bug_task = self.bug.getBugTask(self.product)
+        self.bug_task_before_modification = Snapshot(self.bug_task,
+            providing=providedBy(self.bug_task))
+
+    def test_for_bug_modifier_header(self):
+        """Test X-Launchpad-Bug-Modifier appears when a bug is modified."""
+        self.bug_task.transitionToStatus(BugTaskStatus.CONFIRMED, self.user)
+        notify(ObjectModifiedEvent(
+            self.bug_task, self.bug_task_before_modification,
+            ['status'], user=self.user))
+        transaction.commit()
+        latest_notification = BugNotification.selectFirst(orderBy='-id')
+        notifications, messages = construct_email_notifications(
+            [latest_notification])
+        self.assertEqual(len(notifications), 1,
+                         'email notification not created')
+        headers = [msg['X-Launchpad-Bug-Modifier'] for msg in messages]
+        self.assertEqual(len(headers), len(messages))
+
+
+def test_suite():
+    return TestLoader().loadTestsFromName(__name__)
 
=== modified file 'lib/lp/bugs/scripts/bugnotification.py'
--- lib/lp/bugs/scripts/bugnotification.py	2010-06-23 09:36:02 +0000
+++ lib/lp/bugs/scripts/bugnotification.py	2010-08-17 19:22:48 +0000
@@ -111,7 +111,7 @@
                 (email_person, recipient)
                 for email_person, recipient in recipients.items()
                 if recipient.person.inTeam(comment_syncing_team))
-    bug_notification_builder = BugNotificationBuilder(bug)
+    bug_notification_builder = BugNotificationBuilder(bug, person)
     sorted_recipients = sorted(
         recipients.items(), key=lambda t: t[0].preferredemail.email)
     for email_person, recipient in sorted_recipients: