hooks: deprecate old action_logger

rhodecode/lib/hooks_base.py

@@ -1,421 +1,413 b''
 # -*- coding: utf-8 -*-
 
 # Copyright (C) 2013-2017 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/
 
 
 """
 Set of hooks run by RhodeCode Enterprise
 """
 
 import os
 import collections
 import logging
 
 import rhodecode
 from rhodecode import events
 from rhodecode.lib import helpers as h
 from rhodecode.lib import audit_logger
-from rhodecode.lib.utils import action_logger
 from rhodecode.lib.utils2 import safe_str
 from rhodecode.lib.exceptions import HTTPLockedRC, UserCreationError
 from rhodecode.model.db import Repository, User
 
 log = logging.getLogger(__name__)
 
 
 HookResponse = collections.namedtuple('HookResponse', ('status', 'output'))
 
 
 def is_shadow_repo(extras):
     """
     Returns ``True`` if this is an action executed against a shadow repository.
     """
     return extras['is_shadow_repo']
 
 
 def _get_scm_size(alias, root_path):
 
     if not alias.startswith('.'):
         alias += '.'
 
     size_scm, size_root = 0, 0
     for path, unused_dirs, files in os.walk(safe_str(root_path)):
         if path.find(alias) != -1:
             for f in files:
                 try:
                     size_scm += os.path.getsize(os.path.join(path, f))
                 except OSError:
                     pass
         else:
             for f in files:
                 try:
                     size_root += os.path.getsize(os.path.join(path, f))
                 except OSError:
                     pass
 
     size_scm_f = h.format_byte_size_binary(size_scm)
     size_root_f = h.format_byte_size_binary(size_root)
     size_total_f = h.format_byte_size_binary(size_root + size_scm)
 
     return size_scm_f, size_root_f, size_total_f
 
 
 # actual hooks called by Mercurial internally, and GIT by our Python Hooks
 def repo_size(extras):
     """Present size of repository after push."""
     repo = Repository.get_by_repo_name(extras.repository)
     vcs_part = safe_str(u'.%s' % repo.repo_type)
     size_vcs, size_root, size_total = _get_scm_size(vcs_part,
                                                     repo.repo_full_path)
     msg = ('Repository `%s` size summary %s:%s repo:%s total:%s\n'
            % (repo.repo_name, vcs_part, size_vcs, size_root, size_total))
     return HookResponse(0, msg)
 
 
 def pre_push(extras):
     """
     Hook executed before pushing code.
 
     It bans pushing when the repository is locked.
     """
 
     usr = User.get_by_username(extras.username)
     output = ''
     if extras.locked_by[0] and usr.user_id != int(extras.locked_by[0]):
         locked_by = User.get(extras.locked_by[0]).username
         reason = extras.locked_by[2]
         # this exception is interpreted in git/hg middlewares and based
         # on that proper return code is server to client
         _http_ret = HTTPLockedRC(
             _locked_by_explanation(extras.repository, locked_by, reason))
         if str(_http_ret.code).startswith('2'):
             # 2xx Codes don't raise exceptions
             output = _http_ret.title
         else:
             raise _http_ret
 
     # Propagate to external components. This is done after checking the
     # lock, for consistent behavior.
     if not is_shadow_repo(extras):
         pre_push_extension(repo_store_path=Repository.base_path(), **extras)
         events.trigger(events.RepoPrePushEvent(
             repo_name=extras.repository, extras=extras))
 
     return HookResponse(0, output)
 
 
 def pre_pull(extras):
     """
     Hook executed before pulling the code.
 
     It bans pulling when the repository is locked.
     """
 
     output = ''
     if extras.locked_by[0]:
         locked_by = User.get(extras.locked_by[0]).username
         reason = extras.locked_by[2]
         # this exception is interpreted in git/hg middlewares and based
         # on that proper return code is server to client
         _http_ret = HTTPLockedRC(
             _locked_by_explanation(extras.repository, locked_by, reason))
         if str(_http_ret.code).startswith('2'):
             # 2xx Codes don't raise exceptions
             output = _http_ret.title
         else:
             raise _http_ret
 
     # Propagate to external components. This is done after checking the
     # lock, for consistent behavior.
     if not is_shadow_repo(extras):
         pre_pull_extension(**extras)
         events.trigger(events.RepoPrePullEvent(
             repo_name=extras.repository, extras=extras))
 
     return HookResponse(0, output)
 
 
 def post_pull(extras):
     """Hook executed after client pulls the code."""
-    user = User.get_by_username(extras.username)
-    action = 'pull'
-    action_logger(user, action, extras.repository, extras.ip, commit=True)
 
     audit_user = audit_logger.UserWrap(
         username=extras.username,
         ip_addr=extras.ip)
     repo = audit_logger.RepoWrap(repo_name=extras.repository)
     audit_logger.store(
         action='user.pull', action_data={
             'user_agent': extras.user_agent},
         user=audit_user, repo=repo, commit=True)
 
     # Propagate to external components.
     if not is_shadow_repo(extras):
         post_pull_extension(**extras)
         events.trigger(events.RepoPullEvent(
             repo_name=extras.repository, extras=extras))
 
     output = ''
     # make lock is a tri state False, True, None. We only make lock on True
     if extras.make_lock is True and not is_shadow_repo(extras):
+        user = User.get_by_username(extras.username)
         Repository.lock(Repository.get_by_repo_name(extras.repository),
                         user.user_id,
                         lock_reason=Repository.LOCK_PULL)
         msg = 'Made lock on repo `%s`' % (extras.repository,)
         output += msg
 
     if extras.locked_by[0]:
         locked_by = User.get(extras.locked_by[0]).username
         reason = extras.locked_by[2]
         _http_ret = HTTPLockedRC(
             _locked_by_explanation(extras.repository, locked_by, reason))
         if str(_http_ret.code).startswith('2'):
             # 2xx Codes don't raise exceptions
             output += _http_ret.title
 
     return HookResponse(0, output)
 
 
 def post_push(extras):
     """Hook executed after user pushes to the repository."""
-    action_tmpl = extras.action + ':%s'
-    commit_ids = extras.commit_ids[:29000]
+    commit_ids = extras.commit_ids
 
-    action = action_tmpl % ','.join(commit_ids)
-    action_logger(
-        extras.username, action, extras.repository, extras.ip, commit=True)
-
+    # log the push call
     audit_user = audit_logger.UserWrap(
-        username=extras.username,
-        ip_addr=extras.ip)
+        username=extras.username, ip_addr=extras.ip)
     repo = audit_logger.RepoWrap(repo_name=extras.repository)
     audit_logger.store(
         action='user.push', action_data={
             'user_agent': extras.user_agent,
             'commit_ids': commit_ids[:10000]},
         user=audit_user, repo=repo, commit=True)
 
     # Propagate to external components.
     if not is_shadow_repo(extras):
         post_push_extension(
             repo_store_path=Repository.base_path(),
             pushed_revs=commit_ids,
             **extras)
         events.trigger(events.RepoPushEvent(
             repo_name=extras.repository,
             pushed_commit_ids=commit_ids,
             extras=extras))
 
     output = ''
     # make lock is a tri state False, True, None. We only release lock on False
     if extras.make_lock is False and not is_shadow_repo(extras):
         Repository.unlock(Repository.get_by_repo_name(extras.repository))
         msg = 'Released lock on repo `%s`\n' % extras.repository
         output += msg
 
     if extras.locked_by[0]:
         locked_by = User.get(extras.locked_by[0]).username
         reason = extras.locked_by[2]
         _http_ret = HTTPLockedRC(
             _locked_by_explanation(extras.repository, locked_by, reason))
         # TODO: johbo: if not?
         if str(_http_ret.code).startswith('2'):
             # 2xx Codes don't raise exceptions
             output += _http_ret.title
 
     output += 'RhodeCode: push completed\n'
 
     return HookResponse(0, output)
 
 
 def _locked_by_explanation(repo_name, user_name, reason):
     message = (
         'Repository `%s` locked by user `%s`. Reason:`%s`'
         % (repo_name, user_name, reason))
     return message
 
 
 def check_allowed_create_user(user_dict, created_by, **kwargs):
     # pre create hooks
     if pre_create_user.is_active():
         allowed, reason = pre_create_user(created_by=created_by, **user_dict)
         if not allowed:
             raise UserCreationError(reason)
 
 
 class ExtensionCallback(object):
     """
     Forwards a given call to rcextensions, sanitizes keyword arguments.
 
     Does check if there is an extension active for that hook. If it is
     there, it will forward all `kwargs_keys` keyword arguments to the
     extension callback.
     """
 
     def __init__(self, hook_name, kwargs_keys):
         self._hook_name = hook_name
         self._kwargs_keys = set(kwargs_keys)
 
     def __call__(self, *args, **kwargs):
         log.debug('Calling extension callback for %s', self._hook_name)
 
         kwargs_to_pass = dict((key, kwargs[key]) for key in self._kwargs_keys)
         # backward compat for removed api_key for old hooks. THis was it works
         # with older rcextensions that require api_key present
         if self._hook_name in ['CREATE_USER_HOOK', 'DELETE_USER_HOOK']:
             kwargs_to_pass['api_key'] = '_DEPRECATED_'
 
         callback = self._get_callback()
         if callback:
             return callback(**kwargs_to_pass)
         else:
             log.debug('extensions callback not found skipping...')
 
     def is_active(self):
         return hasattr(rhodecode.EXTENSIONS, self._hook_name)
 
     def _get_callback(self):
         return getattr(rhodecode.EXTENSIONS, self._hook_name, None)
 
 
 pre_pull_extension = ExtensionCallback(
     hook_name='PRE_PULL_HOOK',
     kwargs_keys=(
         'server_url', 'config', 'scm', 'username', 'ip', 'action',
         'repository'))
 
 
 post_pull_extension = ExtensionCallback(
     hook_name='PULL_HOOK',
     kwargs_keys=(
         'server_url', 'config', 'scm', 'username', 'ip', 'action',
         'repository'))
 
 
 pre_push_extension = ExtensionCallback(
     hook_name='PRE_PUSH_HOOK',
     kwargs_keys=(
         'server_url', 'config', 'scm', 'username', 'ip', 'action',
         'repository', 'repo_store_path', 'commit_ids'))
 
 
 post_push_extension = ExtensionCallback(
     hook_name='PUSH_HOOK',
     kwargs_keys=(
         'server_url', 'config', 'scm', 'username', 'ip', 'action',
         'repository', 'repo_store_path', 'pushed_revs'))
 
 
 pre_create_user = ExtensionCallback(
     hook_name='PRE_CREATE_USER_HOOK',
     kwargs_keys=(
         'username', 'password', 'email', 'firstname', 'lastname', 'active',
         'admin', 'created_by'))
 
 
 log_create_pull_request = ExtensionCallback(
     hook_name='CREATE_PULL_REQUEST',
     kwargs_keys=(
         'server_url', 'config', 'scm', 'username', 'ip', 'action',
         'repository', 'pull_request_id', 'url', 'title', 'description',
         'status', 'created_on', 'updated_on', 'commit_ids', 'review_status',
         'mergeable', 'source', 'target', 'author', 'reviewers'))
 
 
 log_merge_pull_request = ExtensionCallback(
     hook_name='MERGE_PULL_REQUEST',
     kwargs_keys=(
         'server_url', 'config', 'scm', 'username', 'ip', 'action',
         'repository', 'pull_request_id', 'url', 'title', 'description',
         'status', 'created_on', 'updated_on', 'commit_ids', 'review_status',
         'mergeable', 'source', 'target', 'author', 'reviewers'))
 
 
 log_close_pull_request = ExtensionCallback(
     hook_name='CLOSE_PULL_REQUEST',
     kwargs_keys=(
         'server_url', 'config', 'scm', 'username', 'ip', 'action',
         'repository', 'pull_request_id', 'url', 'title', 'description',
         'status', 'created_on', 'updated_on', 'commit_ids', 'review_status',
         'mergeable', 'source', 'target', 'author', 'reviewers'))
 
 
 log_review_pull_request = ExtensionCallback(
     hook_name='REVIEW_PULL_REQUEST',
     kwargs_keys=(
         'server_url', 'config', 'scm', 'username', 'ip', 'action',
         'repository', 'pull_request_id', 'url', 'title', 'description',
         'status', 'created_on', 'updated_on', 'commit_ids', 'review_status',
         'mergeable', 'source', 'target', 'author', 'reviewers'))
 
 
 log_update_pull_request = ExtensionCallback(
     hook_name='UPDATE_PULL_REQUEST',
     kwargs_keys=(
         'server_url', 'config', 'scm', 'username', 'ip', 'action',
         'repository', 'pull_request_id', 'url', 'title', 'description',
         'status', 'created_on', 'updated_on', 'commit_ids', 'review_status',
         'mergeable', 'source', 'target', 'author', 'reviewers'))
 
 
 log_create_user = ExtensionCallback(
     hook_name='CREATE_USER_HOOK',
     kwargs_keys=(
         'username', 'full_name_or_username', 'full_contact', 'user_id',
         'name', 'firstname', 'short_contact', 'admin', 'lastname',
         'ip_addresses', 'extern_type', 'extern_name',
         'email', 'api_keys', 'last_login',
         'full_name', 'active', 'password', 'emails',
         'inherit_default_permissions', 'created_by', 'created_on'))
 
 
 log_delete_user = ExtensionCallback(
     hook_name='DELETE_USER_HOOK',
     kwargs_keys=(
         'username', 'full_name_or_username', 'full_contact', 'user_id',
         'name', 'firstname', 'short_contact', 'admin', 'lastname',
         'ip_addresses',
         'email', 'last_login',
         'full_name', 'active', 'password', 'emails',
         'inherit_default_permissions', 'deleted_by'))
 
 
 log_create_repository = ExtensionCallback(
     hook_name='CREATE_REPO_HOOK',
     kwargs_keys=(
         'repo_name', 'repo_type', 'description', 'private', 'created_on',
         'enable_downloads', 'repo_id', 'user_id', 'enable_statistics',
         'clone_uri', 'fork_id', 'group_id', 'created_by'))
 
 
 log_delete_repository = ExtensionCallback(
     hook_name='DELETE_REPO_HOOK',
     kwargs_keys=(
         'repo_name', 'repo_type', 'description', 'private', 'created_on',
         'enable_downloads', 'repo_id', 'user_id', 'enable_statistics',
         'clone_uri', 'fork_id', 'group_id', 'deleted_by', 'deleted_on'))
 
 
 log_create_repository_group = ExtensionCallback(
     hook_name='CREATE_REPO_GROUP_HOOK',
     kwargs_keys=(
         'group_name', 'group_parent_id', 'group_description',
         'group_id', 'user_id', 'created_by', 'created_on',
         'enable_locking'))

rhodecode/tests/functional/test_pullrequests.py

@@ -1,1097 +1,1103 b''
 # -*- coding: utf-8 -*-
 
 # Copyright (C) 2010-2017 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
 from webob.exc import HTTPNotFound
 
 import rhodecode
 from rhodecode.lib.vcs.nodes import FileNode
 from rhodecode.model.changeset_status import ChangesetStatusModel
 from rhodecode.model.db import (
     PullRequest, ChangesetStatus, UserLog, Notification)
 from rhodecode.model.meta import Session
 from rhodecode.model.pull_request import PullRequestModel
 from rhodecode.model.user import UserModel
 from rhodecode.model.repo import RepoModel
 from rhodecode.tests import (
     assert_session_flash, url, TEST_USER_ADMIN_LOGIN, TEST_USER_REGULAR_LOGIN)
 from rhodecode.tests.utils import AssertResponse
 
 
 @pytest.mark.usefixtures('app', 'autologin_user')
 @pytest.mark.backends("git", "hg")
 class TestPullrequestsController:
 
     def test_index(self, backend):
         self.app.get(url(
             controller='pullrequests', action='index',
             repo_name=backend.repo_name))
 
     def test_option_menu_create_pull_request_exists(self, backend):
         repo_name = backend.repo_name
         response = self.app.get(url('summary_home', repo_name=repo_name))
 
         create_pr_link = '<a href="%s">Create Pull Request</a>' % url(
             'pullrequest', repo_name=repo_name)
         response.mustcontain(create_pr_link)
 
     def test_global_redirect_of_pr(self, backend, pr_util):
         pull_request = pr_util.create_pull_request()
 
         response = self.app.get(
             url('pull_requests_global',
                 pull_request_id=pull_request.pull_request_id))
 
         repo_name = pull_request.target_repo.repo_name
         redirect_url = url('pullrequest_show', repo_name=repo_name,
                            pull_request_id=pull_request.pull_request_id)
         assert response.status == '302 Found'
         assert redirect_url in response.location
 
     def test_create_pr_form_with_raw_commit_id(self, backend):
         repo = backend.repo
 
         self.app.get(
             url(controller='pullrequests', action='index',
                 repo_name=repo.repo_name,
                 commit=repo.get_commit().raw_id),
             status=200)
 
     @pytest.mark.parametrize('pr_merge_enabled', [True, False])
     def test_show(self, pr_util, pr_merge_enabled):
         pull_request = pr_util.create_pull_request(
             mergeable=pr_merge_enabled, enable_notifications=False)
 
         response = self.app.get(url(
             controller='pullrequests', action='show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=str(pull_request.pull_request_id)))
 
         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
         assert (
             'Server-side pull request merging is disabled.'
             in response) != pr_merge_enabled
 
     def test_close_status_visibility(self, pr_util, user_util, csrf_token):
         from rhodecode.tests.functional.test_login import login_url, logut_url
         # Logout
         response = self.app.post(
             logut_url,
             params={'csrf_token': csrf_token})
         # Login as regular user
         response = self.app.post(login_url,
                                  {'username': TEST_USER_REGULAR_LOGIN,
                                   'password': 'test12'})
 
         pull_request = pr_util.create_pull_request(
             author=TEST_USER_REGULAR_LOGIN)
 
         response = self.app.get(url(
             controller='pullrequests', action='show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=str(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(url(
             controller='pullrequests', action='show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=str(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(url(
             controller='pullrequests', action='show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=str(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(url(
             controller='pullrequests', action='show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=str(pull_request.pull_request_id)))
 
     def test_edit_title_description(self, pr_util, csrf_token):
         pull_request = pr_util.create_pull_request()
         pull_request_id = pull_request.pull_request_id
 
         response = self.app.post(
             url(controller='pullrequests', action='update',
                 repo_name=pull_request.target_repo.repo_name,
                 pull_request_id=str(pull_request_id)),
             params={
                 'edit_pull_request': 'true',
                 '_method': 'put',
                 '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
         pr_util.close()
 
         response = self.app.post(
             url(controller='pullrequests', action='update',
                 repo_name=pull_request.target_repo.repo_name,
                 pull_request_id=str(pull_request_id)),
             params={
                 'edit_pull_request': 'true',
                 '_method': 'put',
                 'title': 'New title',
                 'description': 'New description',
                 'csrf_token': csrf_token})
 
         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(
             url(controller='pullrequests', action='update',
                 repo_name=pull_request.target_repo.repo_name,
                 pull_request_id=str(pull_request_id)),
             params={'update_commits': 'true', '_method': 'put',
                     'csrf_token': csrf_token})
 
         expected_msg = 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)
         pull_request.target_ref = 'branch:invalid-branch:invalid-commit-id'
         Session().add(pull_request)
         Session().commit()
 
         pull_request_id = pull_request.pull_request_id
         pull_request_url = url(
             controller='pullrequests', action='show',
             repo_name=pull_request.target_repo.repo_name,
             pull_request_id=str(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))
 
     def test_comment_and_close_pull_request(self, pr_util, csrf_token):
         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(
             url(controller='pullrequests',
                 action='comment',
                 repo_name=pull_request.target_repo.scm_instance().name,
                 pull_request_id=str(pull_request_id)),
             params={
                 'changeset_status': ChangesetStatus.STATUS_APPROVED,
                 'close_pull_request': '1',
                 'text': 'Closing a PR',
                 'csrf_token': csrf_token},
             status=302)
 
         action = 'user_closed_pull_request:%d' % pull_request_id
         journal = UserLog.query()\
             .filter(UserLog.user_id == author)\
             .filter(UserLog.repository_id == repo)\
             .filter(UserLog.action == action)\
             .all()
         assert len(journal) == 1
 
         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_APPROVED
 
     def test_reject_and_close_pull_request(self, pr_util, csrf_token):
         pull_request = pr_util.create_pull_request()
         pull_request_id = pull_request.pull_request_id
         response = self.app.post(
             url(controller='pullrequests',
                 action='update',
                 repo_name=pull_request.target_repo.scm_instance().name,
                 pull_request_id=str(pull_request.pull_request_id)),
             params={'close_pull_request': 'true', '_method': 'put',
                     'csrf_token': csrf_token})
 
         pull_request = PullRequest.get(pull_request_id)
 
         assert response.json is True
         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_comment_force_close_pull_request(self, pr_util, csrf_token):
         pull_request = pr_util.create_pull_request()
         pull_request_id = pull_request.pull_request_id
         reviewers_data = [(1, ['reason']), (2, ['reason2'])]
         PullRequestModel().update_reviewers(pull_request_id, reviewers_data)
         author = pull_request.user_id
         repo = pull_request.target_repo.repo_id
         self.app.post(
             url(controller='pullrequests',
                 action='comment',
                 repo_name=pull_request.target_repo.scm_instance().name,
                 pull_request_id=str(pull_request_id)),
             params={
                 'changeset_status': 'rejected',
                 'close_pull_request': '1',
                 'csrf_token': csrf_token},
             status=302)
 
         pull_request = PullRequest.get(pull_request_id)
 
         action = 'user_closed_pull_request:%d' % pull_request_id
         journal = UserLog.query().filter(
             UserLog.user_id == author,
             UserLog.repository_id == repo,
             UserLog.action == action).all()
         assert len(journal) == 1
 
         # 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(
             url(
                 controller='pullrequests',
                 action='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']),
                 ('pullrequest_desc', 'Description'),
                 ('pullrequest_title', 'Title'),
                 ('__start__', 'review_members:sequence'),
                     ('__start__', 'reviewer:mapping'),
                         ('user_id', '1'),
                         ('__start__', 'reasons:sequence'),
                             ('reason', 'Some reason'),
                         ('__end__', 'reasons:sequence'),
                     ('__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 = int(location.rsplit('/', 1)[1])
         pull_request = PullRequest.get(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(
             url(
                 controller='pullrequests',
                 action='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']),
                 ('pullrequest_desc', 'Description'),
                 ('pullrequest_title', 'Title'),
                 ('__start__', 'review_members:sequence'),
                     ('__start__', 'reviewer:mapping'),
                         ('user_id', '2'),
                         ('__start__', 'reasons:sequence'),
                             ('reason', 'Some reason'),
                         ('__end__', 'reasons:sequence'),
                     ('__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 = int(location.rsplit('/', 1)[1])
         pull_request = PullRequest.get(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 #%d"
                                                   % 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, [])])
         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(
             url(
                 controller='pullrequests',
                 action='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']),
                 ('pullrequest_desc', 'Description'),
                 ('pullrequest_title', 'Title'),
                 ('__start__', 'review_members:sequence'),
                     ('__start__', 'reviewer:mapping'),
                         ('user_id', '1'),
                         ('__start__', 'reasons:sequence'),
                             ('reason', 'Some reason'),
                         ('__end__', 'reasons:sequence'),
                     ('__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 = int(location.rsplit('/', 1)[1])
         pull_request = PullRequest.get(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(
             url(controller='pullrequests',
                 action='merge',
                 repo_name=str(repo_name[0]),
                 pull_request_id=str(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() \
             .filter(UserLog.version == UserLog.VERSION_1) \
-            .order_by('-user_log_id').limit(4)
+            .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'user_closed_pull_request:%d' % pull_request_id,
             u'user_merged_pull_request:%d' % pull_request_id,
             # The action below reflect that the post push actions were executed
             u'user_commented_pull_request:%d' % pull_request_id,
-            u'push:%s' % ','.join(pr_commit_ids),
         ]
         assert actions == expected_actions
 
+        user_logs = UserLog.query() \
+            .filter(UserLog.version == UserLog.VERSION_2) \
+            .order_by('-user_log_id').limit(1)
+        actions = [log.action for log in user_logs]
+        assert actions == ['user.push']
+        assert user_logs[0].action_data['commit_ids'] == pr_commit_ids
+
         # Check post_push rcextension was really executed
         push_calls = rhodecode.EXTENSIONS.calls['post_push']
         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['pushed_revs'] == 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(
             url(controller='pullrequests',
                 action='merge',
                 repo_name=pull_request.target_repo.scm_instance().name,
                 pull_request_id=str(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(
             url(controller='pullrequests',
                 action='merge',
                 repo_name=str(repo_name[0]),
                 pull_request_id=str(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_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'])
         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)
         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(
             url(controller='pullrequests', action='update',
                 repo_name=target.repo_name,
                 pull_request_id=str(pull_request_id)),
             params={'update_commits': 'true', '_method': 'put',
                     'csrf_token': csrf_token})
 
         # 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(url(
             controller='pullrequests', action='index',
             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
 
     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'])
         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)
         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(
             url(controller='pullrequests', action='update',
                 repo_name=target.repo_name,
                 pull_request_id=str(pull_request_id)),
             params={'update_commits': 'true', '_method': 'put',
                     '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'])
 
         # TODO: johbo: This should be a test on its own
         response = self.app.get(url(
             controller='pullrequests', action='index',
             repo_name=target.repo_name))
         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_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'])
         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'])
         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)
         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(
             url(controller='pullrequests', action='update',
                 repo_name=target.repo_name,
                 pull_request_id=str(pull_request_id)),
             params={'update_commits': 'true', '_method': 'put',
                     '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'])
         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)
         Session().add(pull_request)
         Session().commit()
 
         vcs = repo.scm_instance()
         vcs.remove_ref('refs/heads/{}'.format(branch_name))
 
         response = self.app.get(url(
             controller='pullrequests', action='show',
             repo_name=repo.repo_name,
             pull_request_id=str(pull_request.pull_request_id)))
 
         assert response.status_int == 200
         assert_response = AssertResponse(response)
         assert_response.element_contains(
             '#changeset_compare_view_content .alert strong',
             'Missing commits')
         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(url(
             controller='pullrequests', action='show',
             repo_name=pr_util.target_repository.repo_name,
             pull_request_id=str(pull_request.pull_request_id)))
 
         assert response.status_int == 200
         assert_response = AssertResponse(response)
         assert_response.element_contains(
             '#changeset_compare_view_content .alert strong',
             'Missing commits')
         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(
             '#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(
             url(controller='pullrequests', action='update',
                 repo_name=pull_request.target_repo.repo_name,
                 pull_request_id=str(pull_request.pull_request_id)),
             params={'update_commits': 'true', '_method': 'put',
                     'csrf_token': csrf_token})
 
         assert response.status_int == 200
         assert response.body == 'true'
 
         # Make sure that after update, it won't raise 500 errors
         response = self.app.get(url(
             controller='pullrequests', action='show',
             repo_name=pr_util.target_repository.repo_name,
             pull_request_id=str(pull_request.pull_request_id)))
 
         assert response.status_int == 200
         assert_response = AssertResponse(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(url(
             controller='pullrequests', action='show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=str(pull_request.pull_request_id)))
         assert response.status_int == 200
         assert_response = AssertResponse(response)
 
         origin = 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_children = target.getchildren()
         assert len(target_children) == 1
 
         expected_origin_link = url(
             'changelog_home',
             repo_name=pull_request.source_repo.scm_instance().name,
             branch='origin')
         expected_target_link = url(
             'changelog_home',
             repo_name=pull_request.target_repo.scm_instance().name,
             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(url(
             controller='pullrequests', action='show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=str(pull_request.pull_request_id)))
         assert response.status_int == 200
         assert_response = AssertResponse(response)
 
         origin = assert_response.get_element('.pr-origininfo .tag')
         assert origin.text.strip() == 'bookmark: origin'
         assert origin.getchildren() == []
 
         target = 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(url(
             controller='pullrequests', action='show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=str(pull_request.pull_request_id)))
         assert response.status_int == 200
         assert_response = AssertResponse(response)
 
         origin = assert_response.get_element('.pr-origininfo .tag')
         assert origin.text.strip() == 'tag: origin'
         assert origin.getchildren() == []
 
         target = assert_response.get_element('.pr-targetinfo .tag')
         assert target.text.strip() == 'tag: target'
         assert target.getchildren() == []
 
     def test_description_is_escaped_on_index_page(self, backend, pr_util):
         xss_description = "<script>alert('Hi!')</script>"
         pull_request = pr_util.create_pull_request(description=xss_description)
         response = self.app.get(url(
             controller='pullrequests', action='show_all',
             repo_name=pull_request.target_repo.repo_name))
         response.mustcontain(
             "&lt;script&gt;alert(&#39;Hi!&#39;)&lt;/script&gt;")
 
     @pytest.mark.parametrize('mergeable', [True, False])
     def test_shadow_repository_link(
             self, mergeable, pr_util, http_host_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_stub, repo=target_repo.name, pr_id=pr_id)
 
         response = self.app.get(url(
             controller='pullrequests', action='show',
             repo_name=target_repo.name,
             pull_request_id=str(pr_id)))
 
         assertr = AssertResponse(response)
         if mergeable:
             assertr.element_value_contains(
                 'div.pr-mergeinfo input', shadow_url)
             assertr.element_value_contains(
                 'div.pr-mergeinfo input', 'pr-merge')
         else:
             assertr.no_element_exists('div.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(url(
             controller='pullrequests', action='show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=str(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(url(
             controller='pullrequests', action='show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=str(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(url(
             controller='pullrequests', action='show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=str(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(url(
             controller='pullrequests', action='show',
             repo_name=pull_request.target_repo.scm_instance().name,
             pull_request_id=str(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 assert_pull_request_status(pull_request, expected_status):
     status = ChangesetStatusModel().calculated_review_status(
         pull_request=pull_request)
     assert status == expected_status
 
 
 @pytest.mark.parametrize('action', ['show_all', 'index', 'create'])
 @pytest.mark.usefixtures("autologin_user")
 def test_redirects_to_repo_summary_for_svn_repositories(
         backend_svn, app, action):
     denied_actions = ['show_all', 'index', 'create']
     for action in denied_actions:
         response = app.get(url(
             controller='pullrequests', action=action,
             repo_name=backend_svn.repo_name))
         assert response.status_int == 302
 
         # Not allowed, redirect to the summary
         redirected = response.follow()
         summary_url = url('summary_home', repo_name=backend_svn.repo_name)
 
         # URL adds leading slash and path doesn't have it
         assert redirected.req.path == summary_url
 
 
 def test_delete_comment_returns_404_if_comment_does_not_exist(pylonsapp):
     # TODO: johbo: Global import not possible because models.forms blows up
     from rhodecode.controllers.pullrequests import PullrequestsController
     controller = PullrequestsController()
     patcher = mock.patch(
         'rhodecode.model.db.BaseModel.get', return_value=None)
     with pytest.raises(HTTPNotFound), patcher:
         controller._delete_comment(1)

rhodecode/tests/lib/test_hooks_base.py

@@ -1,144 +1,141 b''
 # -*- coding: utf-8 -*-
 
 # Copyright (C) 2010-2017 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
-
+from rhodecode.model.db import Session, UserLog
 from rhodecode.lib import hooks_base, utils2
 
 
[e-mail](
-    hooks_base,
-    action_logger=mock.Mock(),
-    post_push_extension=mock.Mock(),
-    Repository=mock.Mock())
 def test_post_push_truncates_commits(user_regular, repo_stub):
     extras = {
         'ip': '127.0.0.1',
         'username': user_regular.username,
         'action': 'push_local',
         'repository': repo_stub.repo_name,
         'scm': 'git',
         'config': '',
         'server_url': 'http://example.com',
         'make_lock': None,
         'user_agent': 'some-client',
         'locked_by': [None],
         'commit_ids': ['abcde12345' * 4] * 30000,
         'is_shadow_repo': False,
     }
     extras = utils2.AttributeDict(extras)
 
     hooks_base.post_push(extras)
 
     # Calculate appropriate action string here
-    expected_action = 'push_local:%s' % ','.join(extras.commit_ids[:29000])
+    commit_ids = extras.commit_ids[:10000]
 
-    hooks_base.action_logger.assert_called_with(
-        extras.username, expected_action, extras.repository, extras.ip,
-        commit=True)
+    entry = UserLog.query().order_by('-user_log_id').first()
+    assert entry.action == 'user.push'
+    assert entry.action_data['commit_ids'] == commit_ids
+    Session().delete(entry)
+    Session().commit()
 
 
 def assert_called_with_mock(callable_, expected_mock_name):
     mock_obj = callable_.call_args[0][0]
     mock_name = mock_obj._mock_new_parent._mock_new_name
     assert mock_name == expected_mock_name
 
 
 @pytest.fixture
 def hook_extras(user_regular, repo_stub):
     extras = utils2.AttributeDict({
         'ip': '127.0.0.1',
         'username': user_regular.username,
         'action': 'push',
         'repository': repo_stub.repo_name,
         'scm': '',
         'config': '',
         'server_url': 'http://example.com',
         'make_lock': None,
         'user_agent': 'some-client',
         'locked_by': [None],
         'commit_ids': [],
         'is_shadow_repo': False,
     })
     return extras
 
 
 @pytest.mark.parametrize('func, extension, event', [
     (hooks_base.pre_push, 'pre_push_extension', 'RepoPrePushEvent'),
     (hooks_base.post_push, 'post_pull_extension', 'RepoPushEvent'),
     (hooks_base.pre_pull, 'pre_pull_extension', 'RepoPrePullEvent'),
     (hooks_base.post_pull, 'post_push_extension', 'RepoPullEvent'),
 ])
 def test_hooks_propagate(func, extension, event, hook_extras):
     """
     Tests that our hook code propagates to rhodecode extensions and triggers
     the appropriate event.
     """
     extension_mock = mock.Mock()
     events_mock = mock.Mock()
     patches = {
         'Repository': mock.Mock(),
         'events': events_mock,
         extension: extension_mock,
     }
 
     # Clear shadow repo flag.
     hook_extras.is_shadow_repo = False
 
     # Execute hook function.
     with mock.patch.multiple(hooks_base, **patches):
         func(hook_extras)
 
     # Assert that extensions are called and event was fired.
     extension_mock.called_once()
     assert_called_with_mock(events_mock.trigger, event)
 
 
 @pytest.mark.parametrize('func, extension, event', [
     (hooks_base.pre_push, 'pre_push_extension', 'RepoPrePushEvent'),
     (hooks_base.post_push, 'post_pull_extension', 'RepoPushEvent'),
     (hooks_base.pre_pull, 'pre_pull_extension', 'RepoPrePullEvent'),
     (hooks_base.post_pull, 'post_push_extension', 'RepoPullEvent'),
 ])
 def test_hooks_propagates_not_on_shadow(func, extension, event, hook_extras):
     """
     If hooks are called by a request to a shadow repo we only want to run our
     internal hooks code but not external ones like rhodecode extensions or
     trigger an event.
     """
     extension_mock = mock.Mock()
     events_mock = mock.Mock()
     patches = {
         'Repository': mock.Mock(),
         'events': events_mock,
         extension: extension_mock,
     }
 
     # Set shadow repo flag.
     hook_extras.is_shadow_repo = True
 
     # Execute hook function.
     with mock.patch.multiple(hooks_base, **patches):
         func(hook_extras)
 
     # Assert that extensions are *not* called and event was *not* fired.
     assert not extension_mock.called
     assert not events_mock.trigger.called