launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/codereviewcomment-mail-newline into lp:launchpad.

Commit message:
Add a missing newline to code review comment mail footers.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/codereviewcomment-mail-newline/+merge/266846

I turned expanded notification footers on for myself recently for testing, and noticed that code review comment mails are missing a newline between their ordinary footer and the expanded one, which has been annoying me.  This fixes that detail.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/codereviewcomment-mail-newline into lp:launchpad.

=== modified file 'lib/lp/code/mail/codereviewcomment.py'
--- lib/lp/code/mail/codereviewcomment.py	2015-07-09 10:43:21 +0000
+++ lib/lp/code/mail/codereviewcomment.py	2015-08-04 09:29:39 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Email notifications for code review comments."""
@@ -125,7 +125,7 @@
         # Include both the canonical_url for the proposal and the reason
         # in the footer to the email.
         reason, rationale = self._recipients.getReason(email)
-        footer = "%(proposal_url)s\n%(reason)s" % {
+        footer = "%(proposal_url)s\n%(reason)s\n" % {
             'proposal_url': self.proposal_url,
             'reason': reason.getReason()}
         return ''.join((

=== modified file 'lib/lp/code/mail/tests/test_codereviewcomment.py'
--- lib/lp/code/mail/tests/test_codereviewcomment.py	2015-07-09 09:53:26 +0000
+++ lib/lp/code/mail/tests/test_codereviewcomment.py	2015-08-04 09:29:39 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test CodeReviewComment emailing functionality."""
@@ -23,6 +23,7 @@
 from lp.testing import (
     login,
     login_person,
+    person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.layers import LaunchpadFunctionalLayer
@@ -192,6 +193,27 @@
              canonical_url(mailer.merge_proposal),
              'You are subscribed to branch %s.' % branch_name])
 
+    def test_appendExpandedFooter(self):
+        """Check that expanded notification footers are sensible."""
+        mailer, subscriber = self.makeMailer(as_reply=True)
+        with person_logged_in(subscriber):
+            subscriber.expanded_notification_footers = True
+        ctrl = mailer.generateEmail(
+            subscriber.preferredemail.email, subscriber)
+        source_branch = mailer.merge_proposal.source_branch
+        rationale = mailer._recipients.getReason('subscriber@xxxxxxxxxxx')[1]
+        expected_footer = [
+            '-- ', canonical_url(mailer.merge_proposal),
+            'You are subscribed to branch %s.' % source_branch.bzr_identity,
+            '',
+            'Launchpad-Message-Rationale: %s' % rationale,
+            'Launchpad-Notification-Type: code-review',
+            'Launchpad-Branch: %s' % source_branch.unique_name,
+            'Launchpad-Project: %s' % source_branch.product.name,
+            ]
+        self.assertEqual(
+            expected_footer, ctrl.body.splitlines()[-len(expected_footer):])
+
     def test_generateEmailWithVote(self):
         """Ensure that votes are displayed."""
         mailer, subscriber = self.makeMailer(