[FIX] forwardport: allow delegates on fw PRs to review followups

The forward-port process currently automatically adds delegates of a
PR as delegates on its forward ports, but that only works for
the *source* pull request.

If a delegate is added to a forward-port, they were not able to
approve the followups to that initial port, which makes little sense.

Fixes #548

forwardport/models/project.py

@@ -326,18 +326,6 @@ class PullRequests(models.Model):
                         'token_field': 'fp_github_token',
                     })
                     continue
-                if not self.source_id._pr_acl(author).is_reviewer:
-                    Feedback.create({
-                        'repository': self.repository.id,
-                        'pull_request': self.number,
-                        'message': "I'm sorry, @{}. You can't review this PR: "
-                                   "you need to be a reviewer on {}.".format(
-                            login,
-                            self.source_id.display_name
-                        ),
-                        'token_field': 'fp_github_token',
-                    })
-                    continue
                 merge_bot = self.repository.project_id.github_prefix
                 # don't update the root ever
                 for pr in (p for p in self._iter_ancestors() if p.parent_id if p.state in RPLUS):
@@ -675,16 +663,16 @@ class PullRequests(models.Model):
             _logger.info("Created forward-port PR %s", new_pr)
             new_batch |= new_pr
 
-            # delegate original author on merged original PR & on new PR so
+            # allows PR author to close or skipci
-            # they can r+ the forward ports (via mergebot or forwardbot)
             source.delegates |= source.author
             new_pr.write({
                 'merge_method': pr.merge_method,
                 'source_id': source.id,
                 # only link to previous PR of sequence if cherrypick passed
                 'parent_id': pr.id if not has_conflicts else False,
-                # copy all delegates of source to new
+                # Copy author & delegates of source as well as delegates of
-                'delegates': [(6, False, source.delegates.ids)]
+                # previous so they can r+ the new forward ports.
+                'delegates': [(6, False, (source.delegates | pr.delegates).ids)]
             })
             if has_conflicts and pr.parent_id and pr.state not in ('merged', 'closed'):
                 message = source._pingline() + """

forwardport/tests/test_simple.py

@@ -499,6 +499,81 @@ def signoff(conf, message):
     raise AssertionError("Failed to find signoff by %s in %s" % (conf, message))
 
 
+def test_delegate_fw(env, config, make_repo, users):
+    """If a user is delegated *on a forward port* they should be able to approve
+    *the followup*.
+    """
+    prod, _ = make_basic(env, config, make_repo)
+    # create a partner for `other` so we can put an email on it
+    env['res.partner'].create({
+        'name': users['other'],
+        'github_login': users['other'],
+        'email': 'other@example.org',
+    })
+    author_token = config['role_self_reviewer']['token']
+    fork = prod.fork(token=author_token)
+    with prod, fork:
+        [c] = fork.make_commits('a', Commit('c_0', tree={'y': '0'}), ref='heads/accessrights')
+        pr = prod.make_pr(
+            target='a', title='my change',
+            head=users['self_reviewer'] + ':accessrights',
+            token=author_token,
+        )
+        prod.post_status(c, 'success', 'legal/cla')
+        prod.post_status(c, 'success', 'ci/runbot')
+        pr.post_comment('hansen r+', token=config['role_reviewer']['token'])
+    env.run_crons()
+
+    with prod:
+        prod.post_status('staging.a', 'success', 'legal/cla')
+        prod.post_status('staging.a', 'success', 'ci/runbot')
+    env.run_crons()
+
+    # ensure pr1 has to be approved to be forward-ported
+    _, pr1_id = env['runbot_merge.pull_requests'].search([], order='number')
+    # detatch from source
+    pr1_id.parent_id = False
+    with prod:
+        prod.post_status(pr1_id.head, 'success', 'legal/cla')
+        prod.post_status(pr1_id.head, 'success', 'ci/runbot')
+    env.run_crons()
+    pr1 = prod.get_pr(pr1_id.number)
+    # delegate review to "other" consider PR fixed, and have "other" approve it
+    with prod:
+        pr1.post_comment('hansen delegate=' + users['other'],
+                         token=config['role_reviewer']['token'])
+        prod.post_status(pr1_id.head, 'success', 'ci/runbot')
+        pr1.post_comment('hansen r+', token=config['role_other']['token'])
+    env.run_crons()
+
+    with prod:
+        prod.post_status('staging.b', 'success', 'legal/cla')
+        prod.post_status('staging.b', 'success', 'ci/runbot')
+    env.run_crons()
+
+    _, _, pr2_id = env['runbot_merge.pull_requests'].search([], order='number')
+    pr2 = prod.get_pr(pr2_id.number)
+    # make "other" also approve this one
+    with prod:
+        prod.post_status(pr2_id.head, 'success', 'ci/runbot')
+        prod.post_status(pr2_id.head, 'success', 'legal/cla')
+        pr2.post_comment('hansen r+', token=config['role_other']['token'])
+    env.run_crons()
+
+    assert pr2.comments == [
+        seen(env, pr2, users),
+        (users['user'], '''Ping @{self_reviewer}, @{reviewer}
+This PR targets c and is the last of the forward-port chain.
+
+To merge the full chain, say
+> @{user} r+
+
+More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
+'''.format_map(users)),
+        (users['other'], 'hansen r+')
+    ]
+
+
 def test_redundant_approval(env, config, make_repo, users):
     """If a forward port sequence has been partially approved, fw-bot r+ should
     not perform redundant approval as that triggers warning messages.