diff --git a/forwardport/models/project.py b/forwardport/models/project.py index 8b3e1f28..ed52bea5 100644 --- a/forwardport/models/project.py +++ b/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 - # they can r+ the forward ports (via mergebot or forwardbot) + # allows PR author to close or skipci 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 - 'delegates': [(6, False, source.delegates.ids)] + # Copy author & delegates of source as well as delegates of + # 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() + """ diff --git a/forwardport/tests/test_simple.py b/forwardport/tests/test_simple.py index fbce44d0..871d6904 100644 --- a/forwardport/tests/test_simple.py +++ b/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.