pull-request: code cleanup

- use stricter checks
- pull_request_id should be taken from object to cast to INT
- unified some code usage
This commit is contained in:
Marcin Kuzminski 2017-07-27 22:48:09 +02:00
parent b8804925c3
commit 015ea42459
6 changed files with 31 additions and 26 deletions

View file

@ -35,13 +35,13 @@ def admin_routes(config):
config.add_route(
name='pull_requests_global_0', # backward compat
pattern='/pull_requests/{pull_request_id:[0-9]+}')
pattern='/pull_requests/{pull_request_id:\d+}')
config.add_route(
name='pull_requests_global_1', # backward compat
pattern='/pull-requests/{pull_request_id:[0-9]+}')
pattern='/pull-requests/{pull_request_id:\d+}')
config.add_route(
name='pull_requests_global',
pattern='/pull-request/{pull_request_id:[0-9]+}')
pattern='/pull-request/{pull_request_id:\d+}')
config.add_route(
name='admin_settings_open_source',

View file

@ -54,8 +54,10 @@ class AdminMainView(BaseAppView):
:param pull_request_id: id of pull requests in the system
"""
pull_request_id = self.request.matchdict.get('pull_request_id')
pull_request = PullRequest.get_or_404(pull_request_id)
pull_request = PullRequest.get_or_404(
self.request.matchdict['pull_request_id'])
pull_request_id = pull_request.pull_request_id
repo_name = pull_request.target_repo.repo_name
raise HTTPFound(

View file

@ -113,7 +113,7 @@ class TestAdminRepoSettingsAdvanced(object):
# mark it as None
response = self.app.post(
route_path('edit_repo_advanced_fork', repo_name=backend.repo_name),
params={'id_fork_of': None, '_method': 'put',
params={'id_fork_of': None,
'csrf_token': csrf_token})
assert_session_flash(
response,

View file

@ -265,7 +265,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
route_name='pullrequest_show', request_method='GET',
renderer='rhodecode:templates/pullrequests/pullrequest_show.mako')
def pull_request_show(self):
pull_request_id = self.request.matchdict.get('pull_request_id')
pull_request_id = self.request.matchdict['pull_request_id']
c = self.load_default_context()
@ -832,8 +832,8 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
route_name='pullrequest_update', request_method='POST',
renderer='json_ext')
def pull_request_update(self):
pull_request_id = self.request.matchdict['pull_request_id']
pull_request = PullRequest.get_or_404(pull_request_id)
pull_request = PullRequest.get_or_404(
self.request.matchdict['pull_request_id'])
# only owner or admin can update it
allowed_to_update = PullRequestModel().check_user_update(
@ -843,7 +843,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
if 'review_members' in controls:
self._update_reviewers(
pull_request_id, controls['review_members'],
pull_request, controls['review_members'],
pull_request.reviewer_data)
elif str2bool(self.request.POST.get('update_commits', 'false')):
self._update_commits(pull_request)
@ -930,8 +930,8 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
After successful merging, the pull request is automatically
closed, with a relevant comment.
"""
pull_request_id = self.request.matchdict['pull_request_id']
pull_request = PullRequest.get_or_404(pull_request_id)
pull_request = PullRequest.get_or_404(
self.request.matchdict['pull_request_id'])
check = MergeCheck.validate(pull_request, self._rhodecode_db_user)
merge_possible = not check.failed
@ -974,7 +974,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
merge_resp.failure_reason)
h.flash(msg, category='error')
def _update_reviewers(self, pull_request_id, review_members, reviewer_rules):
def _update_reviewers(self, pull_request, review_members, reviewer_rules):
_ = self.request.translate
get_default_reviewers_data, validate_default_reviewers = \
PullRequestModel().get_reviewer_functions()
@ -987,7 +987,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
return
PullRequestModel().update_reviewers(
pull_request_id, reviewers, self._rhodecode_user)
pull_request, reviewers, self._rhodecode_user)
h.flash(_('Pull request reviewers updated.'), category='success')
Session().commit()
@ -1002,8 +1002,8 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
def pull_request_delete(self):
_ = self.request.translate
pull_request_id = self.request.matchdict['pull_request_id']
pull_request = PullRequest.get_or_404(pull_request_id)
pull_request = PullRequest.get_or_404(
self.request.matchdict['pull_request_id'])
pr_closed = pull_request.is_closed()
allowed_to_delete = PullRequestModel().check_user_delete(
@ -1031,8 +1031,11 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
renderer='json_ext')
def pull_request_comment_create(self):
_ = self.request.translate
pull_request_id = self.request.matchdict['pull_request_id']
pull_request = PullRequest.get_or_404(pull_request_id)
pull_request = PullRequest.get_or_404(
self.request.matchdict['pull_request_id'])
pull_request_id = pull_request.pull_request_id
if pull_request.is_closed():
log.debug('comment: forbidden because pull request is closed')
raise HTTPForbidden()
@ -1080,7 +1083,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
text=text,
repo=self.db_repo.repo_id,
user=self._rhodecode_user.user_id,
pull_request=pull_request_id,
pull_request=pull_request,
f_path=self.request.POST.get('f_path'),
line_no=self.request.POST.get('line'),
status_change=(ChangesetStatus.get_status_lbl(status)
@ -1102,7 +1105,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
status,
self._rhodecode_user.user_id,
comment,
pull_request=pull_request_id
pull_request=pull_request
)
Session().flush()
@ -1142,15 +1145,17 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
route_name='pullrequest_comment_delete', request_method='POST',
renderer='json_ext')
def pull_request_comment_delete(self):
pull_request_id = self.request.matchdict['pull_request_id']
comment_id = self.request.matchdict['comment_id']
pull_request = PullRequest.get_or_404(
self.request.matchdict['pull_request_id'])
comment = ChangesetComment.get_or_404(
self.request.matchdict['comment_id'])
comment_id = comment.comment_id
pull_request = PullRequest.get_or_404(pull_request_id)
if pull_request.is_closed():
log.debug('comment: forbidden because pull request is closed')
raise HTTPForbidden()
comment = ChangesetComment.get_or_404(comment_id)
if not comment:
log.debug('Comment with id:%s not found, skipping', comment_id)
# comment already deleted in another call probably

View file

@ -544,7 +544,6 @@ var CommentsController = function() {
var comment_id = $comment.attr('data-comment-id');
var url = AJAX_COMMENT_DELETE_URL.replace('__COMMENT_ID__', comment_id);
var postData = {
'_method': 'delete',
'csrf_token': CSRF_TOKEN
};

View file

@ -372,7 +372,6 @@ var _updatePullRequest = function(repo_name, pull_request_id, postData) {
*/
var updateCommits = function(repo_name, pull_request_id) {
var postData = {
'_method': 'put',
'update_commits': true};
_updatePullRequest(repo_name, pull_request_id, postData);
};