[IMP] runbot_merge: a PR can't be reopened if its batch is merged

In that case, ignore the reopen, close the PR, and tell the idiot to
fuck off.

Also the case where a PR is reopened while its batch is staged was
already handled, but probably not tested: it was implicit in
forcefully updating the HEAD of the PR, which triggers an unstage
since c8a06601a7.

Now that scenario is tested, which should lower the odds of breaking
it in the future.

Fixes #965

runbot_merge/controllers/__init__.py

@@ -334,16 +334,17 @@ def handle_pr(env, event):
         return 'Ignored: could not lock rows (probably being merged)'
 
     if event['action'] == 'reopened' :
-        if pr_obj.state == 'merged':
+        if pr_obj.merge_date:
-            feedback(
+            if pr_obj.state == 'merged':
-                close=True,
+                message = env.ref('runbot_merge.handle.pr.merged')._format(event=event)
-                message=env.ref('runbot_merge.handle.pr.merged')._format(event=event),
+            else:
-            )
+                message = env.ref('runbot_merge.handle.pr.mergedbatch')._format(event=event)
+            feedback(close=True, message=message)
         elif pr_obj.closed:
             _logger.info('%s reopening %s', event['sender']['login'], pr_obj.display_name)
             pr_obj.write({
                 'closed': False,
-                # updating the head triggers a revalidation
+                # updating the head triggers a revalidation, and unstages the batch
                 'head': pr['head']['sha'],
                 'squash': pr['commits'] == 1,
             })

runbot_merge/data/runbot_merge.pull_requests.feedback.template.csv

@@ -11,14 +11,17 @@ branch: branch (ref) name
 event: complete pr event"
 runbot_merge.handle.pr.merged,@{event[sender][login]} ya silly goose you can't reopen a merged PR.,"Notifies that a user tried to reopen a merged PR.
 
-Event: complete PR event"
+event: complete PR event"
+runbot_merge.handle.pr.mergedbatch,"Reopening a PR in a merged batch is not allowed, create a new PR.","Notifies that a user tried to reopen a closed PR whose batch is merged.
+
+event: complete PR event"
 runbot_merge.pr.load.unmanaged,"Branch `{pr[base][ref]}` is not within my remit, imma just ignore it.","Notifies that a user tried to load a PR targeting a non-handled branch.
 
 pr: pull request (github object)
 Repository: repository object (???)"
 runbot_merge.pr.load.fetched,"{ping}I didn't know about this PR and had to retrieve its information, you may have to re-approve it as I didn't see previous commands.","Notifies that we did retrieve an unknown PR (either by request or as side effect of an interaction).
 
-Pr: pr object we just created"
+pr: pr object we just created"
 runbot_merge.pr.branch.disabled,"{pr.ping}the target branch {pr.target.name!r} has been disabled, you may want to close this PR.","Notifies that the target branch for this PR was deactivated.
 
 pr: pull request in question"

runbot_merge/tests/test_batch_consistency.py

@@ -3,7 +3,7 @@ without wider relevance and thus other location.
 """
 import pytest
 
-from utils import Commit, to_pr, pr_page
+from utils import Commit, to_pr, pr_page, seen
 
 
 def test_close_single(env, repo):
@@ -199,3 +199,110 @@ Inconsistent targets:
     env.run_crons()
 
     assert env['runbot_merge.stagings'].search_count([])
+
+def test_reopen_pr_in_staged_batch(env, project, make_repo2, config):
+    """Reopening a closed PR from a staged batch should cancel the staging
+    """
+    repo1 = make_repo2('a')
+    repo2 = make_repo2('b')
+
+    with repo1:
+        [m1, _] = repo1.make_commits(
+            None,
+            Commit('a', tree={'a': 'a'}),
+            Commit('b', tree={'b': 'b'}),
+            ref='heads/p',
+        )
+        repo1.make_ref('heads/master', m1)
+        pr1 = repo1.make_pr(target='master', head='p')
+    with repo2:
+        [m2, _] = repo2.make_commits(
+            None,
+            Commit('a', tree={'a': 'a'}),
+            Commit('b', tree={'b': 'b'}),
+            ref='heads/p',
+        )
+        repo2.make_ref('heads/master', m2)
+        pr2 = repo2.make_pr(target='master', head='p')
+
+    pr1_id = to_pr(env, pr1)
+    pr2_id = to_pr(env, pr2)
+    batch_id = pr1_id.batch_id
+    assert batch_id
+    assert batch_id == pr2_id.batch_id
+
+    with repo1:
+        repo1.post_status(pr1.head, 'success')
+        pr1.post_comment("hansen r+", config['role_reviewer']['token'])
+    with repo2:
+        pr2.close()
+
+    env.run_crons(None)
+
+    assert pr2_id.state == 'closed'
+    assert batch_id.staging_ids.filtered(lambda s: s.active)
+
+    with repo2:
+        pr2.open()
+    assert pr2_id.state == 'opened'
+    assert not batch_id.staging_ids.filtered(lambda s: s.active)
+    assert batch_id.blocked
+
+def test_reopen_pr_in_merged_batch(env, project, make_repo2, config, users):
+    """If the batch is merged, the pr should just be re-closed with a message
+    """
+    repo1 = make_repo2('a')
+    repo2 = make_repo2('b')
+
+    with repo1:
+        [m1, _] = repo1.make_commits(
+            None,
+            Commit('a', tree={'a': 'a'}),
+            Commit('b', tree={'b': 'b'}),
+            ref='heads/p',
+        )
+        repo1.make_ref('heads/master', m1)
+        pr1 = repo1.make_pr(target='master', head='p')
+    with repo2:
+        [m2, _] = repo2.make_commits(
+            None,
+            Commit('a', tree={'a': 'a'}),
+            Commit('b', tree={'b': 'b'}),
+            ref='heads/p',
+        )
+        repo2.make_ref('heads/master', m2)
+        pr2 = repo2.make_pr(target='master', head='p')
+
+    pr1_id = to_pr(env, pr1)
+    pr2_id = to_pr(env, pr2)
+    batch_id = pr1_id.batch_id
+    assert batch_id
+    assert batch_id == pr2_id.batch_id
+
+    with repo1:
+        repo1.post_status(pr1.head, 'success')
+        pr1.post_comment("hansen r+", config['role_reviewer']['token'])
+    with repo2:
+        pr2.close()
+
+    env.run_crons(None)
+
+    with repo1, repo2:
+        repo1.post_status('staging.master', 'success')
+        repo2.post_status('staging.master', 'success')
+    env.run_crons(None)
+
+
+    assert pr1_id.state == 'merged'
+    assert pr2_id.state == 'closed'
+
+    with repo2:
+        pr2.open()
+    env.run_crons(None)
+
+    assert pr2_id.closed
+    assert pr2_id.state == 'closed'
+    assert pr2.comments == [
+        seen(env, pr2, users),
+        (users['user'], 'Reopening a PR in a merged batch is not allowed, create a new PR.'),
+    ]