pull-request: code cleanup

- use stricter checks
- pull_request_id should be taken from object to cast to INT
- unified some code usage

rhodecode/apps/admin/__init__.py

@@ -35,13 +35,13 b' 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',

rhodecode/apps/admin/views/main_views.py

@@ -54,8 +54,10 b' 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(

rhodecode/apps/repository/tests/test_repo_settings_advanced.py

@@ -113,7 +113,7 b' 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,

rhodecode/apps/repository/views/repo_pull_requests.py

@@ -265,7 +265,7 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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

rhodecode/public/js/src/rhodecode/comments.js

@@ -544,7 +544,6 b' 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
       };
 

rhodecode/public/js/src/rhodecode/pullrequests.js

@@ -372,7 +372,6 b' 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);
 };