# HG changeset patch # User Marcin Kuzminski # Date 2017-07-27 20:48:09 # Node ID f2f8f5ce30d3ea3a0ffaafa565fafe9ecb623266 # Parent 455f23f1afc3d49364435eca98ead8269b871b1e pull-request: code cleanup - use stricter checks - pull_request_id should be taken from object to cast to INT - unified some code usage diff --git a/rhodecode/apps/admin/__init__.py b/rhodecode/apps/admin/__init__.py --- a/rhodecode/apps/admin/__init__.py +++ b/rhodecode/apps/admin/__init__.py @@ -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', diff --git a/rhodecode/apps/admin/views/main_views.py b/rhodecode/apps/admin/views/main_views.py --- a/rhodecode/apps/admin/views/main_views.py +++ b/rhodecode/apps/admin/views/main_views.py @@ -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( diff --git a/rhodecode/apps/repository/tests/test_repo_settings_advanced.py b/rhodecode/apps/repository/tests/test_repo_settings_advanced.py --- a/rhodecode/apps/repository/tests/test_repo_settings_advanced.py +++ b/rhodecode/apps/repository/tests/test_repo_settings_advanced.py @@ -113,7 +113,7 @@ class TestAdminRepoSettingsAdvanced(obje # 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, 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 @@ -265,7 +265,7 @@ class RepoPullRequestsView(RepoAppView, 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, 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, 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, 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, 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, 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, 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, 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, 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, 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, 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']) - pull_request = PullRequest.get_or_404(pull_request_id) + comment = ChangesetComment.get_or_404( + self.request.matchdict['comment_id']) + comment_id = comment.comment_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 diff --git a/rhodecode/public/js/src/rhodecode/comments.js b/rhodecode/public/js/src/rhodecode/comments.js --- a/rhodecode/public/js/src/rhodecode/comments.js +++ b/rhodecode/public/js/src/rhodecode/comments.js @@ -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 }; diff --git a/rhodecode/public/js/src/rhodecode/pullrequests.js b/rhodecode/public/js/src/rhodecode/pullrequests.js --- a/rhodecode/public/js/src/rhodecode/pullrequests.js +++ b/rhodecode/public/js/src/rhodecode/pullrequests.js @@ -372,7 +372,6 @@ var _updatePullRequest = function(repo_n */ var updateCommits = function(repo_name, pull_request_id) { var postData = { - '_method': 'put', 'update_commits': true}; _updatePullRequest(repo_name, pull_request_id, postData); };