diff --git a/rhodecode/api/tests/test_create_pull_request.py b/rhodecode/api/tests/test_create_pull_request.py --- a/rhodecode/api/tests/test_create_pull_request.py +++ b/rhodecode/api/tests/test_create_pull_request.py @@ -113,11 +113,13 @@ class TestCreatePullRequestApi(object): data = self._prepare_data(backend) reviewers = [ {'username': TEST_USER_REGULAR_LOGIN, - 'reasons': ['added manually']}, + 'reasons': ['{} added manually'.format(TEST_USER_REGULAR_LOGIN)]}, {'username': TEST_USER_ADMIN_LOGIN, - 'reasons': ['added manually']}, + 'reasons': ['{} added manually'.format(TEST_USER_ADMIN_LOGIN)], + 'mandatory': True}, ] data['reviewers'] = reviewers + id_, params = build_data( self.apikey_regular, 'create_pull_request', **data) response = api_call(self.app, params) @@ -128,12 +130,24 @@ class TestCreatePullRequestApi(object): assert result['result']['msg'] == expected_message pull_request_id = result['result']['pull_request_id'] pull_request = PullRequestModel().get(pull_request_id) - actual_reviewers = [ - {'username': r.user.username, - 'reasons': ['added manually'], - } for r in pull_request.reviewers - ] - assert sorted(actual_reviewers) == sorted(reviewers) + + actual_reviewers = [] + for rev in pull_request.reviewers: + entry = { + 'username': rev.user.username, + 'reasons': rev.reasons, + } + if rev.mandatory: + entry['mandatory'] = rev.mandatory + actual_reviewers.append(entry) + + # default reviewer will be added who is an owner of the repo + reviewers.append( + {'username': pull_request.author.username, + 'reasons': [u'Default reviewer', u'Repository owner']}, + ) + assert sorted(actual_reviewers, key=lambda e: e['username']) \ + == sorted(reviewers, key=lambda e: e['username']) @pytest.mark.backends("git", "hg") def test_create_with_reviewers_specified_by_ids( @@ -159,12 +173,23 @@ class TestCreatePullRequestApi(object): assert result['result']['msg'] == expected_message pull_request_id = result['result']['pull_request_id'] pull_request = PullRequestModel().get(pull_request_id) - actual_reviewers = [ - {'username': r.user.user_id, - 'reasons': ['added manually'], - } for r in pull_request.reviewers - ] - assert sorted(actual_reviewers) == sorted(reviewers) + + actual_reviewers = [] + for rev in pull_request.reviewers: + entry = { + 'username': rev.user.user_id, + 'reasons': rev.reasons, + } + if rev.mandatory: + entry['mandatory'] = rev.mandatory + actual_reviewers.append(entry) + # default reviewer will be added who is an owner of the repo + reviewers.append( + {'username': pull_request.author.user_id, + 'reasons': [u'Default reviewer', u'Repository owner']}, + ) + assert sorted(actual_reviewers, key=lambda e: e['username']) \ + == sorted(reviewers, key=lambda e: e['username']) @pytest.mark.backends("git", "hg") def test_create_fails_when_the_reviewer_is_not_found(self, backend): @@ -271,9 +296,10 @@ class TestCreatePullRequestApi(object): def test_create_fails_when_no_permissions(self, backend): data = self._prepare_data(backend) RepoModel().revoke_user_permission( - self.source.repo_name, User.DEFAULT_USER) + self.source.repo_name, self.test_user) RepoModel().revoke_user_permission( - self.source.repo_name, self.test_user) + self.source.repo_name, User.DEFAULT_USER) + id_, params = build_data( self.apikey_regular, 'create_pull_request', **data) response = api_call(self.app, params) 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 @@ -641,6 +641,7 @@ def create_pull_request( reviewer_objects = Optional.extract(reviewers) or [] + # serialize and validate passed in given reviewers if reviewer_objects: schema = ReviewerListSchema() try: @@ -653,20 +654,19 @@ def create_pull_request( user = get_user_or_error(reviewer_object['username']) reviewer_object['user_id'] = user.user_id - get_default_reviewers_data, get_validated_reviewers = \ + get_default_reviewers_data, validate_default_reviewers = \ PullRequestModel().get_reviewer_functions() + # recalculate reviewers logic, to make sure we can validate this reviewer_rules = get_default_reviewers_data( apiuser.get_instance(), source_db_repo, source_commit, target_db_repo, target_commit) - # specified rules are later re-validated, thus we can assume users will - # eventually provide those that meet the reviewer criteria. - if not reviewer_objects: - reviewer_objects = reviewer_rules['reviewers'] + # now MERGE our given with the calculated + reviewer_objects = reviewer_rules['reviewers'] + reviewer_objects try: - reviewers = get_validated_reviewers( + reviewers = validate_default_reviewers( reviewer_objects, reviewer_rules) except ValueError as e: raise JSONRPCError('Reviewers Validation: {}'.format(e)) diff --git a/rhodecode/apps/repository/views/repo_pull_requests.py b/rhodecode/apps/repository/views/repo_pull_requests.py --- a/rhodecode/apps/repository/views/repo_pull_requests.py +++ b/rhodecode/apps/repository/views/repo_pull_requests.py @@ -881,7 +881,8 @@ class RepoPullRequestsView(RepoAppView, source_commit, target_db_repo, target_commit) given_reviewers = _form['review_members'] - reviewers = validate_default_reviewers(given_reviewers, reviewer_rules) + reviewers = validate_default_reviewers( + given_reviewers, reviewer_rules) pullrequest_title = _form['pullrequest_title'] title_source_ref = source_ref.split(':', 2)[1]