From eb91e2371b0deff7806014b5abbd52b58bd14ef2 Mon Sep 17 00:00:00 2001
From: Xavier Morel <xmo@odoo.com>
Date: Tue, 7 May 2019 12:32:02 +0200
Subject: [PATCH] [FIX] runbot_merge: check CI status when reopening a PR

Previously, creating a PR would validate the head (in case it had
already passed CI) but reopening it would not, which is inconvenient
as the CI would not automatically run on a reopened PR.

Update both the state and the head of the PR on reopen to force a
revalidation, that way if the head has already passed CI the PR will
be reopened validated and there won't be an unclear need to perform an
explicit CI run.

Fixes #119
---
 runbot_merge/controllers/__init__.py |  9 ++++++++-
 runbot_merge/tests/test_basic.py     | 27 +++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/runbot_merge/controllers/__init__.py b/runbot_merge/controllers/__init__.py
index 33aeb319..3a0ce2ad 100644
--- a/runbot_merge/controllers/__init__.py
+++ b/runbot_merge/controllers/__init__.py
@@ -193,6 +193,7 @@ def handle_pr(env, event):
         WHERE id = %s AND state != 'merged'
         ''', [pr_obj.id])
         env.cr.commit()
+        pr_obj.invalidate_cache(fnames=['state'], ids=[pr_obj.id])
         if env.cr.rowcount:
             env['runbot_merge.pull_requests.tagging'].create({
                 'pull_request': pr_obj.number,
@@ -205,10 +206,16 @@ def handle_pr(env, event):
                 pr_obj.repository.name, pr_obj.number,
                 event['sender']['login']
             )
+
         return 'Closed {}'.format(pr_obj.id)
 
     if event['action'] == 'reopened' and pr_obj.state == 'closed':
-        pr_obj.state = 'opened'
+        pr_obj.write({
+            'state': 'opened',
+            # updating the head triggers a revalidation
+            'head': pr['head']['sha'],
+        })
+
         return 'Reopened {}'.format(pr_obj.id)
 
     _logger.info("Ignoring event %s on PR %s", event['action'], pr['number'])
diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py
index 051d8ed0..e1a11d60 100644
--- a/runbot_merge/tests/test_basic.py
+++ b/runbot_merge/tests/test_basic.py
@@ -778,6 +778,33 @@ def test_ci_failure_after_review(env, repo, users):
         (users['user'], "'ci/runbot' failed on this reviewed PR.".format_map(users)),
     ]
 
+def test_reopen_state(env, repo):
+    """ The PR should be validated on opening and reopening in case there's
+    already a CI+ stored (as the CI might never trigger unless explicitly
+    re-requested)
+    """
+    m = repo.make_commit(None, 'initial', None, tree={'m': 'm'})
+    repo.make_ref('heads/master', m)
+
+    c = repo.make_commit(m, 'fist', None, tree={'m': 'c1'})
+    repo.post_status(c, 'success', 'legal/cla')
+    repo.post_status(c, 'success', 'ci/runbot')
+    prx = repo.make_pr('title', 'body', target='master', ctid=c, user='user')
+
+    pr = env['runbot_merge.pull_requests'].search([
+        ('repository.name', '=', repo.name),
+        ('number', '=', prx.number),
+    ])
+    assert pr.state == 'validated', \
+        "if a PR is created on a CI'd commit, it should be validated immediately"
+
+    prx.close()
+    assert pr.state == 'closed'
+
+    prx.open()
+    assert pr.state == 'validated', \
+        "if a PR is reopened and had a CI'd head, it should be validated immediately"
+
 class TestRetry:
     @pytest.mark.xfail(reason="This may not be a good idea as it could lead to tons of rebuild spam")
     def test_auto_retry_push(self, env, repo):