Merge lp:~thumper/launchpad/initial-mp-email-text-alignment into lp:launchpad

[Tim Penhey]

Fix the alignment of the related bugs and requested reviews in the initial merge proposal email.

[Tim Penhey (thumper)]

A simple fix to the outgoing email for a new merge proposal to make sure that the related bugs and requested reviews use the same formatting.

[Michael Hudson-Doyle (mwhudson)]

Looks fine to me.

review: Approve

=== modified file 'lib/lp/code/mail/branchmergeproposal.py'
--- lib/lp/code/mail/branchmergeproposal.py	2009-11-12 21:27:16 +0000
+++ lib/lp/code/mail/branchmergeproposal.py	2010-02-14 22:36:21 +0000
@@ -237,9 +237,36 @@
             params['diff_cutoff_warning'] = (
                 "The attached diff has been truncated due to its size.")
 
+        params['reviews'] = self._getRequestedReviews()
         params['related_bugs'] = self._getRelatedBugs()
         return params
 
+    def _formatExtraInformation(self, heading, chunks):
+        """Consistently indent the chunks with the heading.
+
+        Used to provide consistent indentation for requested reviews and
+        related bugs.
+        """
+        if len(chunks) == 0:
+            return ''
+        else:
+            info = ''.join('  %s\n' % value for value in chunks)
+            return '%s\n%s' % (heading, info)
+
+    def _getRequestedReviews(self):
+        """Return a string describing the requested reviews, if any."""
+        requested_reviews = []
+        for review in self.requested_reviews:
+            reviewer = review.reviewer
+            if review.review_type is None:
+                requested_reviews.append(reviewer.unique_displayname)
+            else:
+                requested_reviews.append(
+                    "%s: %s" % (reviewer.unique_displayname,
+                                review.review_type))
+        return self._formatExtraInformation(
+            'Requested reviews:', requested_reviews)
+
     def _getRelatedBugs(self):
         """Return a string describing related bugs, if any.
 
@@ -247,12 +274,9 @@
         """
         bug_chunks = []
         for bug in self.merge_proposal.related_bugs:
-            bug_chunks.append('  #%d %s\n' % (bug.id, bug.title))
-            bug_chunks.append('  %s\n' % canonical_url(bug))
-        if len(bug_chunks) == 0:
-            return ''
-        else:
-            return 'Related bugs:\n' + ''.join(bug_chunks)
+            bug_chunks.append('#%d %s' % (bug.id, bug.title))
+            bug_chunks.append(canonical_url(bug))
+        return self._formatExtraInformation('Related bugs:', bug_chunks)
 
     def _getTemplateParams(self, email):
         """Return a dict of values to use in the body and subject."""

=== modified file 'lib/lp/code/mail/tests/test_branchmergeproposal.py'
--- lib/lp/code/mail/tests/test_branchmergeproposal.py	2009-10-28 20:13:39 +0000
+++ lib/lp/code/mail/tests/test_branchmergeproposal.py	2010-02-14 22:36:21 +0000
@@ -135,9 +135,26 @@
         mailer = BMPMailer.forCreation(bmp, bmp.registrant)
         ctrl = mailer.generateEmail('[e-mail]', subscriber)
         self.assertIn(
-            'Requested reviews:\n    Review-person (review-person)\n\n-- \n',
+            '\nRequested reviews:\n  Review-person (review-person)\n\n-- \n',
             ctrl.body)
 
+    def test_forCreation_with_review_request_and_bug(self):
+        """Correctly format list of reviewers and bug info."""
+        bmp, subscriber = self.makeProposalWithSubscriber()
+        bug = self.factory.makeBug(title='I am a bug')
+        bmp.source_branch.linkBug(bug, bmp.registrant)
+        reviewer = self.factory.makePerson(name='review-person')
+        vote_reference = bmp.nominateReviewer(reviewer, bmp.registrant, None)
+        mailer = BMPMailer.forCreation(bmp, bmp.registrant)
+        ctrl = mailer.generateEmail('[e-mail]', subscriber)
+        expected = (
+            '\nRequested reviews:'
+            '\n  Review-person (review-person)'
+            '\nRelated bugs:'
+            '\n  #%d I am a bug'
+            '\n  %s\n\n--' % (bug.id, canonical_url(bug)))
+        self.assertIn(expected, ctrl.body)
+
     def test_forCreation_with_prerequisite_branch(self):
         """Correctly format list of reviewers."""
         bmp, subscriber = self.makeProposalWithSubscriber(prerequisite=True)