diff --git a/rhodecode/api/tests/test_comment_pull_request.py b/rhodecode/api/tests/test_comment_pull_request.py --- a/rhodecode/api/tests/test_comment_pull_request.py +++ b/rhodecode/api/tests/test_comment_pull_request.py @@ -58,7 +58,7 @@ class TestCommentPullRequest(object): expected = { 'pull_request_id': pull_request.pull_request_id, 'comment_id': comments[-1].comment_id, - 'status': None + 'status': {'given': None, 'was_changed': None} } assert_ok(id_, expected, response.body) @@ -88,7 +88,56 @@ class TestCommentPullRequest(object): expected = { 'pull_request_id': pull_request.pull_request_id, 'comment_id': comments[-1].comment_id, - 'status': 'rejected' + 'status': {'given': 'rejected', 'was_changed': True} + } + assert_ok(id_, expected, response.body) + + @pytest.mark.backends("git", "hg") + def test_api_comment_pull_request_change_status_with_specific_commit_id( + self, pr_util, no_notifications): + pull_request = pr_util.create_pull_request() + pull_request_id = pull_request.pull_request_id + latest_commit_id = 'test_commit' + # inject additional revision, to fail test the status change on + # non-latest commit + pull_request.revisions = pull_request.revisions + ['test_commit'] + + id_, params = build_data( + self.apikey, 'comment_pull_request', + repoid=pull_request.target_repo.repo_name, + pullrequestid=pull_request.pull_request_id, + status='approved', commit_id=latest_commit_id) + response = api_call(self.app, params) + pull_request = PullRequestModel().get(pull_request_id) + + expected = { + 'pull_request_id': pull_request.pull_request_id, + 'comment_id': None, + 'status': {'given': 'approved', 'was_changed': False} + } + assert_ok(id_, expected, response.body) + + @pytest.mark.backends("git", "hg") + def test_api_comment_pull_request_change_status_with_specific_commit_id( + self, pr_util, no_notifications): + pull_request = pr_util.create_pull_request() + pull_request_id = pull_request.pull_request_id + latest_commit_id = pull_request.revisions[0] + + id_, params = build_data( + self.apikey, 'comment_pull_request', + repoid=pull_request.target_repo.repo_name, + pullrequestid=pull_request.pull_request_id, + status='approved', commit_id=latest_commit_id) + response = api_call(self.app, params) + pull_request = PullRequestModel().get(pull_request_id) + + comments = ChangesetCommentsModel().get_comments( + pull_request.target_repo.repo_id, pull_request=pull_request) + expected = { + 'pull_request_id': pull_request.pull_request_id, + 'comment_id': comments[-1].comment_id, + 'status': {'given': 'approved', 'was_changed': True} } assert_ok(id_, expected, response.body) @@ -103,7 +152,7 @@ class TestCommentPullRequest(object): pullrequestid=pull_request_id) response = api_call(self.app, params) - expected = 'message and status parameter missing' + expected = 'Both message and status parameters are missing. At least one is required.' assert_error(id_, expected, given=response.body) @pytest.mark.backends("git", "hg") @@ -118,7 +167,7 @@ class TestCommentPullRequest(object): status='42') response = api_call(self.app, params) - expected = 'unknown comment status`42`' + expected = 'Unknown comment status: `42`' assert_error(id_, expected, given=response.body) @pytest.mark.backends("git", "hg") @@ -144,3 +193,17 @@ class TestCommentPullRequest(object): expected = 'userid is not the same as your user' assert_error(id_, expected, given=response.body) + + @pytest.mark.backends("git", "hg") + def test_api_comment_pull_request_wrong_commit_id_error(self, pr_util): + pull_request = pr_util.create_pull_request() + id_, params = build_data( + self.apikey_regular, 'comment_pull_request', + repoid=pull_request.target_repo.repo_name, + status='approved', + pullrequestid=pull_request.pull_request_id, + commit_id='XXX') + response = api_call(self.app, params) + + expected = 'Invalid commit_id `XXX` for this pull request.' + assert_error(id_, expected, given=response.body) 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 @@ -361,6 +361,7 @@ def close_pull_request(request, apiuser, @jsonrpc_method() def comment_pull_request(request, apiuser, repoid, pullrequestid, message=Optional(None), status=Optional(None), + commit_id=Optional(None), userid=Optional(OAttr('apiuser'))): """ Comment on the pull request specified with the `pullrequestid`, @@ -382,6 +383,10 @@ def comment_pull_request(request, apiuse * rejected * under_review :type status: str + :param commit_id: Specify the commit_id for which to set a comment. If + given commit_id is different than latest in the PR status + change won't be performed. + :type commit_id: str :param userid: Comment on the pull request as this user :type userid: Optional(str or int) @@ -393,7 +398,9 @@ def comment_pull_request(request, apiuse result : { "pull_request_id": "", - "comment_id": "" + "comment_id": "", + "status": {"given": , + "was_changed": }, } error : null """ @@ -412,24 +419,42 @@ def comment_pull_request(request, apiuse raise JSONRPCError('repository `%s` does not exist' % (repoid,)) message = Optional.extract(message) status = Optional.extract(status) + commit_id = Optional.extract(commit_id) + if not message and not status: - raise JSONRPCError('message and status parameter missing') + raise JSONRPCError( + 'Both message and status parameters are missing. ' + 'At least one is required.') if (status not in (st[0] for st in ChangesetStatus.STATUSES) and status is not None): - raise JSONRPCError('unknown comment status`%s`' % status) + raise JSONRPCError('Unknown comment status: `%s`' % status) + + if commit_id and commit_id not in pull_request.revisions: + raise JSONRPCError( + 'Invalid commit_id `%s` for this pull request.' % commit_id) allowed_to_change_status = PullRequestModel().check_user_change_status( pull_request, apiuser) + + # if commit_id is passed re-validated if user is allowed to change status + # based on latest commit_id from the PR + if commit_id: + commit_idx = pull_request.revisions.index(commit_id) + if commit_idx != 0: + allowed_to_change_status = False + text = message + status_label = ChangesetStatus.get_status_lbl(status) if status and allowed_to_change_status: - st_message = (('Status change %(transition_icon)s %(status)s') - % {'transition_icon': '>', - 'status': ChangesetStatus.get_status_lbl(status)}) + st_message = ('Status change %(transition_icon)s %(status)s' + % {'transition_icon': '>', 'status': status_label}) text = message or st_message rc_config = SettingsModel().get_all_settings() renderer = rc_config.get('rhodecode_markup_renderer', 'rst') + + status_change = status and allowed_to_change_status comment = ChangesetCommentsModel().create( text=text, repo=pull_request.target_repo.repo_id, @@ -437,10 +462,8 @@ def comment_pull_request(request, apiuse pull_request=pull_request.pull_request_id, f_path=None, line_no=None, - status_change=(ChangesetStatus.get_status_lbl(status) - if status and allowed_to_change_status else None), - status_change_type=(status - if status and allowed_to_change_status else None), + status_change=(status_label if status_change else None), + status_change_type=(status if status_change else None), closing_pr=False, renderer=renderer ) @@ -458,8 +481,8 @@ def comment_pull_request(request, apiuse Session().commit() data = { 'pull_request_id': pull_request.pull_request_id, - 'comment_id': comment.comment_id, - 'status': status + 'comment_id': comment.comment_id if comment else None, + 'status': {'given': status, 'was_changed': status_change}, } return data