[FIX] forwardport: don't notify of detached child on merged parents

The notification is both noise and confusing: we're telling the
author (and reviewer, and anyone else subscribed) that they need to
merge a merged PR.

Fixes #855

forwardport/models/project.py

@@ -363,12 +363,13 @@ class PullRequests(models.Model):
                     token_field='fp_github_token',
                     format_args={'pr': p},
                 )
-                self.env.ref('runbot_merge.forwardport.update.parent')._send(
+                if parent.state not in ('closed', 'merged'):
-                    repository=parent.repository,
+                    self.env.ref('runbot_merge.forwardport.update.parent')._send(
-                    pull_request=parent.number,
+                        repository=parent.repository,
-                    token_field='fp_github_token',
+                        pull_request=parent.number,
-                    format_args={'pr': parent, 'child': p},
+                        token_field='fp_github_token',
-                )
+                        format_args={'pr': parent, 'child': p},
+                    )
         for p in closed_fp.filtered(lambda p: p.state != 'closed'):
             self.env.ref('runbot_merge.forwardport.reopen.detached')._send(
                 repository=p.repository,

forwardport/tests/test_updates.py

@@ -4,10 +4,13 @@ initial merge has succeeded (and forward-porting has started)
 """
 import re
 
+import pytest
+
 from utils import seen, re_matches, Commit, make_basic, to_pr
 
 
-def test_update_pr(env, config, make_repo, users):
+@pytest.mark.parametrize("merge_parent", [False, True])
+def test_update_pr(env, config, make_repo, users, merge_parent) -> None:
     """ Even for successful cherrypicks, it's possible that e.g. CI doesn't
     pass or the reviewer finds out they need to update the code.
 
@@ -134,7 +137,18 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
         prod.post_status(pr2_id.head, 'success', 'ci/runbot')
         prod.post_status(pr2_id.head, 'success', 'legal/cla')
     env.run_crons()
-    
+
+    pr2 = prod.get_pr(pr2_id.number)
+    if merge_parent:
+        with prod:
+            pr2.post_comment('hansen r+', config['role_reviewer']['token'])
+        env.run_crons()
+        with prod:
+            prod.post_status('staging.c', 'success', 'ci/runbot')
+            prod.post_status('staging.c', 'success', 'legal/cla')
+        env.run_crons()
+        assert pr2_id.state == 'merged'
+
     _0, _1, _2, pr3_id = env['runbot_merge.pull_requests'].search([], order='number')
     assert pr3_id.parent_id == pr2_id
     # don't bother updating heads (?)
@@ -160,21 +174,29 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
                         f"(via @{pr3_id.repository.project_id.github_prefix})"
         )
     ]
-    pr2 = prod.get_pr(pr2_id.number)
+
-    assert pr2.comments == [
+    assert pr2.comments[:2] == [
         seen(env, pr2, users),
         (users['user'], """\
 This PR targets c and is part of the forward-port chain. Further PRs will be created up to d.
 
 More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
 """),
-        (users['user'], f"@{users['user']} @{users['reviewer']} child PR "
-                        f"{pr3_id.display_name} was modified / updated and has "
-                        f"become a normal PR. This PR (and any of its parents) "
-                        f"will need to be merged independently as approvals "
-                        f"won't cross."),
     ]
-    
+
+    if merge_parent:
+        assert pr2.comments[2:] == [
+            (users['reviewer'], "hansen r+"),
+        ]
+    else:
+        assert pr2.comments[2:] == [
+            (users['user'], f"@{users['user']} @{users['reviewer']} child PR "
+                            f"{pr3_id.display_name} was modified / updated and has "
+                            f"become a normal PR. This PR (and any of its parents) "
+                            f"will need to be merged independently as approvals "
+                            f"won't cross."),
+        ]
+
 
 def test_update_merged(env, make_repo, config, users):
     """ Strange things happen when an FP gets closed / merged but then its