api: pull-requests, make the repoid parameter optional. Because pullrequestid is global

we don't want to complicate things more by requiring repoid. Left for backward compat.
This commit is contained in:
Marcin Kuzminski 2017-11-23 21:14:04 +01:00
parent b809d00f88
commit bc23ad316c
8 changed files with 72 additions and 39 deletions

View file

@ -86,7 +86,7 @@ class TestApi(object):
def test_api_non_existing_method_have_similar(self, request):
id_, params = build_data(self.apikey, 'comment', args='xx')
response = api_call(self.app, params)
expected = 'No such method: comment. Similar methods: changeset_comment, comment_pull_request, comment_commit'
expected = 'No such method: comment. Similar methods: changeset_comment, comment_pull_request, get_pull_request_comments, comment_commit'
assert_error(id_, expected, given=response.body)
def test_api_disabled_user(self, request):

View file

@ -70,10 +70,11 @@ class TestClosePullRequest(object):
assert_error(id_, expected, given=response.body)
@pytest.mark.backends("git", "hg")
def test_api_close_pull_request_repo_error(self):
def test_api_close_pull_request_repo_error(self, pr_util):
pull_request = pr_util.create_pull_request()
id_, params = build_data(
self.apikey, 'close_pull_request',
repoid=666, pullrequestid=1)
repoid=666, pullrequestid=pull_request.pull_request_id)
response = api_call(self.app, params)
expected = 'repository `666` does not exist'

View file

@ -170,10 +170,11 @@ class TestCommentPullRequest(object):
assert_error(id_, expected, given=response.body)
@pytest.mark.backends("git", "hg")
def test_api_comment_pull_request_repo_error(self):
def test_api_comment_pull_request_repo_error(self, pr_util):
pull_request = pr_util.create_pull_request()
id_, params = build_data(
self.apikey, 'comment_pull_request',
repoid=666, pullrequestid=1)
repoid=666, pullrequestid=pull_request.pull_request_id)
response = api_call(self.app, params)
expected = 'repository `666` does not exist'

View file

@ -38,11 +38,12 @@ class TestGetMethod(object):
response = api_call(self.app, params)
expected = ['changeset_comment', 'comment_pull_request',
'comment_commit']
'get_pull_request_comments', 'comment_commit']
assert_ok(id_, expected, given=response.body)
def test_get_methods_on_single_match(self):
id_, params = build_data(self.apikey, 'get_method', pattern='*comment_commit*')
id_, params = build_data(self.apikey, 'get_method',
pattern='*comment_commit*')
response = api_call(self.app, params)
expected = ['comment_commit',

View file

@ -38,7 +38,6 @@ class TestGetPullRequest(object):
pull_request = pr_util.create_pull_request(mergeable=True)
id_, params = build_data(
self.apikey, 'get_pull_request',
repoid=pull_request.target_repo.repo_name,
pullrequestid=pull_request.pull_request_id)
response = api_call(self.app, params)
@ -115,10 +114,11 @@ class TestGetPullRequest(object):
}
assert_ok(id_, expected, response.body)
def test_api_get_pull_request_repo_error(self):
def test_api_get_pull_request_repo_error(self, pr_util):
pull_request = pr_util.create_pull_request()
id_, params = build_data(
self.apikey, 'get_pull_request',
repoid=666, pullrequestid=1)
repoid=666, pullrequestid=pull_request.pull_request_id)
response = api_call(self.app, params)
expected = 'repository `666` does not exist'
@ -126,8 +126,16 @@ class TestGetPullRequest(object):
def test_api_get_pull_request_pull_request_error(self):
id_, params = build_data(
self.apikey, 'get_pull_request',
repoid=1, pullrequestid=666)
self.apikey, 'get_pull_request', pullrequestid=666)
response = api_call(self.app, params)
expected = 'pull request `666` does not exist'
assert_error(id_, expected, given=response.body)
def test_api_get_pull_request_pull_request_error_just_pr_id(self):
id_, params = build_data(
self.apikey, 'get_pull_request',
pullrequestid=666)
response = api_call(self.app, params)
expected = 'pull request `666` does not exist'

View file

@ -112,10 +112,11 @@ class TestMergePullRequest(object):
assert_error(id_, expected, given=response.body)
@pytest.mark.backends("git", "hg")
def test_api_merge_pull_request_repo_error(self):
def test_api_merge_pull_request_repo_error(self, pr_util):
pull_request = pr_util.create_pull_request()
id_, params = build_data(
self.apikey, 'merge_pull_request',
repoid=666, pullrequestid=1)
repoid=666, pullrequestid=pull_request.pull_request_id)
response = api_call(self.app, params)
expected = 'repository `666` does not exist'

View file

@ -168,10 +168,11 @@ class TestUpdatePullRequest(object):
@pytest.mark.backends("git", "hg")
def test_api_update_repo_error(self, pr_util):
pull_request = pr_util.create_pull_request()
id_, params = build_data(
self.apikey, 'update_pull_request',
repoid='fake',
pullrequestid='fake',
pullrequestid=pull_request.pull_request_id,
reviewers=[{'username': 'bad_name'}])
response = api_call(self.app, params)

View file

@ -43,14 +43,14 @@ log = logging.getLogger(__name__)
@jsonrpc_method()
def get_pull_request(request, apiuser, repoid, pullrequestid):
def get_pull_request(request, apiuser, pullrequestid, repoid=Optional(None)):
"""
Get a pull request based on the given ID.
:param apiuser: This is filled automatically from the |authtoken|.
:type apiuser: AuthUser
:param repoid: Repository name or repository ID from where the pull
request was opened.
:param repoid: Optional, repository name or repository ID from where
the pull request was opened.
:type repoid: str or int
:param pullrequestid: ID of the requested pull request.
:type pullrequestid: int
@ -121,11 +121,17 @@ def get_pull_request(request, apiuser, repoid, pullrequestid):
},
"error": null
"""
get_repo_or_error(repoid)
pull_request = get_pull_request_or_error(pullrequestid)
if Optional.extract(repoid):
repo = get_repo_or_error(repoid)
else:
repo = pull_request.target_repo
if not PullRequestModel().check_user_read(
pull_request, apiuser, api=True):
raise JSONRPCError('repository `%s` does not exist' % (repoid,))
raise JSONRPCError('repository `%s` or pull request `%s` '
'does not exist' % (repoid, pullrequestid))
data = pull_request.get_api_data()
return data
@ -137,7 +143,7 @@ def get_pull_requests(request, apiuser, repoid, status=Optional('new')):
:param apiuser: This is filled automatically from the |authtoken|.
:type apiuser: AuthUser
:param repoid: Repository name or repository ID.
:param repoid: Optional repository name or repository ID.
:type repoid: str or int
:param status: Only return pull requests with the specified status.
Valid options are.
@ -229,7 +235,7 @@ def get_pull_requests(request, apiuser, repoid, status=Optional('new')):
@jsonrpc_method()
def merge_pull_request(
request, apiuser, repoid, pullrequestid,
request, apiuser, pullrequestid, repoid=Optional(None),
userid=Optional(OAttr('apiuser'))):
"""
Merge the pull request specified by `pullrequestid` into its target
@ -237,7 +243,7 @@ def merge_pull_request(
:param apiuser: This is filled automatically from the |authtoken|.
:type apiuser: AuthUser
:param repoid: The Repository name or repository ID of the
:param repoid: Optional, repository name or repository ID of the
target repository to which the |pr| is to be merged.
:type repoid: str or int
:param pullrequestid: ID of the pull request which shall be merged.
@ -263,7 +269,12 @@ def merge_pull_request(
},
"error": null
"""
repo = get_repo_or_error(repoid)
pull_request = get_pull_request_or_error(pullrequestid)
if Optional.extract(repoid):
repo = get_repo_or_error(repoid)
else:
repo = pull_request.target_repo
if not isinstance(userid, Optional):
if (has_superadmin_permission(apiuser) or
HasRepoPermissionAnyApi('repository.admin')(
@ -272,8 +283,6 @@ def merge_pull_request(
else:
raise JSONRPCError('userid is not the same as your user')
pull_request = get_pull_request_or_error(pullrequestid)
check = MergeCheck.validate(
pull_request, user=apiuser, translator=request.translate)
merge_possible = not check.failed
@ -415,8 +424,8 @@ def get_pull_request_comments(
@jsonrpc_method()
def comment_pull_request(
request, apiuser, repoid, pullrequestid, message=Optional(None),
commit_id=Optional(None), status=Optional(None),
request, apiuser, pullrequestid, repoid=Optional(None),
message=Optional(None), commit_id=Optional(None), status=Optional(None),
comment_type=Optional(ChangesetComment.COMMENT_TYPE_NOTE),
resolves_comment_id=Optional(None),
userid=Optional(OAttr('apiuser'))):
@ -427,7 +436,7 @@ def comment_pull_request(
:param apiuser: This is filled automatically from the |authtoken|.
:type apiuser: AuthUser
:param repoid: The repository name or repository ID.
:param repoid: Optional repository name or repository ID.
:type repoid: str or int
:param pullrequestid: The pull request ID.
:type pullrequestid: int
@ -459,7 +468,12 @@ def comment_pull_request(
},
error : null
"""
repo = get_repo_or_error(repoid)
pull_request = get_pull_request_or_error(pullrequestid)
if Optional.extract(repoid):
repo = get_repo_or_error(repoid)
else:
repo = pull_request.target_repo
if not isinstance(userid, Optional):
if (has_superadmin_permission(apiuser) or
HasRepoPermissionAnyApi('repository.admin')(
@ -468,7 +482,6 @@ def comment_pull_request(
else:
raise JSONRPCError('userid is not the same as your user')
pull_request = get_pull_request_or_error(pullrequestid)
if not PullRequestModel().check_user_read(
pull_request, apiuser, api=True):
raise JSONRPCError('repository `%s` does not exist' % (repoid,))
@ -676,15 +689,15 @@ def create_pull_request(
@jsonrpc_method()
def update_pull_request(
request, apiuser, repoid, pullrequestid, title=Optional(''),
description=Optional(''), reviewers=Optional(None),
request, apiuser, pullrequestid, repoid=Optional(None),
title=Optional(''), description=Optional(''), reviewers=Optional(None),
update_commits=Optional(None)):
"""
Updates a pull request.
:param apiuser: This is filled automatically from the |authtoken|.
:type apiuser: AuthUser
:param repoid: The repository name or repository ID.
:param repoid: Optional repository name or repository ID.
:type repoid: str or int
:param pullrequestid: The pull request ID.
:type pullrequestid: int
@ -729,8 +742,12 @@ def update_pull_request(
error : null
"""
repo = get_repo_or_error(repoid)
pull_request = get_pull_request_or_error(pullrequestid)
if Optional.extract(repoid):
repo = get_repo_or_error(repoid)
else:
repo = pull_request.target_repo
if not PullRequestModel().check_user_update(
pull_request, apiuser, api=True):
raise JSONRPCError(
@ -808,7 +825,7 @@ def update_pull_request(
@jsonrpc_method()
def close_pull_request(
request, apiuser, repoid, pullrequestid,
request, apiuser, pullrequestid, repoid=Optional(None),
userid=Optional(OAttr('apiuser')), message=Optional('')):
"""
Close the pull request specified by `pullrequestid`.
@ -841,7 +858,12 @@ def close_pull_request(
"""
_ = request.translate
repo = get_repo_or_error(repoid)
pull_request = get_pull_request_or_error(pullrequestid)
if Optional.extract(repoid):
repo = get_repo_or_error(repoid)
else:
repo = pull_request.target_repo
if not isinstance(userid, Optional):
if (has_superadmin_permission(apiuser) or
HasRepoPermissionAnyApi('repository.admin')(
@ -850,8 +872,6 @@ def close_pull_request(
else:
raise JSONRPCError('userid is not the same as your user')
pull_request = get_pull_request_or_error(pullrequestid)
if pull_request.is_closed():
raise JSONRPCError(
'pull request `%s` is already closed' % (pullrequestid,))