diff --git a/runbot_merge/controllers/__init__.py b/runbot_merge/controllers/__init__.py index 60692c3f..7fe99d9d 100644 --- a/runbot_merge/controllers/__init__.py +++ b/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': - feedback( - close=True, - message=env.ref('runbot_merge.handle.pr.merged')._format(event=event), - ) + if pr_obj.merge_date: + if pr_obj.state == 'merged': + 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, }) diff --git a/runbot_merge/data/runbot_merge.pull_requests.feedback.template.csv b/runbot_merge/data/runbot_merge.pull_requests.feedback.template.csv index b140ec43..543459de 100644 --- a/runbot_merge/data/runbot_merge.pull_requests.feedback.template.csv +++ b/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" diff --git a/runbot_merge/tests/test_batch_consistency.py b/runbot_merge/tests/test_batch_consistency.py index 0cc3b8b0..d5516786 100644 --- a/runbot_merge/tests/test_batch_consistency.py +++ b/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.'), + ]