pull-requests: introduce operation state for pull requests to prevent from

locks during merge/update operations.

rhodecode/lib/dbmigrate/versions/092_version_4_16_0.py

@@ -0,0 +1,37 b''
+import logging
+
+from sqlalchemy import *
+
+from rhodecode.model import meta
+from rhodecode.lib.dbmigrate.versions import _reset_base, notify
+
+log = logging.getLogger(__name__)
+
+
+def upgrade(migrate_engine):
+    """
+    Upgrade operations go here.
+    Don't create your own engine; bind migrate_engine to your metadata
+    """
+    _reset_base(migrate_engine)
+    from rhodecode.lib.dbmigrate.schema import db_4_13_0_0 as db
+
+    pull_request = db.PullRequest.__table__
+    pull_request_version = db.PullRequestVersion.__table__
+
+    repo_state_1 = Column("pull_request_state", String(255), nullable=True)
+    repo_state_1.create(table=pull_request)
+
+    repo_state_2 = Column("pull_request_state", String(255), nullable=True)
+    repo_state_2.create(table=pull_request_version)
+
+    fixups(db, meta.Session)
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+
+def fixups(models, _SESSION):
+    pass

rhodecode/lib/dbmigrate/versions/093_version_4_16_0.py

@@ -0,0 +1,41 b''
+import logging
+
+from sqlalchemy import *
+
+from rhodecode.model import meta
+from rhodecode.lib.dbmigrate.versions import _reset_base, notify
+
+log = logging.getLogger(__name__)
+
+
+def upgrade(migrate_engine):
+    """
+    Upgrade operations go here.
+    Don't create your own engine; bind migrate_engine to your metadata
+    """
+    _reset_base(migrate_engine)
+    from rhodecode.lib.dbmigrate.schema import db_4_16_0_0 as db
+
+    fixups(db, meta.Session)
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+
+def fixups(models, _SESSION):
+    # move the builtin token to external tokens
+
+    log.info('Updating pull request pull_request_state to %s',
+             models.PullRequest.STATE_CREATED)
+    qry = _SESSION().query(models.PullRequest)
+    qry.update({"pull_request_state": models.PullRequest.STATE_CREATED})
+    _SESSION().commit()
+
+    log.info('Updating pull_request_version pull_request_state to %s',
+             models.PullRequest.STATE_CREATED)
+    qry = _SESSION().query(models.PullRequestVersion)
+    qry.update({"pull_request_state": models.PullRequest.STATE_CREATED})
+    _SESSION().commit()
+

rhodecode/__init__.py

@@ -1,63 +1,57 b''
 # -*- coding: utf-8 -*-
 
 # Copyright (C) 2010-2019 RhodeCode GmbH
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU Affero General Public License, version 3
 # (only), as published by the Free Software Foundation.
 #
 # This program is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 # 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/
 
-"""
-
-RhodeCode, a web based repository management software
-versioning implementation: http://www.python.org/dev/peps/pep-0386/
-"""
-
 import os
 import sys
 import platform
 
 VERSION = tuple(open(os.path.join(
     os.path.dirname(__file__), 'VERSION')).read().split('.'))
 
 BACKENDS = {
     'hg': 'Mercurial repository',
     'git': 'Git repository',
     'svn': 'Subversion repository',
 }
 
 CELERY_ENABLED = False
 CELERY_EAGER = False
 
 # link to config for pyramid
 CONFIG = {}
 
 # Populated with the settings dictionary from application init in
 # rhodecode.conf.environment.load_pyramid_environment
 PYRAMID_SETTINGS = {}
 
 # Linked module for extensions
 EXTENSIONS = {}
 
 __version__ = ('.'.join((str(each) for each in VERSION[:3])))
-__dbversion__ = 91  # defines current db version for migrations
+__dbversion__ = 93  # defines current db version for migrations
 __platform__ = platform.system()
 __license__ = 'AGPLv3, and Commercial License'
 __author__ = 'RhodeCode GmbH'
 __url__ = 'https://code.rhodecode.com'
 
 is_windows = __platform__ in ['Windows']
 is_unix = not is_windows
 is_test = False
 disable_error_handler = False

rhodecode/api/tests/test_get_pull_request.py

@@ -1,142 +1,143 b''
 # -*- coding: utf-8 -*-
 
 # Copyright (C) 2010-2019 RhodeCode GmbH
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU Affero General Public License, version 3
 # (only), as published by the Free Software Foundation.
 #
 # This program is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 # 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 pytest
 import urlobject
 
 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")
 
 
 @pytest.mark.usefixtures("testuser_api", "app")
 class TestGetPullRequest(object):
 
     def test_api_get_pull_request(self, pr_util, http_host_only_stub):
         from rhodecode.model.pull_request import PullRequestModel
         pull_request = pr_util.create_pull_request(mergeable=True)
         id_, params = build_data(
             self.apikey, 'get_pull_request',
             pullrequestid=pull_request.pull_request_id)
 
         response = api_call(self.app, params)
 
         assert response.status == '200 OK'
 
         url_obj = urlobject.URLObject(
             h.route_url(
                 'pullrequest_show',
                 repo_name=pull_request.target_repo.repo_name,
                 pull_request_id=pull_request.pull_request_id))
 
         pr_url = safe_unicode(
             url_obj.with_netloc(http_host_only_stub))
         source_url = safe_unicode(
             pull_request.source_repo.clone_url().with_netloc(http_host_only_stub))
         target_url = safe_unicode(
             pull_request.target_repo.clone_url().with_netloc(http_host_only_stub))
         shadow_url = safe_unicode(
             PullRequestModel().get_shadow_clone_url(pull_request))
 
         expected = {
             'pull_request_id': pull_request.pull_request_id,
             'url': pr_url,
             'title': pull_request.title,
             'description': pull_request.description,
             'status': pull_request.status,
+            'state': pull_request.pull_request_state,
             'created_on': pull_request.created_on,
             'updated_on': pull_request.updated_on,
             'commit_ids': pull_request.revisions,
             'review_status': pull_request.calculated_review_status(),
             'mergeable': {
                 'status': True,
                 'message': 'This pull request can be automatically merged.',
             },
             'source': {
                 'clone_url': source_url,
                 'repository': pull_request.source_repo.repo_name,
                 'reference': {
                     'name': pull_request.source_ref_parts.name,
                     'type': pull_request.source_ref_parts.type,
                     'commit_id': pull_request.source_ref_parts.commit_id,
                 },
             },
             'target': {
                 'clone_url': target_url,
                 'repository': pull_request.target_repo.repo_name,
                 'reference': {
                     'name': pull_request.target_ref_parts.name,
                     'type': pull_request.target_ref_parts.type,
                     'commit_id': pull_request.target_ref_parts.commit_id,
                 },
             },
             'merge': {
                 'clone_url': shadow_url,
                 'reference': {
                     'name': pull_request.shadow_merge_ref.name,
                     'type': pull_request.shadow_merge_ref.type,
                     'commit_id': pull_request.shadow_merge_ref.commit_id,
                 },
             },
             'author': pull_request.author.get_api_data(include_secrets=False,
                                                        details='basic'),
             'reviewers': [
                 {
                     'user': reviewer.get_api_data(include_secrets=False,
                                                   details='basic'),
                     'reasons': reasons,
                     'review_status': st[0][1].status if st else 'not_reviewed',
                 }
                 for obj, reviewer, reasons, mandatory, st in
                 pull_request.reviewers_statuses()
             ]
         }
         assert_ok(id_, expected, response.body)
 
     def test_api_get_pull_request_repo_error(self, pr_util):
         pull_request = pr_util.create_pull_request()
         id_, params = build_data(
             self.apikey, 'get_pull_request',
             repoid=666, pullrequestid=pull_request.pull_request_id)
         response = api_call(self.app, params)
 
         expected = 'repository `666` does not exist'
         assert_error(id_, expected, given=response.body)
 
     def test_api_get_pull_request_pull_request_error(self):
         id_, params = build_data(
             self.apikey, 'get_pull_request', pullrequestid=666)
         response = api_call(self.app, params)
 
         expected = 'pull request `666` does not exist'
         assert_error(id_, expected, given=response.body)
 
     def test_api_get_pull_request_pull_request_error_just_pr_id(self):
         id_, params = build_data(
             self.apikey, 'get_pull_request',
             pullrequestid=666)
         response = api_call(self.app, params)
 
         expected = 'pull request `666` does not exist'
         assert_error(id_, expected, given=response.body)

rhodecode/api/tests/test_merge_pull_request.py

@@ -1,137 +1,157 b''
 # -*- coding: utf-8 -*-
 
 # Copyright (C) 2010-2019 RhodeCode GmbH
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU Affero General Public License, version 3
 # (only), as published by the Free Software Foundation.
 #
 # This program is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 # 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 pytest
 
 from rhodecode.model.db import UserLog, PullRequest
 from rhodecode.model.meta import Session
 from rhodecode.tests import TEST_USER_ADMIN_LOGIN
 from rhodecode.api.tests.utils import (
     build_data, api_call, assert_error, assert_ok)
 
 
 @pytest.mark.usefixtures("testuser_api", "app")
 class TestMergePullRequest(object):
+
     @pytest.mark.backends("git", "hg")
     def test_api_merge_pull_request_merge_failed(self, pr_util, no_notifications):
         pull_request = pr_util.create_pull_request(mergeable=True)
-        author = pull_request.user_id
-        repo = pull_request.target_repo.repo_id
         pull_request_id = pull_request.pull_request_id
         pull_request_repo = pull_request.target_repo.repo_name
 
         id_, params = build_data(
             self.apikey, 'merge_pull_request',
             repoid=pull_request_repo,
             pullrequestid=pull_request_id)
 
         response = api_call(self.app, params)
 
         # The above api call detaches the pull request DB object from the
         # session because of an unconditional transaction rollback in our
-        # middleware. Therefore we need to add it back here if we want to use
-        # it.
+        # middleware. Therefore we need to add it back here if we want to use it.
         Session().add(pull_request)
 
         expected = 'merge not possible for following reasons: ' \
                    'Pull request reviewer approval is pending.'
         assert_error(id_, expected, given=response.body)
 
     @pytest.mark.backends("git", "hg")
+    def test_api_merge_pull_request_merge_failed_disallowed_state(
+            self, pr_util, no_notifications):
+        pull_request = pr_util.create_pull_request(mergeable=True, approved=True)
+        pull_request_id = pull_request.pull_request_id
+        pull_request_repo = pull_request.target_repo.repo_name
+
+        pr = PullRequest.get(pull_request_id)
+        pr.pull_request_state = pull_request.STATE_UPDATING
+        Session().add(pr)
+        Session().commit()
+
+        id_, params = build_data(
+            self.apikey, 'merge_pull_request',
+            repoid=pull_request_repo,
+            pullrequestid=pull_request_id)
+
+        response = api_call(self.app, params)
+        expected = 'Operation forbidden because pull request is in state {}, '\
+                   'only state {} is allowed.'.format(PullRequest.STATE_UPDATING,
+                                                      PullRequest.STATE_CREATED)
+        assert_error(id_, expected, given=response.body)
+
+    @pytest.mark.backends("git", "hg")
     def test_api_merge_pull_request(self, pr_util, no_notifications):
         pull_request = pr_util.create_pull_request(mergeable=True, approved=True)
         author = pull_request.user_id
         repo = pull_request.target_repo.repo_id
         pull_request_id = pull_request.pull_request_id
         pull_request_repo = pull_request.target_repo.repo_name
 
         id_, params = build_data(
             self.apikey, 'comment_pull_request',
             repoid=pull_request_repo,
             pullrequestid=pull_request_id,
             status='approved')
 
         response = api_call(self.app, params)
         expected = {
             'comment_id': response.json.get('result', {}).get('comment_id'),
             'pull_request_id': pull_request_id,
             'status': {'given': 'approved', 'was_changed': True}
         }
         assert_ok(id_, expected, given=response.body)
 
         id_, params = build_data(
             self.apikey, 'merge_pull_request',
             repoid=pull_request_repo,
             pullrequestid=pull_request_id)
 
         response = api_call(self.app, params)
 
         pull_request = PullRequest.get(pull_request_id)
 
         expected = {
             'executed': True,
             'failure_reason': 0,
             'possible': True,
             'merge_commit_id': pull_request.shadow_merge_ref.commit_id,
             'merge_ref': pull_request.shadow_merge_ref._asdict()
         }
 
         assert_ok(id_, expected, response.body)
 
         journal = UserLog.query()\
             .filter(UserLog.user_id == author)\
             .filter(UserLog.repository_id == repo) \
             .order_by('user_log_id') \
             .all()
         assert journal[-2].action == 'repo.pull_request.merge'
         assert journal[-1].action == 'repo.pull_request.close'
 
         id_, params = build_data(
             self.apikey, 'merge_pull_request',
             repoid=pull_request_repo, pullrequestid=pull_request_id)
         response = api_call(self.app, params)
 
         expected = 'merge not possible for following reasons: This pull request is closed.'
         assert_error(id_, expected, given=response.body)
 
     @pytest.mark.backends("git", "hg")
     def test_api_merge_pull_request_repo_error(self, pr_util):
         pull_request = pr_util.create_pull_request()
         id_, params = build_data(
             self.apikey, 'merge_pull_request',
             repoid=666, pullrequestid=pull_request.pull_request_id)
         response = api_call(self.app, params)
 
         expected = 'repository `666` does not exist'
         assert_error(id_, expected, given=response.body)
 
     @pytest.mark.backends("git", "hg")
-    def test_api_merge_pull_request_non_admin_with_userid_error(self,
-                                                                pr_util):
+    def test_api_merge_pull_request_non_admin_with_userid_error(self, pr_util):
         pull_request = pr_util.create_pull_request(mergeable=True)
         id_, params = build_data(
             self.apikey_regular, 'merge_pull_request',
             repoid=pull_request.target_repo.repo_name,
             pullrequestid=pull_request.pull_request_id,
             userid=TEST_USER_ADMIN_LOGIN)
         response = api_call(self.app, params)
 
         expected = 'userid is not the same as your user'
         assert_error(id_, expected, given=response.body)

rhodecode/api/views/pull_request_api.py

@@ -1,937 +1,957 b''
 # -*- coding: utf-8 -*-
 
 # Copyright (C) 2011-2019 RhodeCode GmbH
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU Affero General Public License, version 3
 # (only), as published by the Free Software Foundation.
 #
 # This program is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 # 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 logging
 
 from rhodecode import events
 from rhodecode.api import jsonrpc_method, JSONRPCError, JSONRPCValidationError
 from rhodecode.api.utils import (
     has_superadmin_permission, Optional, OAttr, get_repo_or_error,
     get_pull_request_or_error, get_commit_or_error, get_user_or_error,
     validate_repo_permissions, resolve_ref_or_error)
 from rhodecode.lib.auth import (HasRepoPermissionAnyApi)
 from rhodecode.lib.base import vcs_operation_context
 from rhodecode.lib.utils2 import str2bool
 from rhodecode.model.changeset_status import ChangesetStatusModel
 from rhodecode.model.comment import CommentsModel
-from rhodecode.model.db import Session, ChangesetStatus, ChangesetComment
+from rhodecode.model.db import Session, ChangesetStatus, ChangesetComment, PullRequest
 from rhodecode.model.pull_request import PullRequestModel, MergeCheck
 from rhodecode.model.settings import SettingsModel
 from rhodecode.model.validation_schema import Invalid
 from rhodecode.model.validation_schema.schemas.reviewer_schema import(
     ReviewerListSchema)
 
 log = logging.getLogger(__name__)
 
 
 @jsonrpc_method()
 def get_pull_request(request, apiuser, pullrequestid, repoid=Optional(None)):
     """
     Get a pull request based on the given ID.
 
     :param apiuser: This is filled automatically from the |authtoken|.
     :type apiuser: AuthUser
     :param repoid: Optional, repository name or repository ID from where
         the pull request was opened.
     :type repoid: str or int
     :param pullrequestid: ID of the requested pull request.
     :type pullrequestid: int
 
     Example output:
 
     .. code-block:: bash
 
       "id": <id_given_in_input>,
       "result":
         {
             "pull_request_id":   "<pull_request_id>",
             "url":               "<url>",
             "title":             "<title>",
             "description":       "<description>",
             "status" :           "<status>",
             "created_on":        "<date_time_created>",
             "updated_on":        "<date_time_updated>",
             "commit_ids":        [
                                      ...
                                      "<commit_id>",
                                      "<commit_id>",
                                      ...
                                  ],
             "review_status":    "<review_status>",
             "mergeable":         {
                                      "status":  "<bool>",
                                      "message": "<message>",
                                  },
             "source":            {
                                      "clone_url":     "<clone_url>",
                                      "repository":    "<repository_name>",
                                      "reference":
                                      {
                                          "name":      "<name>",
                                          "type":      "<type>",
                                          "commit_id": "<commit_id>",
                                      }
                                  },
             "target":            {
                                      "clone_url":   "<clone_url>",
                                      "repository":    "<repository_name>",
                                      "reference":
                                      {
                                          "name":      "<name>",
                                          "type":      "<type>",
                                          "commit_id": "<commit_id>",
                                      }
                                  },
             "merge":             {
                                      "clone_url":   "<clone_url>",
                                      "reference":
                                      {
                                          "name":      "<name>",
                                          "type":      "<type>",
                                          "commit_id": "<commit_id>",
                                      }
                                  },
            "author":             <user_obj>,
            "reviewers":          [
                                      ...
                                      {
                                         "user":          "<user_obj>",
                                         "review_status": "<review_status>",
                                      }
                                      ...
                                  ]
         },
        "error": null
     """
 
     pull_request = get_pull_request_or_error(pullrequestid)
     if Optional.extract(repoid):
         repo = get_repo_or_error(repoid)
     else:
         repo = pull_request.target_repo
 
-    if not PullRequestModel().check_user_read(
-            pull_request, apiuser, api=True):
+    if not PullRequestModel().check_user_read(pull_request, apiuser, api=True):
         raise JSONRPCError('repository `%s` or pull request `%s` '
                            'does not exist' % (repoid, pullrequestid))
-    data = pull_request.get_api_data()
+
+    # NOTE(marcink): only calculate and return merge state if the pr state is 'created'
+    # otherwise we can lock the repo on calculation of merge state while update/merge
+    # is happening.
+    merge_state = pull_request.pull_request_state == pull_request.STATE_CREATED
+    data = pull_request.get_api_data(with_merge_state=merge_state)
     return data
 
 
 @jsonrpc_method()
 def get_pull_requests(request, apiuser, repoid, status=Optional('new')):
     """
     Get all pull requests from the repository specified in `repoid`.
 
     :param apiuser: This is filled automatically from the |authtoken|.
     :type apiuser: AuthUser
     :param repoid: Optional repository name or repository ID.
     :type repoid: str or int
     :param status: Only return pull requests with the specified status.
         Valid options are.
         * ``new`` (default)
         * ``open``
         * ``closed``
     :type status: str
 
     Example output:
 
     .. code-block:: bash
 
       "id": <id_given_in_input>,
       "result":
         [
             ...
             {
                 "pull_request_id":   "<pull_request_id>",
                 "url":               "<url>",
                 "title" :            "<title>",
                 "description":       "<description>",
                 "status":            "<status>",
                 "created_on":        "<date_time_created>",
                 "updated_on":        "<date_time_updated>",
                 "commit_ids":        [
                                          ...
                                          "<commit_id>",
                                          "<commit_id>",
                                          ...
                                      ],
                 "review_status":    "<review_status>",
                 "mergeable":         {
                                         "status":      "<bool>",
                                         "message:      "<message>",
                                      },
                 "source":            {
                                          "clone_url":     "<clone_url>",
                                          "reference":
                                          {
                                              "name":      "<name>",
                                              "type":      "<type>",
                                              "commit_id": "<commit_id>",
                                          }
                                      },
                 "target":            {
                                          "clone_url":   "<clone_url>",
                                          "reference":
                                          {
                                              "name":      "<name>",
                                              "type":      "<type>",
                                              "commit_id": "<commit_id>",
                                          }
                                      },
                 "merge":             {
                                          "clone_url":   "<clone_url>",
                                          "reference":
                                          {
                                              "name":      "<name>",
                                              "type":      "<type>",
                                              "commit_id": "<commit_id>",
                                          }
                                      },
                "author":             <user_obj>,
                "reviewers":          [
                                          ...
                                          {
                                             "user":          "<user_obj>",
                                             "review_status": "<review_status>",
                                          }
                                          ...
                                      ]
             }
             ...
         ],
       "error": null
 
     """
     repo = get_repo_or_error(repoid)
     if not has_superadmin_permission(apiuser):
         _perms = (
             'repository.admin', 'repository.write', 'repository.read',)
         validate_repo_permissions(apiuser, repoid, repo, _perms)
 
     status = Optional.extract(status)
     pull_requests = PullRequestModel().get_all(repo, statuses=[status])
     data = [pr.get_api_data() for pr in pull_requests]
     return data
 
 
 @jsonrpc_method()
 def merge_pull_request(
         request, apiuser, pullrequestid, repoid=Optional(None),
         userid=Optional(OAttr('apiuser'))):
     """
     Merge the pull request specified by `pullrequestid` into its target
     repository.
 
     :param apiuser: This is filled automatically from the |authtoken|.
     :type apiuser: AuthUser
     :param repoid: Optional, repository name or repository ID of the
         target repository to which the |pr| is to be merged.
     :type repoid: str or int
     :param pullrequestid: ID of the pull request which shall be merged.
     :type pullrequestid: int
     :param userid: Merge the pull request as this user.
     :type userid: Optional(str or int)
 
     Example output:
 
     .. code-block:: bash
 
         "id": <id_given_in_input>,
         "result": {
             "executed":         "<bool>",
             "failure_reason":   "<int>",
             "merge_commit_id":  "<merge_commit_id>",
             "possible":         "<bool>",
             "merge_ref":        {
                                     "commit_id": "<commit_id>",
                                     "type":      "<type>",
                                     "name":      "<name>"
                                 }
         },
         "error": null
     """
     pull_request = get_pull_request_or_error(pullrequestid)
     if Optional.extract(repoid):
         repo = get_repo_or_error(repoid)
     else:
         repo = pull_request.target_repo
 
     if not isinstance(userid, Optional):
         if (has_superadmin_permission(apiuser) or
                 HasRepoPermissionAnyApi('repository.admin')(
                     user=apiuser, repo_name=repo.repo_name)):
             apiuser = get_user_or_error(userid)
         else:
             raise JSONRPCError('userid is not the same as your user')
 
-    check = MergeCheck.validate(
-        pull_request, auth_user=apiuser, translator=request.translate)
+    if pull_request.pull_request_state != PullRequest.STATE_CREATED:
+        raise JSONRPCError(
+            'Operation forbidden because pull request is in state {}, '
+            'only state {} is allowed.'.format(
+                pull_request.pull_request_state, PullRequest.STATE_CREATED))
+
+    with pull_request.set_state(PullRequest.STATE_UPDATING):
+        check = MergeCheck.validate(
+            pull_request, auth_user=apiuser,
+            translator=request.translate)
     merge_possible = not check.failed
 
     if not merge_possible:
         error_messages = []
         for err_type, error_msg in check.errors:
             error_msg = request.translate(error_msg)
             error_messages.append(error_msg)
 
         reasons = ','.join(error_messages)
         raise JSONRPCError(
             'merge not possible for following reasons: {}'.format(reasons))
 
     target_repo = pull_request.target_repo
     extras = vcs_operation_context(
         request.environ, repo_name=target_repo.repo_name,
         username=apiuser.username, action='push',
         scm=target_repo.repo_type)
-    merge_response = PullRequestModel().merge_repo(
-        pull_request, apiuser, extras=extras)
+    with pull_request.set_state(PullRequest.STATE_UPDATING):
+        merge_response = PullRequestModel().merge_repo(
+            pull_request, apiuser, extras=extras)
     if merge_response.executed:
         PullRequestModel().close_pull_request(
             pull_request.pull_request_id, apiuser)
 
         Session().commit()
 
     # In previous versions the merge response directly contained the merge
     # commit id. It is now contained in the merge reference object. To be
     # backwards compatible we have to extract it again.
     merge_response = merge_response.asdict()
     merge_response['merge_commit_id'] = merge_response['merge_ref'].commit_id
 
     return merge_response
 
 
 @jsonrpc_method()
 def get_pull_request_comments(
         request, apiuser, pullrequestid, repoid=Optional(None)):
     """
     Get all comments of pull request specified with the `pullrequestid`
 
     :param apiuser: This is filled automatically from the |authtoken|.
     :type apiuser: AuthUser
     :param repoid: Optional repository name or repository ID.
     :type repoid: str or int
     :param pullrequestid: The pull request ID.
     :type pullrequestid: int
 
     Example output:
 
     .. code-block:: bash
 
         id : <id_given_in_input>
         result : [
             {
               "comment_author": {
                 "active": true,
                 "full_name_or_username": "Tom Gore",
                 "username": "admin"
               },
               "comment_created_on": "2017-01-02T18:43:45.533",
               "comment_f_path": null,
               "comment_id": 25,
               "comment_lineno": null,
               "comment_status": {
                 "status": "under_review",
                 "status_lbl": "Under Review"
               },
               "comment_text": "Example text",
               "comment_type": null,
               "pull_request_version": null
             }
         ],
         error :  null
     """
 
     pull_request = get_pull_request_or_error(pullrequestid)
     if Optional.extract(repoid):
         repo = get_repo_or_error(repoid)
     else:
         repo = pull_request.target_repo
 
     if not PullRequestModel().check_user_read(
             pull_request, apiuser, api=True):
         raise JSONRPCError('repository `%s` or pull request `%s` '
                            'does not exist' % (repoid, pullrequestid))
 
     (pull_request_latest,
      pull_request_at_ver,
      pull_request_display_obj,
      at_version) = PullRequestModel().get_pr_version(
         pull_request.pull_request_id, version=None)
 
     versions = pull_request_display_obj.versions()
     ver_map = {
         ver.pull_request_version_id: cnt
         for cnt, ver in enumerate(versions, 1)
     }
 
     # GENERAL COMMENTS with versions #
     q = CommentsModel()._all_general_comments_of_pull_request(pull_request)
     q = q.order_by(ChangesetComment.comment_id.asc())
     general_comments = q.all()
 
     # INLINE COMMENTS with versions  #
     q = CommentsModel()._all_inline_comments_of_pull_request(pull_request)
     q = q.order_by(ChangesetComment.comment_id.asc())
     inline_comments = q.all()
 
     data = []
     for comment in inline_comments + general_comments:
         full_data = comment.get_api_data()
         pr_version_id = None
         if comment.pull_request_version_id:
             pr_version_id = 'v{}'.format(
                 ver_map[comment.pull_request_version_id])
 
         # sanitize some entries
 
         full_data['pull_request_version'] = pr_version_id
         full_data['comment_author'] = {
             'username': full_data['comment_author'].username,
             'full_name_or_username': full_data['comment_author'].full_name_or_username,
             'active': full_data['comment_author'].active,
         }
 
         if full_data['comment_status']:
             full_data['comment_status'] = {
                 'status': full_data['comment_status'][0].status,
                 'status_lbl': full_data['comment_status'][0].status_lbl,
             }
         else:
             full_data['comment_status'] = {}
 
         data.append(full_data)
     return data
 
 
 @jsonrpc_method()
 def comment_pull_request(
         request, apiuser, pullrequestid, repoid=Optional(None),
         message=Optional(None), commit_id=Optional(None), status=Optional(None),
         comment_type=Optional(ChangesetComment.COMMENT_TYPE_NOTE),
         resolves_comment_id=Optional(None),
         userid=Optional(OAttr('apiuser'))):
     """
     Comment on the pull request specified with the `pullrequestid`,
     in the |repo| specified by the `repoid`, and optionally change the
     review status.
 
     :param apiuser: This is filled automatically from the |authtoken|.
     :type apiuser: AuthUser
     :param repoid: Optional repository name or repository ID.
     :type repoid: str or int
     :param pullrequestid: The pull request ID.
     :type pullrequestid: int
     :param commit_id: Specify the commit_id for which to set a comment. If
         given commit_id is different than latest in the PR status
         change won't be performed.
     :type commit_id: str
     :param message: The text content of the comment.
     :type message: str
     :param status: (**Optional**) Set the approval status of the pull
         request. One of: 'not_reviewed', 'approved', 'rejected',
         'under_review'
     :type status: str
     :param comment_type: Comment type, one of: 'note', 'todo'
     :type comment_type: Optional(str), default: 'note'
     :param userid: Comment on the pull request as this user
     :type userid: Optional(str or int)
 
     Example output:
 
     .. code-block:: bash
 
         id : <id_given_in_input>
         result : {
             "pull_request_id":  "<Integer>",
             "comment_id":       "<Integer>",
             "status": {"given": <given_status>,
                        "was_changed": <bool status_was_actually_changed> },
         },
         error :  null
     """
     pull_request = get_pull_request_or_error(pullrequestid)
     if Optional.extract(repoid):
         repo = get_repo_or_error(repoid)
     else:
         repo = pull_request.target_repo
 
     if not isinstance(userid, Optional):
         if (has_superadmin_permission(apiuser) or
                 HasRepoPermissionAnyApi('repository.admin')(
                     user=apiuser, repo_name=repo.repo_name)):
             apiuser = get_user_or_error(userid)
         else:
             raise JSONRPCError('userid is not the same as your user')
 
     if not PullRequestModel().check_user_read(
             pull_request, apiuser, api=True):
         raise JSONRPCError('repository `%s` does not exist' % (repoid,))
     message = Optional.extract(message)
     status = Optional.extract(status)
     commit_id = Optional.extract(commit_id)
     comment_type = Optional.extract(comment_type)
     resolves_comment_id = Optional.extract(resolves_comment_id)
 
     if not message and not status:
         raise JSONRPCError(
             'Both message and status parameters are missing. '
             'At least one is required.')
 
     if (status not in (st[0] for st in ChangesetStatus.STATUSES) and
             status is not None):
         raise JSONRPCError('Unknown comment status: `%s`' % status)
 
     if commit_id and commit_id not in pull_request.revisions:
         raise JSONRPCError(
             'Invalid commit_id `%s` for this pull request.' % commit_id)
 
     allowed_to_change_status = PullRequestModel().check_user_change_status(
         pull_request, apiuser)
 
     # if commit_id is passed re-validated if user is allowed to change status
     # based on latest commit_id from the PR
     if commit_id:
         commit_idx = pull_request.revisions.index(commit_id)
         if commit_idx != 0:
             allowed_to_change_status = False
 
     if resolves_comment_id:
         comment = ChangesetComment.get(resolves_comment_id)
         if not comment:
             raise JSONRPCError(
                 'Invalid resolves_comment_id `%s` for this pull request.'
                 % resolves_comment_id)
         if comment.comment_type != ChangesetComment.COMMENT_TYPE_TODO:
             raise JSONRPCError(
                 'Comment `%s` is wrong type for setting status to resolved.'
                 % resolves_comment_id)
 
     text = message
     status_label = ChangesetStatus.get_status_lbl(status)
     if status and allowed_to_change_status:
         st_message = ('Status change %(transition_icon)s %(status)s'
                       % {'transition_icon': '>', 'status': status_label})
         text = message or st_message
 
     rc_config = SettingsModel().get_all_settings()
     renderer = rc_config.get('rhodecode_markup_renderer', 'rst')
 
     status_change = status and allowed_to_change_status
     comment = CommentsModel().create(
         text=text,
         repo=pull_request.target_repo.repo_id,
         user=apiuser.user_id,
         pull_request=pull_request.pull_request_id,
         f_path=None,
         line_no=None,
         status_change=(status_label if status_change else None),
         status_change_type=(status if status_change else None),
         closing_pr=False,
         renderer=renderer,
         comment_type=comment_type,
         resolves_comment_id=resolves_comment_id,
         auth_user=apiuser
     )
 
     if allowed_to_change_status and status:
         ChangesetStatusModel().set_status(
             pull_request.target_repo.repo_id,
             status,
             apiuser.user_id,
             comment,
             pull_request=pull_request.pull_request_id
         )
         Session().flush()
 
     Session().commit()
     data = {
         'pull_request_id': pull_request.pull_request_id,
         'comment_id': comment.comment_id if comment else None,
         'status': {'given': status, 'was_changed': status_change},
     }
     return data
 
 
 @jsonrpc_method()
 def create_pull_request(
         request, apiuser, source_repo, target_repo, source_ref, target_ref,
         title=Optional(''), description=Optional(''), description_renderer=Optional(''),
         reviewers=Optional(None)):
     """
     Creates a new pull request.
 
     Accepts refs in the following formats:
 
         * branch:<branch_name>:<sha>
         * branch:<branch_name>
         * bookmark:<bookmark_name>:<sha> (Mercurial only)
         * bookmark:<bookmark_name> (Mercurial only)
 
     :param apiuser: This is filled automatically from the |authtoken|.
     :type apiuser: AuthUser
     :param source_repo: Set the source repository name.
     :type source_repo: str
     :param target_repo: Set the target repository name.
     :type target_repo: str
     :param source_ref: Set the source ref name.
     :type source_ref: str
     :param target_ref: Set the target ref name.
     :type target_ref: str
     :param title: Optionally Set the pull request title, it's generated otherwise
     :type title: str
     :param description: Set the pull request description.
     :type description: Optional(str)
     :type description_renderer: Optional(str)
     :param description_renderer: Set pull request renderer for the description.
         It should be 'rst', 'markdown' or 'plain'. If not give default
         system renderer will be used
     :param reviewers: Set the new pull request reviewers list.
         Reviewer defined by review rules will be added automatically to the
         defined list.
     :type reviewers: Optional(list)
         Accepts username strings or objects of the format:
 
             [{'username': 'nick', 'reasons': ['original author'], 'mandatory': <bool>}]
     """
 
     source_db_repo = get_repo_or_error(source_repo)
     target_db_repo = get_repo_or_error(target_repo)
     if not has_superadmin_permission(apiuser):
         _perms = ('repository.admin', 'repository.write', 'repository.read',)
         validate_repo_permissions(apiuser, source_repo, source_db_repo, _perms)
 
     full_source_ref = resolve_ref_or_error(source_ref, source_db_repo)
     full_target_ref = resolve_ref_or_error(target_ref, target_db_repo)
 
     source_scm = source_db_repo.scm_instance()
     target_scm = target_db_repo.scm_instance()
 
     source_commit = get_commit_or_error(full_source_ref, source_db_repo)
     target_commit = get_commit_or_error(full_target_ref, target_db_repo)
 
     ancestor = source_scm.get_common_ancestor(
         source_commit.raw_id, target_commit.raw_id, target_scm)
     if not ancestor:
         raise JSONRPCError('no common ancestor found')
 
     # recalculate target ref based on ancestor
     target_ref_type, target_ref_name, __ = full_target_ref.split(':')
     full_target_ref = ':'.join((target_ref_type, target_ref_name, ancestor))
 
     commit_ranges = target_scm.compare(
         target_commit.raw_id, source_commit.raw_id, source_scm,
         merge=True, pre_load=[])
 
     if not commit_ranges:
         raise JSONRPCError('no commits found')
 
     reviewer_objects = Optional.extract(reviewers) or []
 
     # serialize and validate passed in given reviewers
     if reviewer_objects:
         schema = ReviewerListSchema()
         try:
             reviewer_objects = schema.deserialize(reviewer_objects)
         except Invalid as err:
             raise JSONRPCValidationError(colander_exc=err)
 
         # validate users
         for reviewer_object in reviewer_objects:
             user = get_user_or_error(reviewer_object['username'])
             reviewer_object['user_id'] = user.user_id
 
     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(
         apiuser.get_instance(), source_db_repo,
         source_commit, target_db_repo, target_commit)
 
     # now MERGE our given with the calculated
     reviewer_objects = reviewer_rules['reviewers'] + reviewer_objects
 
     try:
         reviewers = validate_default_reviewers(
             reviewer_objects, reviewer_rules)
     except ValueError as e:
         raise JSONRPCError('Reviewers Validation: {}'.format(e))
 
     title = Optional.extract(title)
     if not title:
         title_source_ref = source_ref.split(':', 2)[1]
         title = PullRequestModel().generate_pullrequest_title(
             source=source_repo,
             source_ref=title_source_ref,
             target=target_repo
         )
     # fetch renderer, if set fallback to plain in case of PR
     rc_config = SettingsModel().get_all_settings()
     default_system_renderer = rc_config.get('rhodecode_markup_renderer', 'plain')
     description = Optional.extract(description)
     description_renderer = Optional.extract(description_renderer) or default_system_renderer
 
     pull_request = PullRequestModel().create(
         created_by=apiuser.user_id,
         source_repo=source_repo,
         source_ref=full_source_ref,
         target_repo=target_repo,
         target_ref=full_target_ref,
         revisions=[commit.raw_id for commit in reversed(commit_ranges)],
         reviewers=reviewers,
         title=title,
         description=description,
         description_renderer=description_renderer,
         reviewer_data=reviewer_rules,
         auth_user=apiuser
     )
 
     Session().commit()
     data = {
         'msg': 'Created new pull request `{}`'.format(title),
         'pull_request_id': pull_request.pull_request_id,
     }
     return data
 
 
 @jsonrpc_method()
 def update_pull_request(
         request, apiuser, pullrequestid, repoid=Optional(None),
         title=Optional(''), description=Optional(''), description_renderer=Optional(''),
         reviewers=Optional(None), update_commits=Optional(None)):
     """
     Updates a pull request.
 
     :param apiuser: This is filled automatically from the |authtoken|.
     :type apiuser: AuthUser
     :param repoid: Optional repository name or repository ID.
     :type repoid: str or int
     :param pullrequestid: The pull request ID.
     :type pullrequestid: int
     :param title: Set the pull request title.
     :type title: str
     :param description: Update pull request description.
     :type description: Optional(str)
     :type description_renderer: Optional(str)
     :param description_renderer: Update pull request renderer for the description.
         It should be 'rst', 'markdown' or 'plain'
     :param reviewers: Update pull request reviewers list with new value.
     :type reviewers: Optional(list)
         Accepts username strings or objects of the format:
 
             [{'username': 'nick', 'reasons': ['original author'], 'mandatory': <bool>}]
 
     :param update_commits: Trigger update of commits for this pull request
     :type: update_commits: Optional(bool)
 
     Example output:
 
     .. code-block:: bash
 
         id : <id_given_in_input>
         result : {
             "msg": "Updated pull request `63`",
             "pull_request": <pull_request_object>,
             "updated_reviewers": {
               "added": [
                 "username"
               ],
               "removed": []
             },
             "updated_commits": {
               "added": [
                 "<sha1_hash>"
               ],
               "common": [
                 "<sha1_hash>",
                 "<sha1_hash>",
               ],
               "removed": []
             }
         }
         error :  null
     """
 
     pull_request = get_pull_request_or_error(pullrequestid)
     if Optional.extract(repoid):
         repo = get_repo_or_error(repoid)
     else:
         repo = pull_request.target_repo
 
     if not PullRequestModel().check_user_update(
             pull_request, apiuser, api=True):
         raise JSONRPCError(
             'pull request `%s` update failed, no permission to update.' % (
                 pullrequestid,))
     if pull_request.is_closed():
         raise JSONRPCError(
             'pull request `%s` update failed, pull request is closed' % (
                 pullrequestid,))
 
     reviewer_objects = Optional.extract(reviewers) or []
 
     if reviewer_objects:
         schema = ReviewerListSchema()
         try:
             reviewer_objects = schema.deserialize(reviewer_objects)
         except Invalid as err:
             raise JSONRPCValidationError(colander_exc=err)
 
         # validate users
         for reviewer_object in reviewer_objects:
             user = get_user_or_error(reviewer_object['username'])
             reviewer_object['user_id'] = user.user_id
 
         get_default_reviewers_data, get_validated_reviewers = \
             PullRequestModel().get_reviewer_functions()
 
         # re-use stored rules
         reviewer_rules = pull_request.reviewer_data
         try:
             reviewers = get_validated_reviewers(
                 reviewer_objects, reviewer_rules)
         except ValueError as e:
             raise JSONRPCError('Reviewers Validation: {}'.format(e))
     else:
         reviewers = []
 
     title = Optional.extract(title)
     description = Optional.extract(description)
     description_renderer = Optional.extract(description_renderer)
 
     if title or description:
         PullRequestModel().edit(
             pull_request,
             title or pull_request.title,
             description or pull_request.description,
             description_renderer or pull_request.description_renderer,
             apiuser)
         Session().commit()
 
     commit_changes = {"added": [], "common": [], "removed": []}
     if str2bool(Optional.extract(update_commits)):
-        if PullRequestModel().has_valid_update_type(pull_request):
-            update_response = PullRequestModel().update_commits(
-                pull_request)
-            commit_changes = update_response.changes or commit_changes
-        Session().commit()
+
+        if pull_request.pull_request_state != PullRequest.STATE_CREATED:
+            raise JSONRPCError(
+                'Operation forbidden because pull request is in state {}, '
+                'only state {} is allowed.'.format(
+                    pull_request.pull_request_state, PullRequest.STATE_CREATED))
+
+        with pull_request.set_state(PullRequest.STATE_UPDATING):
+            if PullRequestModel().has_valid_update_type(pull_request):
+                update_response = PullRequestModel().update_commits(pull_request)
+                commit_changes = update_response.changes or commit_changes
+            Session().commit()
 
     reviewers_changes = {"added": [], "removed": []}
     if reviewers:
         added_reviewers, removed_reviewers = \
             PullRequestModel().update_reviewers(pull_request, reviewers, apiuser)
 
         reviewers_changes['added'] = sorted(
             [get_user_or_error(n).username for n in added_reviewers])
         reviewers_changes['removed'] = sorted(
             [get_user_or_error(n).username for n in removed_reviewers])
         Session().commit()
 
     data = {
         'msg': 'Updated pull request `{}`'.format(
             pull_request.pull_request_id),
         'pull_request': pull_request.get_api_data(),
         'updated_commits': commit_changes,
         'updated_reviewers': reviewers_changes
     }
 
     return data
 
 
 @jsonrpc_method()
 def close_pull_request(
         request, apiuser, pullrequestid, repoid=Optional(None),
         userid=Optional(OAttr('apiuser')), message=Optional('')):
     """
     Close the pull request specified by `pullrequestid`.
 
     :param apiuser: This is filled automatically from the |authtoken|.
     :type apiuser: AuthUser
     :param repoid: Repository name or repository ID to which the pull
         request belongs.
     :type repoid: str or int
     :param pullrequestid: ID of the pull request to be closed.
     :type pullrequestid: int
     :param userid: Close the pull request as this user.
     :type userid: Optional(str or int)
     :param message: Optional message to close the Pull Request with. If not
         specified it will be generated automatically.
     :type message: Optional(str)
 
     Example output:
 
     .. code-block:: bash
 
         "id": <id_given_in_input>,
         "result": {
             "pull_request_id":  "<int>",
             "close_status":     "<str:status_lbl>,
             "closed":           "<bool>"
         },
         "error": null
 
     """
     _ = request.translate
 
     pull_request = get_pull_request_or_error(pullrequestid)
     if Optional.extract(repoid):
         repo = get_repo_or_error(repoid)
     else:
         repo = pull_request.target_repo
 
     if not isinstance(userid, Optional):
         if (has_superadmin_permission(apiuser) or
                 HasRepoPermissionAnyApi('repository.admin')(
                     user=apiuser, repo_name=repo.repo_name)):
             apiuser = get_user_or_error(userid)
         else:
             raise JSONRPCError('userid is not the same as your user')
 
     if pull_request.is_closed():
         raise JSONRPCError(
             'pull request `%s` is already closed' % (pullrequestid,))
 
     # only owner or admin or person with write permissions
     allowed_to_close = PullRequestModel().check_user_update(
             pull_request, apiuser, api=True)
 
     if not allowed_to_close:
         raise JSONRPCError(
             'pull request `%s` close failed, no permission to close.' % (
                 pullrequestid,))
 
     # message we're using to close the PR, else it's automatically generated
     message = Optional.extract(message)
 
     # finally close the PR, with proper message comment
     comment, status = PullRequestModel().close_pull_request_with_comment(
         pull_request, apiuser, repo, message=message, auth_user=apiuser)
     status_lbl = ChangesetStatus.get_status_lbl(status)
 
     Session().commit()
 
     data = {
         'pull_request_id': pull_request.pull_request_id,
         'close_status': status_lbl,
         'closed': True,
     }
     return data

rhodecode/apps/repository/tests/test_repo_pullrequests.py

@@ -1,1233 +1,1216 b''
 # -*- coding: utf-8 -*-
 
 # Copyright (C) 2010-2019 RhodeCode GmbH
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU Affero General Public License, version 3
 # (only), as published by the Free Software Foundation.
 #
 # This program is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 # 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
 
 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
 from rhodecode.model.db import (
     PullRequest, ChangesetStatus, UserLog, Notification, ChangesetComment, Repository)
 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, TEST_USER_ADMIN_LOGIN, TEST_USER_REGULAR_LOGIN)
 
 
 def route_path(name, params=None, **kwargs):
     import urllib
 
     base_url = {
         '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_targets': '/{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:
         base_url = '{}?{}'.format(base_url, urllib.urlencode(params))
     return base_url
 
 
 @pytest.mark.usefixtures('app', 'autologin_user')
 @pytest.mark.backends("git", "hg")
 class TestPullrequestsView(object):
 
     def test_index(self, backend):
         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>' % 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(
             route_path('pullrequest_new', repo_name=repo.repo_name,
                        commit=repo.get_commit().raw_id),
             status=200)
 
     @pytest.mark.parametrize('pr_merge_enabled', [True, False])
     @pytest.mark.parametrize('range_diff', ["0", "1"])
     def test_show(self, pr_util, pr_merge_enabled, range_diff):
         pull_request = pr_util.create_pull_request(
             mergeable=pr_merge_enabled, enable_notifications=False)
 
         response = self.app.get(route_path(
             'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id,
             params={'range-diff': range_diff}))
 
         for commit_id in pull_request.revisions:
             response.mustcontain(commit_id)
 
         assert pull_request.target_ref_parts.type in response
         assert pull_request.target_ref_parts.name in response
         target_clone_url = pull_request.target_repo.clone_url()
         assert target_clone_url in response
 
         assert 'class="pull-request-merge"' in response
         if pr_merge_enabled:
             response.mustcontain('Pull request reviewer approval is pending')
         else:
             response.mustcontain('Server-side pull request merging is disabled.')
 
         if range_diff == "1":
             response.mustcontain('Turn off: Show the diff as commit range')
 
     def test_close_status_visibility(self, pr_util, user_util, csrf_token):
         # Logout
         response = self.app.post(
             h.route_path('logout'),
             params={'csrf_token': csrf_token})
         # Login as regular user
         response = self.app.post(h.route_path('login'),
                                  {'username': TEST_USER_REGULAR_LOGIN,
                                   'password': 'test12'})
 
         pull_request = pr_util.create_pull_request(
             author=TEST_USER_REGULAR_LOGIN)
 
         response = self.app.get(route_path(
             'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id))
 
         response.mustcontain('Server-side pull request merging is disabled.')
 
         assert_response = response.assert_response()
         # for regular user without a merge permissions, we don't see it
         assert_response.no_element_exists('#close-pull-request-action')
 
         user_util.grant_user_permission_to_repo(
             pull_request.target_repo,
             UserModel().get_by_username(TEST_USER_REGULAR_LOGIN),
             'repository.write')
         response = self.app.get(route_path(
             'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id))
 
         response.mustcontain('Server-side pull request merging is disabled.')
 
         assert_response = response.assert_response()
         # now regular user has a merge permissions, we have CLOSE button
         assert_response.one_element_exists('#close-pull-request-action')
 
     def test_show_invalid_commit_id(self, pr_util):
         # Simulating invalid revisions which will cause a lookup error
         pull_request = pr_util.create_pull_request()
         pull_request.revisions = ['invalid']
         Session().add(pull_request)
         Session().commit()
 
         response = self.app.get(route_path(
             'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id))
 
         for commit_id in pull_request.revisions:
             response.mustcontain(commit_id)
 
     def test_show_invalid_source_reference(self, pr_util):
         pull_request = pr_util.create_pull_request()
         pull_request.source_ref = 'branch:b:invalid'
         Session().add(pull_request)
         Session().commit()
 
         self.app.get(route_path(
             'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
             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(
             route_path('pullrequest_update',
                 repo_name=pull_request.target_repo.repo_name,
                 pull_request_id=pull_request_id),
             params={
                 'edit_pull_request': 'true',
                 'title': 'New title',
                 'description': 'New description',
                 'csrf_token': csrf_token})
 
         assert_session_flash(
             response, u'Pull request title & description updated.',
             category='success')
 
         pull_request = PullRequest.get(pull_request_id)
         assert pull_request.title == 'New title'
         assert pull_request.description == 'New description'
 
     def test_edit_title_description_closed(self, pr_util, csrf_token):
         pull_request = pr_util.create_pull_request()
         pull_request_id = pull_request.pull_request_id
         repo_name = pull_request.target_repo.repo_name
         pr_util.close()
 
         response = self.app.post(
             route_path('pullrequest_update',
                        repo_name=repo_name, pull_request_id=pull_request_id),
             params={
                 'edit_pull_request': 'true',
                 'title': 'New title',
                 'description': 'New description',
                 'csrf_token': csrf_token}, status=200)
         assert_session_flash(
             response, u'Cannot update closed pull requests.',
             category='error')
 
     def test_update_invalid_source_reference(self, pr_util, csrf_token):
         from rhodecode.lib.vcs.backends.base import UpdateFailureReason
 
         pull_request = pr_util.create_pull_request()
         pull_request.source_ref = 'branch:invalid-branch:invalid-commit-id'
         Session().add(pull_request)
         Session().commit()
 
         pull_request_id = pull_request.pull_request_id
 
         response = self.app.post(
             route_path('pullrequest_update',
                 repo_name=pull_request.target_repo.repo_name,
                 pull_request_id=pull_request_id),
             params={'update_commits': 'true', 'csrf_token': csrf_token})
 
         expected_msg = str(PullRequestModel.UPDATE_STATUS_MESSAGES[
             UpdateFailureReason.MISSING_SOURCE_REF])
         assert_session_flash(response, expected_msg, category='error')
 
     def test_missing_target_reference(self, pr_util, csrf_token):
         from rhodecode.lib.vcs.backends.base import MergeFailureReason
         pull_request = pr_util.create_pull_request(
             approved=True, mergeable=True)
         unicode_reference = u'branch:invalid-branch:invalid-commit-id'
         pull_request.target_ref = unicode_reference
         Session().add(pull_request)
         Session().commit()
 
         pull_request_id = pull_request.pull_request_id
         pull_request_url = route_path(
             'pullrequest_show',
             repo_name=pull_request.target_repo.repo_name,
             pull_request_id=pull_request_id)
 
         response = self.app.get(pull_request_url)
         target_ref_id = 'invalid-branch'
         merge_resp = MergeResponse(
             True, True, '', MergeFailureReason.MISSING_TARGET_REF,
             metadata={'target_ref': PullRequest.unicode_to_reference(unicode_reference)})
         response.assert_response().element_contains(
             'span[data-role="merge-message"]', merge_resp.merge_status_message)
 
     def test_comment_and_close_pull_request_custom_message_approved(
             self, pr_util, csrf_token, xhr_header):
 
         pull_request = pr_util.create_pull_request(approved=True)
         pull_request_id = pull_request.pull_request_id
         author = pull_request.user_id
         repo = pull_request.target_repo.repo_id
 
         self.app.post(
             route_path('pullrequest_comment_create',
                        repo_name=pull_request.target_repo.scm_instance().name,
                        pull_request_id=pull_request_id),
             params={
                 'close_pull_request': '1',
                 'text': 'Closing a PR',
                 'csrf_token': csrf_token},
             extra_environ=xhr_header,)
 
         journal = UserLog.query()\
             .filter(UserLog.user_id == author)\
             .filter(UserLog.repository_id == repo) \
             .order_by('user_log_id') \
             .all()
         assert journal[-1].action == 'repo.pull_request.close'
 
         pull_request = PullRequest.get(pull_request_id)
         assert pull_request.is_closed()
 
         status = ChangesetStatusModel().get_status(
             pull_request.source_repo, pull_request=pull_request)
         assert status == ChangesetStatus.STATUS_APPROVED
         comments = ChangesetComment().query() \
             .filter(ChangesetComment.pull_request == pull_request) \
             .order_by(ChangesetComment.comment_id.asc())\
             .all()
         assert comments[-1].text == 'Closing a PR'
 
     def test_comment_force_close_pull_request_rejected(
             self, pr_util, csrf_token, xhr_header):
         pull_request = pr_util.create_pull_request()
         pull_request_id = pull_request.pull_request_id
         PullRequestModel().update_reviewers(
             pull_request_id, [(1, ['reason'], False, []), (2, ['reason2'], False, [])],
             pull_request.author)
         author = pull_request.user_id
         repo = pull_request.target_repo.repo_id
 
         self.app.post(
             route_path('pullrequest_comment_create',
                 repo_name=pull_request.target_repo.scm_instance().name,
                 pull_request_id=pull_request_id),
             params={
                 'close_pull_request': '1',
                 'csrf_token': csrf_token},
             extra_environ=xhr_header)
 
         pull_request = PullRequest.get(pull_request_id)
 
         journal = UserLog.query()\
             .filter(UserLog.user_id == author, UserLog.repository_id == repo) \
             .order_by('user_log_id') \
             .all()
         assert journal[-1].action == 'repo.pull_request.close'
 
         # check only the latest status, not the review status
         status = ChangesetStatusModel().get_status(
             pull_request.source_repo, pull_request=pull_request)
         assert status == ChangesetStatus.STATUS_REJECTED
 
     def test_comment_and_close_pull_request(
             self, pr_util, csrf_token, xhr_header):
         pull_request = pr_util.create_pull_request()
         pull_request_id = pull_request.pull_request_id
 
         response = self.app.post(
             route_path('pullrequest_comment_create',
                 repo_name=pull_request.target_repo.scm_instance().name,
                 pull_request_id=pull_request.pull_request_id),
             params={
                 'close_pull_request': 'true',
                 'csrf_token': csrf_token},
             extra_environ=xhr_header)
 
         assert response.json
 
         pull_request = PullRequest.get(pull_request_id)
         assert pull_request.is_closed()
 
         # check only the latest status, not the review status
         status = ChangesetStatusModel().get_status(
             pull_request.source_repo, pull_request=pull_request)
         assert status == ChangesetStatus.STATUS_REJECTED
 
     def test_create_pull_request(self, backend, csrf_token):
         commits = [
             {'message': 'ancestor'},
             {'message': 'change'},
             {'message': 'change2'},
         ]
         commit_ids = backend.create_master_repo(commits)
         target = backend.create_repo(heads=['ancestor'])
         source = backend.create_repo(heads=['change2'])
 
         response = self.app.post(
             route_path('pullrequest_create', repo_name=source.repo_name),
             [
                 ('source_repo', source.repo_name),
                 ('source_ref', 'branch:default:' + commit_ids['change2']),
                 ('target_repo', target.repo_name),
                 ('target_ref',  'branch:default:' + commit_ids['ancestor']),
                 ('common_ancestor', commit_ids['ancestor']),
                 ('pullrequest_title', 'Title'),
                 ('pullrequest_desc', 'Description'),
                 ('description_renderer', 'markdown'),
                 ('__start__', 'review_members:sequence'),
                     ('__start__', 'reviewer:mapping'),
                         ('user_id', '1'),
                         ('__start__', 'reasons:sequence'),
                             ('reason', 'Some reason'),
                         ('__end__', 'reasons:sequence'),
                         ('__start__', 'rules:sequence'),
                         ('__end__', 'rules:sequence'),
                         ('mandatory', 'False'),
                     ('__end__', 'reviewer:mapping'),
                 ('__end__', 'review_members:sequence'),
                 ('__start__', 'revisions:sequence'),
                     ('revisions', commit_ids['change']),
                     ('revisions', commit_ids['change2']),
                 ('__end__', 'revisions:sequence'),
                 ('user', ''),
                 ('csrf_token', csrf_token),
             ],
             status=302)
 
         location = response.headers['Location']
         pull_request_id = location.rsplit('/', 1)[1]
         assert pull_request_id != 'new'
         pull_request = PullRequest.get(int(pull_request_id))
 
         # check that we have now both revisions
         assert pull_request.revisions == [commit_ids['change2'], commit_ids['change']]
         assert pull_request.source_ref == 'branch:default:' + commit_ids['change2']
         expected_target_ref = 'branch:default:' + commit_ids['ancestor']
         assert pull_request.target_ref == expected_target_ref
 
     def test_reviewer_notifications(self, backend, csrf_token):
         # We have to use the app.post for this test so it will create the
         # notifications properly with the new PR
         commits = [
             {'message': 'ancestor',
              'added': [FileNode('file_A', content='content_of_ancestor')]},
             {'message': 'change',
              'added': [FileNode('file_a', content='content_of_change')]},
             {'message': 'change-child'},
             {'message': 'ancestor-child', 'parents': ['ancestor'],
              'added': [
                 FileNode('file_B', content='content_of_ancestor_child')]},
             {'message': 'ancestor-child-2'},
         ]
         commit_ids = backend.create_master_repo(commits)
         target = backend.create_repo(heads=['ancestor-child'])
         source = backend.create_repo(heads=['change'])
 
         response = self.app.post(
             route_path('pullrequest_create', repo_name=source.repo_name),
             [
                 ('source_repo', source.repo_name),
                 ('source_ref', 'branch:default:' + commit_ids['change']),
                 ('target_repo', target.repo_name),
                 ('target_ref',  'branch:default:' + commit_ids['ancestor-child']),
                 ('common_ancestor', commit_ids['ancestor']),
                 ('pullrequest_title', 'Title'),
                 ('pullrequest_desc', 'Description'),
                 ('description_renderer', 'markdown'),
                 ('__start__', 'review_members:sequence'),
                     ('__start__', 'reviewer:mapping'),
                         ('user_id', '2'),
                         ('__start__', 'reasons:sequence'),
                             ('reason', 'Some reason'),
                         ('__end__', 'reasons:sequence'),
                         ('__start__', 'rules:sequence'),
                         ('__end__', 'rules:sequence'),
                         ('mandatory', 'False'),
                     ('__end__', 'reviewer:mapping'),
                 ('__end__', 'review_members:sequence'),
                 ('__start__', 'revisions:sequence'),
                     ('revisions', commit_ids['change']),
                 ('__end__', 'revisions:sequence'),
                 ('user', ''),
                 ('csrf_token', csrf_token),
             ],
             status=302)
 
         location = response.headers['Location']
 
         pull_request_id = location.rsplit('/', 1)[1]
         assert pull_request_id != 'new'
         pull_request = PullRequest.get(int(pull_request_id))
 
         # Check that a notification was made
         notifications = Notification.query()\
             .filter(Notification.created_by == pull_request.author.user_id,
                     Notification.type_ == Notification.TYPE_PULL_REQUEST,
                     Notification.subject.contains(
                         "wants you to review pull request #%s" % pull_request_id))
         assert len(notifications.all()) == 1
 
         # Change reviewers and check that a notification was made
         PullRequestModel().update_reviewers(
             pull_request.pull_request_id, [(1, [], False, [])],
             pull_request.author)
         assert len(notifications.all()) == 2
 
     def test_create_pull_request_stores_ancestor_commit_id(self, backend,
                                                            csrf_token):
         commits = [
             {'message': 'ancestor',
              'added': [FileNode('file_A', content='content_of_ancestor')]},
             {'message': 'change',
              'added': [FileNode('file_a', content='content_of_change')]},
             {'message': 'change-child'},
             {'message': 'ancestor-child', 'parents': ['ancestor'],
              'added': [
                 FileNode('file_B', content='content_of_ancestor_child')]},
             {'message': 'ancestor-child-2'},
         ]
         commit_ids = backend.create_master_repo(commits)
         target = backend.create_repo(heads=['ancestor-child'])
         source = backend.create_repo(heads=['change'])
 
         response = self.app.post(
             route_path('pullrequest_create', repo_name=source.repo_name),
             [
                 ('source_repo', source.repo_name),
                 ('source_ref', 'branch:default:' + commit_ids['change']),
                 ('target_repo', target.repo_name),
                 ('target_ref',  'branch:default:' + commit_ids['ancestor-child']),
                 ('common_ancestor', commit_ids['ancestor']),
                 ('pullrequest_title', 'Title'),
                 ('pullrequest_desc', 'Description'),
                 ('description_renderer', 'markdown'),
                 ('__start__', 'review_members:sequence'),
                     ('__start__', 'reviewer:mapping'),
                         ('user_id', '1'),
                         ('__start__', 'reasons:sequence'),
                             ('reason', 'Some reason'),
                         ('__end__', 'reasons:sequence'),
                         ('__start__', 'rules:sequence'),
                         ('__end__', 'rules:sequence'),
                         ('mandatory', 'False'),
                     ('__end__', 'reviewer:mapping'),
                 ('__end__', 'review_members:sequence'),
                 ('__start__', 'revisions:sequence'),
                     ('revisions', commit_ids['change']),
                 ('__end__', 'revisions:sequence'),
                 ('user', ''),
                 ('csrf_token', csrf_token),
             ],
             status=302)
 
         location = response.headers['Location']
 
         pull_request_id = location.rsplit('/', 1)[1]
         assert pull_request_id != 'new'
         pull_request = PullRequest.get(int(pull_request_id))
 
         # target_ref has to point to the ancestor's commit_id in order to
         # show the correct diff
         expected_target_ref = 'branch:default:' + commit_ids['ancestor']
         assert pull_request.target_ref == expected_target_ref
 
         # Check generated diff contents
         response = response.follow()
         assert 'content_of_ancestor' not in response.body
         assert 'content_of_ancestor-child' not in response.body
         assert 'content_of_change' in response.body
 
     def test_merge_pull_request_enabled(self, pr_util, csrf_token):
         # Clear any previous calls to rcextensions
         rhodecode.EXTENSIONS.calls.clear()
 
         pull_request = pr_util.create_pull_request(
             approved=True, mergeable=True)
         pull_request_id = pull_request.pull_request_id
         repo_name = pull_request.target_repo.scm_instance().name,
 
         response = self.app.post(
             route_path('pullrequest_merge',
                 repo_name=str(repo_name[0]),
                 pull_request_id=pull_request_id),
             params={'csrf_token': csrf_token}).follow()
 
         pull_request = PullRequest.get(pull_request_id)
 
         assert response.status_int == 200
         assert pull_request.is_closed()
         assert_pull_request_status(
             pull_request, ChangesetStatus.STATUS_APPROVED)
 
         # Check the relevant log entries were added
         user_logs = UserLog.query().order_by('-user_log_id').limit(3)
         actions = [log.action for log in user_logs]
         pr_commit_ids = PullRequestModel()._get_commit_ids(pull_request)
         expected_actions = [
              u'repo.pull_request.close',
              u'repo.pull_request.merge',
              u'repo.pull_request.comment.create'
         ]
         assert actions == expected_actions
 
         user_logs = UserLog.query().order_by('-user_log_id').limit(4)
         actions = [log for log in user_logs]
         assert actions[-1].action == 'user.push'
         assert actions[-1].action_data['commit_ids'] == pr_commit_ids
 
         # Check post_push rcextension was really executed
         push_calls = rhodecode.EXTENSIONS.calls['_push_hook']
         assert len(push_calls) == 1
         unused_last_call_args, last_call_kwargs = push_calls[0]
         assert last_call_kwargs['action'] == 'push'
         assert last_call_kwargs['commit_ids'] == pr_commit_ids
 
     def test_merge_pull_request_disabled(self, pr_util, csrf_token):
         pull_request = pr_util.create_pull_request(mergeable=False)
         pull_request_id = pull_request.pull_request_id
         pull_request = PullRequest.get(pull_request_id)
 
         response = self.app.post(
             route_path('pullrequest_merge',
                 repo_name=pull_request.target_repo.scm_instance().name,
                 pull_request_id=pull_request.pull_request_id),
             params={'csrf_token': csrf_token}).follow()
 
         assert response.status_int == 200
         response.mustcontain(
             'Merge is not currently possible because of below failed checks.')
         response.mustcontain('Server-side pull request merging is disabled.')
 
     @pytest.mark.skip_backends('svn')
     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
 
         response = self.app.post(
             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
 
         response.mustcontain(
             '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
 
         merge_resp = MergeResponse(True, False, 'STUB_COMMIT_ID',
                                    MergeFailureReason.PUSH_FAILED,
                                    metadata={'target': 'shadow repo',
                                              'merge_commit': 'xxx'})
         model_patcher = mock.patch.multiple(
             PullRequestModel,
             merge_repo=mock.Mock(return_value=merge_resp),
             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)
 
             merge_resp = MergeResponse(True, True, '', MergeFailureReason.PUSH_FAILED,
                                        metadata={'target': 'shadow repo',
                                                  'merge_commit': 'xxx'})
         assert_session_flash(response, merge_resp.merge_status_message)
 
     def test_update_source_revision(self, backend, csrf_token):
         commits = [
             {'message': 'ancestor'},
             {'message': 'change'},
             {'message': 'change-2'},
         ]
         commit_ids = backend.create_master_repo(commits)
         target = backend.create_repo(heads=['ancestor'])
         source = backend.create_repo(heads=['change'])
 
         # create pr from a in source to A in target
         pull_request = PullRequest()
+
         pull_request.source_repo = source
-        # TODO: johbo: Make sure that we write the source ref this way!
         pull_request.source_ref = 'branch:{branch}:{commit_id}'.format(
             branch=backend.default_branch_name, commit_id=commit_ids['change'])
+
         pull_request.target_repo = target
-
         pull_request.target_ref = 'branch:{branch}:{commit_id}'.format(
-            branch=backend.default_branch_name,
-            commit_id=commit_ids['ancestor'])
+            branch=backend.default_branch_name, commit_id=commit_ids['ancestor'])
+
         pull_request.revisions = [commit_ids['change']]
         pull_request.title = u"Test"
         pull_request.description = u"Description"
-        pull_request.author = UserModel().get_by_username(
-            TEST_USER_ADMIN_LOGIN)
+        pull_request.author = UserModel().get_by_username(TEST_USER_ADMIN_LOGIN)
+        pull_request.pull_request_state = PullRequest.STATE_CREATED
         Session().add(pull_request)
         Session().commit()
         pull_request_id = pull_request.pull_request_id
 
         # source has ancestor - change - change-2
         backend.pull_heads(source, heads=['change-2'])
 
         # update PR
         self.app.post(
             route_path('pullrequest_update',
-                repo_name=target.repo_name,
-                pull_request_id=pull_request_id),
-            params={'update_commits': 'true',
-                    'csrf_token': csrf_token})
+                       repo_name=target.repo_name, pull_request_id=pull_request_id),
+            params={'update_commits': 'true', 'csrf_token': csrf_token})
+
+        response = self.app.get(
+            route_path('pullrequest_show',
+                       repo_name=target.repo_name,
+                       pull_request_id=pull_request.pull_request_id))
+
+        assert response.status_int == 200
+        assert 'Pull request updated to' in response.body
+        assert 'with 1 added, 0 removed commits.' in response.body
 
         # check that we have now both revisions
         pull_request = PullRequest.get(pull_request_id)
-        assert pull_request.revisions == [
-            commit_ids['change-2'], commit_ids['change']]
-
-        # TODO: johbo: this should be a test on its own
-        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
-        assert 'with 1 added, 0 removed commits.' in response.body
+        assert pull_request.revisions == [commit_ids['change-2'], commit_ids['change']]
 
     def test_update_target_revision(self, backend, csrf_token):
         commits = [
             {'message': 'ancestor'},
             {'message': 'change'},
             {'message': 'ancestor-new', 'parents': ['ancestor']},
             {'message': 'change-rebased'},
         ]
         commit_ids = backend.create_master_repo(commits)
         target = backend.create_repo(heads=['ancestor'])
         source = backend.create_repo(heads=['change'])
 
         # create pr from a in source to A in target
         pull_request = PullRequest()
+
         pull_request.source_repo = source
-        # TODO: johbo: Make sure that we write the source ref this way!
         pull_request.source_ref = 'branch:{branch}:{commit_id}'.format(
             branch=backend.default_branch_name, commit_id=commit_ids['change'])
+
         pull_request.target_repo = target
-        # TODO: johbo: Target ref should be branch based, since tip can jump
-        # from branch to branch
         pull_request.target_ref = 'branch:{branch}:{commit_id}'.format(
-            branch=backend.default_branch_name,
-            commit_id=commit_ids['ancestor'])
+            branch=backend.default_branch_name, commit_id=commit_ids['ancestor'])
+
         pull_request.revisions = [commit_ids['change']]
         pull_request.title = u"Test"
         pull_request.description = u"Description"
-        pull_request.author = UserModel().get_by_username(
-            TEST_USER_ADMIN_LOGIN)
+        pull_request.author = UserModel().get_by_username(TEST_USER_ADMIN_LOGIN)
+        pull_request.pull_request_state = PullRequest.STATE_CREATED
+
         Session().add(pull_request)
         Session().commit()
         pull_request_id = pull_request.pull_request_id
 
         # target has ancestor - ancestor-new
         # source has ancestor - ancestor-new - change-rebased
         backend.pull_heads(target, heads=['ancestor-new'])
         backend.pull_heads(source, heads=['change-rebased'])
 
         # update PR
         self.app.post(
             route_path('pullrequest_update',
-                repo_name=target.repo_name,
-                pull_request_id=pull_request_id),
-            params={'update_commits': 'true',
-                    'csrf_token': csrf_token},
+                       repo_name=target.repo_name,
+                       pull_request_id=pull_request_id),
+            params={'update_commits': 'true', 'csrf_token': csrf_token},
             status=200)
 
         # check that we have now both revisions
         pull_request = PullRequest.get(pull_request_id)
         assert pull_request.revisions == [commit_ids['change-rebased']]
         assert pull_request.target_ref == 'branch:{branch}:{commit_id}'.format(
-            branch=backend.default_branch_name,
-            commit_id=commit_ids['ancestor-new'])
+            branch=backend.default_branch_name, commit_id=commit_ids['ancestor-new'])
 
-        # TODO: johbo: This should be a test on its own
-        response = self.app.get(route_path(
-            'pullrequest_new',
-            repo_name=target.repo_name))
+        response = self.app.get(
+            route_path('pullrequest_show',
+                       repo_name=target.repo_name,
+                       pull_request_id=pull_request.pull_request_id))
         assert response.status_int == 200
         assert 'Pull request updated to' in response.body
         assert 'with 1 added, 1 removed commits.' in response.body
 
     def test_update_target_revision_with_removal_of_1_commit_git(self, backend_git, csrf_token):
         backend = backend_git
         commits = [
             {'message': 'master-commit-1'},
             {'message': 'master-commit-2-change-1'},
             {'message': 'master-commit-3-change-2'},
 
             {'message': 'feat-commit-1', 'parents': ['master-commit-1']},
             {'message': 'feat-commit-2'},
         ]
         commit_ids = backend.create_master_repo(commits)
         target = backend.create_repo(heads=['master-commit-3-change-2'])
         source = backend.create_repo(heads=['feat-commit-2'])
 
         # create pr from a in source to A in target
         pull_request = PullRequest()
         pull_request.source_repo = source
-        # TODO: johbo: Make sure that we write the source ref this way!
+
         pull_request.source_ref = 'branch:{branch}:{commit_id}'.format(
             branch=backend.default_branch_name,
             commit_id=commit_ids['master-commit-3-change-2'])
 
         pull_request.target_repo = target
-        # TODO: johbo: Target ref should be branch based, since tip can jump
-        # from branch to branch
         pull_request.target_ref = 'branch:{branch}:{commit_id}'.format(
-            branch=backend.default_branch_name,
-            commit_id=commit_ids['feat-commit-2'])
+            branch=backend.default_branch_name, commit_id=commit_ids['feat-commit-2'])
 
         pull_request.revisions = [
             commit_ids['feat-commit-1'],
             commit_ids['feat-commit-2']
         ]
         pull_request.title = u"Test"
         pull_request.description = u"Description"
-        pull_request.author = UserModel().get_by_username(
-            TEST_USER_ADMIN_LOGIN)
+        pull_request.author = UserModel().get_by_username(TEST_USER_ADMIN_LOGIN)
+        pull_request.pull_request_state = PullRequest.STATE_CREATED
         Session().add(pull_request)
         Session().commit()
         pull_request_id = pull_request.pull_request_id
 
         # PR is created, now we simulate a force-push into target,
         # that drops a 2 last commits
         vcsrepo = target.scm_instance()
         vcsrepo.config.clear_section('hooks')
         vcsrepo.run_git_command(['reset', '--soft', 'HEAD~2'])
 
         # update PR
         self.app.post(
             route_path('pullrequest_update',
                 repo_name=target.repo_name,
                 pull_request_id=pull_request_id),
-            params={'update_commits': 'true',
-                    'csrf_token': csrf_token},
+            params={'update_commits': 'true', 'csrf_token': csrf_token},
             status=200)
 
-        response = self.app.get(route_path(
-            'pullrequest_new',
-            repo_name=target.repo_name))
+        response = self.app.get(route_path('pullrequest_new', repo_name=target.repo_name))
         assert response.status_int == 200
         response.mustcontain('Pull request updated to')
         response.mustcontain('with 0 added, 0 removed commits.')
 
     def test_update_of_ancestor_reference(self, backend, csrf_token):
         commits = [
             {'message': 'ancestor'},
             {'message': 'change'},
             {'message': 'change-2'},
             {'message': 'ancestor-new', 'parents': ['ancestor']},
             {'message': 'change-rebased'},
         ]
         commit_ids = backend.create_master_repo(commits)
         target = backend.create_repo(heads=['ancestor'])
         source = backend.create_repo(heads=['change'])
 
         # create pr from a in source to A in target
         pull_request = PullRequest()
         pull_request.source_repo = source
-        # TODO: johbo: Make sure that we write the source ref this way!
+
         pull_request.source_ref = 'branch:{branch}:{commit_id}'.format(
-            branch=backend.default_branch_name,
-            commit_id=commit_ids['change'])
+            branch=backend.default_branch_name, commit_id=commit_ids['change'])
         pull_request.target_repo = target
-        # TODO: johbo: Target ref should be branch based, since tip can jump
-        # from branch to branch
         pull_request.target_ref = 'branch:{branch}:{commit_id}'.format(
-            branch=backend.default_branch_name,
-            commit_id=commit_ids['ancestor'])
+            branch=backend.default_branch_name, commit_id=commit_ids['ancestor'])
         pull_request.revisions = [commit_ids['change']]
         pull_request.title = u"Test"
         pull_request.description = u"Description"
-        pull_request.author = UserModel().get_by_username(
-            TEST_USER_ADMIN_LOGIN)
+        pull_request.author = UserModel().get_by_username(TEST_USER_ADMIN_LOGIN)
+        pull_request.pull_request_state = PullRequest.STATE_CREATED
         Session().add(pull_request)
         Session().commit()
         pull_request_id = pull_request.pull_request_id
 
         # target has ancestor - ancestor-new
         # source has ancestor - ancestor-new - change-rebased
         backend.pull_heads(target, heads=['ancestor-new'])
         backend.pull_heads(source, heads=['change-rebased'])
 
         # update PR
         self.app.post(
             route_path('pullrequest_update',
-                repo_name=target.repo_name,
-                pull_request_id=pull_request_id),
-            params={'update_commits': 'true',
-                    'csrf_token': csrf_token},
+                       repo_name=target.repo_name, pull_request_id=pull_request_id),
+            params={'update_commits': 'true', 'csrf_token': csrf_token},
             status=200)
 
         # Expect the target reference to be updated correctly
         pull_request = PullRequest.get(pull_request_id)
         assert pull_request.revisions == [commit_ids['change-rebased']]
         expected_target_ref = 'branch:{branch}:{commit_id}'.format(
             branch=backend.default_branch_name,
             commit_id=commit_ids['ancestor-new'])
         assert pull_request.target_ref == expected_target_ref
 
     def test_remove_pull_request_branch(self, backend_git, csrf_token):
         branch_name = 'development'
         commits = [
             {'message': 'initial-commit'},
             {'message': 'old-feature'},
             {'message': 'new-feature', 'branch': branch_name},
         ]
         repo = backend_git.create_repo(commits)
         commit_ids = backend_git.commit_ids
 
         pull_request = PullRequest()
         pull_request.source_repo = repo
         pull_request.target_repo = repo
         pull_request.source_ref = 'branch:{branch}:{commit_id}'.format(
             branch=branch_name, commit_id=commit_ids['new-feature'])
         pull_request.target_ref = 'branch:{branch}:{commit_id}'.format(
-            branch=backend_git.default_branch_name,
-            commit_id=commit_ids['old-feature'])
+            branch=backend_git.default_branch_name, commit_id=commit_ids['old-feature'])
         pull_request.revisions = [commit_ids['new-feature']]
         pull_request.title = u"Test"
         pull_request.description = u"Description"
-        pull_request.author = UserModel().get_by_username(
-            TEST_USER_ADMIN_LOGIN)
+        pull_request.author = UserModel().get_by_username(TEST_USER_ADMIN_LOGIN)
+        pull_request.pull_request_state = PullRequest.STATE_CREATED
         Session().add(pull_request)
         Session().commit()
 
         vcs = repo.scm_instance()
         vcs.remove_ref('refs/heads/{}'.format(branch_name))
 
         response = self.app.get(route_path(
             'pullrequest_show',
             repo_name=repo.repo_name,
             pull_request_id=pull_request.pull_request_id))
 
         assert response.status_int == 200
 
         response.assert_response().element_contains(
             '#changeset_compare_view_content .alert strong',
             'Missing commits')
         response.assert_response().element_contains(
             '#changeset_compare_view_content .alert',
             'This pull request cannot be displayed, because one or more'
             ' commits no longer exist in the source repository.')
 
     def test_strip_commits_from_pull_request(
             self, backend, pr_util, csrf_token):
         commits = [
             {'message': 'initial-commit'},
             {'message': 'old-feature'},
             {'message': 'new-feature', 'parents': ['initial-commit']},
         ]
         pull_request = pr_util.create_pull_request(
             commits, target_head='initial-commit', source_head='new-feature',
             revisions=['new-feature'])
 
         vcs = pr_util.source_repository.scm_instance()
         if backend.alias == 'git':
             vcs.strip(pr_util.commit_ids['new-feature'], branch_name='master')
         else:
             vcs.strip(pr_util.commit_ids['new-feature'])
 
         response = self.app.get(route_path(
             'pullrequest_show',
             repo_name=pr_util.target_repository.repo_name,
             pull_request_id=pull_request.pull_request_id))
 
         assert response.status_int == 200
 
         response.assert_response().element_contains(
             '#changeset_compare_view_content .alert strong',
             'Missing commits')
         response.assert_response().element_contains(
             '#changeset_compare_view_content .alert',
             'This pull request cannot be displayed, because one or more'
             ' commits no longer exist in the source repository.')
         response.assert_response().element_contains(
             '#update_commits',
             'Update commits')
 
     def test_strip_commits_and_update(
             self, backend, pr_util, csrf_token):
         commits = [
             {'message': 'initial-commit'},
             {'message': 'old-feature'},
             {'message': 'new-feature', 'parents': ['old-feature']},
         ]
         pull_request = pr_util.create_pull_request(
             commits, target_head='old-feature', source_head='new-feature',
             revisions=['new-feature'], mergeable=True)
 
         vcs = pr_util.source_repository.scm_instance()
         if backend.alias == 'git':
             vcs.strip(pr_util.commit_ids['new-feature'], branch_name='master')
         else:
             vcs.strip(pr_util.commit_ids['new-feature'])
 
         response = self.app.post(
             route_path('pullrequest_update',
                 repo_name=pull_request.target_repo.repo_name,
                 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(route_path(
             'pullrequest_show',
             repo_name=pr_util.target_repository.repo_name,
             pull_request_id=pull_request.pull_request_id))
 
         assert response.status_int == 200
         response.assert_response().element_contains(
             '#changeset_compare_view_content .alert strong',
             'Missing commits')
 
     def test_branch_is_a_link(self, pr_util):
         pull_request = pr_util.create_pull_request()
         pull_request.source_ref = 'branch:origin:1234567890abcdef'
         pull_request.target_ref = 'branch:target:abcdef1234567890'
         Session().add(pull_request)
         Session().commit()
 
         response = self.app.get(route_path(
             'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id))
         assert response.status_int == 200
 
         origin = response.assert_response().get_element('.pr-origininfo .tag')
         origin_children = origin.getchildren()
         assert len(origin_children) == 1
         target = response.assert_response().get_element('.pr-targetinfo .tag')
         target_children = target.getchildren()
         assert len(target_children) == 1
 
         expected_origin_link = route_path(
             'repo_changelog',
             repo_name=pull_request.source_repo.scm_instance().name,
             params=dict(branch='origin'))
         expected_target_link = route_path(
             'repo_changelog',
             repo_name=pull_request.target_repo.scm_instance().name,
             params=dict(branch='target'))
         assert origin_children[0].attrib['href'] == expected_origin_link
         assert origin_children[0].text == 'branch: origin'
         assert target_children[0].attrib['href'] == expected_target_link
         assert target_children[0].text == 'branch: target'
 
     def test_bookmark_is_not_a_link(self, pr_util):
         pull_request = pr_util.create_pull_request()
         pull_request.source_ref = 'bookmark:origin:1234567890abcdef'
         pull_request.target_ref = 'bookmark:target:abcdef1234567890'
         Session().add(pull_request)
         Session().commit()
 
         response = self.app.get(route_path(
             'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id))
         assert response.status_int == 200
 
         origin = response.assert_response().get_element('.pr-origininfo .tag')
         assert origin.text.strip() == 'bookmark: origin'
         assert origin.getchildren() == []
 
         target = response.assert_response().get_element('.pr-targetinfo .tag')
         assert target.text.strip() == 'bookmark: target'
         assert target.getchildren() == []
 
     def test_tag_is_not_a_link(self, pr_util):
         pull_request = pr_util.create_pull_request()
         pull_request.source_ref = 'tag:origin:1234567890abcdef'
         pull_request.target_ref = 'tag:target:abcdef1234567890'
         Session().add(pull_request)
         Session().commit()
 
         response = self.app.get(route_path(
             'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id))
         assert response.status_int == 200
 
         origin = response.assert_response().get_element('.pr-origininfo .tag')
         assert origin.text.strip() == 'tag: origin'
         assert origin.getchildren() == []
 
         target = response.assert_response().get_element('.pr-targetinfo .tag')
         assert target.text.strip() == 'tag: target'
         assert target.getchildren() == []
 
     @pytest.mark.parametrize('mergeable', [True, False])
     def test_shadow_repository_link(
             self, mergeable, pr_util, http_host_only_stub):
         """
         Check that the pull request summary page displays a link to the shadow
         repository if the pull request is mergeable. If it is not mergeable
         the link should not be displayed.
         """
         pull_request = pr_util.create_pull_request(
             mergeable=mergeable, enable_notifications=False)
         target_repo = pull_request.target_repo.scm_instance()
         pr_id = pull_request.pull_request_id
         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(route_path(
             'pullrequest_show',
             repo_name=target_repo.name,
             pull_request_id=pr_id))
 
         if mergeable:
             response.assert_response().element_value_contains(
                 'input.pr-mergeinfo', shadow_url)
             response.assert_response().element_value_contains(
                 'input.pr-mergeinfo ', 'pr-merge')
         else:
             response.assert_response().no_element_exists('.pr-mergeinfo')
 
 
 @pytest.mark.usefixtures('app')
 @pytest.mark.backends("git", "hg")
 class TestPullrequestsControllerDelete(object):
     def test_pull_request_delete_button_permissions_admin(
             self, autologin_user, user_admin, pr_util):
         pull_request = pr_util.create_pull_request(
             author=user_admin.username, enable_notifications=False)
 
         response = self.app.get(route_path(
             'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id))
 
         response.mustcontain('id="delete_pullrequest"')
         response.mustcontain('Confirm to delete this pull request')
 
     def test_pull_request_delete_button_permissions_owner(
             self, autologin_regular_user, user_regular, pr_util):
         pull_request = pr_util.create_pull_request(
             author=user_regular.username, enable_notifications=False)
 
         response = self.app.get(route_path(
             'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id))
 
         response.mustcontain('id="delete_pullrequest"')
         response.mustcontain('Confirm to delete this pull request')
 
     def test_pull_request_delete_button_permissions_forbidden(
             self, autologin_regular_user, user_regular, user_admin, pr_util):
         pull_request = pr_util.create_pull_request(
             author=user_admin.username, enable_notifications=False)
 
         response = self.app.get(route_path(
             'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id))
         response.mustcontain(no=['id="delete_pullrequest"'])
         response.mustcontain(no=['Confirm to delete this pull request'])
 
     def test_pull_request_delete_button_permissions_can_update_cannot_delete(
             self, autologin_regular_user, user_regular, user_admin, pr_util,
             user_util):
 
         pull_request = pr_util.create_pull_request(
             author=user_admin.username, enable_notifications=False)
 
         user_util.grant_user_permission_to_repo(
             pull_request.target_repo, user_regular,
             'repository.write')
 
         response = self.app.get(route_path(
             'pullrequest_show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id))
 
         response.mustcontain('id="open_edit_pullrequest"')
         response.mustcontain('id="delete_pullrequest"')
         response.mustcontain(no=['Confirm to delete this pull request'])
 
     def test_delete_comment_returns_404_if_comment_does_not_exist(
             self, autologin_user, pr_util, user_admin, csrf_token, xhr_header):
 
         pull_request = pr_util.create_pull_request(
             author=user_admin.username, enable_notifications=False)
 
         self.app.post(
             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),
             extra_environ=xhr_header,
             params={'csrf_token': csrf_token},
             status=404
         )
 
     def test_delete_comment(
             self, autologin_user, pr_util, user_admin, csrf_token, xhr_header):
 
         pull_request = pr_util.create_pull_request(
             author=user_admin.username, enable_notifications=False)
         comment = pr_util.create_comment()
         comment_id = comment.comment_id
 
         response = self.app.post(
             route_path(
             'pullrequest_comment_delete',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id,
             comment_id=comment_id),
             extra_environ=xhr_header,
             params={'csrf_token': csrf_token},
             status=200
         )
         assert response.body == 'true'
 
     @pytest.mark.parametrize('url_type', [
         'pullrequest_new',
         'pullrequest_create',
         'pullrequest_update',
         'pullrequest_merge',
     ])
     def test_pull_request_is_forbidden_on_archived_repo(
             self, autologin_user, backend, xhr_header, user_util, url_type):
 
         # create a temporary repo
         source = user_util.create_repo(repo_type=backend.alias)
         repo_name = source.repo_name
         repo = Repository.get_by_repo_name(repo_name)
         repo.archived = True
         Session().commit()
 
         response = self.app.get(
             route_path(url_type, repo_name=repo_name, pull_request_id=1), status=302)
 
         msg = 'Action not supported for archived repository.'
         assert_session_flash(response, msg)
 
 
 def assert_pull_request_status(pull_request, expected_status):
     status = ChangesetStatusModel().calculated_review_status(
         pull_request=pull_request)
     assert status == expected_status
 
 
 @pytest.mark.parametrize('route', ['pullrequest_new', 'pullrequest_create'])
 @pytest.mark.usefixtures("autologin_user")
 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)
 

rhodecode/apps/repository/views/repo_pull_requests.py

@@ -1,1413 +1,1456 b''
 # -*- coding: utf-8 -*-
 
 # Copyright (C) 2011-2019 RhodeCode GmbH
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU Affero General Public License, version 3
 # (only), as published by the Free Software Foundation.
 #
 # This program is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 # 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 logging
 import collections
 
 import formencode
 import formencode.htmlfill
 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, channelstream
 from rhodecode.lib.base import vcs_operation_context
 from rhodecode.lib.diffs import load_cached_diff, cache_diff, diff_cache_exist
 from rhodecode.lib.ext_json import json
 from rhodecode.lib.auth import (
     LoginRequired, HasRepoPermissionAny, 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, EmptyRepositoryError)
 from rhodecode.model.changeset_status import ChangesetStatusModel
 from rhodecode.model.comment import CommentsModel
 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__)
 
 
 class RepoPullRequestsView(RepoAppView, DataGridAppView):
 
     def load_default_context(self):
         c = self._get_local_tmpl_context(include_app_defaults=True)
         c.REVIEW_STATUS_APPROVED = ChangesetStatus.STATUS_APPROVED
         c.REVIEW_STATUS_REJECTED = ChangesetStatus.STATUS_REJECTED
         # backward compat., we use for OLD PRs a plain renderer
         c.renderer = 'plain'
         return c
 
     def _get_pull_requests_list(
             self, repo_name, source, filter_type, opened_by, statuses):
 
         draw, start, limit = self._extract_chunk(self.request)
         search_q, order_by, order_dir = self._extract_ordering(self.request)
         _render = self.request.get_partial_renderer(
             'rhodecode:templates/data_table/_dt_elements.mako')
 
         # pagination
 
         if filter_type == 'awaiting_review':
             pull_requests = PullRequestModel().get_awaiting_review(
                 repo_name, source=source, opened_by=opened_by,
                 statuses=statuses, offset=start, length=limit,
                 order_by=order_by, order_dir=order_dir)
             pull_requests_total_count = PullRequestModel().count_awaiting_review(
                 repo_name, source=source, statuses=statuses,
                 opened_by=opened_by)
         elif filter_type == 'awaiting_my_review':
             pull_requests = PullRequestModel().get_awaiting_my_review(
                 repo_name, source=source, opened_by=opened_by,
                 user_id=self._rhodecode_user.user_id, statuses=statuses,
                 offset=start, length=limit, order_by=order_by,
                 order_dir=order_dir)
             pull_requests_total_count = PullRequestModel().count_awaiting_my_review(
                 repo_name, source=source, user_id=self._rhodecode_user.user_id,
                 statuses=statuses, opened_by=opened_by)
         else:
             pull_requests = PullRequestModel().get_all(
                 repo_name, source=source, opened_by=opened_by,
                 statuses=statuses, offset=start, length=limit,
                 order_by=order_by, order_dir=order_dir)
             pull_requests_total_count = PullRequestModel().count_all(
                 repo_name, source=source, statuses=statuses,
                 opened_by=opened_by)
 
         data = []
         comments_model = CommentsModel()
         for pr in pull_requests:
             comments = comments_model.get_all_comments(
                 self.db_repo.repo_id, pull_request=pr)
 
             data.append({
                 'name': _render('pullrequest_name',
                                 pr.pull_request_id, pr.target_repo.repo_name),
                 'name_raw': pr.pull_request_id,
                 'status': _render('pullrequest_status',
                                   pr.calculated_review_status()),
                 'title': _render(
                     'pullrequest_title', pr.title, pr.description),
                 'description': h.escape(pr.description),
                 'updated_on': _render('pullrequest_updated_on',
                                       h.datetime_to_time(pr.updated_on)),
                 'updated_on_raw': h.datetime_to_time(pr.updated_on),
                 'created_on': _render('pullrequest_updated_on',
                                       h.datetime_to_time(pr.created_on)),
                 'created_on_raw': h.datetime_to_time(pr.created_on),
                 'author': _render('pullrequest_author',
                                   pr.author.full_contact, ),
                 'author_raw': pr.author.full_name,
                 'comments': _render('pullrequest_comments', len(comments)),
                 'comments_raw': len(comments),
                 'closed': pr.is_closed(),
             })
 
         data = ({
             'draw': draw,
             'data': data,
             'recordsTotal': pull_requests_total_count,
             'recordsFiltered': pull_requests_total_count,
         })
         return data
 
     def get_recache_flag(self):
         for flag_name in ['force_recache', 'force-recache', 'no-cache']:
             flag_val = self.request.GET.get(flag_name)
             if str2bool(flag_val):
                 return True
         return False
 
     @LoginRequired()
     @HasRepoPermissionAnyDecorator(
         'repository.read', 'repository.write', 'repository.admin')
     @view_config(
         route_name='pullrequest_show_all', request_method='GET',
         renderer='rhodecode:templates/pullrequests/pullrequests.mako')
     def pull_request_list(self):
         c = self.load_default_context()
 
         req_get = self.request.GET
         c.source = str2bool(req_get.get('source'))
         c.closed = str2bool(req_get.get('closed'))
         c.my = str2bool(req_get.get('my'))
         c.awaiting_review = str2bool(req_get.get('awaiting_review'))
         c.awaiting_my_review = str2bool(req_get.get('awaiting_my_review'))
 
         c.active = 'open'
         if c.my:
             c.active = 'my'
         if c.closed:
             c.active = 'closed'
         if c.awaiting_review and not c.source:
             c.active = 'awaiting'
         if c.source and not c.awaiting_review:
             c.active = 'source'
         if c.awaiting_my_review:
             c.active = 'awaiting_my'
 
         return self._get_template_context(c)
 
     @LoginRequired()
     @HasRepoPermissionAnyDecorator(
         'repository.read', 'repository.write', 'repository.admin')
     @view_config(
         route_name='pullrequest_show_all_data', request_method='GET',
         renderer='json_ext', xhr=True)
     def pull_request_list_data(self):
         self.load_default_context()
 
         # additional filters
         req_get = self.request.GET
         source = str2bool(req_get.get('source'))
         closed = str2bool(req_get.get('closed'))
         my = str2bool(req_get.get('my'))
         awaiting_review = str2bool(req_get.get('awaiting_review'))
         awaiting_my_review = str2bool(req_get.get('awaiting_my_review'))
 
         filter_type = 'awaiting_review' if awaiting_review \
             else 'awaiting_my_review' if awaiting_my_review \
             else None
 
         opened_by = None
         if my:
             opened_by = [self._rhodecode_user.user_id]
 
         statuses = [PullRequest.STATUS_NEW, PullRequest.STATUS_OPEN]
         if closed:
             statuses = [PullRequest.STATUS_CLOSED]
 
         data = self._get_pull_requests_list(
             repo_name=self.db_repo_name, source=source,
             filter_type=filter_type, opened_by=opened_by, statuses=statuses)
 
         return data
 
     def _is_diff_cache_enabled(self, target_repo):
         caching_enabled = self._get_general_setting(
             target_repo, 'rhodecode_diff_cache')
         log.debug('Diff caching enabled: %s', caching_enabled)
         return caching_enabled
 
     def _get_diffset(self, source_repo_name, source_repo,
                      source_ref_id, target_ref_id,
                      target_commit, source_commit, diff_limit, file_limit,
                      fulldiff, hide_whitespace_changes, diff_context):
 
         vcs_diff = PullRequestModel().get_diff(
             source_repo, source_ref_id, target_ref_id,
             hide_whitespace_changes, diff_context)
 
         diff_processor = diffs.DiffProcessor(
             vcs_diff, format='newdiff', diff_limit=diff_limit,
             file_limit=file_limit, show_full_diff=fulldiff)
 
         _parsed = diff_processor.prepare()
 
         diffset = codeblocks.DiffSet(
             repo_name=self.db_repo_name,
             source_repo_name=source_repo_name,
             source_node_getter=codeblocks.diffset_node_getter(target_commit),
             target_node_getter=codeblocks.diffset_node_getter(source_commit),
         )
         diffset = self.path_filter.render_patchset_filtered(
             diffset, _parsed, target_commit.raw_id, source_commit.raw_id)
 
         return diffset
 
     def _get_range_diffset(self, source_scm, source_repo,
                            commit1, commit2, diff_limit, file_limit,
                            fulldiff, hide_whitespace_changes, diff_context):
         vcs_diff = source_scm.get_diff(
             commit1, commit2,
             ignore_whitespace=hide_whitespace_changes,
             context=diff_context)
 
         diff_processor = diffs.DiffProcessor(
             vcs_diff, format='newdiff', diff_limit=diff_limit,
             file_limit=file_limit, show_full_diff=fulldiff)
 
         _parsed = diff_processor.prepare()
 
         diffset = codeblocks.DiffSet(
             repo_name=source_repo.repo_name,
             source_node_getter=codeblocks.diffset_node_getter(commit1),
             target_node_getter=codeblocks.diffset_node_getter(commit2))
 
         diffset = self.path_filter.render_patchset_filtered(
             diffset, _parsed, commit1.raw_id, commit2.raw_id)
 
         return diffset
 
     @LoginRequired()
     @HasRepoPermissionAnyDecorator(
         'repository.read', 'repository.write', 'repository.admin')
     @view_config(
         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['pull_request_id']
+        _ = self.request.translate
+        c = self.load_default_context()
+
+        pull_request = PullRequest.get_or_404(
+            self.request.matchdict['pull_request_id'])
+        pull_request_id = pull_request.pull_request_id
 
-        c = self.load_default_context()
+        if pull_request.pull_request_state != PullRequest.STATE_CREATED:
+            log.debug('show: forbidden because pull request is in state %s',
+                      pull_request.pull_request_state)
+            msg = _(u'Cannot show pull requests in state other than `{}`. '
+                    u'Current state is: `{}`').format(PullRequest.STATE_CREATED,
+                                                      pull_request.pull_request_state)
+            h.flash(msg, category='error')
+            raise HTTPFound(h.route_path('pullrequest_show_all',
+                                         repo_name=self.db_repo_name))
 
         version = self.request.GET.get('version')
         from_version = self.request.GET.get('from_version') or version
         merge_checks = self.request.GET.get('merge_checks')
         c.fulldiff = str2bool(self.request.GET.get('fulldiff'))
 
         # fetch global flags of ignore ws or context lines
         diff_context = diffs.get_diff_context(self.request)
         hide_whitespace_changes = diffs.get_diff_whitespace_flag(self.request)
 
         force_refresh = str2bool(self.request.GET.get('force_refresh'))
 
         (pull_request_latest,
          pull_request_at_ver,
          pull_request_display_obj,
          at_version) = PullRequestModel().get_pr_version(
             pull_request_id, version=version)
         pr_closed = pull_request_latest.is_closed()
 
         if pr_closed and (version or from_version):
             # not allow to browse versions
             raise HTTPFound(h.route_path(
                 'pullrequest_show', repo_name=self.db_repo_name,
                 pull_request_id=pull_request_id))
 
         versions = pull_request_display_obj.versions()
         # used to store per-commit range diffs
         c.changes = collections.OrderedDict()
         c.range_diff_on = self.request.GET.get('range-diff') == "1"
 
         c.at_version = at_version
         c.at_version_num = (at_version
                             if at_version and at_version != 'latest'
                             else None)
         c.at_version_pos = ChangesetComment.get_index_from_version(
             c.at_version_num, versions)
 
         (prev_pull_request_latest,
          prev_pull_request_at_ver,
          prev_pull_request_display_obj,
          prev_at_version) = PullRequestModel().get_pr_version(
             pull_request_id, version=from_version)
 
         c.from_version = prev_at_version
         c.from_version_num = (prev_at_version
                               if prev_at_version and prev_at_version != 'latest'
                               else None)
         c.from_version_pos = ChangesetComment.get_index_from_version(
             c.from_version_num, versions)
 
         # define if we're in COMPARE mode or VIEW at version mode
         compare = at_version != prev_at_version
 
         # pull_requests repo_name we opened it against
         # ie. target_repo must match
         if self.db_repo_name != pull_request_at_ver.target_repo.repo_name:
             raise HTTPNotFound()
 
         c.shadow_clone_url = PullRequestModel().get_shadow_clone_url(
             pull_request_at_ver)
 
         c.pull_request = pull_request_display_obj
         c.renderer = pull_request_at_ver.description_renderer or c.renderer
         c.pull_request_latest = pull_request_latest
 
         if compare or (at_version and not at_version == 'latest'):
             c.allowed_to_change_status = False
             c.allowed_to_update = False
             c.allowed_to_merge = False
             c.allowed_to_delete = False
             c.allowed_to_comment = False
             c.allowed_to_close = False
         else:
             can_change_status = PullRequestModel().check_user_change_status(
                 pull_request_at_ver, self._rhodecode_user)
             c.allowed_to_change_status = can_change_status and not pr_closed
 
             c.allowed_to_update = PullRequestModel().check_user_update(
                 pull_request_latest, self._rhodecode_user) and not pr_closed
             c.allowed_to_merge = PullRequestModel().check_user_merge(
                 pull_request_latest, self._rhodecode_user) and not pr_closed
             c.allowed_to_delete = PullRequestModel().check_user_delete(
                 pull_request_latest, self._rhodecode_user) and not pr_closed
             c.allowed_to_comment = not pr_closed
             c.allowed_to_close = c.allowed_to_merge and not pr_closed
 
         c.forbid_adding_reviewers = False
         c.forbid_author_to_review = False
         c.forbid_commit_author_to_review = False
 
         if pull_request_latest.reviewer_data and \
                         'rules' in pull_request_latest.reviewer_data:
             rules = pull_request_latest.reviewer_data['rules'] or {}
             try:
                 c.forbid_adding_reviewers = rules.get(
                     'forbid_adding_reviewers')
                 c.forbid_author_to_review = rules.get(
                     'forbid_author_to_review')
                 c.forbid_commit_author_to_review = rules.get(
                     'forbid_commit_author_to_review')
             except Exception:
                 pass
 
         # check merge capabilities
         _merge_check = MergeCheck.validate(
             pull_request_latest, auth_user=self._rhodecode_user,
             translator=self.request.translate,
             force_shadow_repo_refresh=force_refresh)
         c.pr_merge_errors = _merge_check.error_details
         c.pr_merge_possible = not _merge_check.failed
         c.pr_merge_message = _merge_check.merge_msg
 
         c.pr_merge_info = MergeCheck.get_merge_conditions(
             pull_request_latest, translator=self.request.translate)
 
         c.pull_request_review_status = _merge_check.review_status
         if merge_checks:
             self.request.override_renderer = \
                 'rhodecode:templates/pullrequests/pullrequest_merge_checks.mako'
             return self._get_template_context(c)
 
         comments_model = CommentsModel()
 
         # reviewers and statuses
         c.pull_request_reviewers = pull_request_at_ver.reviewers_statuses()
         allowed_reviewers = [x[0].user_id for x in c.pull_request_reviewers]
 
         # GENERAL COMMENTS with versions #
         q = comments_model._all_general_comments_of_pull_request(pull_request_latest)
         q = q.order_by(ChangesetComment.comment_id.asc())
         general_comments = q
 
         # pick comments we want to render at current version
         c.comment_versions = comments_model.aggregate_comments(
             general_comments, versions, c.at_version_num)
         c.comments = c.comment_versions[c.at_version_num]['until']
 
         # INLINE COMMENTS with versions  #
         q = comments_model._all_inline_comments_of_pull_request(pull_request_latest)
         q = q.order_by(ChangesetComment.comment_id.asc())
         inline_comments = q
 
         c.inline_versions = comments_model.aggregate_comments(
             inline_comments, versions, c.at_version_num, inline=True)
 
         # inject latest version
         latest_ver = PullRequest.get_pr_display_object(
             pull_request_latest, pull_request_latest)
 
         c.versions = versions + [latest_ver]
 
         # if we use version, then do not show later comments
         # than current version
         display_inline_comments = collections.defaultdict(
             lambda: collections.defaultdict(list))
         for co in inline_comments:
             if c.at_version_num:
                 # pick comments that are at least UPTO given version, so we
                 # don't render comments for higher version
                 should_render = co.pull_request_version_id and \
                                 co.pull_request_version_id <= c.at_version_num
             else:
                 # showing all, for 'latest'
                 should_render = True
 
             if should_render:
                 display_inline_comments[co.f_path][co.line_no].append(co)
 
         # load diff data into template context, if we use compare mode then
         # diff is calculated based on changes between versions of PR
 
         source_repo = pull_request_at_ver.source_repo
         source_ref_id = pull_request_at_ver.source_ref_parts.commit_id
 
         target_repo = pull_request_at_ver.target_repo
         target_ref_id = pull_request_at_ver.target_ref_parts.commit_id
 
         if compare:
             # in compare switch the diff base to latest commit from prev version
             target_ref_id = prev_pull_request_display_obj.revisions[0]
 
         # despite opening commits for bookmarks/branches/tags, we always
         # convert this to rev to prevent changes after bookmark or branch change
         c.source_ref_type = 'rev'
         c.source_ref = source_ref_id
 
         c.target_ref_type = 'rev'
         c.target_ref = target_ref_id
 
         c.source_repo = source_repo
         c.target_repo = target_repo
 
         c.commit_ranges = []
         source_commit = EmptyCommit()
         target_commit = EmptyCommit()
         c.missing_requirements = False
 
         source_scm = source_repo.scm_instance()
         target_scm = target_repo.scm_instance()
 
         shadow_scm = None
         try:
             shadow_scm = pull_request_latest.get_shadow_repo()
         except Exception:
             log.debug('Failed to get shadow repo', exc_info=True)
         # try first the existing source_repo, and then shadow
         # repo if we can obtain one
         commits_source_repo = source_scm or shadow_scm
 
         c.commits_source_repo = commits_source_repo
         c.ancestor = None  # set it to None, to hide it from PR view
 
         # empty version means latest, so we keep this to prevent
         # double caching
         version_normalized = version or 'latest'
         from_version_normalized = from_version or 'latest'
 
         cache_path = self.rhodecode_vcs_repo.get_create_shadow_cache_pr_path(target_repo)
         cache_file_path = diff_cache_exist(
             cache_path, 'pull_request', pull_request_id, version_normalized,
             from_version_normalized, source_ref_id, target_ref_id,
             hide_whitespace_changes, diff_context, c.fulldiff)
 
         caching_enabled = self._is_diff_cache_enabled(c.target_repo)
         force_recache = self.get_recache_flag()
 
         cached_diff = None
         if caching_enabled:
             cached_diff = load_cached_diff(cache_file_path)
 
         has_proper_commit_cache = (
                 cached_diff and cached_diff.get('commits')
                 and len(cached_diff.get('commits', [])) == 5
                 and cached_diff.get('commits')[0]
                 and cached_diff.get('commits')[3])
 
         if not force_recache and not c.range_diff_on and has_proper_commit_cache:
             diff_commit_cache = \
                 (ancestor_commit, commit_cache, missing_requirements,
                  source_commit, target_commit) = cached_diff['commits']
         else:
             diff_commit_cache = \
                 (ancestor_commit, commit_cache, missing_requirements,
                  source_commit, target_commit) = self.get_commits(
                     commits_source_repo,
                     pull_request_at_ver,
                     source_commit,
                     source_ref_id,
                     source_scm,
                     target_commit,
                     target_ref_id,
                     target_scm)
 
         # register our commit range
         for comm in commit_cache.values():
             c.commit_ranges.append(comm)
 
         c.missing_requirements = missing_requirements
         c.ancestor_commit = ancestor_commit
         c.statuses = source_repo.statuses(
             [x.raw_id for x in c.commit_ranges])
 
         # auto collapse if we have more than limit
         collapse_limit = diffs.DiffProcessor._collapse_commits_over
         c.collapse_all_commits = len(c.commit_ranges) > collapse_limit
         c.compare_mode = compare
 
         # diff_limit is the old behavior, will cut off the whole diff
         # if the limit is applied  otherwise will just hide the
         # big files from the front-end
         diff_limit = c.visual.cut_off_limit_diff
         file_limit = c.visual.cut_off_limit_file
 
         c.missing_commits = False
         if (c.missing_requirements
             or isinstance(source_commit, EmptyCommit)
             or source_commit == target_commit):
 
             c.missing_commits = True
         else:
             c.inline_comments = display_inline_comments
 
             has_proper_diff_cache = cached_diff and cached_diff.get('commits')
             if not force_recache and has_proper_diff_cache:
                 c.diffset = cached_diff['diff']
                 (ancestor_commit, commit_cache, missing_requirements,
                  source_commit, target_commit) = cached_diff['commits']
             else:
                 c.diffset = self._get_diffset(
                     c.source_repo.repo_name, commits_source_repo,
                     source_ref_id, target_ref_id,
                     target_commit, source_commit,
                     diff_limit, file_limit, c.fulldiff,
                     hide_whitespace_changes, diff_context)
 
                 # save cached diff
                 if caching_enabled:
                     cache_diff(cache_file_path, c.diffset, diff_commit_cache)
 
             c.limited_diff = c.diffset.limited_diff
 
             # calculate removed files that are bound to comments
             comment_deleted_files = [
                 fname for fname in display_inline_comments
                 if fname not in c.diffset.file_stats]
 
             c.deleted_files_comments = collections.defaultdict(dict)
             for fname, per_line_comments in display_inline_comments.items():
                 if fname in comment_deleted_files:
                     c.deleted_files_comments[fname]['stats'] = 0
                     c.deleted_files_comments[fname]['comments'] = list()
                     for lno, comments in per_line_comments.items():
                         c.deleted_files_comments[fname]['comments'].extend(comments)
 
             # maybe calculate the range diff
             if c.range_diff_on:
                 # TODO(marcink): set whitespace/context
                 context_lcl = 3
                 ign_whitespace_lcl = False
 
                 for commit in c.commit_ranges:
                     commit2 = commit
                     commit1 = commit.first_parent
 
                     range_diff_cache_file_path = diff_cache_exist(
                         cache_path, 'diff', commit.raw_id,
                         ign_whitespace_lcl, context_lcl, c.fulldiff)
 
                     cached_diff = None
                     if caching_enabled:
                         cached_diff = load_cached_diff(range_diff_cache_file_path)
 
                     has_proper_diff_cache = cached_diff and cached_diff.get('diff')
                     if not force_recache and has_proper_diff_cache:
                         diffset = cached_diff['diff']
                     else:
                         diffset = self._get_range_diffset(
                             source_scm, source_repo,
                             commit1, commit2, diff_limit, file_limit,
                             c.fulldiff, ign_whitespace_lcl, context_lcl
                         )
 
                     # save cached diff
                     if caching_enabled:
                         cache_diff(range_diff_cache_file_path, diffset, None)
 
                     c.changes[commit.raw_id] = diffset
 
         # this is a hack to properly display links, when creating PR, the
         # compare view and others uses different notation, and
         # compare_commits.mako renders links based on the target_repo.
         # We need to swap that here to generate it properly on the html side
         c.target_repo = c.source_repo
 
         c.commit_statuses = ChangesetStatus.STATUSES
 
         c.show_version_changes = not pr_closed
         if c.show_version_changes:
             cur_obj = pull_request_at_ver
             prev_obj = prev_pull_request_at_ver
 
             old_commit_ids = prev_obj.revisions
             new_commit_ids = cur_obj.revisions
             commit_changes = PullRequestModel()._calculate_commit_id_changes(
                 old_commit_ids, new_commit_ids)
             c.commit_changes_summary = commit_changes
 
             # calculate the diff for commits between versions
             c.commit_changes = []
             mark = lambda cs, fw: list(
                 h.itertools.izip_longest([], cs, fillvalue=fw))
             for c_type, raw_id in mark(commit_changes.added, 'a') \
                                 + mark(commit_changes.removed, 'r') \
                                 + mark(commit_changes.common, 'c'):
 
                 if raw_id in commit_cache:
                     commit = commit_cache[raw_id]
                 else:
                     try:
                         commit = commits_source_repo.get_commit(raw_id)
                     except CommitDoesNotExistError:
                         # in case we fail extracting still use "dummy" commit
                         # for display in commit diff
                         commit = h.AttributeDict(
                             {'raw_id': raw_id,
                              'message': 'EMPTY or MISSING COMMIT'})
                 c.commit_changes.append([c_type, commit])
 
             # current user review statuses for each version
             c.review_versions = {}
             if self._rhodecode_user.user_id in allowed_reviewers:
                 for co in general_comments:
                     if co.author.user_id == self._rhodecode_user.user_id:
                         status = co.status_change
                         if status:
                             _ver_pr = status[0].comment.pull_request_version_id
                             c.review_versions[_ver_pr] = status[0]
 
         return self._get_template_context(c)
 
     def get_commits(
             self, commits_source_repo, pull_request_at_ver, source_commit,
             source_ref_id, source_scm, target_commit, target_ref_id, target_scm):
         commit_cache = collections.OrderedDict()
         missing_requirements = False
         try:
             pre_load = ["author", "branch", "date", "message", "parents"]
             show_revs = pull_request_at_ver.revisions
             for rev in show_revs:
                 comm = commits_source_repo.get_commit(
                     commit_id=rev, pre_load=pre_load)
                 commit_cache[comm.raw_id] = comm
 
             # Order here matters, we first need to get target, and then
             # the source
             target_commit = commits_source_repo.get_commit(
                 commit_id=safe_str(target_ref_id))
 
             source_commit = commits_source_repo.get_commit(
                 commit_id=safe_str(source_ref_id))
         except CommitDoesNotExistError:
             log.warning(
                 'Failed to get commit from `{}` repo'.format(
                     commits_source_repo), exc_info=True)
         except RepositoryRequirementError:
             log.warning(
                 'Failed to get all required data from repo', exc_info=True)
             missing_requirements = True
         ancestor_commit = None
         try:
             ancestor_id = source_scm.get_common_ancestor(
                 source_commit.raw_id, target_commit.raw_id, target_scm)
             ancestor_commit = source_scm.get_commit(ancestor_id)
         except Exception:
             ancestor_commit = None
         return ancestor_commit, commit_cache, missing_requirements, source_commit, target_commit
 
     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,
                 translator=self.request.translate)
         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 and c.has_origin_repo_read_perm:
             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, translator=self.request.translate)
 
         selected_source_ref = source_repo_data['refs']['selected_ref']
         title_source_ref = ''
         if selected_source_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):
         self.load_default_context()
         target_repo_name = self.request.matchdict['target_repo_name']
         repo = Repository.get_by_repo_name(target_repo_name)
         if not repo:
             raise HTTPNotFound()
 
         target_perm = HasRepoPermissionAny(
             'repository.read', 'repository.write', 'repository.admin')(
             target_repo_name)
         if not target_perm:
             raise HTTPNotFound()
 
         return PullRequestModel().generate_repo_data(
             repo, translator=self.request.translate)
 
     @LoginRequired()
     @NotAnonymous()
     @HasRepoPermissionAnyDecorator(
         'repository.read', 'repository.write', 'repository.admin')
     @view_config(
         route_name='pullrequest_repo_targets', request_method='GET',
         renderer='json_ext', xhr=True)
     def pullrequest_repo_targets(self):
         _ = self.request.translate
         filter_query = self.request.GET.get('query')
 
         # get the parents
         parent_target_repos = []
         if self.db_repo.parent:
             parents_query = Repository.query() \
                 .order_by(func.length(Repository.repo_name)) \
                 .filter(Repository.fork_id == self.db_repo.parent.repo_id)
 
             if filter_query:
                 ilike_expression = u'%{}%'.format(safe_unicode(filter_query))
                 parents_query = parents_query.filter(
                     Repository.repo_name.ilike(ilike_expression))
             parents = parents_query.limit(20).all()
 
             for parent in parents:
                 parent_vcs_obj = parent.scm_instance()
                 if parent_vcs_obj and not parent_vcs_obj.is_empty():
                     parent_target_repos.append(parent)
 
         # get other forks, and repo itself
         query = Repository.query() \
             .order_by(func.length(Repository.repo_name)) \
             .filter(
                 or_(Repository.repo_id == self.db_repo.repo_id,  # repo itself
                     Repository.fork_id == self.db_repo.repo_id)  # forks of this repo
             )  \
             .filter(~Repository.repo_id.in_([x.repo_id for x in parent_target_repos]))
 
         if filter_query:
             ilike_expression = u'%{}%'.format(safe_unicode(filter_query))
             query = query.filter(Repository.repo_name.ilike(ilike_expression))
 
         limit = max(20 - len(parent_target_repos), 5)  # not less then 5
         target_repos = query.limit(limit).all()
 
         all_target_repos = target_repos + parent_target_repos
 
         repos = []
         # This checks permissions to the repositories
         for obj in ScmModel().get_repos(all_target_repos):
             repos.append({
                 'id': obj['name'],
                 'text': obj['name'],
                 'type': 'repo',
                 'repo_id': obj['dbrepo']['repo_id'],
                 'repo_type': obj['dbrepo']['repo_type'],
                 'private': obj['dbrepo']['private'],
 
             })
 
         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()
         self.load_default_context()
 
         controls = peppercorn.parse(self.request.POST.items())
 
         try:
             form = PullRequestForm(
                 self.request.translate, self.db_repo.repo_id)()
             _form = form.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 = errors.error_dict.get('pullrequest_title')
             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'])
 
+        if not (source_db_repo or target_db_repo):
+            h.flash(_('source_repo or target repo not found'), category='error')
+            raise HTTPFound(
+                h.route_path('pullrequest_new', repo_name=self.db_repo_name))
+
         # re-check permissions again here
         # source_repo we must have read permissions
 
         source_perm = HasRepoPermissionAny(
-            'repository.read',
-            'repository.write', 'repository.admin')(source_db_repo.repo_name)
+            'repository.read', 'repository.write', 'repository.admin')(
+            source_db_repo.repo_name)
         if not source_perm:
             msg = _('Not Enough permissions to source repo `{}`.'.format(
                 source_db_repo.repo_name))
             h.flash(msg, category='error')
             # copy the args back to redirect
             org_query = self.request.GET.mixed()
             raise HTTPFound(
                 h.route_path('pullrequest_new', repo_name=self.db_repo_name,
                              _query=org_query))
 
         # target repo we must have read permissions, and also later on
         # we want to check branch permissions here
         target_perm = HasRepoPermissionAny(
-            'repository.read',
-            'repository.write', 'repository.admin')(target_db_repo.repo_name)
+            'repository.read', 'repository.write', 'repository.admin')(
+            target_db_repo.repo_name)
         if not target_perm:
             msg = _('Not Enough permissions to target repo `{}`.'.format(
                 target_db_repo.repo_name))
             h.flash(msg, category='error')
             # copy the args back to redirect
             org_query = self.request.GET.mixed()
             raise HTTPFound(
                 h.route_path('pullrequest_new', repo_name=self.db_repo_name,
                              _query=org_query))
 
         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)
 
         # recalculate target ref based on ancestor
         target_ref_type, target_ref_name, __ = _form['target_ref'].split(':')
         target_ref = ':'.join((target_ref_type, target_ref_name, ancestor))
 
         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)
 
         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']
         description_renderer = _form['description_renderer']
 
         try:
             pull_request = PullRequestModel().create(
                 created_by=self._rhodecode_user.user_id,
                 source_repo=source_repo,
                 source_ref=source_ref,
                 target_repo=target_repo,
                 target_ref=target_ref,
                 revisions=commit_ids,
                 reviewers=reviewers,
                 title=pullrequest_title,
                 description=description,
                 description_renderer=description_renderer,
                 reviewer_data=reviewer_rules,
                 auth_user=self._rhodecode_user
             )
             Session().commit()
 
             h.flash(_('Successfully opened new pull request'),
                     category='success')
         except Exception:
             msg = _('Error occurred during creation of this pull request.')
             log.exception(msg)
             h.flash(msg, category='error')
 
             # copy the args back to redirect
             org_query = self.request.GET.mixed()
             raise HTTPFound(
                 h.route_path('pullrequest_new', repo_name=self.db_repo_name,
                              _query=org_query))
 
         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 = PullRequest.get_or_404(
             self.request.matchdict['pull_request_id'])
         _ = self.request.translate
 
         self.load_default_context()
 
         if pull_request.is_closed():
             log.debug('update: forbidden because pull request is closed')
             msg = _(u'Cannot update closed pull requests.')
             h.flash(msg, category='error')
             return True
 
+        if pull_request.pull_request_state != PullRequest.STATE_CREATED:
+            log.debug('update: forbidden because pull request is in state %s',
+                      pull_request.pull_request_state)
+            msg = _(u'Cannot update pull requests in state other than `{}`. '
+                    u'Current state is: `{}`').format(PullRequest.STATE_CREATED,
+                                                      pull_request.pull_request_state)
+            h.flash(msg, category='error')
+            return True
+
         # 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, 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.request.POST.get('description_renderer'),
                 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)
+
+        with pull_request.set_state(PullRequest.STATE_UPDATING):
+            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 = _(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)
+                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 = PullRequest.get_or_404(
             self.request.matchdict['pull_request_id'])
+        _ = self.request.translate
+
+        if pull_request.pull_request_state != PullRequest.STATE_CREATED:
+            log.debug('show: forbidden because pull request is in state %s',
+                      pull_request.pull_request_state)
+            msg = _(u'Cannot merge pull requests in state other than `{}`. '
+                    u'Current state is: `{}`').format(PullRequest.STATE_CREATED,
+                                                      pull_request.pull_request_state)
+            h.flash(msg, category='error')
+            raise HTTPFound(
+                h.route_path('pullrequest_show',
+                             repo_name=pull_request.target_repo.repo_name,
+                             pull_request_id=pull_request.pull_request_id))
 
         self.load_default_context()
-        check = MergeCheck.validate(
-            pull_request, auth_user=self._rhodecode_user,
-            translator=self.request.translate)
+
+        with pull_request.set_state(PullRequest.STATE_UPDATING):
+            check = MergeCheck.validate(
+                pull_request, auth_user=self._rhodecode_user,
+                translator=self.request.translate)
         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)
+            with pull_request.set_state(PullRequest.STATE_UPDATING):
+                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_repo(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 = merge_resp.merge_status_message
             h.flash(msg, category='error')
 
     def _update_reviewers(self, pull_request, 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, 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 = PullRequest.get_or_404(
             self.request.matchdict['pull_request_id'])
         self.load_default_context()
 
         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('pullrequest_show_all',
                                          repo_name=self.db_repo_name))
 
         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 = 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()
 
         allowed_to_comment = PullRequestModel().check_user_comment(
             pull_request, self._rhodecode_user)
         if not allowed_to_comment:
             log.debug(
                 'comment: forbidden because pull request is from forbidden repo')
             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,
                 auth_user=self._rhodecode_user)
             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,
                 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,
                 auth_user=self._rhodecode_user
             )
 
             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
                     )
 
                 Session().flush()
                 # this is somehow required to get access to some relationship
                 # loaded on comment
                 Session().refresh(comment)
 
                 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):
         pull_request = PullRequest.get_or_404(
             self.request.matchdict['pull_request_id'])
 
         comment = ChangesetComment.get_or_404(
             self.request.matchdict['comment_id'])
         comment_id = comment.comment_id
 
         if pull_request.is_closed():
             log.debug('comment: forbidden because pull request is closed')
             raise HTTPForbidden()
 
         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, auth_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()