[ADD] forwardport: message on parent of detached PR

Currently a user is not notified that the parent of a detached PR
needs to be independently approved and may miss that information. Add
a notification to *that* PR as well.

Fixes #788

forwardport/models/project.py

@@ -29,7 +29,7 @@ from pathlib import Path
 import dateutil.relativedelta
 import requests
 
-from odoo import _, models, fields, api
+from odoo import models, fields, api
 from odoo.osv import expression
 from odoo.exceptions import UserError
 from odoo.tools.misc import topological_sort, groupby
@@ -303,7 +303,11 @@ class PullRequests(models.Model):
         # also a bit odd to only handle updating 1 head at a time, but then
         # again 2 PRs with same head is weird so...
         newhead = vals.get('head')
-        with_parents = self.filtered('parent_id')
+        with_parents = {
+            p: p.parent_id
+            for p in self
+            if p.parent_id
+        }
         closed_fp = self.filtered(lambda p: p.state == 'closed' and p.source_id)
         if newhead and not self.env.context.get('ignore_head_update') and newhead != self.head:
             vals.setdefault('parent_id', False)
@@ -323,14 +327,21 @@ class PullRequests(models.Model):
             vals['merge_date'] = fields.Datetime.now()
         r = super().write(vals)
         if self.env.context.get('forwardport_detach_warn', True):
-            for p in with_parents:
+            for p, parent in with_parents.items():
-                if not p.parent_id:
+                if p.parent_id:
-                    self.env.ref('runbot_merge.forwardport.update.detached')._send(
+                    continue
-                        repository=p.repository,
+                self.env.ref('runbot_merge.forwardport.update.detached')._send(
-                        pull_request=p.number,
+                    repository=p.repository,
-                        token_field='fp_github_token',
+                    pull_request=p.number,
-                        format_args={'pr': p},
+                    token_field='fp_github_token',
-                    )
+                    format_args={'pr': p},
+                )
+                self.env.ref('runbot_merge.forwardport.update.parent')._send(
+                    repository=parent.repository,
+                    pull_request=parent.number,
+                    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,
@@ -927,11 +938,8 @@ class PullRequests(models.Model):
             head_commit = commits[-1]['commit']
 
             to_tuple = operator.itemgetter('name', 'email')
-            to_dict = lambda term, vals: {
+            def to_dict(term, vals):
-                'GIT_%s_NAME' % term: vals[0],
+                return {'GIT_%s_NAME' % term: vals[0], 'GIT_%s_EMAIL' % term: vals[1], 'GIT_%s_DATE' % term: vals[2]}
-                'GIT_%s_EMAIL' % term: vals[1],
-                'GIT_%s_DATE' % term: vals[2],
-            }
             authors, committers = set(), set()
             for c in (c['commit'] for c in commits):
                 authors.add(to_tuple(c['author']))

forwardport/tests/test_updates.py

@@ -15,6 +15,14 @@ def test_update_pr(env, config, make_repo, users):
     only this one and its dependent should be updated?
     """
     prod, _ = make_basic(env, config, make_repo)
+    # create a branch d from c so we can have 3 forward ports PRs, not just 2,
+    # for additional checks
+    env['runbot_merge.project'].search([]).write({
+        'branch_ids': [(0, 0, {'name': 'd', 'sequence': 40})]
+    })
+    with prod:
+        prod.make_commits('c', Commit('1111', tree={'i': 'a'}), ref='heads/d')
+    
     with prod:
         [p_1] = prod.make_commits(
             'a',
@@ -37,7 +45,7 @@ def test_update_pr(env, config, make_repo, users):
     pr0_id, pr1_id = env['runbot_merge.pull_requests'].search([], order='number')
 
     fp_intermediate = (users['user'], '''\
-This PR targets b and is part of the forward-port chain. Further PRs will be created up to c.
+This PR targets b 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
 ''')
@@ -121,6 +129,52 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
         'h': 'a',
         'x': '5'
     }, "the followup FP should also have the update"
+    
+    with prod:
+        prod.post_status(pr2_id.head, 'success', 'ci/runbot')
+        prod.post_status(pr2_id.head, 'success', 'legal/cla')
+    env.run_crons()
+    
+    _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 (?)
+    pr3_id.write({'parent_id': False, 'detach_reason': "testing"})
+    # pump feedback messages
+    env.run_crons()
+    
+    pr3 = prod.get_pr(pr3_id.number)
+    assert pr3.comments == [
+        seen(env, pr3, users),
+        (users['user'], f"""\
+@{users['user']} @{users['reviewer']} this PR targets d and is the last of the forward-port chain containing:
+* {pr2_id.display_name}
+
+To merge the full chain, use
+> @{pr3_id.repository.project_id.fp_github_name} r+
+
+More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
+"""),
+        (users['user'], f"@{users['user']} @{users['reviewer']} this PR was "
+                        f"modified / updated and has become a normal PR. It "
+                        f"should be merged the normal way "
+                        f"(via @{pr3_id.repository.project_id.github_prefix})"
+        )
+    ]
+    pr2 = prod.get_pr(pr2_id.number)
+    assert pr2.comments == [
+        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."),
+    ]
+    
 
 def test_update_merged(env, make_repo, config, users):
     """ Strange things happen when an FP gets closed / merged but then its

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

@@ -110,6 +110,10 @@ stderr: markdown-formatted stderr of git, if any"
 runbot_merge.forwardport.update.detached,{pr.ping}this PR was modified / updated and has become a normal PR. It should be merged the normal way (via @{pr.repository.project_id.github_prefix}),"Comment when a forwardport PR gets updated, documents that the PR now needs to be merged the “normal” way.
 
 pr: the pr in question "
+runbot_merge.forwardport.update.parent,{pr.ping}child PR {child.display_name} was modified / updated and has become a normal PR. This PR (and any of its parents) will need to be merged independently as approvals won't cross.,"Sent to an open PR when its direct child has been detached.
+
+pr: the pr
+child: its detached child"
 runbot_merge.forwardport.reopen.detached,{pr.ping}this PR was closed then reopened. It should be merged the normal way (via @{pr.repository.project_id.github_prefix}),"Comment when a forwardport PR gets closed then reopened, documents that the PR is now in a detached state.
 
 pr: the pr in question"