diff --git a/rhodecode/api/tests/test_merge_pull_request.py b/rhodecode/api/tests/test_merge_pull_request.py --- a/rhodecode/api/tests/test_merge_pull_request.py +++ b/rhodecode/api/tests/test_merge_pull_request.py @@ -134,6 +134,107 @@ class TestMergePullRequest(object): assert_error(id_, expected, given=response.body) @pytest.mark.backends("git", "hg") + def test_api_merge_pull_request_as_another_user_no_perms_to_merge( + self, pr_util, no_notifications, user_util): + merge_user = user_util.create_user() + merge_user_id = merge_user.user_id + merge_user_username = merge_user.username + + pull_request = pr_util.create_pull_request(mergeable=True, approved=True) + + pull_request_id = pull_request.pull_request_id + pull_request_repo = pull_request.target_repo.repo_name + + id_, params = build_data( + self.apikey, 'comment_pull_request', + repoid=pull_request_repo, + pullrequestid=pull_request_id, + status='approved') + + response = api_call(self.app, params) + expected = { + 'comment_id': response.json.get('result', {}).get('comment_id'), + 'pull_request_id': pull_request_id, + 'status': {'given': 'approved', 'was_changed': True} + } + assert_ok(id_, expected, given=response.body) + id_, params = build_data( + self.apikey, 'merge_pull_request', + repoid=pull_request_repo, + pullrequestid=pull_request_id, + userid=merge_user_id + ) + + response = api_call(self.app, params) + expected = 'merge not possible for following reasons: User `{}` ' \ + 'not allowed to perform merge.'.format(merge_user_username) + assert_error(id_, expected, response.body) + + @pytest.mark.backends("git", "hg") + def test_api_merge_pull_request_as_another_user(self, pr_util, no_notifications, user_util): + merge_user = user_util.create_user() + merge_user_id = merge_user.user_id + pull_request = pr_util.create_pull_request(mergeable=True, approved=True) + user_util.grant_user_permission_to_repo( + pull_request.target_repo, merge_user, 'repository.write') + author = pull_request.user_id + repo = pull_request.target_repo.repo_id + pull_request_id = pull_request.pull_request_id + pull_request_repo = pull_request.target_repo.repo_name + + id_, params = build_data( + self.apikey, 'comment_pull_request', + repoid=pull_request_repo, + pullrequestid=pull_request_id, + status='approved') + + response = api_call(self.app, params) + expected = { + 'comment_id': response.json.get('result', {}).get('comment_id'), + 'pull_request_id': pull_request_id, + 'status': {'given': 'approved', 'was_changed': True} + } + assert_ok(id_, expected, given=response.body) + + id_, params = build_data( + self.apikey, 'merge_pull_request', + repoid=pull_request_repo, + pullrequestid=pull_request_id, + userid=merge_user_id + ) + + response = api_call(self.app, params) + + pull_request = PullRequest.get(pull_request_id) + + expected = { + 'executed': True, + 'failure_reason': 0, + 'merge_status_message': 'This pull request can be automatically merged.', + 'possible': True, + 'merge_commit_id': pull_request.shadow_merge_ref.commit_id, + 'merge_ref': pull_request.shadow_merge_ref._asdict() + } + + assert_ok(id_, expected, response.body) + + journal = UserLog.query() \ + .filter(UserLog.user_id == merge_user_id) \ + .filter(UserLog.repository_id == repo) \ + .order_by('user_log_id') \ + .all() + assert journal[-2].action == 'repo.pull_request.merge' + assert journal[-1].action == 'repo.pull_request.close' + + id_, params = build_data( + self.apikey, 'merge_pull_request', + repoid=pull_request_repo, pullrequestid=pull_request_id, userid=merge_user_id) + response = api_call(self.app, params) + + expected = 'merge not possible for following reasons: This pull request is closed.' + assert_error(id_, expected, given=response.body) + + @pytest.mark.backends("git", "hg") def test_api_merge_pull_request_repo_error(self, pr_util): pull_request = pr_util.create_pull_request() id_, params = build_data( diff --git a/rhodecode/api/views/pull_request_api.py b/rhodecode/api/views/pull_request_api.py --- a/rhodecode/api/views/pull_request_api.py +++ b/rhodecode/api/views/pull_request_api.py @@ -285,12 +285,13 @@ def merge_pull_request( repo = get_repo_or_error(repoid) else: repo = pull_request.target_repo - + auth_user = apiuser if not isinstance(userid, Optional): if (has_superadmin_permission(apiuser) or HasRepoPermissionAnyApi('repository.admin')( user=apiuser, repo_name=repo.repo_name)): apiuser = get_user_or_error(userid) + auth_user = apiuser.AuthUser() else: raise JSONRPCError('userid is not the same as your user') @@ -301,9 +302,8 @@ def merge_pull_request( pull_request.pull_request_state, PullRequest.STATE_CREATED)) with pull_request.set_state(PullRequest.STATE_UPDATING): - check = MergeCheck.validate( - pull_request, auth_user=apiuser, - translator=request.translate) + check = MergeCheck.validate(pull_request, auth_user=auth_user, + translator=request.translate) merge_possible = not check.failed if not merge_possible: @@ -319,14 +319,13 @@ def merge_pull_request( target_repo = pull_request.target_repo extras = vcs_operation_context( request.environ, repo_name=target_repo.repo_name, - username=apiuser.username, action='push', + username=auth_user.username, action='push', scm=target_repo.repo_type) with pull_request.set_state(PullRequest.STATE_UPDATING): merge_response = PullRequestModel().merge_repo( pull_request, apiuser, extras=extras) if merge_response.executed: - PullRequestModel().close_pull_request( - pull_request.pull_request_id, apiuser) + PullRequestModel().close_pull_request(pull_request.pull_request_id, auth_user) Session().commit() @@ -494,11 +493,13 @@ def comment_pull_request( else: repo = pull_request.target_repo + auth_user = apiuser if not isinstance(userid, Optional): if (has_superadmin_permission(apiuser) or HasRepoPermissionAnyApi('repository.admin')( user=apiuser, repo_name=repo.repo_name)): apiuser = get_user_or_error(userid) + auth_user = apiuser.AuthUser() else: raise JSONRPCError('userid is not the same as your user') @@ -574,7 +575,7 @@ def comment_pull_request( renderer=renderer, comment_type=comment_type, resolves_comment_id=resolves_comment_id, - auth_user=apiuser + auth_user=auth_user ) if allowed_to_change_status and status: