From aa2248e379cab7ff24b08997c66247eed84afd9f Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 2 Mar 2020 13:42:07 +0100 Subject: [PATCH] [IMP] forwardport: close command This is useful as the author of the original PR doesn't necessarily have (write) access to the repository where the forward-port PR was created. As a result, while they can r+ the PR they're unable to close it (via github's interface). Since the forwardport bot created the PR, it can also close it, which seems like a useful feature. Closes #341 --- conftest.py | 4 ++-- forwardport/models/project.py | 14 +++++++++++ forwardport/tests/test_weird.py | 41 +++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/conftest.py b/conftest.py index 2bdf05cc..07f76596 100644 --- a/conftest.py +++ b/conftest.py @@ -855,8 +855,8 @@ class PR: def open(self, token=None): self._set_prop('state', 'open', token=token) - def close(self): - self._set_prop('state', 'closed') + def close(self, token=None): + self._set_prop('state', 'closed', token=token) @property def branch(self): diff --git a/forwardport/models/project.py b/forwardport/models/project.py index fe2d8ce9..0c2cd329 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -324,6 +324,20 @@ class PullRequests(models.Model): pr.state = newstate pr.reviewed_by = author # TODO: logging & feedback + elif token == 'close': + close = False + message = "I'm sorry, @{}. I can't close this PR for you." + if self.source_id._pr_acl(author).is_reviewer: + close = True + message = None + + Feedback.create({ + 'repository': self.repository.id, + 'pull_request': self.number, + 'message': message, + 'close': close, + 'token_field': 'fp_github_token', + }) elif token == 'up' and next(tokens, None) == 'to': limit = next(tokens, None) if not self._pr_acl(author).is_author: diff --git a/forwardport/tests/test_weird.py b/forwardport/tests/test_weird.py index 1037a27d..7e884a22 100644 --- a/forwardport/tests/test_weird.py +++ b/forwardport/tests/test_weird.py @@ -557,3 +557,44 @@ def test_new_intermediate_branch(env, config, make_repo): '0': '0', '1': '1', '2': '2', # updates from PRs 'x': 'x', }, "check that new got all the updates (should be in the same state as c really)" + +def test_author_can_close_via_fwbot(env, config, make_repo, users): + project, prod, xxx = make_basic(env, config, make_repo, fp_token=True, fp_remote=True) + other_user = config['role_other'] + other_token = other_user['token'] + other = prod.fork(token=other_token) + + with prod, other: + [c] = other.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/change') + pr = prod.make_pr( + target='a', title='my change', + head=other_user['user'] + ':change', + token=other_token + ) + # should be able to close and open own PR + pr.close(other_token) + pr.open(other_token) + prod.post_status(c, 'success', 'legal/cla') + prod.post_status(c, 'success', 'ci/runbot') + pr.post_comment('%s close' % project.fp_github_name, other_token) + pr.post_comment('hansen r+', config['role_reviewer']['token']) + env.run_crons() + assert pr.state == 'open' + + 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') + assert pr0_id.number == pr.number + pr1 = prod.get_pr(pr1_id.number) + # user can't close PR directly + with prod, pytest.raises(Exception): + pr1.close(other_token) # what the fuck? + # use can close via fwbot + with prod: + pr1.post_comment('%s close' % project.fp_github_name, other_token) + env.run_crons() + assert pr1.state == 'closed' + assert pr1_id.state == 'closed'