launchpad-reviewers team mailing list archive

[Nigel Jones <dev@xxxxxxxxxx>]

Nigel Jones has proposed merging lp:~dev-nigelj/launchpad/bug-697157 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #697157 in Launchpad itself: "Streamline "You have assigned this bug to yourself" message "
  https://bugs.launchpad.net/launchpad/+bug/697157

For more details, see:
https://code.launchpad.net/~dev-nigelj/launchpad/bug-697157/+merge/74035

Summary

If you assign yourself to a bug, under certain circumstances an e-mail will be sent saying "Nigel Jones (dev-nigelj) has assigned this bug to you for Launchpad itself:" when a better/more logical message would be "You have assigned this bug to yourself for Launchpad itself:"

Proposed Fix/Implementation Details

The Proposed and Implemented fix is to include an if statement, to verify if that the event creator is the same as the bug task assignee and generate the new message, otherwise to use the existing

Tests

All tests related to this change pass, and I couldn't see any new failures on a full test natty run.

The change also introduces a new test in lib/lp/bugs/mail/tests/test_bug_task_assignment.py (test_self_assignee_notification_message()) which tests for this new message in the situations that it should be generated.

Demo/QA

w/ AppServer mail setup in a way that you can read all e-mails sent
Create a new bug for "Ubuntu" under one username
Logout & login as a second username (not one that is a member of Ubuntu Team)
Assign the bug to yourself, verify the correct statement regarding bug assignment is used in the [NEW] e-mail.
-- 
https://code.launchpad.net/~dev-nigelj/launchpad/bug-697157/+merge/74035
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~dev-nigelj/launchpad/bug-697157 into lp:launchpad.

=== modified file 'lib/lp/bugs/mail/newbug.py'
--- lib/lp/bugs/mail/newbug.py	2010-12-21 23:49:34 +0000
+++ lib/lp/bugs/mail/newbug.py	2011-09-05 00:43:26 +0000
@@ -62,8 +62,13 @@
 
     if new_recipients:
         if "assignee" in reason and event_creator is not None:
-            contents += (
-                "%(assigner)s has assigned this bug to you for %(target)s")
+            if event_creator == bugtask.assignee:
+                contents += (
+                    "You have assigned this bug to yourself for %(target)s")
+            else:
+                contents += (
+                    "%(assigner)s has assigned this bug to you for " +
+                    "%(target)s")
             content_substitutions['assigner'] = (
                 event_creator.unique_displayname)
             content_substitutions['target'] = bugtask.target.displayname

=== modified file 'lib/lp/bugs/mail/tests/test_bug_task_assignment.py'
--- lib/lp/bugs/mail/tests/test_bug_task_assignment.py	2011-08-12 11:37:08 +0000
+++ lib/lp/bugs/mail/tests/test_bug_task_assignment.py	2011-09-05 00:43:26 +0000
@@ -65,6 +65,22 @@
         self.assertTrue(rationale in msg,
                         '%s not in\n%s\n' % (rationale, msg))
 
+    def test_self_assignee_notification_message(self):
+        """Test notification string when a person is assigned a task by
+           someone else."""
+        self.assertEqual(len(stub.test_emails), 0, 'emails in queue')
+        self.bug_task.transitionToAssignee(self.user)
+        notify(ObjectModifiedEvent(
+            self.bug_task, self.bug_task_before_modification,
+            edited_fields=['assignee']))
+        transaction.commit()
+        self.assertEqual(len(stub.test_emails), 1, 'email not sent')
+        rationale = (
+            'You have assigned this bug to yourself for Rebirth')
+        msg = stub.test_emails[-1][2]
+        self.assertTrue(rationale in msg,
+                        '%s not in\n%s\n' % (rationale, msg))
+
     def test_assignee_not_a_subscriber(self):
         """Test that a new recipient being assigned a bug task does send
            a NEW message."""