pull-requests: ensure merge response provide more details

- refactor merge responses with detailed messages
- fixes  #5520
- code cleanups

rhodecode/api/views/pull_request_api.py

@@ -313,7 +313,7 b' def merge_pull_request('
     # 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_response.asdict()
     merge_response['merge_commit_id'] = merge_response['merge_ref'].commit_id
 
     return merge_response

rhodecode/apps/repository/tests/test_repo_pullrequests.py

@@ -32,7 +32,6 b' from rhodecode.model.pull_request import'
 from rhodecode.model.user import UserModel
 from rhodecode.tests import (
     assert_session_flash, TEST_USER_ADMIN_LOGIN, TEST_USER_REGULAR_LOGIN)
-from rhodecode.tests.utils import AssertResponse
 
 
 def route_path(name, params=None, **kwargs):
@@ -233,8 +232,7 b' class TestPullrequestsView(object):'
             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})
+            params={'update_commits': 'true', 'csrf_token': csrf_token})
 
         expected_msg = str(PullRequestModel.UPDATE_STATUS_MESSAGES[
             UpdateFailureReason.MISSING_SOURCE_REF])
@@ -244,7 +242,8 b' class TestPullrequestsView(object):'
         from rhodecode.lib.vcs.backends.base import MergeFailureReason
         pull_request = pr_util.create_pull_request(
             approved=True, mergeable=True)
-        pull_request.target_ref = 'branch:invalid-branch:invalid-commit-id'
+        unicode_reference = u'branch:invalid-branch:invalid-commit-id'
+        pull_request.target_ref = unicode_reference
         Session().add(pull_request)
         Session().commit()
 
@@ -255,12 +254,12 b' class TestPullrequestsView(object):'
             pull_request_id=pull_request_id)
 
         response = self.app.get(pull_request_url)
-
-        assertr = AssertResponse(response)
-        expected_msg = PullRequestModel.MERGE_STATUS_MESSAGES[
-            MergeFailureReason.MISSING_TARGET_REF]
-        assertr.element_contains(
-            'span[data-role="merge-message"]', str(expected_msg))
+        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):
@@ -272,8 +271,8 b' class TestPullrequestsView(object):'
 
         self.app.post(
             route_path('pullrequest_comment_create',
-                repo_name=pull_request.target_repo.scm_instance().name,
-                pull_request_id=pull_request_id),
+                       repo_name=pull_request.target_repo.scm_instance().name,
+                       pull_request_id=pull_request_id),
             params={
                 'close_pull_request': '1',
                 'text': 'Closing a PR',
@@ -608,8 +607,7 b' class TestPullrequestsView(object):'
 
         response = self.app.post(
             route_path('pullrequest_merge',
-                repo_name=repo_name,
-                pull_request_id=pull_request_id),
+                       repo_name=repo_name, pull_request_id=pull_request_id),
             params={'csrf_token': csrf_token}).follow()
 
         assert response.status_int == 200
@@ -624,10 +622,13 b' class TestPullrequestsView(object):'
         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=MergeResponse(
-                True, False, 'STUB_COMMIT_ID', MergeFailureReason.PUSH_FAILED)),
+            merge_repo=mock.Mock(return_value=merge_resp),
             merge_status=mock.Mock(return_value=(True, 'WRONG_MESSAGE')))
 
         with model_patcher:
@@ -637,8 +638,10 b' class TestPullrequestsView(object):'
                            pull_request_id=pull_request_id),
                 params={'csrf_token': csrf_token}, status=302)
 
-        assert_session_flash(response, PullRequestModel.MERGE_STATUS_MESSAGES[
-            MergeFailureReason.PUSH_FAILED])
+            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 = [
@@ -909,11 +912,11 b' class TestPullrequestsView(object):'
             pull_request_id=pull_request.pull_request_id))
 
         assert response.status_int == 200
-        assert_response = AssertResponse(response)
-        assert_response.element_contains(
+
+        response.assert_response().element_contains(
             '#changeset_compare_view_content .alert strong',
             'Missing commits')
-        assert_response.element_contains(
+        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.')
@@ -941,15 +944,15 b' class TestPullrequestsView(object):'
             pull_request_id=pull_request.pull_request_id))
 
         assert response.status_int == 200
-        assert_response = AssertResponse(response)
-        assert_response.element_contains(
+
+        response.assert_response().element_contains(
             '#changeset_compare_view_content .alert strong',
             'Missing commits')
-        assert_response.element_contains(
+        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.')
-        assert_response.element_contains(
+        response.assert_response().element_contains(
             '#update_commits',
             'Update commits')
 
@@ -987,8 +990,7 b' class TestPullrequestsView(object):'
             pull_request_id=pull_request.pull_request_id))
 
         assert response.status_int == 200
-        assert_response = AssertResponse(response)
-        assert_response.element_contains(
+        response.assert_response().element_contains(
             '#changeset_compare_view_content .alert strong',
             'Missing commits')
 
@@ -1004,12 +1006,11 b' class TestPullrequestsView(object):'
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id))
         assert response.status_int == 200
-        assert_response = AssertResponse(response)
 
-        origin = assert_response.get_element('.pr-origininfo .tag')
+        origin = response.assert_response().get_element('.pr-origininfo .tag')
         origin_children = origin.getchildren()
         assert len(origin_children) == 1
-        target = assert_response.get_element('.pr-targetinfo .tag')
+        target = response.assert_response().get_element('.pr-targetinfo .tag')
         target_children = target.getchildren()
         assert len(target_children) == 1
 
@@ -1038,13 +1039,12 b' class TestPullrequestsView(object):'
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id))
         assert response.status_int == 200
-        assert_response = AssertResponse(response)
 
-        origin = assert_response.get_element('.pr-origininfo .tag')
+        origin = response.assert_response().get_element('.pr-origininfo .tag')
         assert origin.text.strip() == 'bookmark: origin'
         assert origin.getchildren() == []
 
-        target = assert_response.get_element('.pr-targetinfo .tag')
+        target = response.assert_response().get_element('.pr-targetinfo .tag')
         assert target.text.strip() == 'bookmark: target'
         assert target.getchildren() == []
 
@@ -1060,13 +1060,12 b' class TestPullrequestsView(object):'
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=pull_request.pull_request_id))
         assert response.status_int == 200
-        assert_response = AssertResponse(response)
 
-        origin = assert_response.get_element('.pr-origininfo .tag')
+        origin = response.assert_response().get_element('.pr-origininfo .tag')
         assert origin.text.strip() == 'tag: origin'
         assert origin.getchildren() == []
 
-        target = assert_response.get_element('.pr-targetinfo .tag')
+        target = response.assert_response().get_element('.pr-targetinfo .tag')
         assert target.text.strip() == 'tag: target'
         assert target.getchildren() == []
 
@@ -1090,12 +1089,13 b' class TestPullrequestsView(object):'
             repo_name=target_repo.name,
             pull_request_id=pr_id))
 
-        assertr = AssertResponse(response)
         if mergeable:
-            assertr.element_value_contains('input.pr-mergeinfo', shadow_url)
-            assertr.element_value_contains('input.pr-mergeinfo ', 'pr-merge')
+            response.assert_response().element_value_contains(
+                'input.pr-mergeinfo', shadow_url)
+            response.assert_response().element_value_contains(
+                'input.pr-mergeinfo ', 'pr-merge')
         else:
-            assertr.no_element_exists('.pr-mergeinfo')
+            response.assert_response().no_element_exists('.pr-mergeinfo')
 
 
 @pytest.mark.usefixtures('app')

rhodecode/apps/repository/views/repo_pull_requests.py

@@ -1181,10 +1181,8 b' class RepoPullRequestsView(RepoAppView, '
             h.flash(msg, category='success')
         else:
             log.debug(
-                "The merge was not successful. Merge response: %s",
-                merge_resp)
-            msg = PullRequestModel().merge_status_message(
-                merge_resp.failure_reason)
+                "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):

rhodecode/lib/vcs/backends/base.py

@@ -21,20 +21,20 b''
 """
 Base module for all VCS systems
 """
-
-import collections
+import os
+import re
+import time
+import shutil
 import datetime
 import fnmatch
 import itertools
 import logging
-import os
-import re
-import time
+import collections
 import warnings
-import shutil
 
 from zope.cachedescriptors.property import Lazy as LazyProperty
 
+from rhodecode.translation import lazy_ugettext
 from rhodecode.lib.utils2 import safe_str, safe_unicode
 from rhodecode.lib.vcs import connection
 from rhodecode.lib.vcs.utils import author_name, author_email
@@ -54,9 +54,6 b' FILEMODE_DEFAULT = 0o100644'
 FILEMODE_EXECUTABLE = 0o100755
 
 Reference = collections.namedtuple('Reference', ('type', 'name', 'commit_id'))
-MergeResponse = collections.namedtuple(
-    'MergeResponse',
-    ('possible', 'executed', 'merge_ref', 'failure_reason'))
 
 
 class MergeFailureReason(object):
@@ -142,6 +139,92 b' class UpdateFailureReason(object):'
     MISSING_SOURCE_REF = 5
 
 
+class MergeResponse(object):
+
+    # uses .format(**metadata) for variables
+    MERGE_STATUS_MESSAGES = {
+        MergeFailureReason.NONE: lazy_ugettext(
+            u'This pull request can be automatically merged.'),
+        MergeFailureReason.UNKNOWN: lazy_ugettext(
+            u'This pull request cannot be merged because of an unhandled exception. '
+            u'{exception}'),
+        MergeFailureReason.MERGE_FAILED: lazy_ugettext(
+            u'This pull request cannot be merged because of merge conflicts.'),
+        MergeFailureReason.PUSH_FAILED: lazy_ugettext(
+            u'This pull request could not be merged because push to '
+            u'target:`{target}@{merge_commit}` failed.'),
+        MergeFailureReason.TARGET_IS_NOT_HEAD: lazy_ugettext(
+            u'This pull request cannot be merged because the target '
+            u'`{target_ref.name}` is not a head.'),
+        MergeFailureReason.HG_SOURCE_HAS_MORE_BRANCHES: lazy_ugettext(
+            u'This pull request cannot be merged because the source contains '
+            u'more branches than the target.'),
+        MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS: lazy_ugettext(
+            u'This pull request cannot be merged because the target '
+            u'has multiple heads: `{heads}`.'),
+        MergeFailureReason.TARGET_IS_LOCKED: lazy_ugettext(
+            u'This pull request cannot be merged because the target repository is '
+            u'locked by {locked_by}.'),
+
+        MergeFailureReason.MISSING_TARGET_REF: lazy_ugettext(
+            u'This pull request cannot be merged because the target '
+            u'reference `{target_ref.name}` is missing.'),
+        MergeFailureReason.MISSING_SOURCE_REF: lazy_ugettext(
+            u'This pull request cannot be merged because the source '
+            u'reference `{source_ref.name}` is missing.'),
+        MergeFailureReason.SUBREPO_MERGE_FAILED: lazy_ugettext(
+            u'This pull request cannot be merged because of conflicts related '
+            u'to sub repositories.'),
+
+        # Deprecations
+        MergeFailureReason._DEPRECATED_MISSING_COMMIT: lazy_ugettext(
+            u'This pull request cannot be merged because the target or the '
+            u'source reference is missing.'),
+
+    }
+
+    def __init__(self, possible, executed, merge_ref, failure_reason, metadata=None):
+        self.possible = possible
+        self.executed = executed
+        self.merge_ref = merge_ref
+        self.failure_reason = failure_reason
+        self.metadata = metadata or {}
+
+    def __repr__(self):
+        return '<MergeResponse:{} {}>'.format(self.label, self.failure_reason)
+
+    def __eq__(self, other):
+        same_instance = isinstance(other, self.__class__)
+        return same_instance \
+               and self.possible == other.possible \
+               and self.executed == other.executed \
+               and self.failure_reason == other.failure_reason
+
+    @property
+    def label(self):
+        label_dict = dict((v, k) for k, v in MergeFailureReason.__dict__.items() if
+                          not k.startswith('_'))
+        return label_dict.get(self.failure_reason)
+
+    @property
+    def merge_status_message(self):
+        """
+        Return a human friendly error message for the given merge status code.
+        """
+        msg = safe_unicode(self.MERGE_STATUS_MESSAGES[self.failure_reason])
+        try:
+            return msg.format(**self.metadata)
+        except Exception:
+            log.exception('Failed to format %s message', self)
+            return msg
+
+    def asdict(self):
+        data = {}
+        for k in ['possible', 'executed', 'merge_ref', 'failure_reason']:
+            data[k] = getattr(self, k)
+        return data
+
+
 class BaseRepository(object):
     """
     Base Repository for final backends
@@ -501,12 +584,11 b' class BaseRepository(object):'
                 repo_id, workspace_id, target_ref, source_repo,
                 source_ref, message, user_name, user_email, dry_run=dry_run,
                 use_rebase=use_rebase, close_branch=close_branch)
-        except RepositoryError:
-            log.exception(
-                'Unexpected failure when running merge, dry-run=%s',
-                dry_run)
+        except RepositoryError as exc:
+            log.exception('Unexpected failure when running merge, dry-run=%s', dry_run)
             return MergeResponse(
-                False, False, None, MergeFailureReason.UNKNOWN)
+                False, False, None, MergeFailureReason.UNKNOWN,
+                metadata={'exception': str(exc)})
 
     def _merge_repo(self, repo_id, workspace_id, target_ref,
                     source_repo, source_ref, merge_message,

rhodecode/lib/vcs/backends/git/repository.py

@@ -911,11 +911,15 b' class GitRepository(BaseRepository):'
                     source_repo, source_ref, merge_message,
                     merger_name, merger_email, dry_run=False,
                     use_rebase=False, close_branch=False):
+
+        log.debug('Executing merge_repo with %s strategy, dry_run mode:%s',
+                  'rebase' if use_rebase else 'merge', dry_run)
         if target_ref.commit_id != self.branches[target_ref.name]:
             log.warning('Target ref %s commit mismatch %s vs %s', target_ref,
                         target_ref.commit_id, self.branches[target_ref.name])
             return MergeResponse(
-                False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD)
+                False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD,
+                metadata={'target_ref': target_ref})
 
         shadow_repository_path = self._maybe_prepare_merge_workspace(
             repo_id, workspace_id, target_ref, source_ref)
@@ -943,7 +947,8 b' class GitRepository(BaseRepository):'
                         target_ref, target_ref.commit_id,
                         shadow_repo.branches[target_ref.name])
             return MergeResponse(
-                False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD)
+                False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD,
+                metadata={'target_ref': target_ref})
 
         # calculate new branch
         pr_branch = shadow_repo._get_new_pr_branch(
@@ -954,12 +959,15 b' class GitRepository(BaseRepository):'
         try:
             shadow_repo._local_fetch(source_repo.path, source_ref.name)
         except RepositoryError:
-            log.exception('Failure when doing local fetch on git shadow repo')
+            log.exception('Failure when doing local fetch on '
+                          'shadow repo: %s', shadow_repo)
             return MergeResponse(
-                False, False, None, MergeFailureReason.MISSING_SOURCE_REF)
+                False, False, None, MergeFailureReason.MISSING_SOURCE_REF,
+                metadata={'source_ref': source_ref})
 
         merge_ref = None
         merge_failure_reason = MergeFailureReason.NONE
+        metadata = {}
         try:
             shadow_repo._local_merge(merge_message, merger_name, merger_email,
                                      [source_ref.commit_id])
@@ -988,12 +996,15 b' class GitRepository(BaseRepository):'
                 merge_succeeded = True
             except RepositoryError:
                 log.exception(
-                    'Failure when doing local push on git shadow repo')
+                    'Failure when doing local push from the shadow '
+                    'repository to the target repository at %s.', self.path)
                 merge_succeeded = False
                 merge_failure_reason = MergeFailureReason.PUSH_FAILED
+                metadata['target'] = 'git shadow repo'
+                metadata['merge_commit'] = pr_branch
         else:
             merge_succeeded = False
 
         return MergeResponse(
-            merge_possible, merge_succeeded, merge_ref,
-            merge_failure_reason)
+            merge_possible, merge_succeeded, merge_ref, merge_failure_reason,
+            metadata=metadata)

rhodecode/lib/vcs/backends/hg/repository.py

@@ -610,7 +610,7 b' class MercurialRepository(BaseRepository'
         Returns the commit id of the merge and a boolean indicating if the
         commit needs to be pushed.
         """
-        self._update(target_ref.commit_id)
+        self._update(target_ref.commit_id, clean=True)
 
         ancestor = self._ancestor(target_ref.commit_id, source_ref.commit_id)
         is_the_same_branch = self._is_the_same_branch(target_ref, source_ref)
@@ -631,7 +631,7 b' class MercurialRepository(BaseRepository'
                 self._remote.rebase(
                     source=source_ref.commit_id, dest=target_ref.commit_id)
                 self._remote.invalidate_vcs_cache()
-                self._update(bookmark_name)
+                self._update(bookmark_name, clean=True)
                 return self._identify(), True
             except RepositoryError:
                 # The rebase-abort may raise another exception which 'hides'
@@ -710,18 +710,21 b' class MercurialRepository(BaseRepository'
                   'rebase' if use_rebase else 'merge', dry_run)
         if target_ref.commit_id not in self._heads():
             return MergeResponse(
-                False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD)
+                False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD,
+                metadata={'target_ref': target_ref})
 
         try:
-            if (target_ref.type == 'branch' and
-                    len(self._heads(target_ref.name)) != 1):
+            if target_ref.type == 'branch' and len(self._heads(target_ref.name)) != 1:
+                heads = ','.join(self._heads(target_ref.name))
                 return MergeResponse(
                     False, False, None,
-                    MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS)
+                    MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS,
+                    metadata={'heads': heads})
         except CommitDoesNotExistError:
             log.exception('Failure when looking up branch heads on hg target')
             return MergeResponse(
-                False, False, None, MergeFailureReason.MISSING_TARGET_REF)
+                False, False, None, MergeFailureReason.MISSING_TARGET_REF,
+                metadata={'target_ref': target_ref})
 
         shadow_repository_path = self._maybe_prepare_merge_workspace(
             repo_id, workspace_id, target_ref, source_ref)
@@ -730,6 +733,7 b' class MercurialRepository(BaseRepository'
         log.debug('Pulling in target reference %s', target_ref)
         self._validate_pull_reference(target_ref)
         shadow_repo._local_pull(self.path, target_ref)
+
         try:
             log.debug('Pulling in source reference %s', source_ref)
             source_repo._validate_pull_reference(source_ref)
@@ -737,12 +741,14 b' class MercurialRepository(BaseRepository'
         except CommitDoesNotExistError:
             log.exception('Failure when doing local pull on hg shadow repo')
             return MergeResponse(
-                False, False, None, MergeFailureReason.MISSING_SOURCE_REF)
+                False, False, None, MergeFailureReason.MISSING_SOURCE_REF,
+                metadata={'source_ref': source_ref})
 
         merge_ref = None
         merge_commit_id = None
         close_commit_id = None
         merge_failure_reason = MergeFailureReason.NONE
+        metadata = {}
 
         # enforce that close branch should be used only in case we source from
         # an actual Branch
@@ -758,8 +764,8 b' class MercurialRepository(BaseRepository'
                     target_ref, merger_name, merger_email, source_ref)
                 merge_possible = True
             except RepositoryError:
-                log.exception(
-                    'Failure when doing close branch on hg shadow repo')
+                log.exception('Failure when doing close branch on '
+                              'shadow repo: %s', shadow_repo)
                 merge_possible = False
                 merge_failure_reason = MergeFailureReason.MERGE_FAILED
         else:
@@ -824,19 +830,21 b' class MercurialRepository(BaseRepository'
                 except RepositoryError:
                     log.exception(
                         'Failure when doing local push from the shadow '
-                        'repository to the target repository.')
+                        'repository to the target repository at %s.', self.path)
                     merge_succeeded = False
                     merge_failure_reason = MergeFailureReason.PUSH_FAILED
+                    metadata['target'] = 'hg shadow repo'
+                    metadata['merge_commit'] = merge_commit_id
             else:
                 merge_succeeded = True
         else:
             merge_succeeded = False
 
         return MergeResponse(
-            merge_possible, merge_succeeded, merge_ref, merge_failure_reason)
+            merge_possible, merge_succeeded, merge_ref, merge_failure_reason,
+            metadata=metadata)
 
-    def _get_shadow_instance(
-            self, shadow_repository_path, enable_hooks=False):
+    def _get_shadow_instance(self, shadow_repository_path, enable_hooks=False):
         config = self.config.copy()
         if not enable_hooks:
             config.clear_section('hooks')

rhodecode/model/pull_request.py

@@ -33,7 +33,7 b' import collections'
 from pyramid.threadlocal import get_current_request
 
 from rhodecode import events
-from rhodecode.translation import lazy_ugettext#, _
+from rhodecode.translation import lazy_ugettext
 from rhodecode.lib import helpers as h, hooks_utils, diffs
 from rhodecode.lib import audit_logger
 from rhodecode.lib.compat import OrderedDict
@@ -75,43 +75,6 b' class PullRequestModel(BaseModel):'
 
     DIFF_CONTEXT = diffs.DEFAULT_CONTEXT
 
-    MERGE_STATUS_MESSAGES = {
-        MergeFailureReason.NONE: lazy_ugettext(
-            'This pull request can be automatically merged.'),
-        MergeFailureReason.UNKNOWN: lazy_ugettext(
-            'This pull request cannot be merged because of an unhandled'
-            ' exception.'),
-        MergeFailureReason.MERGE_FAILED: lazy_ugettext(
-            'This pull request cannot be merged because of merge conflicts.'),
-        MergeFailureReason.PUSH_FAILED: lazy_ugettext(
-            'This pull request could not be merged because push to target'
-            ' failed.'),
-        MergeFailureReason.TARGET_IS_NOT_HEAD: lazy_ugettext(
-            'This pull request cannot be merged because the target is not a'
-            ' head.'),
-        MergeFailureReason.HG_SOURCE_HAS_MORE_BRANCHES: lazy_ugettext(
-            'This pull request cannot be merged because the source contains'
-            ' more branches than the target.'),
-        MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS: lazy_ugettext(
-            'This pull request cannot be merged because the target has'
-            ' multiple heads.'),
-        MergeFailureReason.TARGET_IS_LOCKED: lazy_ugettext(
-            'This pull request cannot be merged because the target repository'
-            ' is locked.'),
-        MergeFailureReason._DEPRECATED_MISSING_COMMIT: lazy_ugettext(
-            'This pull request cannot be merged because the target or the '
-            'source reference is missing.'),
-        MergeFailureReason.MISSING_TARGET_REF: lazy_ugettext(
-            'This pull request cannot be merged because the target '
-            'reference is missing.'),
-        MergeFailureReason.MISSING_SOURCE_REF: lazy_ugettext(
-            'This pull request cannot be merged because the source '
-            'reference is missing.'),
-        MergeFailureReason.SUBREPO_MERGE_FAILED: lazy_ugettext(
-            'This pull request cannot be merged because of conflicts related '
-            'to sub repositories.'),
-    }
-
     UPDATE_STATUS_MESSAGES = {
         UpdateFailureReason.NONE: lazy_ugettext(
             'Pull request update successful.'),
@@ -593,8 +556,7 b' class PullRequestModel(BaseModel):'
         extras['user_agent'] = 'internal-merge'
         merge_state = self._merge_pull_request(pull_request, user, extras)
         if merge_state.executed:
-            log.debug(
-                "Merge was successful, updating the pull request comments.")
+            log.debug("Merge was successful, updating the pull request comments.")
             self._comment_and_close_pr(pull_request, user, merge_state)
 
             self._log_audit_action(
@@ -1254,8 +1216,7 b' class PullRequestModel(BaseModel):'
                 pull_request,
                 force_shadow_repo_refresh=force_shadow_repo_refresh)
             log.debug("Merge response: %s", resp)
-            status = resp.possible, self.merge_status_message(
-                resp.failure_reason)
+            status = resp.possible, resp.merge_status_message
         except NotImplementedError:
             status = False, _('Pull request merging is not supported.')
 
@@ -1297,21 +1258,23 b' class PullRequestModel(BaseModel):'
             "Trying out if the pull request %s can be merged. Force_refresh=%s",
             pull_request.pull_request_id, force_shadow_repo_refresh)
         target_vcs = pull_request.target_repo.scm_instance()
-
         # Refresh the target reference.
         try:
             target_ref = self._refresh_reference(
                 pull_request.target_ref_parts, target_vcs)
         except CommitDoesNotExistError:
             merge_state = MergeResponse(
-                False, False, None, MergeFailureReason.MISSING_TARGET_REF)
+                False, False, None, MergeFailureReason.MISSING_TARGET_REF,
+                metadata={'target_ref': pull_request.target_ref_parts})
             return merge_state
 
         target_locked = pull_request.target_repo.locked
         if target_locked and target_locked[0]:
-            log.debug("The target repository is locked.")
+            locked_by = 'user:{}'.format(target_locked[0])
+            log.debug("The target repository is locked by %s.", locked_by)
             merge_state = MergeResponse(
-                False, False, None, MergeFailureReason.TARGET_IS_LOCKED)
+                False, False, None, MergeFailureReason.TARGET_IS_LOCKED,
+                metadata={'locked_by': locked_by})
         elif force_shadow_repo_refresh or self._needs_merge_state_refresh(
                 pull_request, target_ref):
             log.debug("Refreshing the merge status of the repository.")
@@ -1369,12 +1332,6 b' class PullRequestModel(BaseModel):'
         workspace_id = 'pr-%s' % pull_request.pull_request_id
         return workspace_id
 
-    def merge_status_message(self, status_code):
-        """
-        Return a human friendly error message for the given merge status code.
-        """
-        return self.MERGE_STATUS_MESSAGES[status_code]
-
     def generate_repo_data(self, repo, commit_id=None, branch=None,
                            bookmark=None, translator=None):
         from rhodecode.model.repo import RepoModel

rhodecode/tests/models/test_pullrequest.py

@@ -50,9 +50,11 b' class TestPullRequestModel(object):'
         A pull request combined with multiples patches.
         """
         BackendClass = get_backend(backend.alias)
+        merge_resp = MergeResponse(
+            False, False, None, MergeFailureReason.UNKNOWN,
+            metadata={'exception': 'MockError'})
         self.merge_patcher = mock.patch.object(
-            BackendClass, 'merge', return_value=MergeResponse(
-            False, False, None, MergeFailureReason.UNKNOWN))
+            BackendClass, 'merge', return_value=merge_resp)
         self.workspace_remove_patcher = mock.patch.object(
             BackendClass, 'cleanup_merge_workspace')
 
@@ -162,7 +164,7 b' class TestPullRequestModel(object):'
 
         status, msg = PullRequestModel().merge_status(pull_request)
         assert status is True
-        assert msg.eval() == 'This pull request can be automatically merged.'
+        assert msg == 'This pull request can be automatically merged.'
         self.merge_mock.assert_called_with(
             self.repo_id, self.workspace_id,
             pull_request.target_ref_parts,
@@ -177,7 +179,7 b' class TestPullRequestModel(object):'
         self.merge_mock.reset_mock()
         status, msg = PullRequestModel().merge_status(pull_request)
         assert status is True
-        assert msg.eval() == 'This pull request can be automatically merged.'
+        assert msg == 'This pull request can be automatically merged.'
         assert self.merge_mock.called is False
 
     def test_merge_status_known_failure(self, pull_request):
@@ -190,9 +192,7 b' class TestPullRequestModel(object):'
 
         status, msg = PullRequestModel().merge_status(pull_request)
         assert status is False
-        assert (
-            msg.eval() ==
-            'This pull request cannot be merged because of merge conflicts.')
+        assert msg == 'This pull request cannot be merged because of merge conflicts.'
         self.merge_mock.assert_called_with(
             self.repo_id, self.workspace_id,
             pull_request.target_ref_parts,
@@ -208,14 +208,13 b' class TestPullRequestModel(object):'
         self.merge_mock.reset_mock()
         status, msg = PullRequestModel().merge_status(pull_request)
         assert status is False
-        assert (
-            msg.eval() ==
-            'This pull request cannot be merged because of merge conflicts.')
+        assert msg == 'This pull request cannot be merged because of merge conflicts.'
         assert self.merge_mock.called is False
 
     def test_merge_status_unknown_failure(self, pull_request):
         self.merge_mock.return_value = MergeResponse(
-            False, False, None, MergeFailureReason.UNKNOWN)
+            False, False, None, MergeFailureReason.UNKNOWN,
+            metadata={'exception': 'MockError'})
 
         assert pull_request._last_merge_source_rev is None
         assert pull_request._last_merge_target_rev is None
@@ -223,9 +222,9 b' class TestPullRequestModel(object):'
 
         status, msg = PullRequestModel().merge_status(pull_request)
         assert status is False
-        assert msg.eval() == (
-            'This pull request cannot be merged because of an unhandled'
-            ' exception.')
+        assert msg == (
+            'This pull request cannot be merged because of an unhandled exception. '
+            'MockError')
         self.merge_mock.assert_called_with(
             self.repo_id, self.workspace_id,
             pull_request.target_ref_parts,
@@ -240,18 +239,18 b' class TestPullRequestModel(object):'
         self.merge_mock.reset_mock()
         status, msg = PullRequestModel().merge_status(pull_request)
         assert status is False
-        assert msg.eval() == (
-            'This pull request cannot be merged because of an unhandled'
-            ' exception.')
+        assert msg == (
+            'This pull request cannot be merged because of an unhandled exception. '
+            'MockError')
         assert self.merge_mock.called is True
 
     def test_merge_status_when_target_is_locked(self, pull_request):
         pull_request.target_repo.locked = [1, u'12345.50', 'lock_web']
         status, msg = PullRequestModel().merge_status(pull_request)
         assert status is False
-        assert msg.eval() == (
-            'This pull request cannot be merged because the target repository'
-            ' is locked.')
+        assert msg == (
+            'This pull request cannot be merged because the target repository '
+            'is locked by user:1.')
 
     def test_merge_status_requirements_check_target(self, pull_request):
 
@@ -460,6 +459,46 b' def test_outdated_comments('
     outdated_comment_mock.assert_called_with(pull_request)
 
 
[e-mail]('mr_type, expected_msg', [
+    (MergeFailureReason.NONE,
+     'This pull request can be automatically merged.'),
+    (MergeFailureReason.UNKNOWN,
+     'This pull request cannot be merged because of an unhandled exception. CRASH'),
+    (MergeFailureReason.MERGE_FAILED,
+     'This pull request cannot be merged because of merge conflicts.'),
+    (MergeFailureReason.PUSH_FAILED,
+     'This pull request could not be merged because push to target:`some-repo@merge_commit` failed.'),
+    (MergeFailureReason.TARGET_IS_NOT_HEAD,
+     'This pull request cannot be merged because the target `ref_name` is not a head.'),
+    (MergeFailureReason.HG_SOURCE_HAS_MORE_BRANCHES,
+     'This pull request cannot be merged because the source contains more branches than the target.'),
+    (MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS,
+     'This pull request cannot be merged because the target has multiple heads: `a,b,c`.'),
+    (MergeFailureReason.TARGET_IS_LOCKED,
+     'This pull request cannot be merged because the target repository is locked by user:123.'),
+    (MergeFailureReason.MISSING_TARGET_REF,
+     'This pull request cannot be merged because the target reference `ref_name` is missing.'),
+    (MergeFailureReason.MISSING_SOURCE_REF,
+     'This pull request cannot be merged because the source reference `ref_name` is missing.'),
+    (MergeFailureReason.SUBREPO_MERGE_FAILED,
+     'This pull request cannot be merged because of conflicts related to sub repositories.'),
+
+])
+def test_merge_response_message(mr_type, expected_msg):
+    merge_ref = Reference('type', 'ref_name', '6126b7bfcc82ad2d3deaee22af926b082ce54cc6')
+    metadata = {
+        'exception': "CRASH",
+        'target': 'some-repo',
+        'merge_commit': 'merge_commit',
+        'target_ref': merge_ref,
+        'source_ref': merge_ref,
+        'heads': ','.join(['a', 'b', 'c']),
+        'locked_by': 'user:123'}
+
+    merge_response = MergeResponse(True, True, merge_ref, mr_type, metadata=metadata)
+    assert merge_response.merge_status_message == expected_msg
+
+
 @pytest.fixture
 def merge_extras(user_regular):
     """

rhodecode/tests/plugin.py

@@ -661,8 +661,7 b' class Backend(object):'
 
     def _next_repo_name(self):
         return u"%s_%s" % (
-            self.invalid_repo_name.sub(u'_', self._test_name),
-            len(self._cleanup_repos))
+            self.invalid_repo_name.sub(u'_', self._test_name), len(self._cleanup_repos))
 
     def ensure_file(self, filename, content='Test content\n'):
         assert self._cleanup_repos, "Avoid writing into vcs_test repos"

rhodecode/tests/vcs/test_hg.py

@@ -681,9 +681,11 b' TODO: To be written...'
         workspace_id = 'test-merge'
 
         assert len(target_repo._heads(branch='default')) == 2
+        heads = target_repo._heads(branch='default')
         expected_merge_response = MergeResponse(
             False, False, None,
-            MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS)
+            MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS,
+            metadata={'heads': heads})
         repo_id = repo_id_generator(target_repo.path)
         merge_response = target_repo.merge(
             repo_id, workspace_id, target_ref, source_repo, source_ref,

rhodecode/tests/vcs/test_repository.py

@@ -284,11 +284,9 b' class TestRepositoryMerge(object):'
         self.source_commit = self.source_repo.get_commit()
         # This only works for Git and Mercurial
         default_branch = self.target_repo.DEFAULT_BRANCH_NAME
-        self.target_ref = Reference(
-            'branch', default_branch, self.target_commit.raw_id)
-        self.source_ref = Reference(
-            'branch', default_branch, self.source_commit.raw_id)
-        self.workspace_id = 'test-merge'
+        self.target_ref = Reference('branch', default_branch, self.target_commit.raw_id)
+        self.source_ref = Reference('branch', default_branch, self.source_commit.raw_id)
+        self.workspace_id = 'test-merge-{}'.format(vcsbackend.alias)
         self.repo_id = repo_id_generator(self.target_repo.path)
 
     def prepare_for_conflict(self, vcsbackend):
@@ -300,11 +298,9 b' class TestRepositoryMerge(object):'
         self.source_commit = self.source_repo.get_commit()
         # This only works for Git and Mercurial
         default_branch = self.target_repo.DEFAULT_BRANCH_NAME
-        self.target_ref = Reference(
-            'branch', default_branch, self.target_commit.raw_id)
-        self.source_ref = Reference(
-            'branch', default_branch, self.source_commit.raw_id)
-        self.workspace_id = 'test-merge'
+        self.target_ref = Reference('branch', default_branch, self.target_commit.raw_id)
+        self.source_ref = Reference('branch', default_branch, self.source_commit.raw_id)
+        self.workspace_id = 'test-merge-{}'.format(vcsbackend.alias)
         self.repo_id = repo_id_generator(self.target_repo.path)
 
     def test_merge_success(self, vcsbackend):
@@ -367,10 +363,11 b' class TestRepositoryMerge(object):'
 
         # Multiple merges may differ in their commit id. Therefore we set the
         # commit id to `None` before comparing the merge responses.
-        merge_response = merge_response._replace(
-            merge_ref=merge_response.merge_ref._replace(commit_id=None))
-        merge_response_update = merge_response_update._replace(
-            merge_ref=merge_response_update.merge_ref._replace(commit_id=None))
+        new_merge_ref = merge_response.merge_ref._replace(commit_id=None)
+        merge_response.merge_ref = new_merge_ref
+
+        new_update_merge_ref = merge_response_update.merge_ref._replace(commit_id=None)
+        merge_response_update.merge_ref = new_update_merge_ref
 
         assert merge_response == merge_response_update
         assert merge_response.possible is True
@@ -381,6 +378,7 b' class TestRepositoryMerge(object):'
     @pytest.mark.parametrize('dry_run', [True, False])
     def test_merge_conflict(self, vcsbackend, dry_run):
         self.prepare_for_conflict(vcsbackend)
+
         expected_merge_response = MergeResponse(
             False, False, None, MergeFailureReason.MERGE_FAILED)
 
@@ -399,12 +397,11 b' class TestRepositoryMerge(object):'
 
     def test_merge_target_is_not_head(self, vcsbackend):
         self.prepare_for_success(vcsbackend)
-        expected_merge_response = MergeResponse(
-            False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD)
-
         target_ref = Reference(
             self.target_ref.type, self.target_ref.name, '0' * 40)
-
+        expected_merge_response = MergeResponse(
+            False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD,
+            metadata={'target_ref': target_ref})
         merge_response = self.target_repo.merge(
             self.repo_id, self.workspace_id, target_ref, self.source_repo,
             self.source_ref, dry_run=True)
@@ -413,11 +410,12 b' class TestRepositoryMerge(object):'
 
     def test_merge_missing_source_reference(self, vcsbackend):
         self.prepare_for_success(vcsbackend)
-        expected_merge_response = MergeResponse(
-            False, False, None, MergeFailureReason.MISSING_SOURCE_REF)
 
         source_ref = Reference(
             self.source_ref.type, 'not_existing', self.source_ref.commit_id)
+        expected_merge_response = MergeResponse(
+            False, False, None, MergeFailureReason.MISSING_SOURCE_REF,
+            metadata={'source_ref': source_ref})
 
         merge_response = self.target_repo.merge(
             self.repo_id, self.workspace_id, self.target_ref,
@@ -429,7 +427,8 b' class TestRepositoryMerge(object):'
     def test_merge_raises_exception(self, vcsbackend):
         self.prepare_for_success(vcsbackend)
         expected_merge_response = MergeResponse(
-            False, False, None, MergeFailureReason.UNKNOWN)
+            False, False, None, MergeFailureReason.UNKNOWN,
+            metadata={'exception': 'ErrorForTest'})
 
         with mock.patch.object(self.target_repo, '_merge_repo',
                                side_effect=RepositoryError()):