From 5f08100f3adacb4f51e79093843095d9efc52d9d Mon Sep 17 00:00:00 2001
From: Xavier Morel <xmo@odoo.com>
Date: Wed, 2 Nov 2022 09:24:46 +0100
Subject: [PATCH] [REV] runbot_merge: don't close PRs when deactivating
 branches

Partially revert 0c882fc0df3f342851ed2399d34565c4477c3b55

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
---
 runbot_merge/changelog/2022-10/branch.md   |  4 ++++
 runbot_merge/models/pull_requests.py       |  7 +++++--
 runbot_merge/tests/test_disabled_branch.py | 20 +++++++-------------
 3 files changed, 16 insertions(+), 15 deletions(-)
 create mode 100644 runbot_merge/changelog/2022-10/branch.md

diff --git a/runbot_merge/changelog/2022-10/branch.md b/runbot_merge/changelog/2022-10/branch.md
new file mode 100644
index 00000000..b745d15f
--- /dev/null
+++ b/runbot_merge/changelog/2022-10/branch.md
@@ -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.
diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py
index 0b6168e1..783456a4 100644
--- a/runbot_merge/models/pull_requests.py
+++ b/runbot_merge/models/pull_requests.py
@@ -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
 
diff --git a/runbot_merge/tests/test_disabled_branch.py b/runbot_merge/tests/test_disabled_branch.py
index 1d6be201..2cff73ce 100644
--- a/runbot_merge/tests/test_disabled_branch.py
+++ b/runbot_merge/tests/test_disabled_branch.py
@@ -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: