From 21077c690a885784f28f1e8c4927de78e075c7e3 Mon Sep 17 00:00:00 2001
From: Xavier Morel <xmo@odoo.com>
Date: Thu, 21 Jan 2021 13:15:32 +0100
Subject: [PATCH] [FIX] runbot_merge: incorrect processing of rebased commit
 messages

5cf3617eef9ce848d19d8d06c22e47586889df36 intended to create merge
messages with only the content of PR descriptions before the first
thematic break. However this processing was incorrectly applied
to all messages being processed (meaning rebased / squashed commit
messages as well).

Properly apply thematic break processing to only commit messages
created from PR descriptions.
---
 runbot_merge/models/pull_requests.py |  8 ++++---
 runbot_merge/tests/test_basic.py     | 33 ++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py
index e955be29..f317fa64 100644
--- a/runbot_merge/models/pull_requests.py
+++ b/runbot_merge/models/pull_requests.py
@@ -1343,7 +1343,7 @@ class PullRequests(models.Model):
         return head
 
     def _stage_rebase_merge(self, gh, target, commits, related_prs=()):
-        msg = self._build_merge_message(self.message, related_prs=related_prs)
+        msg = self._build_merge_message(self, related_prs=related_prs)
         h, mapping = gh.rebase(self.number, target, reset=True, commits=commits)
         merge_head = gh.merge(h, target, str(msg))['sha']
         self.commits_map = json.dumps({**mapping, '': merge_head})
@@ -1384,7 +1384,7 @@ class PullRequests(models.Model):
             return copy['sha']
         else:
             # otherwise do a regular merge
-            msg = self._build_merge_message(self.message)
+            msg = self._build_merge_message(self)
             merge_head = gh.merge(self.head, target, str(msg))['sha']
             # and the merge commit is the normal merge head
             commits_map[''] = merge_head
@@ -2081,6 +2081,8 @@ class Message:
     def from_message(cls, msg):
         in_headers = True
         maybe_setex = None
+        # creating from PR message -> remove content following break
+        msg, handle_break = (msg, False) if isinstance(msg, str) else (msg.message, True)
         headers = []
         body = []
         for line in reversed(msg.splitlines()):
@@ -2102,7 +2104,7 @@ class Message:
                     body.append(line)
                 continue
 
-            if BREAK.match(line):
+            if handle_break and BREAK.match(line):
                 if SETEX_UNDERLINE.match(line):
                     maybe_setex = line
                 else:
diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py
index d484e086..accd91b6 100644
--- a/runbot_merge/tests/test_basic.py
+++ b/runbot_merge/tests/test_basic.py
@@ -1718,6 +1718,39 @@ removed
         Signed-off-by: {reviewer}
         """).strip(), "should not break the SETEX titles"
 
+    def test_rebase_no_edit(self, repo, env, users, config):
+        """ Only the merge messages should be de-breaked
+        """
+        reviewer = get_partner(env, users["reviewer"]).formatted_email
+        with repo:
+            root = repo.make_commits(None, Commit("root", tree={'a': 'a'}), ref='heads/master')
+
+            repo.make_commits(root, Commit('Commit\n\nfirst\n***\nsecond', tree={'a': 'b'}), ref=f'heads/change')
+            pr = repo.make_pr(title="PR", body=f'first\n***\nsecond',
+                              target='master', head=f'change')
+            repo.post_status(pr.head, 'success', 'legal/cla')
+            repo.post_status(pr.head, 'success', 'ci/runbot')
+            pr.post_comment('hansen r+', config['role_reviewer']['token'])
+        env.run_crons()
+
+        with repo:
+            repo.post_status('heads/staging.master', 'success', 'ci/runbot')
+            repo.post_status('heads/staging.master', 'success', 'legal/cla')
+        env.run_crons()
+
+        head = repo.commit('heads/master')
+        assert head.message == textwrap.dedent(f"""\
+        Commit
+
+        first
+        ***
+        second
+
+        closes {repo.name}#{pr.number}
+
+        Signed-off-by: {reviewer}
+        """).strip(), "squashed / rebased messages should not be stripped"
+
     def test_pr_mergehead(self, repo, env, config):
         """ if the head of the PR is a merge commit and one of the parents is
         in the target, replicate the merge commit instead of merging