[FIX] forwardport: don't re-approve approved PRs during an fw-bot r+

When using the forwardport's shortcut, the bot would not skip
already-approved PRs leading to a warning from the mergebot that the
PR was already approved (out of nowhere which was weird).

During the walk to the ancestors, skip any PR which is not
approvable (either already approved or in a state where that makes no
sense e.g. closed).

Fixes #543

forwardport/models/project.py

@@ -34,6 +34,7 @@ from odoo.exceptions import UserError
 from odoo.tools import topological_sort, groupby
 from odoo.tools.appdirs import user_cache_dir
 from odoo.addons.runbot_merge import utils
+from odoo.addons.runbot_merge.models.pull_requests import RPLUS
 
 footer = '\nMore info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port\n'
 
@@ -339,7 +340,7 @@ class PullRequests(models.Model):
                     continue
                 merge_bot = self.repository.project_id.github_prefix
                 # don't update the root ever
-                for pr in filter(lambda p: p.parent_id, self._iter_ancestors()):
+                for pr in (p for p in self._iter_ancestors() if p.parent_id if p.state in RPLUS):
                     # only the author is delegated explicitely on the
                     pr._parse_commands(author, {**comment, 'body': merge_bot + ' r+'}, login)
             elif token == 'close':

forwardport/tests/test_simple.py

@@ -498,6 +498,54 @@ def signoff(conf, message):
             return signoff
     raise AssertionError("Failed to find signoff by %s in %s" % (conf, message))
 
+
+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.
+    """
+    prod, _ = make_basic(env, config, make_repo)
+    [project] = env['runbot_merge.project'].search([])
+    with prod:
+        prod.make_commits(
+            'a', Commit('p', tree={'x': '0'}),
+            ref='heads/early'
+        )
+        pr0 = prod.make_pr(target='a', head='early')
+        prod.post_status('heads/early', 'success', 'legal/cla')
+        prod.post_status('heads/early', 'success', 'ci/runbot')
+        pr0.post_comment('hansen r+', 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()
+    pr0_id, pr1_id = env['runbot_merge.pull_requests'].search([], order='number asc')
+    with prod:
+        prod.post_status(pr1_id.head, 'success', 'legal/cla')
+        prod.post_status(pr1_id.head, 'success', 'ci/runbot')
+    env.run_crons()
+
+    _, _, pr2_id = env['runbot_merge.pull_requests'].search([], order='number asc')
+    assert pr2_id.parent_id == pr1_id
+    assert pr1_id.parent_id == pr0_id
+
+    pr1 = prod.get_pr(pr1_id.number)
+    pr2 = prod.get_pr(pr2_id.number)
+    with prod:
+        pr1.post_comment('hansen r+', config['role_reviewer']['token'])
+    with prod:
+        pr2.post_comment(f'{project.fp_github_name} r+', config['role_reviewer']['token'])
+    env.run_crons()
+
+    assert pr1.comments == [
+        seen(env, pr1, users),
+        (users['user'], 'This PR targets b and is part of the forward-port chain. '
+                        'Further PRs will be created up to c.\n\n'
+                        'More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port\n'),
+        (users['reviewer'], 'hansen r+'),
+    ]
+
+
 def test_batched(env, config, make_repo, users):
     """ Tests for projects with multiple repos & sync'd branches. Batches
     should be FP'd to batches