From 76ea2a4f5d1f17f92c105ce702cd937d9740b600 Mon Sep 17 00:00:00 2001
From: Xavier Morel <xmo@odoo.com>
Date: Tue, 7 May 2019 13:22:13 +0200
Subject: [PATCH] [FIX] runbot_merge: ensure co-authored-by are at the bottom

Sometimes people add co-authored-by lines in the middle of their
message, where github ignores them.

Since we previously added properly handling existing (correct) C-A-B
lines in the case where we're adding fixes and signed-off-by, we might
as well fix-up existing but mispalced co-authored-by lines.

Fixes #107
---
 runbot_merge/models/pull_requests.py | 16 ++++++++--------
 runbot_merge/tests/test_basic.py     |  7 +++++--
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py
index 0b87a899..be57f995 100644
--- a/runbot_merge/models/pull_requests.py
+++ b/runbot_merge/models/pull_requests.py
@@ -901,18 +901,18 @@ class PullRequests(models.Model):
 
     def _build_merge_message(self, message):
         # handle co-authored commits (https://help.github.com/articles/creating-a-commit-with-multiple-authors/)
-        lines = message.splitlines()
+        original = message.splitlines()
+        lines = []
         coauthors = []
-        for idx, line in enumerate(reversed(lines)):
+        for line in original:
             if line.startswith('Co-authored-by:'):
+                # remove all empty lines before C-A-B
                 coauthors.append(line)
-                continue
-            if not line.strip():
+                while lines and not lines[-1]:
+                    lines.pop()
                 continue
 
-            if idx:
-                del lines[-idx:]
-            break
+            lines.append(line.strip())
 
         m = re.search(r'( |{repository})#{pr.number}\b'.format(
             pr=self,
@@ -925,7 +925,7 @@ class PullRequests(models.Model):
 
         if coauthors:
             lines.extend(['', ''])
-            lines.extend(reversed(coauthors))
+            lines.extend(coauthors)
         return '\n'.join(lines)
 
     def _stage(self, gh, target):
diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py
index e1a11d60..04b560d1 100644
--- a/runbot_merge/tests/test_basic.py
+++ b/runbot_merge/tests/test_basic.py
@@ -220,10 +220,13 @@ class TestCommitMessage:
 
     def test_commit_coauthored(self, env, repo, users):
         """ verify 'closes ...' and 'Signed-off-by' are added before co-authored-by tags.
+
+        Also checks that all co-authored-by are moved at the end of the
+        message
         """
         c1 = repo.make_commit(None, 'first!', None, tree={'f': 'm1'})
         repo.make_ref('heads/master', c1)
-        c2 = repo.make_commit(c1, 'simple commit message\n\n\nCo-authored-by: Bob <bob@example.com>', None, tree={'f': 'm2'})
+        c2 = repo.make_commit(c1, 'simple commit message\n\n\nCo-authored-by: Bob <bob@example.com>\n\nFixes a thing', None, tree={'f': 'm2'})
 
         prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user')
         repo.post_status(prx.head, 'success', 'ci/runbot')
@@ -237,7 +240,7 @@ class TestCommitMessage:
         run_crons(env)
 
         master = repo.commit('heads/master')
-        assert master.message == "simple commit message\n\ncloses {repo.name}#1"\
+        assert master.message == "simple commit message\n\nFixes a thing\n\ncloses {repo.name}#1"\
                                  "\n\nSigned-off-by: {reviewer.formatted_email}"\
                                  "\n\n\nCo-authored-by: Bob <bob@example.com>"\
                                  .format(repo=repo, reviewer=get_partner(env, users['reviewer']))