Show More
@@ -252,8 +252,9 b' class TestCreatePullRequestApi(object):' | |||||
252 | id_, params = build_data( |
|
252 | id_, params = build_data( | |
253 | self.apikey_regular, 'create_pull_request', **data) |
|
253 | self.apikey_regular, 'create_pull_request', **data) | |
254 | response = api_call(self.app, params) |
|
254 | response = api_call(self.app, params) | |
255 |
expected_message = 'The specified |
|
255 | expected_message = 'The specified value:{type}:`{name}` ' \ | |
256 | branch_name) |
|
256 | 'does not exist, or is not allowed.'.format(type='branch', | |
|
257 | name=branch_name) | |||
257 | assert_error(id_, expected_message, given=response.body) |
|
258 | assert_error(id_, expected_message, given=response.body) | |
258 |
|
259 | |||
259 | @pytest.mark.backends("git", "hg") |
|
260 | @pytest.mark.backends("git", "hg") |
@@ -87,7 +87,8 b' class TestResolveRefOrError(object):' | |||||
87 | ref = 'ancestor:ref' |
|
87 | ref = 'ancestor:ref' | |
88 | with pytest.raises(JSONRPCError) as excinfo: |
|
88 | with pytest.raises(JSONRPCError) as excinfo: | |
89 | utils.resolve_ref_or_error(ref, repo) |
|
89 | utils.resolve_ref_or_error(ref, repo) | |
90 | expected_message = 'The specified ancestor `ref` does not exist' |
|
90 | expected_message = ( | |
|
91 | 'The specified value:ancestor:`ref` does not exist, or is not allowed.') | |||
91 | assert excinfo.value.message == expected_message |
|
92 | assert excinfo.value.message == expected_message | |
92 |
|
93 | |||
93 | def test_branch_is_not_found(self): |
|
94 | def test_branch_is_not_found(self): | |
@@ -99,7 +100,7 b' class TestResolveRefOrError(object):' | |||||
99 | with pytest.raises(JSONRPCError) as excinfo: |
|
100 | with pytest.raises(JSONRPCError) as excinfo: | |
100 | utils.resolve_ref_or_error(ref, repo) |
|
101 | utils.resolve_ref_or_error(ref, repo) | |
101 | expected_message = ( |
|
102 | expected_message = ( | |
102 |
'The specified branch |
|
103 | 'The specified value:branch:`non-existing-one` does not exist, or is not allowed.') | |
103 | assert excinfo.value.message == expected_message |
|
104 | assert excinfo.value.message == expected_message | |
104 |
|
105 | |||
105 | def test_bookmark_is_not_found(self): |
|
106 | def test_bookmark_is_not_found(self): | |
@@ -111,7 +112,7 b' class TestResolveRefOrError(object):' | |||||
111 | with pytest.raises(JSONRPCError) as excinfo: |
|
112 | with pytest.raises(JSONRPCError) as excinfo: | |
112 | utils.resolve_ref_or_error(ref, repo) |
|
113 | utils.resolve_ref_or_error(ref, repo) | |
113 | expected_message = ( |
|
114 | expected_message = ( | |
114 |
'The specified bookmark |
|
115 | 'The specified value:bookmark:`non-existing-one` does not exist, or is not allowed.') | |
115 | assert excinfo.value.message == expected_message |
|
116 | assert excinfo.value.message == expected_message | |
116 |
|
117 | |||
117 | @pytest.mark.parametrize("ref", ['ref', '12345', 'a:b:c:d']) |
|
118 | @pytest.mark.parametrize("ref", ['ref', '12345', 'a:b:c:d']) |
@@ -403,7 +403,7 b' def resolve_ref_or_error(ref, repo):' | |||||
403 | ref_hash = ref_hash or _get_ref_hash(repo, ref_type, ref_name) |
|
403 | ref_hash = ref_hash or _get_ref_hash(repo, ref_type, ref_name) | |
404 | except (KeyError, ValueError): |
|
404 | except (KeyError, ValueError): | |
405 | raise JSONRPCError( |
|
405 | raise JSONRPCError( | |
406 |
'The specified {type} |
|
406 | 'The specified value:{type}:`{name}` does not exist, or is not allowed.'.format( | |
407 | type=ref_type, name=ref_name)) |
|
407 | type=ref_type, name=ref_name)) | |
408 |
|
408 | |||
409 | return ':'.join([ref_type, ref_name, ref_hash]) |
|
409 | return ':'.join([ref_type, ref_name, ref_hash]) | |
@@ -431,7 +431,6 b' def _get_commit_dict(' | |||||
431 | } |
|
431 | } | |
432 |
|
432 | |||
433 |
|
433 | |||
434 | # TODO: mikhail: Think about moving this function to some library |
|
|||
435 | def _get_ref_hash(repo, type_, name): |
|
434 | def _get_ref_hash(repo, type_, name): | |
436 | vcs_repo = repo.scm_instance() |
|
435 | vcs_repo = repo.scm_instance() | |
437 | if type_ == 'branch' and vcs_repo.alias in ('hg', 'git'): |
|
436 | if type_ == 'branch' and vcs_repo.alias in ('hg', 'git'): |
@@ -284,7 +284,7 b' def merge_pull_request(' | |||||
284 | raise JSONRPCError('userid is not the same as your user') |
|
284 | raise JSONRPCError('userid is not the same as your user') | |
285 |
|
285 | |||
286 | check = MergeCheck.validate( |
|
286 | check = MergeCheck.validate( | |
287 | pull_request, user=apiuser, translator=request.translate) |
|
287 | pull_request, auth_user=apiuser, translator=request.translate) | |
288 | merge_possible = not check.failed |
|
288 | merge_possible = not check.failed | |
289 |
|
289 | |||
290 | if not merge_possible: |
|
290 | if not merge_possible: |
@@ -341,7 +341,7 b' class RepoPullRequestsView(RepoAppView, ' | |||||
341 |
|
341 | |||
342 | # check merge capabilities |
|
342 | # check merge capabilities | |
343 | _merge_check = MergeCheck.validate( |
|
343 | _merge_check = MergeCheck.validate( | |
344 | pull_request_latest, user=self._rhodecode_user, |
|
344 | pull_request_latest, auth_user=self._rhodecode_user, | |
345 | translator=self.request.translate, |
|
345 | translator=self.request.translate, | |
346 | force_shadow_repo_refresh=force_refresh) |
|
346 | force_shadow_repo_refresh=force_refresh) | |
347 | c.pr_merge_errors = _merge_check.error_details |
|
347 | c.pr_merge_errors = _merge_check.error_details | |
@@ -1056,8 +1056,9 b' class RepoPullRequestsView(RepoAppView, ' | |||||
1056 | self.request.matchdict['pull_request_id']) |
|
1056 | self.request.matchdict['pull_request_id']) | |
1057 |
|
1057 | |||
1058 | self.load_default_context() |
|
1058 | self.load_default_context() | |
1059 |
check = MergeCheck.validate( |
|
1059 | check = MergeCheck.validate( | |
1060 | translator=self.request.translate) |
|
1060 | pull_request, auth_user=self._rhodecode_user, | |
|
1061 | translator=self.request.translate) | |||
1061 | merge_possible = not check.failed |
|
1062 | merge_possible = not check.failed | |
1062 |
|
1063 | |||
1063 | for err_type, error_msg in check.errors: |
|
1064 | for err_type, error_msg in check.errors: |
@@ -449,7 +449,7 b' class PullRequestModel(BaseModel):' | |||||
449 | translator = translator or get_current_request().translate |
|
449 | translator = translator or get_current_request().translate | |
450 |
|
450 | |||
451 | created_by_user = self._get_user(created_by) |
|
451 | created_by_user = self._get_user(created_by) | |
452 | auth_user = auth_user or created_by_user |
|
452 | auth_user = auth_user or created_by_user.AuthUser() | |
453 | source_repo = self._get_repo(source_repo) |
|
453 | source_repo = self._get_repo(source_repo) | |
454 | target_repo = self._get_repo(target_repo) |
|
454 | target_repo = self._get_repo(target_repo) | |
455 |
|
455 | |||
@@ -534,7 +534,7 b' class PullRequestModel(BaseModel):' | |||||
534 |
|
534 | |||
535 | # prepare workspace, and run initial merge simulation |
|
535 | # prepare workspace, and run initial merge simulation | |
536 | MergeCheck.validate( |
|
536 | MergeCheck.validate( | |
537 |
pull_request, user= |
|
537 | pull_request, auth_user=auth_user, translator=translator) | |
538 |
|
538 | |||
539 | self.notify_reviewers(pull_request, reviewer_ids) |
|
539 | self.notify_reviewers(pull_request, reviewer_ids) | |
540 | self._trigger_pull_request_hook( |
|
540 | self._trigger_pull_request_hook( | |
@@ -1599,19 +1599,38 b' class MergeCheck(object):' | |||||
1599 | ) |
|
1599 | ) | |
1600 |
|
1600 | |||
1601 | @classmethod |
|
1601 | @classmethod | |
1602 | def validate(cls, pull_request, user, translator, fail_early=False, |
|
1602 | def validate(cls, pull_request, auth_user, translator, fail_early=False, | |
1603 | force_shadow_repo_refresh=False): |
|
1603 | force_shadow_repo_refresh=False): | |
1604 | _ = translator |
|
1604 | _ = translator | |
1605 | merge_check = cls() |
|
1605 | merge_check = cls() | |
1606 |
|
1606 | |||
1607 | # permissions to merge |
|
1607 | # permissions to merge | |
1608 | user_allowed_to_merge = PullRequestModel().check_user_merge( |
|
1608 | user_allowed_to_merge = PullRequestModel().check_user_merge( | |
1609 | pull_request, user) |
|
1609 | pull_request, auth_user) | |
1610 | if not user_allowed_to_merge: |
|
1610 | if not user_allowed_to_merge: | |
1611 | log.debug("MergeCheck: cannot merge, approval is pending.") |
|
1611 | log.debug("MergeCheck: cannot merge, approval is pending.") | |
1612 |
|
1612 | |||
1613 | msg = _('User `{}` not allowed to perform merge.').format(user.username) |
|
1613 | msg = _('User `{}` not allowed to perform merge.').format(auth_user.username) | |
1614 | merge_check.push_error('error', msg, cls.PERM_CHECK, user.username) |
|
1614 | merge_check.push_error('error', msg, cls.PERM_CHECK, auth_user.username) | |
|
1615 | if fail_early: | |||
|
1616 | return merge_check | |||
|
1617 | ||||
|
1618 | # permission to merge into the target branch | |||
|
1619 | target_commit_id = pull_request.target_ref_parts.commit_id | |||
|
1620 | if pull_request.target_ref_parts.type == 'branch': | |||
|
1621 | branch_name = pull_request.target_ref_parts.name | |||
|
1622 | else: | |||
|
1623 | # for mercurial we can always figure out the branch from the commit | |||
|
1624 | # in case of bookmark | |||
|
1625 | target_commit = pull_request.target_repo.get_commit(target_commit_id) | |||
|
1626 | branch_name = target_commit.branch | |||
|
1627 | ||||
|
1628 | rule, branch_perm = auth_user.get_rule_and_branch_permission( | |||
|
1629 | pull_request.target_repo.repo_name, branch_name) | |||
|
1630 | if branch_perm and branch_perm == 'branch.none': | |||
|
1631 | msg = _('Target branch `{}` changes rejected by rule {}.').format( | |||
|
1632 | branch_name, rule) | |||
|
1633 | merge_check.push_error('error', msg, cls.PERM_CHECK, auth_user.username) | |||
1615 | if fail_early: |
|
1634 | if fail_early: | |
1616 | return merge_check |
|
1635 | return merge_check | |
1617 |
|
1636 |
General Comments 0
You need to be logged in to leave comments.
Login now