pull-requests: migrated code from pylons to pyramid

rhodecode/api/tests/test_get_pull_request.py

@@ -21,10 +21,10 b''
 
 import pytest
 import urlobject
-from pylons import url
 
 from rhodecode.api.tests.utils import (
     build_data, api_call, assert_error, assert_ok)
+from rhodecode.lib import helpers as h
 from rhodecode.lib.utils2 import safe_unicode
 
 pytestmark = pytest.mark.backends("git", "hg")
@@ -46,10 +46,10 b' class TestGetPullRequest(object):'
         assert response.status == '200 OK'
 
         url_obj = urlobject.URLObject(
-            url(
+            h.route_url(
                 'pullrequest_show',
                 repo_name=pull_request.target_repo.repo_name,
-                pull_request_id=pull_request.pull_request_id, qualified=True))
+                pull_request_id=pull_request.pull_request_id))
 
         pr_url = safe_unicode(
             url_obj.with_netloc(http_host_only_stub))

rhodecode/apps/repository/__init__.py

@@ -217,7 +217,7 b' def includeme(config):'
     # Pull Requests
     config.add_route(
         name='pullrequest_show',
-        pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id}',
+        pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}',
         repo_route=True)
 
     config.add_route(
@@ -230,6 +230,51 b' def includeme(config):'
         pattern='/{repo_name:.*?[^/]}/pull-request-data',
         repo_route=True, repo_accepted_types=['hg', 'git'])
 
+    config.add_route(
+        name='pullrequest_repo_refs',
+        pattern='/{repo_name:.*?[^/]}/pull-request/refs/{target_repo_name:.*?[^/]}',
+        repo_route=True)
+
+    config.add_route(
+        name='pullrequest_repo_destinations',
+        pattern='/{repo_name:.*?[^/]}/pull-request/repo-destinations',
+        repo_route=True)
+
+    config.add_route(
+        name='pullrequest_new',
+        pattern='/{repo_name:.*?[^/]}/pull-request/new',
+        repo_route=True, repo_accepted_types=['hg', 'git'])
+
+    config.add_route(
+        name='pullrequest_create',
+        pattern='/{repo_name:.*?[^/]}/pull-request/create',
+        repo_route=True, repo_accepted_types=['hg', 'git'])
+
+    config.add_route(
+        name='pullrequest_update',
+        pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}/update',
+        repo_route=True)
+
+    config.add_route(
+        name='pullrequest_merge',
+        pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}/merge',
+        repo_route=True)
+
+    config.add_route(
+        name='pullrequest_delete',
+        pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}/delete',
+        repo_route=True)
+
+    config.add_route(
+        name='pullrequest_comment_create',
+        pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}/comment',
+        repo_route=True)
+
+    config.add_route(
+        name='pullrequest_comment_delete',
+        pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}/comment/{comment_id}/delete',
+        repo_route=True, repo_accepted_types=['hg', 'git'])
+
     # Settings
     config.add_route(
         name='edit_repo',

rhodecode/apps/repository/tests/test_repo_pullrequests.py

@@ -17,12 +17,11 b''
 # This program is dual-licensed. If you wish to learn more about the
 # RhodeCode Enterprise Edition, including its added features, Support services,
 # and proprietary license terms, please see https://rhodecode.com/licenses/
-
 import mock
 import pytest
-from webob.exc import HTTPNotFound
 
 import rhodecode
+from rhodecode.lib.vcs.backends.base import MergeResponse, MergeFailureReason
 from rhodecode.lib.vcs.nodes import FileNode
 from rhodecode.lib import helpers as h
 from rhodecode.model.changeset_status import ChangesetStatusModel
@@ -32,7 +31,7 b' from rhodecode.model.meta import Session'
 from rhodecode.model.pull_request import PullRequestModel
 from rhodecode.model.user import UserModel
 from rhodecode.tests import (
-    assert_session_flash, url, TEST_USER_ADMIN_LOGIN, TEST_USER_REGULAR_LOGIN)
+    assert_session_flash, TEST_USER_ADMIN_LOGIN, TEST_USER_REGULAR_LOGIN)
 from rhodecode.tests.utils import AssertResponse
 
 
@@ -40,8 +39,20 b' def route_path(name, params=None, **kwar'
     import urllib
 
     base_url = {
-        'repo_changelog':'/{repo_name}/changelog',
-        'repo_changelog_file':'/{repo_name}/changelog/{commit_id}/{f_path}',
+        'repo_changelog': '/{repo_name}/changelog',
+        'repo_changelog_file': '/{repo_name}/changelog/{commit_id}/{f_path}',
+        'pullrequest_show': '/{repo_name}/pull-request/{pull_request_id}',
+        'pullrequest_show_all': '/{repo_name}/pull-request',
+        'pullrequest_show_all_data': '/{repo_name}/pull-request-data',
+        'pullrequest_repo_refs': '/{repo_name}/pull-request/refs/{target_repo_name:.*?[^/]}',
+        'pullrequest_repo_destinations': '/{repo_name}/pull-request/repo-destinations',
+        'pullrequest_new': '/{repo_name}/pull-request/new',
+        'pullrequest_create': '/{repo_name}/pull-request/create',
+        'pullrequest_update': '/{repo_name}/pull-request/{pull_request_id}/update',
+        'pullrequest_merge': '/{repo_name}/pull-request/{pull_request_id}/merge',
+        'pullrequest_delete': '/{repo_name}/pull-request/{pull_request_id}/delete',
+        'pullrequest_comment_create': '/{repo_name}/pull-request/{pull_request_id}/comment',
+        'pullrequest_comment_delete': '/{repo_name}/pull-request/{pull_request_id}/comment/{comment_id}/delete',
     }[name].format(**kwargs)
 
     if params:
@@ -51,26 +62,26 b' def route_path(name, params=None, **kwar'
 
 @pytest.mark.usefixtures('app', 'autologin_user')
 @pytest.mark.backends("git", "hg")
-class TestPullrequestsController(object):
+class TestPullrequestsView(object):
 
     def test_index(self, backend):
-        self.app.get(url(
-            controller='pullrequests', action='index',
+        self.app.get(route_path(
+            'pullrequest_new',
             repo_name=backend.repo_name))
 
     def test_option_menu_create_pull_request_exists(self, backend):
         repo_name = backend.repo_name
         response = self.app.get(h.route_path('repo_summary', repo_name=repo_name))
 
-        create_pr_link = '<a href="%s">Create Pull Request</a>' % url(
-            'pullrequest', repo_name=repo_name)
+        create_pr_link = '<a href="%s">Create Pull Request</a>' % route_path(
+            'pullrequest_new', repo_name=repo_name)
         response.mustcontain(create_pr_link)
 
     def test_create_pr_form_with_raw_commit_id(self, backend):
         repo = backend.repo
 
         self.app.get(
-            url(controller='pullrequests', action='index',
+            route_path('pullrequest_new',
                 repo_name=repo.repo_name,
                 commit=repo.get_commit().raw_id),
             status=200)
@@ -80,10 +91,10 b' class TestPullrequestsController(object)'
         pull_request = pr_util.create_pull_request(
             mergeable=pr_merge_enabled, enable_notifications=False)
 
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
 
         for commit_id in pull_request.revisions:
             response.mustcontain(commit_id)
@@ -111,10 +122,10 b' class TestPullrequestsController(object)'
         pull_request = pr_util.create_pull_request(
             author=TEST_USER_REGULAR_LOGIN)
 
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
 
         response.mustcontain('Server-side pull request merging is disabled.')
 
@@ -126,10 +137,10 b' class TestPullrequestsController(object)'
             pull_request.target_repo,
             UserModel().get_by_username(TEST_USER_REGULAR_LOGIN),
             'repository.write')
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
 
         response.mustcontain('Server-side pull request merging is disabled.')
 
@@ -144,10 +155,10 b' class TestPullrequestsController(object)'
         Session().add(pull_request)
         Session().commit()
 
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
 
         for commit_id in pull_request.revisions:
             response.mustcontain(commit_id)
@@ -158,22 +169,21 b' class TestPullrequestsController(object)'
         Session().add(pull_request)
         Session().commit()
 
-        self.app.get(url(
-            controller='pullrequests', action='show',
+        self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
 
     def test_edit_title_description(self, pr_util, csrf_token):
         pull_request = pr_util.create_pull_request()
         pull_request_id = pull_request.pull_request_id
 
         response = self.app.post(
-            url(controller='pullrequests', action='update',
+            route_path('pullrequest_update',
                 repo_name=pull_request.target_repo.repo_name,
-                pull_request_id=str(pull_request_id)),
+                pull_request_id=pull_request_id),
             params={
                 'edit_pull_request': 'true',
-                '_method': 'put',
                 'title': 'New title',
                 'description': 'New description',
                 'csrf_token': csrf_token})
@@ -192,12 +202,11 b' class TestPullrequestsController(object)'
         pr_util.close()
 
         response = self.app.post(
-            url(controller='pullrequests', action='update',
+            route_path('pullrequest_update',
                 repo_name=pull_request.target_repo.repo_name,
-                pull_request_id=str(pull_request_id)),
+                pull_request_id=pull_request_id),
             params={
                 'edit_pull_request': 'true',
-                '_method': 'put',
                 'title': 'New title',
                 'description': 'New description',
                 'csrf_token': csrf_token})
@@ -217,10 +226,10 b' class TestPullrequestsController(object)'
         pull_request_id = pull_request.pull_request_id
 
         response = self.app.post(
-            url(controller='pullrequests', action='update',
+            route_path('pullrequest_update',
                 repo_name=pull_request.target_repo.repo_name,
-                pull_request_id=str(pull_request_id)),
-            params={'update_commits': 'true', '_method': 'put',
+                pull_request_id=pull_request_id),
+            params={'update_commits': 'true',
                     'csrf_token': csrf_token})
 
         expected_msg = PullRequestModel.UPDATE_STATUS_MESSAGES[
@@ -236,10 +245,10 b' class TestPullrequestsController(object)'
         Session().commit()
 
         pull_request_id = pull_request.pull_request_id
-        pull_request_url = url(
-            controller='pullrequests', action='show',
+        pull_request_url = route_path(
+            'pullrequest_show',
             repo_name=pull_request.target_repo.repo_name,
-            pull_request_id=str(pull_request_id))
+            pull_request_id=pull_request_id)
 
         response = self.app.get(pull_request_url)
 
@@ -258,10 +267,9 b' class TestPullrequestsController(object)'
         repo = pull_request.target_repo.repo_id
 
         self.app.post(
-            url(controller='pullrequests',
-                action='comment',
+            route_path('pullrequest_comment_create',
                 repo_name=pull_request.target_repo.scm_instance().name,
-                pull_request_id=str(pull_request_id)),
+                pull_request_id=pull_request_id),
             params={
                 'close_pull_request': '1',
                 'text': 'Closing a PR',
@@ -298,10 +306,9 b' class TestPullrequestsController(object)'
         repo = pull_request.target_repo.repo_id
 
         self.app.post(
-            url(controller='pullrequests',
-                action='comment',
+            route_path('pullrequest_comment_create',
                 repo_name=pull_request.target_repo.scm_instance().name,
-                pull_request_id=str(pull_request_id)),
+                pull_request_id=pull_request_id),
             params={
                 'close_pull_request': '1',
                 'csrf_token': csrf_token},
@@ -326,10 +333,9 b' class TestPullrequestsController(object)'
         pull_request_id = pull_request.pull_request_id
 
         response = self.app.post(
-            url(controller='pullrequests',
-                action='comment',
+            route_path('pullrequest_comment_create',
                 repo_name=pull_request.target_repo.scm_instance().name,
-                pull_request_id=str(pull_request.pull_request_id)),
+                pull_request_id=pull_request.pull_request_id),
             params={
                 'close_pull_request': 'true',
                 'csrf_token': csrf_token},
@@ -356,11 +362,7 b' class TestPullrequestsController(object)'
         source = backend.create_repo(heads=['change2'])
 
         response = self.app.post(
-            url(
-                controller='pullrequests',
-                action='create',
-                repo_name=source.repo_name
-            ),
+            route_path('pullrequest_create', repo_name=source.repo_name),
             [
                 ('source_repo', source.repo_name),
                 ('source_ref', 'branch:default:' + commit_ids['change2']),
@@ -417,11 +419,7 b' class TestPullrequestsController(object)'
         source = backend.create_repo(heads=['change'])
 
         response = self.app.post(
-            url(
-                controller='pullrequests',
-                action='create',
-                repo_name=source.repo_name
-            ),
+            route_path('pullrequest_create', repo_name=source.repo_name),
             [
                 ('source_repo', source.repo_name),
                 ('source_ref', 'branch:default:' + commit_ids['change']),
@@ -485,11 +483,7 b' class TestPullrequestsController(object)'
         source = backend.create_repo(heads=['change'])
 
         response = self.app.post(
-            url(
-                controller='pullrequests',
-                action='create',
-                repo_name=source.repo_name
-            ),
+            route_path('pullrequest_create', repo_name=source.repo_name),
             [
                 ('source_repo', source.repo_name),
                 ('source_ref', 'branch:default:' + commit_ids['change']),
@@ -542,10 +536,9 b' class TestPullrequestsController(object)'
         repo_name = pull_request.target_repo.scm_instance().name,
 
         response = self.app.post(
-            url(controller='pullrequests',
-                action='merge',
+            route_path('pullrequest_merge',
                 repo_name=str(repo_name[0]),
-                pull_request_id=str(pull_request_id)),
+                pull_request_id=pull_request_id),
             params={'csrf_token': csrf_token}).follow()
 
         pull_request = PullRequest.get(pull_request_id)
@@ -584,10 +577,9 b' class TestPullrequestsController(object)'
         pull_request = PullRequest.get(pull_request_id)
 
         response = self.app.post(
-            url(controller='pullrequests',
-                action='merge',
+            route_path('pullrequest_merge',
                 repo_name=pull_request.target_repo.scm_instance().name,
-                pull_request_id=str(pull_request.pull_request_id)),
+                pull_request_id=pull_request.pull_request_id),
             params={'csrf_token': csrf_token}).follow()
 
         assert response.status_int == 200
@@ -599,13 +591,12 b' class TestPullrequestsController(object)'
     def test_merge_pull_request_not_approved(self, pr_util, csrf_token):
         pull_request = pr_util.create_pull_request(mergeable=True)
         pull_request_id = pull_request.pull_request_id
-        repo_name = pull_request.target_repo.scm_instance().name,
+        repo_name = pull_request.target_repo.scm_instance().name
 
         response = self.app.post(
-            url(controller='pullrequests',
-                action='merge',
-                repo_name=str(repo_name[0]),
-                pull_request_id=str(pull_request_id)),
+            route_path('pullrequest_merge',
+                repo_name=repo_name,
+                pull_request_id=pull_request_id),
             params={'csrf_token': csrf_token}).follow()
 
         assert response.status_int == 200
@@ -614,6 +605,28 b' class TestPullrequestsController(object)'
             'Merge is not currently possible because of below failed checks.')
         response.mustcontain('Pull request reviewer approval is pending.')
 
+    def test_merge_pull_request_renders_failure_reason(
+            self, user_regular, csrf_token, pr_util):
+        pull_request = pr_util.create_pull_request(mergeable=True, approved=True)
+        pull_request_id = pull_request.pull_request_id
+        repo_name = pull_request.target_repo.scm_instance().name
+
+        model_patcher = mock.patch.multiple(
+            PullRequestModel,
+            merge=mock.Mock(return_value=MergeResponse(
+                True, False, 'STUB_COMMIT_ID', MergeFailureReason.PUSH_FAILED)),
+            merge_status=mock.Mock(return_value=(True, 'WRONG_MESSAGE')))
+
+        with model_patcher:
+            response = self.app.post(
+                route_path('pullrequest_merge',
+                           repo_name=repo_name,
+                           pull_request_id=pull_request_id),
+                params={'csrf_token': csrf_token}, status=302)
+
+        assert_session_flash(response, PullRequestModel.MERGE_STATUS_MESSAGES[
+            MergeFailureReason.PUSH_FAILED])
+
     def test_update_source_revision(self, backend, csrf_token):
         commits = [
             {'message': 'ancestor'},
@@ -649,10 +662,10 b' class TestPullrequestsController(object)'
 
         # update PR
         self.app.post(
-            url(controller='pullrequests', action='update',
+            route_path('pullrequest_update',
                 repo_name=target.repo_name,
-                pull_request_id=str(pull_request_id)),
-            params={'update_commits': 'true', '_method': 'put',
+                pull_request_id=pull_request_id),
+            params={'update_commits': 'true',
                     'csrf_token': csrf_token})
 
         # check that we have now both revisions
@@ -661,8 +674,8 b' class TestPullrequestsController(object)'
             commit_ids['change-2'], commit_ids['change']]
 
         # TODO: johbo: this should be a test on its own
-        response = self.app.get(url(
-            controller='pullrequests', action='index',
+        response = self.app.get(route_path(
+            'pullrequest_new',
             repo_name=target.repo_name))
         assert response.status_int == 200
         assert 'Pull request updated to' in response.body
@@ -707,10 +720,10 b' class TestPullrequestsController(object)'
 
         # update PR
         self.app.post(
-            url(controller='pullrequests', action='update',
+            route_path('pullrequest_update',
                 repo_name=target.repo_name,
-                pull_request_id=str(pull_request_id)),
-            params={'update_commits': 'true', '_method': 'put',
+                pull_request_id=pull_request_id),
+            params={'update_commits': 'true',
                     'csrf_token': csrf_token},
             status=200)
 
@@ -722,8 +735,8 b' class TestPullrequestsController(object)'
             commit_id=commit_ids['ancestor-new'])
 
         # TODO: johbo: This should be a test on its own
-        response = self.app.get(url(
-            controller='pullrequests', action='index',
+        response = self.app.get(route_path(
+            'pullrequest_new',
             repo_name=target.repo_name))
         assert response.status_int == 200
         assert 'Pull request updated to' in response.body
@@ -770,10 +783,10 b' class TestPullrequestsController(object)'
 
         # update PR
         self.app.post(
-            url(controller='pullrequests', action='update',
+            route_path('pullrequest_update',
                 repo_name=target.repo_name,
-                pull_request_id=str(pull_request_id)),
-            params={'update_commits': 'true', '_method': 'put',
+                pull_request_id=pull_request_id),
+            params={'update_commits': 'true',
                     'csrf_token': csrf_token},
             status=200)
 
@@ -814,10 +827,10 b' class TestPullrequestsController(object)'
         vcs = repo.scm_instance()
         vcs.remove_ref('refs/heads/{}'.format(branch_name))
 
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=repo.repo_name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
 
         assert response.status_int == 200
         assert_response = AssertResponse(response)
@@ -846,10 +859,10 b' class TestPullrequestsController(object)'
         else:
             vcs.strip(pr_util.commit_ids['new-feature'])
 
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pr_util.target_repository.repo_name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
 
         assert response.status_int == 200
         assert_response = AssertResponse(response)
@@ -882,20 +895,20 b' class TestPullrequestsController(object)'
             vcs.strip(pr_util.commit_ids['new-feature'])
 
         response = self.app.post(
-            url(controller='pullrequests', action='update',
+            route_path('pullrequest_update',
                 repo_name=pull_request.target_repo.repo_name,
-                pull_request_id=str(pull_request.pull_request_id)),
-            params={'update_commits': 'true', '_method': 'put',
+                pull_request_id=pull_request.pull_request_id),
+            params={'update_commits': 'true',
                     'csrf_token': csrf_token})
 
         assert response.status_int == 200
         assert response.body == 'true'
 
         # Make sure that after update, it won't raise 500 errors
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pr_util.target_repository.repo_name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
 
         assert response.status_int == 200
         assert_response = AssertResponse(response)
@@ -910,10 +923,10 b' class TestPullrequestsController(object)'
         Session().add(pull_request)
         Session().commit()
 
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
         assert response.status_int == 200
         assert_response = AssertResponse(response)
 
@@ -944,10 +957,10 b' class TestPullrequestsController(object)'
         Session().add(pull_request)
         Session().commit()
 
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
         assert response.status_int == 200
         assert_response = AssertResponse(response)
 
@@ -966,10 +979,10 b' class TestPullrequestsController(object)'
         Session().add(pull_request)
         Session().commit()
 
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
         assert response.status_int == 200
         assert_response = AssertResponse(response)
 
@@ -996,10 +1009,10 b' class TestPullrequestsController(object)'
         shadow_url = '{host}/{repo}/pull-request/{pr_id}/repository'.format(
             host=http_host_only_stub, repo=target_repo.name, pr_id=pr_id)
 
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=target_repo.name,
-            pull_request_id=str(pr_id)))
+            pull_request_id=pr_id))
 
         assertr = AssertResponse(response)
         if mergeable:
@@ -1019,10 +1032,10 b' class TestPullrequestsControllerDelete(o'
         pull_request = pr_util.create_pull_request(
             author=user_admin.username, enable_notifications=False)
 
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
 
         response.mustcontain('id="delete_pullrequest"')
         response.mustcontain('Confirm to delete this pull request')
@@ -1032,10 +1045,10 b' class TestPullrequestsControllerDelete(o'
         pull_request = pr_util.create_pull_request(
             author=user_regular.username, enable_notifications=False)
 
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
 
         response.mustcontain('id="delete_pullrequest"')
         response.mustcontain('Confirm to delete this pull request')
@@ -1045,10 +1058,10 b' class TestPullrequestsControllerDelete(o'
         pull_request = pr_util.create_pull_request(
             author=user_admin.username, enable_notifications=False)
 
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
         response.mustcontain(no=['id="delete_pullrequest"'])
         response.mustcontain(no=['Confirm to delete this pull request'])
 
@@ -1063,10 +1076,10 b' class TestPullrequestsControllerDelete(o'
             pull_request.target_repo, user_regular,
             'repository.write')
 
-        response = self.app.get(url(
-            controller='pullrequests', action='show',
+        response = self.app.get(route_path(
+            'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
-            pull_request_id=str(pull_request.pull_request_id)))
+            pull_request_id=pull_request.pull_request_id))
 
         response.mustcontain('id="open_edit_pullrequest"')
         response.mustcontain('id="delete_pullrequest"')
@@ -1078,9 +1091,10 b' class TestPullrequestsControllerDelete(o'
         pull_request = pr_util.create_pull_request(
             author=user_admin.username, enable_notifications=False)
 
-        self.app.get(url(
-            controller='pullrequests', action='delete_comment',
+        self.app.get(route_path(
+            'pullrequest_comment_delete',
             repo_name=pull_request.target_repo.scm_instance().name,
+            pull_request_id=pull_request.pull_request_id,
             comment_id=1024404), status=404)
 
 
@@ -1090,17 +1104,9 b' def assert_pull_request_status(pull_requ'
     assert status == expected_status
 
 
[e-mail]('action', ['index', 'create'])
[e-mail]('route', ['pullrequest_new', 'pullrequest_create'])
 @pytest.mark.usefixtures("autologin_user")
-def test_redirects_to_repo_summary_for_svn_repositories(backend_svn, app, action):
-    response = app.get(url(
-        controller='pullrequests', action=action,
-        repo_name=backend_svn.repo_name))
-    assert response.status_int == 302
+def test_forbidde_to_repo_summary_for_svn_repositories(backend_svn, app, route):
+    response = app.get(
+        route_path(route, repo_name=backend_svn.repo_name), status=404)
 
-    # Not allowed, redirect to the summary
-    redirected = response.follow()
-    summary_url = h.route_path('repo_summary', repo_name=backend_svn.repo_name)
-
-    # URL adds leading slash and path doesn't have it
-    assert redirected.request.path == summary_url

rhodecode/apps/repository/views/repo_pull_requests.py

@@ -19,23 +19,35 b''
 # and proprietary license terms, please see https://rhodecode.com/licenses/
 
 import logging
+import collections
 
-import collections
-from pyramid.httpexceptions import HTTPFound, HTTPNotFound
+import formencode
+import peppercorn
+from pyramid.httpexceptions import (
+    HTTPFound, HTTPNotFound, HTTPForbidden, HTTPBadRequest)
 from pyramid.view import view_config
+from pyramid.renderers import render
 
+from rhodecode import events
 from rhodecode.apps._base import RepoAppView, DataGridAppView
-from rhodecode.lib import helpers as h, diffs, codeblocks
+
+from rhodecode.lib import helpers as h, diffs, codeblocks, channelstream
+from rhodecode.lib.base import vcs_operation_context
+from rhodecode.lib.ext_json import json
 from rhodecode.lib.auth import (
-    LoginRequired, HasRepoPermissionAnyDecorator)
-from rhodecode.lib.utils2 import str2bool, safe_int, safe_str
-from rhodecode.lib.vcs.backends.base import EmptyCommit
-from rhodecode.lib.vcs.exceptions import CommitDoesNotExistError, \
-    RepositoryRequirementError, NodeDoesNotExistError
+    LoginRequired, HasRepoPermissionAnyDecorator, NotAnonymous, CSRFRequired)
+from rhodecode.lib.utils2 import str2bool, safe_str, safe_unicode
+from rhodecode.lib.vcs.backends.base import EmptyCommit, UpdateFailureReason
+from rhodecode.lib.vcs.exceptions import (CommitDoesNotExistError,
+    RepositoryRequirementError, NodeDoesNotExistError, EmptyRepositoryError)
+from rhodecode.model.changeset_status import ChangesetStatusModel
 from rhodecode.model.comment import CommentsModel
-from rhodecode.model.db import PullRequest, PullRequestVersion, \
-    ChangesetComment, ChangesetStatus
+from rhodecode.model.db import (func, or_, PullRequest, PullRequestVersion,
+    ChangesetComment, ChangesetStatus, Repository)
+from rhodecode.model.forms import PullRequestForm
+from rhodecode.model.meta import Session
 from rhodecode.model.pull_request import PullRequestModel, MergeCheck
+from rhodecode.model.scm import ScmModel
 
 log = logging.getLogger(__name__)
 
@@ -189,7 +201,6 b' class RepoPullRequestsView(RepoAppView, '
         return data
 
     def _get_pr_version(self, pull_request_id, version=None):
-        pull_request_id = safe_int(pull_request_id)
         at_version = None
 
         if version and version == 'latest':
@@ -250,12 +261,12 b' class RepoPullRequestsView(RepoAppView, '
     @LoginRequired()
     @HasRepoPermissionAnyDecorator(
         'repository.read', 'repository.write', 'repository.admin')
-    # @view_config(
-    #     route_name='pullrequest_show', request_method='GET',
-    #     renderer='rhodecode:templates/pullrequests/pullrequest_show.mako')
+    @view_config(
+        route_name='pullrequest_show', request_method='GET',
+        renderer='rhodecode:templates/pullrequests/pullrequest_show.mako')
     def pull_request_show(self):
-        pull_request_id = safe_int(
-            self.request.matchdict.get('pull_request_id'))
+        pull_request_id = self.request.matchdict.get('pull_request_id')
+
         c = self.load_default_context()
 
         version = self.request.GET.get('version')
@@ -582,3 +593,590 b' class RepoPullRequestsView(RepoAppView, '
                             c.review_versions[_ver_pr] = status[0]
 
         return self._get_template_context(c)
+
+    def assure_not_empty_repo(self):
+        _ = self.request.translate
+
+        try:
+            self.db_repo.scm_instance().get_commit()
+        except EmptyRepositoryError:
+            h.flash(h.literal(_('There are no commits yet')),
+                    category='warning')
+            raise HTTPFound(
+                h.route_path('repo_summary', repo_name=self.db_repo.repo_name))
+
+    @LoginRequired()
+    @NotAnonymous()
+    @HasRepoPermissionAnyDecorator(
+        'repository.read', 'repository.write', 'repository.admin')
+    @view_config(
+        route_name='pullrequest_new', request_method='GET',
+        renderer='rhodecode:templates/pullrequests/pullrequest.mako')
+    def pull_request_new(self):
+        _ = self.request.translate
+        c = self.load_default_context()
+
+        self.assure_not_empty_repo()
+        source_repo = self.db_repo
+
+        commit_id = self.request.GET.get('commit')
+        branch_ref = self.request.GET.get('branch')
+        bookmark_ref = self.request.GET.get('bookmark')
+
+        try:
+            source_repo_data = PullRequestModel().generate_repo_data(
+                source_repo, commit_id=commit_id,
+                branch=branch_ref, bookmark=bookmark_ref)
+        except CommitDoesNotExistError as e:
+            log.exception(e)
+            h.flash(_('Commit does not exist'), 'error')
+            raise HTTPFound(
+                h.route_path('pullrequest_new', repo_name=source_repo.repo_name))
+
+        default_target_repo = source_repo
+
+        if source_repo.parent:
+            parent_vcs_obj = source_repo.parent.scm_instance()
+            if parent_vcs_obj and not parent_vcs_obj.is_empty():
+                # change default if we have a parent repo
+                default_target_repo = source_repo.parent
+
+        target_repo_data = PullRequestModel().generate_repo_data(
+            default_target_repo)
+
+        selected_source_ref = source_repo_data['refs']['selected_ref']
+
+        title_source_ref = selected_source_ref.split(':', 2)[1]
+        c.default_title = PullRequestModel().generate_pullrequest_title(
+            source=source_repo.repo_name,
+            source_ref=title_source_ref,
+            target=default_target_repo.repo_name
+        )
+
+        c.default_repo_data = {
+            'source_repo_name': source_repo.repo_name,
+            'source_refs_json': json.dumps(source_repo_data),
+            'target_repo_name': default_target_repo.repo_name,
+            'target_refs_json': json.dumps(target_repo_data),
+        }
+        c.default_source_ref = selected_source_ref
+
+        return self._get_template_context(c)
+
+    @LoginRequired()
+    @NotAnonymous()
+    @HasRepoPermissionAnyDecorator(
+        'repository.read', 'repository.write', 'repository.admin')
+    @view_config(
+        route_name='pullrequest_repo_refs', request_method='GET',
+        renderer='json_ext', xhr=True)
+    def pull_request_repo_refs(self):
+        target_repo_name = self.request.matchdict['target_repo_name']
+        repo = Repository.get_by_repo_name(target_repo_name)
+        if not repo:
+            raise HTTPNotFound()
+        return PullRequestModel().generate_repo_data(repo)
+
+    @LoginRequired()
+    @NotAnonymous()
+    @HasRepoPermissionAnyDecorator(
+        'repository.read', 'repository.write', 'repository.admin')
+    @view_config(
+        route_name='pullrequest_repo_destinations', request_method='GET',
+        renderer='json_ext', xhr=True)
+    def pull_request_repo_destinations(self):
+        _ = self.request.translate
+        filter_query = self.request.GET.get('query')
+
+        query = Repository.query() \
+            .order_by(func.length(Repository.repo_name)) \
+            .filter(
+                or_(Repository.repo_name == self.db_repo.repo_name,
+                    Repository.fork_id == self.db_repo.repo_id))
+
+        if filter_query:
+            ilike_expression = u'%{}%'.format(safe_unicode(filter_query))
+            query = query.filter(
+                Repository.repo_name.ilike(ilike_expression))
+
+        add_parent = False
+        if self.db_repo.parent:
+            if filter_query in self.db_repo.parent.repo_name:
+                parent_vcs_obj = self.db_repo.parent.scm_instance()
+                if parent_vcs_obj and not parent_vcs_obj.is_empty():
+                    add_parent = True
+
+        limit = 20 - 1 if add_parent else 20
+        all_repos = query.limit(limit).all()
+        if add_parent:
+            all_repos += [self.db_repo.parent]
+
+        repos = []
+        for obj in ScmModel().get_repos(all_repos):
+            repos.append({
+                'id': obj['name'],
+                'text': obj['name'],
+                'type': 'repo',
+                'obj': obj['dbrepo']
+            })
+
+        data = {
+            'more': False,
+            'results': [{
+                'text': _('Repositories'),
+                'children': repos
+            }] if repos else []
+        }
+        return data
+
+    @LoginRequired()
+    @NotAnonymous()
+    @HasRepoPermissionAnyDecorator(
+        'repository.read', 'repository.write', 'repository.admin')
+    @CSRFRequired()
+    @view_config(
+        route_name='pullrequest_create', request_method='POST',
+        renderer=None)
+    def pull_request_create(self):
+        _ = self.request.translate
+        self.assure_not_empty_repo()
+
+        controls = peppercorn.parse(self.request.POST.items())
+
+        try:
+            _form = PullRequestForm(self.db_repo.repo_id)().to_python(controls)
+        except formencode.Invalid as errors:
+            if errors.error_dict.get('revisions'):
+                msg = 'Revisions: %s' % errors.error_dict['revisions']
+            elif errors.error_dict.get('pullrequest_title'):
+                msg = _('Pull request requires a title with min. 3 chars')
+            else:
+                msg = _('Error creating pull request: {}').format(errors)
+            log.exception(msg)
+            h.flash(msg, 'error')
+
+            # would rather just go back to form ...
+            raise HTTPFound(
+                h.route_path('pullrequest_new', repo_name=self.db_repo_name))
+
+        source_repo = _form['source_repo']
+        source_ref = _form['source_ref']
+        target_repo = _form['target_repo']
+        target_ref = _form['target_ref']
+        commit_ids = _form['revisions'][::-1]
+
+        # find the ancestor for this pr
+        source_db_repo = Repository.get_by_repo_name(_form['source_repo'])
+        target_db_repo = Repository.get_by_repo_name(_form['target_repo'])
+
+        source_scm = source_db_repo.scm_instance()
+        target_scm = target_db_repo.scm_instance()
+
+        source_commit = source_scm.get_commit(source_ref.split(':')[-1])
+        target_commit = target_scm.get_commit(target_ref.split(':')[-1])
+
+        ancestor = source_scm.get_common_ancestor(
+            source_commit.raw_id, target_commit.raw_id, target_scm)
+
+        target_ref_type, target_ref_name, __ = _form['target_ref'].split(':')
+        target_ref = ':'.join((target_ref_type, target_ref_name, ancestor))
+
+        pullrequest_title = _form['pullrequest_title']
+        title_source_ref = source_ref.split(':', 2)[1]
+        if not pullrequest_title:
+            pullrequest_title = PullRequestModel().generate_pullrequest_title(
+                source=source_repo,
+                source_ref=title_source_ref,
+                target=target_repo
+            )
+
+        description = _form['pullrequest_desc']
+
+        get_default_reviewers_data, validate_default_reviewers = \
+            PullRequestModel().get_reviewer_functions()
+
+        # recalculate reviewers logic, to make sure we can validate this
+        reviewer_rules = get_default_reviewers_data(
+            self._rhodecode_db_user, source_db_repo,
+            source_commit, target_db_repo, target_commit)
+
+        given_reviewers = _form['review_members']
+        reviewers = validate_default_reviewers(given_reviewers, reviewer_rules)
+
+        try:
+            pull_request = PullRequestModel().create(
+                self._rhodecode_user.user_id, source_repo, source_ref, target_repo,
+                target_ref, commit_ids, reviewers, pullrequest_title,
+                description, reviewer_rules
+            )
+            Session().commit()
+            h.flash(_('Successfully opened new pull request'),
+                    category='success')
+        except Exception as e:
+            msg = _('Error occurred during creation of this pull request.')
+            log.exception(msg)
+            h.flash(msg, category='error')
+            raise HTTPFound(
+                h.route_path('pullrequest_new', repo_name=self.db_repo_name))
+
+        raise HTTPFound(
+            h.route_path('pullrequest_show', repo_name=target_repo,
+                         pull_request_id=pull_request.pull_request_id))
+
+    @LoginRequired()
+    @NotAnonymous()
+    @HasRepoPermissionAnyDecorator(
+        'repository.read', 'repository.write', 'repository.admin')
+    @CSRFRequired()
+    @view_config(
+        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)
+
+        # only owner or admin can update it
+        allowed_to_update = PullRequestModel().check_user_update(
+            pull_request, self._rhodecode_user)
+        if allowed_to_update:
+            controls = peppercorn.parse(self.request.POST.items())
+
+            if 'review_members' in controls:
+                self._update_reviewers(
+                    pull_request_id, controls['review_members'],
+                    pull_request.reviewer_data)
+            elif str2bool(self.request.POST.get('update_commits', 'false')):
+                self._update_commits(pull_request)
+            elif str2bool(self.request.POST.get('edit_pull_request', 'false')):
+                self._edit_pull_request(pull_request)
+            else:
+                raise HTTPBadRequest()
+            return True
+        raise HTTPForbidden()
+
+    def _edit_pull_request(self, pull_request):
+        _ = self.request.translate
+        try:
+            PullRequestModel().edit(
+                pull_request, self.request.POST.get('title'),
+                self.request.POST.get('description'), self._rhodecode_user)
+        except ValueError:
+            msg = _(u'Cannot update closed pull requests.')
+            h.flash(msg, category='error')
+            return
+        else:
+            Session().commit()
+
+        msg = _(u'Pull request title & description updated.')
+        h.flash(msg, category='success')
+        return
+
+    def _update_commits(self, pull_request):
+        _ = self.request.translate
+        resp = PullRequestModel().update_commits(pull_request)
+
+        if resp.executed:
+
+            if resp.target_changed and resp.source_changed:
+                changed = 'target and source repositories'
+            elif resp.target_changed and not resp.source_changed:
+                changed = 'target repository'
+            elif not resp.target_changed and resp.source_changed:
+                changed = 'source repository'
+            else:
+                changed = 'nothing'
+
+            msg = _(
+                u'Pull request updated to "{source_commit_id}" with '
+                u'{count_added} added, {count_removed} removed commits. '
+                u'Source of changes: {change_source}')
+            msg = msg.format(
+                source_commit_id=pull_request.source_ref_parts.commit_id,
+                count_added=len(resp.changes.added),
+                count_removed=len(resp.changes.removed),
+                change_source=changed)
+            h.flash(msg, category='success')
+
+            channel = '/repo${}$/pr/{}'.format(
+                pull_request.target_repo.repo_name,
+                pull_request.pull_request_id)
+            message = msg + (
+                ' - <a onclick="window.location.reload()">'
+                '<strong>{}</strong></a>'.format(_('Reload page')))
+            channelstream.post_message(
+                channel, message, self._rhodecode_user.username,
+                registry=self.request.registry)
+        else:
+            msg = PullRequestModel.UPDATE_STATUS_MESSAGES[resp.reason]
+            warning_reasons = [
+                UpdateFailureReason.NO_CHANGE,
+                UpdateFailureReason.WRONG_REF_TYPE,
+            ]
+            category = 'warning' if resp.reason in warning_reasons else 'error'
+            h.flash(msg, category=category)
+
+    @LoginRequired()
+    @NotAnonymous()
+    @HasRepoPermissionAnyDecorator(
+        'repository.read', 'repository.write', 'repository.admin')
+    @CSRFRequired()
+    @view_config(
+        route_name='pullrequest_merge', request_method='POST',
+        renderer='json_ext')
+    def pull_request_merge(self):
+        """
+        Merge will perform a server-side merge of the specified
+        pull request, if the pull request is approved and mergeable.
+        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)
+
+        check = MergeCheck.validate(pull_request, self._rhodecode_db_user)
+        merge_possible = not check.failed
+
+        for err_type, error_msg in check.errors:
+            h.flash(error_msg, category=err_type)
+
+        if merge_possible:
+            log.debug("Pre-conditions checked, trying to merge.")
+            extras = vcs_operation_context(
+                self.request.environ, repo_name=pull_request.target_repo.repo_name,
+                username=self._rhodecode_db_user.username, action='push',
+                scm=pull_request.target_repo.repo_type)
+            self._merge_pull_request(
+                pull_request, self._rhodecode_db_user, extras)
+        else:
+            log.debug("Pre-conditions failed, NOT merging.")
+
+        raise HTTPFound(
+            h.route_path('pullrequest_show',
+                         repo_name=pull_request.target_repo.repo_name,
+                         pull_request_id=pull_request.pull_request_id))
+
+    def _merge_pull_request(self, pull_request, user, extras):
+        _ = self.request.translate
+        merge_resp = PullRequestModel().merge(pull_request, user, extras=extras)
+
+        if merge_resp.executed:
+            log.debug("The merge was successful, closing the pull request.")
+            PullRequestModel().close_pull_request(
+                pull_request.pull_request_id, user)
+            Session().commit()
+            msg = _('Pull request was successfully merged and closed.')
+            h.flash(msg, category='success')
+        else:
+            log.debug(
+                "The merge was not successful. Merge response: %s",
+                merge_resp)
+            msg = PullRequestModel().merge_status_message(
+                merge_resp.failure_reason)
+            h.flash(msg, category='error')
+
+    def _update_reviewers(self, pull_request_id, review_members, reviewer_rules):
+        _ = self.request.translate
+        get_default_reviewers_data, validate_default_reviewers = \
+            PullRequestModel().get_reviewer_functions()
+
+        try:
+            reviewers = validate_default_reviewers(review_members, reviewer_rules)
+        except ValueError as e:
+            log.error('Reviewers Validation: {}'.format(e))
+            h.flash(e, category='error')
+            return
+
+        PullRequestModel().update_reviewers(
+            pull_request_id, reviewers, self._rhodecode_user)
+        h.flash(_('Pull request reviewers updated.'), category='success')
+        Session().commit()
+
+    @LoginRequired()
+    @NotAnonymous()
+    @HasRepoPermissionAnyDecorator(
+        'repository.read', 'repository.write', 'repository.admin')
+    @CSRFRequired()
+    @view_config(
+        route_name='pullrequest_delete', request_method='POST',
+        renderer='json_ext')
+    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)
+
+        pr_closed = pull_request.is_closed()
+        allowed_to_delete = PullRequestModel().check_user_delete(
+            pull_request, self._rhodecode_user) and not pr_closed
+
+        # only owner can delete it !
+        if allowed_to_delete:
+            PullRequestModel().delete(pull_request, self._rhodecode_user)
+            Session().commit()
+            h.flash(_('Successfully deleted pull request'),
+                    category='success')
+            raise HTTPFound(h.route_path('my_account_pullrequests'))
+
+        log.warning('user %s tried to delete pull request without access',
+                    self._rhodecode_user)
+        raise HTTPNotFound()
+
+    @LoginRequired()
+    @NotAnonymous()
+    @HasRepoPermissionAnyDecorator(
+        'repository.read', 'repository.write', 'repository.admin')
+    @CSRFRequired()
+    @view_config(
+        route_name='pullrequest_comment_create', request_method='POST',
+        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)
+        if pull_request.is_closed():
+            log.debug('comment: forbidden because pull request is closed')
+            raise HTTPForbidden()
+
+        c = self.load_default_context()
+
+        status = self.request.POST.get('changeset_status', None)
+        text = self.request.POST.get('text')
+        comment_type = self.request.POST.get('comment_type')
+        resolves_comment_id = self.request.POST.get('resolves_comment_id', None)
+        close_pull_request = self.request.POST.get('close_pull_request')
+
+        # the logic here should work like following, if we submit close
+        # pr comment, use `close_pull_request_with_comment` function
+        # else handle regular comment logic
+
+        if close_pull_request:
+            # only owner or admin or person with write permissions
+            allowed_to_close = PullRequestModel().check_user_update(
+                pull_request, self._rhodecode_user)
+            if not allowed_to_close:
+                log.debug('comment: forbidden because not allowed to close '
+                          'pull request %s', pull_request_id)
+                raise HTTPForbidden()
+            comment, status = PullRequestModel().close_pull_request_with_comment(
+                pull_request, self._rhodecode_user, self.db_repo, message=text)
+            Session().flush()
+            events.trigger(
+                events.PullRequestCommentEvent(pull_request, comment))
+
+        else:
+            # regular comment case, could be inline, or one with status.
+            # for that one we check also permissions
+
+            allowed_to_change_status = PullRequestModel().check_user_change_status(
+                pull_request, self._rhodecode_user)
+
+            if status and allowed_to_change_status:
+                message = (_('Status change %(transition_icon)s %(status)s')
+                           % {'transition_icon': '>',
+                              'status': ChangesetStatus.get_status_lbl(status)})
+                text = text or message
+
+            comment = CommentsModel().create(
+                text=text,
+                repo=self.db_repo.repo_id,
+                user=self._rhodecode_user.user_id,
+                pull_request=pull_request_id,
+                f_path=self.request.POST.get('f_path'),
+                line_no=self.request.POST.get('line'),
+                status_change=(ChangesetStatus.get_status_lbl(status)
+                               if status and allowed_to_change_status else None),
+                status_change_type=(status
+                                    if status and allowed_to_change_status else None),
+                comment_type=comment_type,
+                resolves_comment_id=resolves_comment_id
+            )
+
+            if allowed_to_change_status:
+                # calculate old status before we change it
+                old_calculated_status = pull_request.calculated_review_status()
+
+                # get status if set !
+                if status:
+                    ChangesetStatusModel().set_status(
+                        self.db_repo.repo_id,
+                        status,
+                        self._rhodecode_user.user_id,
+                        comment,
+                        pull_request=pull_request_id
+                    )
+
+                Session().flush()
+                events.trigger(
+                    events.PullRequestCommentEvent(pull_request, comment))
+
+                # we now calculate the status of pull request, and based on that
+                # calculation we set the commits status
+                calculated_status = pull_request.calculated_review_status()
+                if old_calculated_status != calculated_status:
+                    PullRequestModel()._trigger_pull_request_hook(
+                        pull_request, self._rhodecode_user, 'review_status_change')
+
+        Session().commit()
+
+        data = {
+            'target_id': h.safeid(h.safe_unicode(
+                self.request.POST.get('f_path'))),
+        }
+        if comment:
+            c.co = comment
+            rendered_comment = render(
+                'rhodecode:templates/changeset/changeset_comment_block.mako',
+                self._get_template_context(c), self.request)
+
+            data.update(comment.get_dict())
+            data.update({'rendered_text': rendered_comment})
+
+        return data
+
+    @LoginRequired()
+    @NotAnonymous()
+    @HasRepoPermissionAnyDecorator(
+        'repository.read', 'repository.write', 'repository.admin')
+    @CSRFRequired()
+    @view_config(
+        route_name='pullrequest_comment_delete', request_method='POST',
+        renderer='json_ext')
+    def pull_request_comment_delete(self):
+        commit_id = self.request.matchdict['commit_id']
+        comment_id = self.request.matchdict['comment_id']
+        pull_request_id = self.request.matchdict['pull_request_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
+            return True
+
+        if comment.pull_request.is_closed():
+            # don't allow deleting comments on closed pull request
+            raise HTTPForbidden()
+
+        is_repo_admin = h.HasRepoPermissionAny('repository.admin')(self.db_repo_name)
+        super_admin = h.HasPermissionAny('hg.admin')()
+        comment_owner = comment.author.user_id == self._rhodecode_user.user_id
+        is_repo_comment = comment.repo.repo_name == self.db_repo_name
+        comment_repo_admin = is_repo_admin and is_repo_comment
+
+        if super_admin or comment_owner or comment_repo_admin:
+            old_calculated_status = comment.pull_request.calculated_review_status()
+            CommentsModel().delete(comment=comment, user=self._rhodecode_user)
+            Session().commit()
+            calculated_status = comment.pull_request.calculated_review_status()
+            if old_calculated_status != calculated_status:
+                PullRequestModel()._trigger_pull_request_hook(
+                    comment.pull_request, self._rhodecode_user, 'review_status_change')
+            return True
+        else:
+            log.warning('No permissions for user %s to delete comment_id: %s',
+                        self._rhodecode_db_user, comment_id)
+            raise HTTPNotFound()

rhodecode/config/routing.py

@@ -503,73 +503,6 b' def make_map(config):'
                  requirements=URL_NAME_REQUIREMENTS)
 
 
-    rmap.connect('pullrequest_home',
-                 '/{repo_name}/pull-request/new', controller='pullrequests',
-                 action='index', conditions={'function': check_repo,
-                                             'method': ['GET']},
-                 requirements=URL_NAME_REQUIREMENTS, jsroute=True)
-
-    rmap.connect('pullrequest',
-                 '/{repo_name}/pull-request/new', controller='pullrequests',
-                 action='create', conditions={'function': check_repo,
-                                              'method': ['POST']},
-                 requirements=URL_NAME_REQUIREMENTS, jsroute=True)
-
-    rmap.connect('pullrequest_repo_refs',
-                 '/{repo_name}/pull-request/refs/{target_repo_name:.*?[^/]}',
-                 controller='pullrequests',
-                 action='get_repo_refs',
-                 conditions={'function': check_repo, 'method': ['GET']},
-                 requirements=URL_NAME_REQUIREMENTS, jsroute=True)
-
-    rmap.connect('pullrequest_repo_destinations',
-                 '/{repo_name}/pull-request/repo-destinations',
-                 controller='pullrequests',
-                 action='get_repo_destinations',
-                 conditions={'function': check_repo, 'method': ['GET']},
-                 requirements=URL_NAME_REQUIREMENTS, jsroute=True)
-
-    rmap.connect('pullrequest_show',
-                 '/{repo_name}/pull-request/{pull_request_id}',
-                 controller='pullrequests',
-                 action='show', conditions={'function': check_repo,
-                                            'method': ['GET']},
-                 requirements=URL_NAME_REQUIREMENTS, jsroute=True)
-
-    rmap.connect('pullrequest_update',
-                 '/{repo_name}/pull-request/{pull_request_id}',
-                 controller='pullrequests',
-                 action='update', conditions={'function': check_repo,
-                                              'method': ['PUT']},
-                 requirements=URL_NAME_REQUIREMENTS, jsroute=True)
-
-    rmap.connect('pullrequest_merge',
-                 '/{repo_name}/pull-request/{pull_request_id}',
-                 controller='pullrequests',
-                 action='merge', conditions={'function': check_repo,
-                                             'method': ['POST']},
-                 requirements=URL_NAME_REQUIREMENTS)
-
-    rmap.connect('pullrequest_delete',
-                 '/{repo_name}/pull-request/{pull_request_id}',
-                 controller='pullrequests',
-                 action='delete', conditions={'function': check_repo,
-                                              'method': ['DELETE']},
-                 requirements=URL_NAME_REQUIREMENTS)
-
-    rmap.connect('pullrequest_comment',
-                 '/{repo_name}/pull-request-comment/{pull_request_id}',
-                 controller='pullrequests',
-                 action='comment', conditions={'function': check_repo,
-                                               'method': ['POST']},
-                 requirements=URL_NAME_REQUIREMENTS, jsroute=True)
-
-    rmap.connect('pullrequest_comment_delete',
-                 '/{repo_name}/pull-request-comment/{comment_id}/delete',
-                 controller='pullrequests', action='delete_comment',
-                 conditions={'function': check_repo, 'method': ['DELETE']},
-                 requirements=URL_NAME_REQUIREMENTS, jsroute=True)
-
     rmap.connect('repo_fork_create_home', '/{repo_name}/fork',
                  controller='forks', action='fork_create',
                  conditions={'function': check_repo, 'method': ['POST']},

rhodecode/model/comment.py

@@ -298,12 +298,11 b' class CommentsModel(BaseModel):'
             pr_target_repo = pull_request_obj.target_repo
             pr_source_repo = pull_request_obj.source_repo
 
-            pr_comment_url = h.url(
+            pr_comment_url = h.route_url(
                 'pullrequest_show',
                 repo_name=pr_target_repo.repo_name,
                 pull_request_id=pull_request_obj.pull_request_id,
-                anchor='comment-%s' % comment.comment_id,
-                qualified=True,)
+                anchor='comment-%s' % comment.comment_id)
 
             # set some variables for email notification
             pr_target_repo_url = h.route_url(

rhodecode/model/pull_request.py

@@ -1541,6 +1541,7 b' class MergeCheck(object):'
             if fail_early:
                 return merge_check
 
+        log.debug('MergeCheck: is failed: %s', merge_check.failed)
         return merge_check
 
 

rhodecode/public/js/rhodecode/routes.js

@@ -15,14 +15,6 b' function registerRCRoutes() {'
     pyroutes.register('new_repo', '/_admin/create_repository', []);
     pyroutes.register('edit_user', '/_admin/users/%(user_id)s/edit', ['user_id']);
     pyroutes.register('edit_user_group_members', '/_admin/user_groups/%(user_group_id)s/edit/members', ['user_group_id']);
-    pyroutes.register('pullrequest_home', '/%(repo_name)s/pull-request/new', ['repo_name']);
-    pyroutes.register('pullrequest', '/%(repo_name)s/pull-request/new', ['repo_name']);
-    pyroutes.register('pullrequest_repo_refs', '/%(repo_name)s/pull-request/refs/%(target_repo_name)s', ['repo_name', 'target_repo_name']);
-    pyroutes.register('pullrequest_repo_destinations', '/%(repo_name)s/pull-request/repo-destinations', ['repo_name']);
-    pyroutes.register('pullrequest_show', '/%(repo_name)s/pull-request/%(pull_request_id)s', ['repo_name', 'pull_request_id']);
-    pyroutes.register('pullrequest_update', '/%(repo_name)s/pull-request/%(pull_request_id)s', ['repo_name', 'pull_request_id']);
-    pyroutes.register('pullrequest_comment', '/%(repo_name)s/pull-request-comment/%(pull_request_id)s', ['repo_name', 'pull_request_id']);
-    pyroutes.register('pullrequest_comment_delete', '/%(repo_name)s/pull-request-comment/%(comment_id)s/delete', ['repo_name', 'comment_id']);
     pyroutes.register('favicon', '/favicon.ico', []);
     pyroutes.register('robots', '/robots.txt', []);
     pyroutes.register('auth_home', '/_admin/auth*traverse', []);
@@ -152,6 +144,15 b' function registerRCRoutes() {'
     pyroutes.register('pullrequest_show', '/%(repo_name)s/pull-request/%(pull_request_id)s', ['repo_name', 'pull_request_id']);
     pyroutes.register('pullrequest_show_all', '/%(repo_name)s/pull-request', ['repo_name']);
     pyroutes.register('pullrequest_show_all_data', '/%(repo_name)s/pull-request-data', ['repo_name']);
+    pyroutes.register('pullrequest_repo_refs', '/%(repo_name)s/pull-request/refs/%(target_repo_name)s', ['repo_name', 'target_repo_name']);
+    pyroutes.register('pullrequest_repo_destinations', '/%(repo_name)s/pull-request/repo-destinations', ['repo_name']);
+    pyroutes.register('pullrequest_new', '/%(repo_name)s/pull-request/new', ['repo_name']);
+    pyroutes.register('pullrequest_create', '/%(repo_name)s/pull-request/create', ['repo_name']);
+    pyroutes.register('pullrequest_update', '/%(repo_name)s/pull-request/%(pull_request_id)s/update', ['repo_name', 'pull_request_id']);
+    pyroutes.register('pullrequest_merge', '/%(repo_name)s/pull-request/%(pull_request_id)s/merge', ['repo_name', 'pull_request_id']);
+    pyroutes.register('pullrequest_delete', '/%(repo_name)s/pull-request/%(pull_request_id)s/delete', ['repo_name', 'pull_request_id']);
+    pyroutes.register('pullrequest_comment_create', '/%(repo_name)s/pull-request/%(pull_request_id)s/comment', ['repo_name', 'pull_request_id']);
+    pyroutes.register('pullrequest_comment_delete', '/%(repo_name)s/pull-request/%(pull_request_id)s/comment/%(comment_id)s/delete', ['repo_name', 'pull_request_id', 'comment_id']);
     pyroutes.register('edit_repo', '/%(repo_name)s/settings', ['repo_name']);
     pyroutes.register('edit_repo_advanced', '/%(repo_name)s/settings/advanced', ['repo_name']);
     pyroutes.register('edit_repo_advanced_delete', '/%(repo_name)s/settings/advanced/delete', ['repo_name']);

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

@@ -138,7 +138,7 b' var bindToggleButtons = function() {'
                  'commit_id': this.commitId});
 
         } else if (this.pullRequestId) {
-            this.submitUrl = pyroutes.url('pullrequest_comment',
+            this.submitUrl = pyroutes.url('pullrequest_comment_create',
                 {'repo_name': templateContext.repo_name,
                  'pull_request_id': this.pullRequestId});
             this.selfUrl = pyroutes.url('pullrequest_show',

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

@@ -387,7 +387,6 b' var editPullRequest = function(repo_name'
         {"repo_name": repo_name, "pull_request_id": pull_request_id});
 
     var postData = {
-        '_method': 'put',
         'title': title,
         'description': description,
         'edit_pull_request': true,

rhodecode/templates/base/base.mako

@@ -276,7 +276,7 b''
               %if c.rhodecode_user.username != h.DEFAULT_USER:
                 %if c.rhodecode_db_repo.repo_type in ['git','hg']:
                   <li><a href="${h.url('repo_fork_home',repo_name=c.repo_name)}">${_('Fork')}</a></li>
-                  <li><a href="${h.url('pullrequest_home',repo_name=c.repo_name)}">${_('Create Pull Request')}</a></li>
+                  <li><a href="${h.route_path('pullrequest_new',repo_name=c.repo_name)}">${_('Create Pull Request')}</a></li>
                 %endif
               %endif
              </ul>

rhodecode/templates/changelog/changelog.mako

@@ -55,7 +55,7 b''
                 ## pr open link
                 %if h.is_hg(c.rhodecode_repo) or h.is_git(c.rhodecode_repo):
                     <span>
-                        <a id="open_new_pull_request" class="btn btn-small btn-success" href="${h.url('pullrequest_home',repo_name=c.repo_name)}">
+                        <a id="open_new_pull_request" class="btn btn-small btn-success" href="${h.route_path('pullrequest_new',repo_name=c.repo_name)}">
                             ${_('Open new pull request')}
                         </a>
                     </span>
@@ -176,7 +176,7 b''
                         .show();
 
                     $commitRangeClear.show();
-                    var _url = pyroutes.url('pullrequest_home',
+                    var _url = pyroutes.url('pullrequest_new',
                                     {'repo_name': '${c.repo_name}',
                                      'commit': revEnd});
                     open_new_pull_request.attr('href', _url);
@@ -186,12 +186,12 b''
                     $commitRangeClear.hide();
 
                     %if c.branch_name:
-                        var _url = pyroutes.url('pullrequest_home',
+                        var _url = pyroutes.url('pullrequest_new',
                                         {'repo_name': '${c.repo_name}',
                                          'branch':'${c.branch_name}'});
                         open_new_pull_request.attr('href', _url);
                     %else:
-                        var _url = pyroutes.url('pullrequest_home',
+                        var _url = pyroutes.url('pullrequest_new',
                                         {'repo_name': '${c.repo_name}'});
                         open_new_pull_request.attr('href', _url);
                     %endif

rhodecode/templates/files/files_delete.mako

@@ -27,7 +27,7 b''
     <div class="edit-file-title">
         ${self.breadcrumbs()}
     </div>
-    ${h.secure_form(h.route_path('repo_files_delete_file', repo_name=c.repo_name, commit_id=c.commit.raw_id, f_path=c.f_path), id='eform', method='POST', class_="form-horizontal")}
+    ${h.secure_form(h.route_path('repo_files_delete_file', repo_name=c.repo_name, commit_id=c.commit.raw_id, f_path=c.f_path), id='eform', method='POST', class_="form-horizontal", request=request)}
     <div class="edit-file-fieldset">
         <div class="fieldset">
             <div id="destination-label" class="left-label">

rhodecode/templates/files/files_edit.mako

@@ -42,7 +42,7 b''
     </div>
 
     <div class="table">
-        ${h.secure_form(h.route_path('repo_files_update_file', repo_name=c.repo_name, commit_id=c.commit.raw_id, f_path=c.f_path), id='eform', method='POST')}
+        ${h.secure_form(h.route_path('repo_files_update_file', repo_name=c.repo_name, commit_id=c.commit.raw_id, f_path=c.f_path), id='eform', method='POST', request=request)}
         <div id="codeblock" class="codeblock" >
         <div class="code-header">
             <div class="stats">

rhodecode/templates/pullrequests/pullrequest.mako

@@ -22,7 +22,7 b''
         ${self.repo_page_title(c.rhodecode_db_repo)}
     </div>
 
-    ${h.secure_form(h.url('pullrequest', repo_name=c.repo_name), method='post', id='pull_request_form')}
+    ${h.secure_form(h.route_path('pullrequest_create', repo_name=c.repo_name), id='pull_request_form', method='POST', request=request)}
 
         ${self.breadcrumbs()}
 

rhodecode/templates/pullrequests/pullrequest_merge_checks.mako

@@ -32,7 +32,7 b''
     <div class="pull-request-merge-actions">
         % if c.allowed_to_merge:
         <div class="pull-right">
-          ${h.secure_form(h.url('pullrequest_merge', repo_name=c.repo_name, pull_request_id=c.pull_request.pull_request_id), id='merge_pull_request_form')}
+          ${h.secure_form(h.route_path('pullrequest_merge', repo_name=c.repo_name, pull_request_id=c.pull_request.pull_request_id), id='merge_pull_request_form', method='POST', request=request)}
           <% merge_disabled = ' disabled' if c.pr_merge_possible is False else '' %>
           <a class="btn" href="#" onclick="refreshMergeChecks(); return false;">${_('refresh checks')}</a>
           <input type="submit" id="merge_pull_request" value="${_('Merge Pull Request')}" class="btn${merge_disabled}"${merge_disabled}>

rhodecode/templates/pullrequests/pullrequest_show.mako

@@ -32,7 +32,7 b''
 
 <script type="text/javascript">
     // TODO: marcink switch this to pyroutes
-    AJAX_COMMENT_DELETE_URL = "${h.url('pullrequest_comment_delete',repo_name=c.repo_name,comment_id='__COMMENT_ID__')}";
+    AJAX_COMMENT_DELETE_URL = "${h.route_path('pullrequest_comment_delete',repo_name=c.repo_name,pull_request_id=c.pull_request.pull_request_id,comment_id='__COMMENT_ID__')}";
     templateContext.pull_request_data.pull_request_id = ${c.pull_request.pull_request_id};
 </script>
 <div class="box">
@@ -52,7 +52,7 b''
             %if c.allowed_to_update:
               <div id="delete_pullrequest" class="pull-right action_button ${'' if c.allowed_to_delete else 'disabled' }" style="clear:inherit;padding: 0">
                   % if c.allowed_to_delete:
-                      ${h.secure_form(h.url('pullrequest_delete', repo_name=c.pull_request.target_repo.repo_name, pull_request_id=c.pull_request.pull_request_id),method='delete')}
+                      ${h.secure_form(h.route_path('pullrequest_delete', repo_name=c.pull_request.target_repo.repo_name, pull_request_id=c.pull_request.pull_request_id), method='POST', request=request)}
                           ${h.submit('remove_%s' % c.pull_request.pull_request_id, _('Delete'),
                         class_="btn btn-link btn-danger no-margin",onclick="return confirm('"+_('Confirm to delete this pull request')+"');")}
                       ${h.end_form()}
@@ -200,7 +200,7 b''
                            <tr class="version-pr" style="display: ${display_row}">
                                <td>
                                     <code>
-                                        <a href="${h.url.current(version=ver_pr or 'latest')}">v${ver_pos}</a>
+                                        <a href="${request.current_route_path(_query=dict(version=ver_pr or 'latest'))}">v${ver_pos}</a>
                                     </code>
                                </td>
                                <td>
@@ -616,8 +616,8 b''
         </div>
 
         ## main comment form and it status
-        ${comment.comments(h.url('pullrequest_comment', repo_name=c.repo_name,
-                                  pull_request_id=c.pull_request.pull_request_id),
+        ${comment.comments(h.route_path('pullrequest_comment_create', repo_name=c.repo_name,
+                                        pull_request_id=c.pull_request.pull_request_id),
                            c.pull_request_review_status,
                            is_pull_request=True, change_status=c.allowed_to_change_status)}
       %endif
@@ -730,7 +730,7 b''
             };
 
             refreshMergeChecks = function(){
-                var loadUrl = "${h.url.current(merge_checks=1)}";
+                var loadUrl = "${request.current_route_path(_query=dict(merge_checks=1))}";
                 $('.pull-request-merge').css('opacity', 0.3);
                 $('.action-buttons-extra').css('opacity', 0.3);
 

rhodecode/templates/pullrequests/pullrequests.mako

@@ -30,7 +30,7 b''
         <li>
            %if c.rhodecode_user.username != h.DEFAULT_USER:
             <span>
-                <a id="open_new_pull_request" class="btn btn-small btn-success" href="${h.url('pullrequest_home',repo_name=c.repo_name)}">
+                <a id="open_new_pull_request" class="btn btn-small btn-success" href="${h.route_path('pullrequest_new',repo_name=c.repo_name)}">
                     ${_('Open new Pull Request')}
                 </a>
             </span>

rhodecode/tests/functional/test_admin_user_groups.py

@@ -200,13 +200,14 b' class TestAdminUsersGroupsController(Tes'
         assert response.body == '{"members": []}'
         fixture.destroy_user_group(TEST_USER_GROUP)
 
-    def test_usergroup_escape(self):
-        user = User.get_by_username('test_admin')
-        user.name = '<img src="/image1" onload="alert(\'Hello, World!\');">'
-        user.lastname = (
-            '<img src="/image2" onload="alert(\'Hello, World!\');">')
-        Session().add(user)
-        Session().commit()
+    def test_usergroup_escape(self, user_util):
+        user = user_util.create_user(
+            username='escape_user',
+            firstname='<img src="/image2" onload="alert(\'Hello, World!\');">',
+            lastname='<img src="/image2" onload="alert(\'Hello, World!\');">'
+        )
+
+        user_util.create_user_group(owner=user.username)
 
         self.log_user()
         users_group_name = 'samplegroup'