Merge lp:~blr/launchpad/recycle-commit-message into lp:launchpad

[Kit Randel]

Preserve commit message on branch merge proposal resubmission.

Preserve commit message on branch merge proposal resubmission.

Support for commit_message has also been added to the BranchMergeProposal and BranchMergeProposalForGit factories.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py	2015-06-10 10:25:28 +0000
+++ lib/lp/code/browser/branchmergeproposal.py	2015-06-12 03:51:02 +0000
@@ -1012,6 +1012,7 @@
                 ]
         field_names.extend([
             'description',
+            'commit_message',
             'break_link',
             ])
         return field_names
@@ -1044,7 +1045,8 @@
                     merge_prerequisite = None
             proposal = self.context.resubmit(
                 self.user, merge_source, merge_target, merge_prerequisite,
-                data['description'], data['break_link'])
+                data['commit_message'], data['description'],
+                data['break_link'])
         except BranchMergeProposalExists as e:
             message = structured(
                 'Cannot resubmit because <a href="%(url)s">a similar merge'

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py	2015-06-05 17:49:22 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py	2015-06-12 03:51:02 +0000
@@ -925,6 +925,7 @@
         new_proposal = view.resubmit_action.success(self._getFormValues(
             context.merge_source, context.merge_target,
             context.merge_prerequisite, {
+                'commit_message': None,
                 'description': None,
                 'break_link': False,
                 }))
@@ -943,6 +944,7 @@
             view.context.merge_source)
         new_proposal = view.resubmit_action.success(self._getFormValues(
             new_source, new_target, new_prerequisite, {
+                'commit_message': 'a commit',
                 'description': 'description',
                 'break_link': False,
                 }))
@@ -964,6 +966,7 @@
             merge_prerequisite = context.merge_prerequisite
         return view.resubmit_action.success(cls._getFormValues(
             context.merge_source, context.merge_target, merge_prerequisite, {
+                'commit_message': None,
                 'description': None,
                 'break_link': break_link,
                 }))
@@ -1108,9 +1111,11 @@
         """Proposals can be resubmitted using the browser."""
         bmp = self.factory.makeBranchMergeProposalForGit(registrant=self.user)
         browser = self.getViewBrowser(bmp, '+resubmit')
+        browser.getControl('Commit Message').value = 'dribble'
         browser.getControl('Description').value = 'flibble'
         browser.getControl('Resubmit').click()
         with person_logged_in(self.user):
+            self.assertEqual('dribble', bmp.superseded_by.commit_message)
             self.assertEqual('flibble', bmp.superseded_by.description)
 
 

=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
--- lib/lp/code/interfaces/branchmergeproposal.py	2015-06-10 10:25:28 +0000
+++ lib/lp/code/interfaces/branchmergeproposal.py	2015-06-12 03:51:02 +0000
@@ -581,7 +581,8 @@
         """
 
     def resubmit(registrant, merge_source=None, merge_target=None,
-                 merge_prerequisite=DEFAULT):
+                 merge_prerequisite=DEFAULT, commit_message=None,
+                 description=None):
         """Mark the branch merge proposal as superseded and return a new one.
 
         The new proposal is created as work-in-progress, and copies across
@@ -596,6 +597,8 @@
             to the current merge_target).
         :param merge_prerequisite: The merge_prerequisite for the new
             proposal (defaults to the current merge_prerequisite).
+        :param commit_message: The commit message for the new proposal (defaults
+            to the current commit message).
         :param description: The description for the new proposal (defaults to
             the current description).
         """

=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py	2015-06-10 10:25:28 +0000
+++ lib/lp/code/model/branchmergeproposal.py	2015-06-12 03:51:02 +0000
@@ -634,8 +634,8 @@
         self.date_merged = date_merged
 
     def resubmit(self, registrant, merge_source=None, merge_target=None,
-                 merge_prerequisite=DEFAULT, description=None,
-                 break_link=False):
+                 merge_prerequisite=DEFAULT, commit_message=None,
+                 description=None, break_link=False):
         """See `IBranchMergeProposal`."""
         if merge_source is None:
             merge_source = self.merge_source
@@ -649,6 +649,8 @@
                 raise BranchMergeProposalExists(proposal)
         if merge_prerequisite is DEFAULT:
             merge_prerequisite = self.merge_prerequisite
+        if commit_message is None:
+            commit_message = self.commit_message
         if description is None:
             description = self.description
         # You can transition from REJECTED to SUPERSEDED, but
@@ -665,6 +667,7 @@
             registrant=registrant,
             merge_target=merge_target,
             merge_prerequisite=merge_prerequisite,
+            commit_message=commit_message,
             description=description,
             needs_review=True, review_requests=review_requests)
         if not break_link:

=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py	2015-05-26 07:39:49 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py	2015-06-12 03:51:02 +0000
@@ -1689,6 +1689,13 @@
         revised = original.resubmit(original.registrant, description='foo')
         self.assertEqual('foo', revised.description)
 
+    def test_resubmit_preserves_commit(self):
+        """Resubmit preserves commit message."""
+        original = self.factory.makeBranchMergeProposal()
+        self.useContext(person_logged_in(original.registrant))
+        revised = original.resubmit(original.registrant)
+        self.assertEqual(original.commit_message, revised.commit_message)
+
     def test_resubmit_breaks_link(self):
         """Resubmit breaks link, if specified."""
         original = self.factory.makeBranchMergeProposal()

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2015-06-11 08:06:28 +0000
+++ lib/lp/testing/factory.py	2015-06-12 03:51:02 +0000
@@ -1435,8 +1435,8 @@
                                 set_state=None, prerequisite_branch=None,
                                 product=None, initial_comment=None,
                                 source_branch=None, date_created=None,
-                                description=None, reviewer=None,
-                                merged_revno=None):
+                                commit_message=None, description=None,
+                                reviewer=None, merged_revno=None):
         """Create a proposal to merge based on anonymous branches."""
         if target_branch is not None:
             target_branch = removeSecurityProxy(target_branch)
@@ -1452,9 +1452,11 @@
             target_branch = self.makeProductBranch(product)
             target = target_branch.target
 
-        # Fall back to initial_comment for description.
+        # Fall back to initial_comment for description and commit_message.
         if description is None:
             description = initial_comment
+        if commit_message is None:
+            commit_message = initial_comment
 
         if target_branch is None:
             target_branch = self.makeBranchTargetBranch(target)
@@ -1468,7 +1470,7 @@
         proposal = source_branch.addLandingTarget(
             registrant, target_branch, review_requests=review_requests,
             merge_prerequisite=prerequisite_branch, description=description,
-            date_created=date_created)
+            commit_message=commit_message, date_created=date_created)
 
         unsafe_proposal = removeSecurityProxy(proposal)
         unsafe_proposal.merged_revno = merged_revno
@@ -1497,8 +1499,8 @@
                                       set_state=None, prerequisite_ref=None,
                                       target=_DEFAULT, initial_comment=None,
                                       source_ref=None, date_created=None,
-                                      description=None, reviewer=None,
-                                      merged_revision_id=None):
+                                      commit_message=None, description=None,
+                                      reviewer=None, merged_revision_id=None):
         """Create a proposal to merge based on anonymous branches."""
         if target is not _DEFAULT:
             pass
@@ -1514,9 +1516,11 @@
             [target_ref] = self.makeGitRefs(target=target)
             target = target_ref.target
 
-        # Fall back to initial_comment for description.
+        # Fall back to initial_comment for description and commit_message.
         if description is None:
             description = initial_comment
+        if commit_message is None:
+            commit_message = initial_comment
 
         if target_ref is None:
             [target_ref] = self.makeGitRefs(target=target)
@@ -1530,7 +1534,7 @@
         proposal = source_ref.addLandingTarget(
             registrant, target_ref, review_requests=review_requests,
             merge_prerequisite=prerequisite_ref, description=description,
-            date_created=date_created)
+            commit_message=commit_message, date_created=date_created)
 
         unsafe_proposal = removeSecurityProxy(proposal)
         unsafe_proposal.merged_revision_id = merged_revision_id