Merge lp:~gmb/launchpad/prompt-user-to-subscribe-to-master-bug-485627 into lp:launchpad

[Graham Binns]

Users will now be prompted to subscribe to the master bug when they are notified that one bug has been marked as a duplicate of another.

This branch adds a link to notifications about bugs being marked as a
duplicate, prompting the user to subscribe to the master bug.

Changes made:

== lib/lp/bugs/adapters/bugchange.py ==

 - I've updated the BugDuplicateChange class to add the 'you can
   subscribe by following this link' footer to duplication
   notifications.
 - I've cleaned up some lint.

== lib/lp/bugs/doc/bugnotification-sending.txt ==

 - Weirdly, this test failed after I'd made my change, but in a code
   path not related to my changes. Adding the switch_db_user calls to
   the point of failure fixed it; AFAICT this test should never have
   passed in the first place.
 - I've taken the opportunity to move this test to ReST format and to
   clean up some lint.

== lib/lp/bugs/tests/test_bugchanges.py ==

 - I've updated the tests for bug duplicate notifications to reflect the
   changes made to bugchange.py above.

[Edwin Grubbs (edwin-grubbs)]

Hi Graham,

This branch looks good.

-Edwin

review: Approve (code)

=== modified file 'lib/lp/bugs/adapters/bugchange.py'
--- lib/lp/bugs/adapters/bugchange.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/adapters/bugchange.py	2010-09-02 10:32:47 +0000
@@ -213,7 +213,7 @@
         lines.append(u"%13s: %s" % (
             u"Status", self.bug_task.status.title))
         return {
-            'text': '\n'.join(lines)
+            'text': '\n'.join(lines),
             }
 
 
@@ -381,8 +381,16 @@
                     "%d" % self.new_value.id)
             else:
                 new_value_text = (
-                    "** This bug has been marked a duplicate of bug %d\n"
-                    "   %s" % (self.new_value.id, self.new_value.title))
+                    "** This bug has been marked a duplicate of bug "
+                    "%(bug_id)d\n   %(bug_title)s\n"
+                    " * You can subscribe to bug %(bug_id)d by following "
+                    "this link: %(subscribe_link)s" % {
+                        'bug_id': self.new_value.id,
+                        'bug_title': self.new_value.title,
+                        'subscribe_link': canonical_url(
+                            self.new_value.default_bugtask,
+                            view_name='+subscribe'),
+                        })
 
             text = "\n".join((old_value_text, new_value_text))
 
@@ -393,8 +401,16 @@
                     "%d" % self.new_value.id)
             else:
                 text = (
-                    "** This bug has been marked a duplicate of bug %d\n"
-                    "   %s" % (self.new_value.id, self.new_value.title))
+                    "** This bug has been marked a duplicate of bug "
+                    "%(bug_id)d\n   %(bug_title)s\n"
+                    " * You can subscribe to bug %(bug_id)d by following "
+                    "this link: %(subscribe_link)s" % {
+                        'bug_id': self.new_value.id,
+                        'bug_title': self.new_value.title,
+                        'subscribe_link': canonical_url(
+                            self.new_value.default_bugtask,
+                            view_name='+subscribe'),
+                        })
 
         elif self.new_value is None:
             if self.old_value.private:
@@ -492,7 +508,7 @@
     def getBugNotification(self):
         return {
             'text': self.notification_mapping[
-                (self.old_value, self.new_value)]
+                (self.old_value, self.new_value)],
             }
 
 
@@ -685,9 +701,9 @@
             u"** Changed in: %(bug_target_name)s\n"
             "%(label)13s: %(oldval)s => %(newval)s\n" % {
                 'bug_target_name': self.bug_task.bugtargetname,
-                'label' : self.display_notification_label,
-                'oldval' : self.display_old_value,
-                'newval' : self.display_new_value,
+                'label': self.display_notification_label,
+                'oldval': self.display_old_value,
+                'newval': self.display_new_value,
             })
 
         return {'text': text.rstrip()}
 
=== modified file 'lib/lp/bugs/doc/bug-change.txt'
--- lib/lp/bugs/doc/bug-change.txt	2010-08-16 13:50:28 +0000
+++ lib/lp/bugs/doc/bug-change.txt	2010-09-02 10:32:47 +0000
@@ -300,6 +300,7 @@
     >>> print bug_duplicate_change.getBugNotification()['text']
     ** This bug has been marked a duplicate of bug ...
        Fish can't walk
+     * You can subscribe to bug ... by following this link:...
 
 BugDuplicateChange overrides getBugActivity() to customize all the
 returned fields.
 
=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
--- lib/lp/bugs/doc/bugnotification-sending.txt	2010-08-23 09:28:02 +0000
+++ lib/lp/bugs/doc/bugnotification-sending.txt	2010-09-02 10:32:47 +0000
@@ -1,4 +1,5 @@
-= Sending the Bug Notifications =
+Sending the Bug Notifications
+=============================
 
 As explained in bugnotifications.txt, a change to a bug causes a bug
 notification to be added. These notifications should be assembled into
@@ -29,6 +30,21 @@
     ...     print email_notification.get_payload(decode=True)
     ...     print "-" * 70
 
+We'll also define some helper functions to help us with database users.
+
+    >>> from canonical.config import config
+    >>> from canonical.database.sqlbase import commit
+    >>> from canonical.testing import LaunchpadZopelessLayer
+
+    >>> def switch_db_to_launchpad():
+    ...     commit()
+    ...     LaunchpadZopelessLayer.switchDbUser('launchpad')
+
+    >>> def switch_db_to_bugnotification():
+    ...     commit()
+    ...     LaunchpadZopelessLayer.switchDbUser(
+    ...         config.malone.bugnotification_dbuser)
+
 You'll note that we are printing out an X-Launchpad-Message-Rationale
 header. This header is a simple string that allows people to filter
 bugmail according to the reason they are getting emailed. For instance,
@@ -46,7 +62,8 @@
     ...     datecreated=ten_minutes_ago)
     >>> bug_one.addCommentNotification(comment)
 
-    >>> notifications = getUtility(IBugNotificationSet).getNotificationsToSend()
+    >>> notifications = getUtility(
+    ...     IBugNotificationSet).getNotificationsToSend()
     >>> len(notifications)
     1
 
@@ -323,6 +340,7 @@
     ...     print member.preferredemail.email
     [e-mail]
 
+    >>> switch_db_to_launchpad()
     >>> bug_one.subscribe(shipit_admins, shipit_admins)
     <...>
 
@@ -330,6 +348,8 @@
     ...     'subject', 'a comment.', sample_person,
     ...     datecreated=ten_minutes_ago)
     >>> bug_one.addCommentNotification(comment)
+
+    >>> switch_db_to_bugnotification()
     >>> pending_notifications = getUtility(
     ...     IBugNotificationSet).getNotificationsToSend()
     >>> len(pending_notifications)
@@ -348,22 +368,8 @@
     >>> flush_notifications()
 
 
-== Duplicates ==
-
-We will need some helper functions.
-
-    >>> from canonical.config import config
-    >>> from canonical.database.sqlbase import commit
-    >>> from canonical.testing import LaunchpadZopelessLayer
-
-    >>> def switch_db_to_launchpad():
-    ...     commit()
-    ...     LaunchpadZopelessLayer.switchDbUser('launchpad')
-
-    >>> def switch_db_to_bugnotification():
-    ...     commit()
-    ...     LaunchpadZopelessLayer.switchDbUser(
-    ...         config.malone.bugnotification_dbuser)
+Duplicates
+----------
 
 We will also need a fresh new bug.
 
@@ -391,11 +397,13 @@
     ...     'subject', 'a comment.', sample_person,
     ...     datecreated=ten_minutes_ago)
     >>> new_bug.addCommentNotification(comment)
-    >>> notifications = getUtility(IBugNotificationSet).getNotificationsToSend()
+    >>> notifications = getUtility(
+    ...     IBugNotificationSet).getNotificationsToSend()
     >>> len(notifications)
     1
 
-    >>> for bug_notifications, messages in get_email_notifications(notifications):
+    >>> for bug_notifications, messages in (
+    ...     get_email_notifications(notifications)):
     ...     for message in messages:
     ...         print_notification(message)
     To: [e-mail]
@@ -436,7 +444,8 @@
     >>> flush_notifications()
 
 
-== Security Vulnerabilities ==
+Security Vulnerabilities
+------------------------
 
 When a new security related bug is filed, a small notification is
 inserted at the top of the message body.
@@ -499,7 +508,8 @@
     >>> flush_notifications()
 
 
-== The cronscript ==
+The cronscript
+--------------
 
 There's a cronsript which does the sending of the email. Let's add a
 few notifications to show that it works.
@@ -528,7 +538,8 @@
     ...     '** Summary changed to: New summary.', sample_person,
     ...     when=ten_minutes_ago)
 
-    >>> notifications = getUtility(IBugNotificationSet).getNotificationsToSend()
+    >>> notifications = getUtility(
+    ...     IBugNotificationSet).getNotificationsToSend()
     >>> len(notifications)
     6
 
@@ -620,7 +631,8 @@
     >>> flush_notifications()
 
 
-== The X-Launchpad-Bug header ==
+The X-Launchpad-Bug header
+--------------------------
 
 When a notification is sent out about a bug, the X-Launchpad-Bug header is
 filled with data about that bug:
@@ -658,7 +670,8 @@
     False
 
 
-== The X-Launchpad-Bug-Tags header ==
+The X-Launchpad-Bug-Tags header
+-------------------------------
 
 First, a helper function that triggers notifications by adding a
 comment to a given bug, another that returns a sorted list of new
@@ -723,7 +736,8 @@
     ...     message.get_all('X-Launchpad-Bug-Tags')
 
 
-== The X-Launchpad-Bug-Private header ==
+The X-Launchpad-Bug-Private header
+----------------------------------
 
 When a notification is sent out about a bug, the
 X-Launchpad-Bug-Private header shows if the bug is marked as
@@ -748,7 +762,8 @@
     [e-mail] ['yes']
 
 
-== The X-Launchpad-Bug-Security-Vulnerability header ==
+The X-Launchpad-Bug-Security-Vulnerability header
+-------------------------------------------------
 
 When a notification is sent out about a bug, the
 X-Launchpad-Bug-Security-Vulnerability header records if the bug is a
@@ -776,7 +791,8 @@
     [e-mail] ['yes']
 
 
-== The X-Launchpad-Bug-Commenters header ==
+The X-Launchpad-Bug-Commenters header
+-------------------------------------
 
 The X-Launchpad-Bug-Recipient-Commented header lists all user IDs of
 people who have ever commented on the bug. It's a space-separated
@@ -809,7 +825,8 @@
     name12 name16
 
 
-== The X-Launchpad-Bug-Reporter header ==
+The X-Launchpad-Bug-Reporter header
+-----------------------------------
 
 The X-Launchpad-Bug-Reporter header contains information about the Launchpad
 user who originally reported the bug and opened the bug's first bug task.
@@ -819,7 +836,8 @@
     Foo Bar (name16)
 
 
-== Verbose bug notifications ==
+Verbose bug notifications
+-------------------------
 
 It is possible for users to have all the bug notifications which they
 receive include the bug description and status. This helps in those
@@ -1016,7 +1034,8 @@
     ----------------------------------------------------------------------
 
 
-== Notification Recipients ==
+Notification Recipients
+-----------------------
 
 Bug notifications are sent to direct subscribers of a bug as well as to
 structural subscribers. Structural subcribers can select the
 
=== modified file 'lib/lp/bugs/tests/test_bugchanges.py'
--- lib/lp/bugs/tests/test_bugchanges.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/tests/test_bugchanges.py	2010-09-02 10:32:47 +0000
@@ -1292,8 +1292,17 @@
 
         expected_notification = {
             'person': self.user,
-            'text': ("** This bug has been marked a duplicate of bug %d\n"
-                     "   %s" % (self.bug.id, self.bug.title)),
+            'text': (
+                "** This bug has been marked a duplicate of bug "
+                "%(bug_id)d\n   %(bug_title)s\n"
+                " * You can subscribe to bug %(bug_id)d by following "
+                "this link: %(subscribe_link)s" % {
+                    'bug_id': self.bug.id,
+                    'bug_title': self.bug.title,
+                    'subscribe_link': canonical_url(
+                        self.bug.default_bugtask,
+                        view_name='+subscribe'),
+                    }),
             'recipients': duplicate_bug_recipients,
             }
 
@@ -1361,11 +1370,21 @@
 
         expected_notification = {
             'person': self.user,
-            'text': ("** This bug is no longer a duplicate of bug %d\n"
-                     "   %s\n"
-                     "** This bug has been marked a duplicate of bug %d\n"
-                     "   %s" % (bug_one.id, bug_one.title,
-                                bug_two.id, bug_two.title)),
+            'text': (
+                "** This bug is no longer a duplicate of bug "
+                "%(bug_one_id)d\n   %(bug_one_title)s\n"
+                "** This bug has been marked a duplicate of bug "
+                "%(bug_two_id)d\n   %(bug_two_title)s\n"
+                " * You can subscribe to bug %(bug_two_id)d by following "
+                "this link: %(subscribe_link)s" % {
+                    'bug_one_id': bug_one.id,
+                    'bug_one_title': bug_one.title,
+                    'bug_two_id': bug_two.id,
+                    'bug_two_title': bug_two.title,
+                    'subscribe_link': canonical_url(
+                        bug_two.default_bugtask,
+                        view_name='+subscribe'),
+                    }),
             'recipients': bug_recipients,
             }
 
@@ -1474,10 +1493,20 @@
         expected_notification = {
             'person': self.user,
             'text': (
-                "** This bug is no longer a duplicate of private bug %d\n"
-                "** This bug has been marked a duplicate of bug %d\n"
-                "   %s" % (private_bug.id, public_bug.id,
-                           public_bug.title)),
+                "** This bug is no longer a duplicate of private bug "
+                "%(private_bug_id)d\n"
+                "** This bug has been marked a duplicate of bug "
+                "%(public_bug_id)d\n   %(public_bug_title)s\n"
+                " * You can subscribe to bug %(public_bug_id)d by following "
+                "this link: %(subscribe_link)s" % {
+                    'private_bug_id': private_bug.id,
+                    'private_bug_title': private_bug.title,
+                    'public_bug_id': public_bug.id,
+                    'public_bug_title': public_bug.title,
+                    'subscribe_link': canonical_url(
+                        public_bug.default_bugtask,
+                        view_name='+subscribe'),
+                    }),
             'recipients': bug_recipients,
             }