[REV] runbot_merge: don't close PRs when deactivating branches

Partially revert 0c882fc0df

This turns out to be more bane than boon, as it breaks forward-port
chains and confuses people (despite the message). Update notification
message and don't close the PR anymore.

While at it, disable any pending staging on the branch being deactivated.

Fixes #654
This commit is contained in:
Xavier Morel 2022-11-02 09:24:46 +01:00
parent b45ecf08f9
commit 5f08100f3a
3 changed files with 16 additions and 15 deletions

View File

@ -0,0 +1,4 @@
REV: don't automatically close PRs when their branch is disabled
Turns out that breaks FW chains and makes existing forward ports harder to
manage, so revert that bit. Do keep sending a message on the PR tho.

View File

@ -258,11 +258,14 @@ class Branch(models.Model):
def write(self, vals):
super().write(vals)
if vals.get('active') is False:
self.active_staging_id.cancel(
"Target branch deactivated by %r.",
self.env.user.login,
)
self.env['runbot_merge.pull_requests.feedback'].create([{
'repository': pr.repository.id,
'pull_request': pr.number,
'close': True,
'message': f'{pr.ping()}the target branch {pr.target.name!r} has been disabled, closing this PR.',
'message': f'{pr.ping()}the target branch {pr.target.name!r} has been disabled, you may want to close this PR.',
} for pr in self.prs])
return True

View File

@ -42,9 +42,15 @@ def test_existing_pr_disabled_branch(env, project, make_repo, setreviewers, conf
branch_id.active = False
env.run_crons()
# the PR should not have been closed implicitly
assert pr_id.state == 'ready'
# but it should be unstaged
assert not pr_id.staging_id
assert not branch_id.active_staging_id
assert staging_id.state == 'cancelled', \
"closing the PRs should have canceled the staging"
assert staging_id.reason == f"Target branch deactivated by 'admin'."
p = pr_page(page, pr)
target = dict(zip(
@ -54,22 +60,10 @@ def test_existing_pr_disabled_branch(env, project, make_repo, setreviewers, conf
assert target.text_content() == 'other (inactive)'
assert target.get('class') == 'text-muted bg-warning'
# the PR should have been closed implicitly
assert pr_id.state == 'closed'
assert not pr_id.staging_id
with repo:
pr.open()
pr.post_comment('hansen r+', config['role_reviewer']['token'])
assert pr_id.state == 'ready', "pr should be reopenable"
env.run_crons()
assert pr.comments == [
(users['reviewer'], "hansen r+"),
seen(env, pr, users),
(users['user'], "@%(user)s @%(reviewer)s the target branch 'other' has been disabled, closing this PR." % users),
(users['reviewer'], "hansen r+"),
(users['user'], "This PR targets the disabled branch %s:other, it needs to be retargeted before it can be merged." % repo.name),
(users['user'], "@%(user)s @%(reviewer)s the target branch 'other' has been disabled, you may want to close this PR." % users),
]
with repo: