diff --git a/rhodecode/api/tests/test_close_pull_request.py b/rhodecode/api/tests/test_close_pull_request.py --- a/rhodecode/api/tests/test_close_pull_request.py +++ b/rhodecode/api/tests/test_close_pull_request.py @@ -47,13 +47,12 @@ class TestClosePullRequest(object): 'closed': True, } assert_ok(id_, expected, response.body) - action = 'user_closed_pull_request:%d' % pull_request_id journal = UserLog.query()\ - .filter(UserLog.user_id == author)\ + .filter(UserLog.user_id == author) \ + .order_by('user_log_id') \ .filter(UserLog.repository_id == repo)\ - .filter(UserLog.action == action)\ .all() - assert len(journal) == 1 + assert journal[-1].action == 'repo.pull_request.close' @pytest.mark.backends("git", "hg") def test_api_close_pull_request_already_closed_error(self, pr_util): diff --git a/rhodecode/api/tests/test_comment_pull_request.py b/rhodecode/api/tests/test_comment_pull_request.py --- a/rhodecode/api/tests/test_comment_pull_request.py +++ b/rhodecode/api/tests/test_comment_pull_request.py @@ -62,13 +62,12 @@ class TestCommentPullRequest(object): } assert_ok(id_, expected, response.body) - action = 'user_commented_pull_request:%d' % pull_request_id journal = UserLog.query()\ .filter(UserLog.user_id == author)\ - .filter(UserLog.repository_id == repo)\ - .filter(UserLog.action == action)\ + .filter(UserLog.repository_id == repo) \ + .order_by('user_log_id') \ .all() - assert len(journal) == 2 + assert journal[-1].action == 'repo.pull_request.comment.create' @pytest.mark.backends("git", "hg") def test_api_comment_pull_request_change_status( diff --git a/rhodecode/api/tests/test_get_pull_request.py b/rhodecode/api/tests/test_get_pull_request.py --- a/rhodecode/api/tests/test_get_pull_request.py +++ b/rhodecode/api/tests/test_get_pull_request.py @@ -33,7 +33,7 @@ pytestmark = pytest.mark.backends("git", @pytest.mark.usefixtures("testuser_api", "app") class TestGetPullRequest(object): - def test_api_get_pull_request(self, pr_util, http_host_stub, http_host_only_stub): + def test_api_get_pull_request(self, pr_util, http_host_only_stub): from rhodecode.model.pull_request import PullRequestModel pull_request = pr_util.create_pull_request(mergeable=True) id_, params = build_data( @@ -52,7 +52,7 @@ class TestGetPullRequest(object): pull_request_id=pull_request.pull_request_id, qualified=True)) pr_url = safe_unicode( - url_obj.with_netloc(http_host_stub)) + url_obj.with_netloc(http_host_only_stub)) source_url = safe_unicode( pull_request.source_repo.clone_url().with_netloc(http_host_only_stub)) target_url = safe_unicode( diff --git a/rhodecode/api/tests/test_merge_pull_request.py b/rhodecode/api/tests/test_merge_pull_request.py --- a/rhodecode/api/tests/test_merge_pull_request.py +++ b/rhodecode/api/tests/test_merge_pull_request.py @@ -95,13 +95,13 @@ class TestMergePullRequest(object): assert_ok(id_, expected, response.body) - action = 'user_merged_pull_request:%d' % (pull_request_id, ) journal = UserLog.query()\ .filter(UserLog.user_id == author)\ - .filter(UserLog.repository_id == repo)\ - .filter(UserLog.action == action)\ + .filter(UserLog.repository_id == repo) \ + .order_by('user_log_id') \ .all() - assert len(journal) == 1 + assert journal[-2].action == 'repo.pull_request.merge' + assert journal[-1].action == 'repo.pull_request.close' id_, params = build_data( self.apikey, 'merge_pull_request', diff --git a/rhodecode/api/tests/test_update_pull_request.py b/rhodecode/api/tests/test_update_pull_request.py --- a/rhodecode/api/tests/test_update_pull_request.py +++ b/rhodecode/api/tests/test_update_pull_request.py @@ -33,7 +33,7 @@ class TestUpdatePullRequest(object): @pytest.mark.backends("git", "hg") def test_api_update_pull_request_title_or_description( - self, pr_util, silence_action_logger, no_notifications): + self, pr_util, no_notifications): pull_request = pr_util.create_pull_request() id_, params = build_data( @@ -61,7 +61,7 @@ class TestUpdatePullRequest(object): @pytest.mark.backends("git", "hg") def test_api_try_update_closed_pull_request( - self, pr_util, silence_action_logger, no_notifications): + self, pr_util, no_notifications): pull_request = pr_util.create_pull_request() PullRequestModel().close_pull_request( pull_request, TEST_USER_ADMIN_LOGIN) @@ -78,8 +78,7 @@ class TestUpdatePullRequest(object): assert_error(id_, expected, response.body) @pytest.mark.backends("git", "hg") - def test_api_update_update_commits( - self, pr_util, silence_action_logger, no_notifications): + def test_api_update_update_commits(self, pr_util, no_notifications): commits = [ {'message': 'a'}, {'message': 'b', 'added': [FileNode('file_b', 'test_content\n')]}, @@ -119,7 +118,7 @@ class TestUpdatePullRequest(object): @pytest.mark.backends("git", "hg") def test_api_update_change_reviewers( - self, user_util, pr_util, silence_action_logger, no_notifications): + self, user_util, pr_util, no_notifications): a = user_util.create_user() b = user_util.create_user() c = user_util.create_user() diff --git a/rhodecode/api/views/pull_request_api.py b/rhodecode/api/views/pull_request_api.py --- a/rhodecode/api/views/pull_request_api.py +++ b/rhodecode/api/views/pull_request_api.py @@ -669,7 +669,7 @@ def update_pull_request( if title or description: PullRequestModel().edit( pull_request, title or pull_request.title, - description or pull_request.description) + description or pull_request.description, apiuser) Session().commit() commit_changes = {"added": [], "common": [], "removed": []} @@ -683,7 +683,7 @@ def update_pull_request( reviewers_changes = {"added": [], "removed": []} if reviewers: added_reviewers, removed_reviewers = \ - PullRequestModel().update_reviewers(pull_request, reviewers) + PullRequestModel().update_reviewers(pull_request, reviewers, apiuser) reviewers_changes['added'] = sorted( [get_user_or_error(n).username for n in added_reviewers]) diff --git a/rhodecode/controllers/admin/users.py b/rhodecode/controllers/admin/users.py --- a/rhodecode/controllers/admin/users.py +++ b/rhodecode/controllers/admin/users.py @@ -628,8 +628,8 @@ class UsersController(BaseController): ip_id = request.POST.get('del_ip_id') user_model = UserModel() + user_data = c.user.get_api_data() ip = UserIpMap.query().get(ip_id).ip_addr - user_data = c.user.get_api_data() user_model.delete_extra_ip(user_id, ip_id) audit_logger.store_web( 'user.edit.ip.delete', diff --git a/rhodecode/controllers/changeset.py b/rhodecode/controllers/changeset.py --- a/rhodecode/controllers/changeset.py +++ b/rhodecode/controllers/changeset.py @@ -440,7 +440,7 @@ class ChangesetController(BaseRepoContro owner = (comment.author.user_id == c.rhodecode_user.user_id) is_repo_admin = h.HasRepoPermissionAny('repository.admin')(c.repo_name) if h.HasPermissionAny('hg.admin')() or is_repo_admin or owner: - CommentsModel().delete(comment=comment) + CommentsModel().delete(comment=comment, user=c.rhodecode_user) Session().commit() return True else: diff --git a/rhodecode/controllers/files.py b/rhodecode/controllers/files.py --- a/rhodecode/controllers/files.py +++ b/rhodecode/controllers/files.py @@ -38,7 +38,7 @@ from rhodecode.lib import diffs, helpers from rhodecode.lib import audit_logger from rhodecode.lib.codeblocks import ( filenode_as_lines_tokens, filenode_as_annotated_lines_tokens) -from rhodecode.lib.utils import jsonify, action_logger +from rhodecode.lib.utils import jsonify from rhodecode.lib.utils2 import ( convert_line_endings, detect_mode, safe_str, str2bool) from rhodecode.lib.auth import ( diff --git a/rhodecode/controllers/pullrequests.py b/rhodecode/controllers/pullrequests.py --- a/rhodecode/controllers/pullrequests.py +++ b/rhodecode/controllers/pullrequests.py @@ -317,7 +317,7 @@ class PullrequestsController(BaseRepoCon try: PullRequestModel().edit( pull_request, request.POST.get('title'), - request.POST.get('description')) + request.POST.get('description'), c.rhodecode_user) except ValueError: msg = _(u'Cannot update closed pull requests.') h.flash(msg, category='error') @@ -456,7 +456,8 @@ class PullrequestsController(BaseRepoCon h.flash(e, category='error') return - PullRequestModel().update_reviewers(pull_request_id, reviewers) + PullRequestModel().update_reviewers( + pull_request_id, reviewers, c.rhodecode_user) h.flash(_('Pull request reviewers updated.'), category='success') Session().commit() @@ -476,7 +477,7 @@ class PullrequestsController(BaseRepoCon # only owner can delete it ! if allowed_to_delete: - PullRequestModel().delete(pull_request) + PullRequestModel().delete(pull_request, c.rhodecode_user) Session().commit() h.flash(_('Successfully deleted pull request'), category='success') @@ -997,7 +998,7 @@ class PullrequestsController(BaseRepoCon is_repo_admin = h.HasRepoPermissionAny('repository.admin')(c.repo_name) if h.HasPermissionAny('hg.admin')() or is_repo_admin or is_owner: old_calculated_status = co.pull_request.calculated_review_status() - CommentsModel().delete(comment=co) + CommentsModel().delete(comment=co, user=c.rhodecode_user) Session().commit() calculated_status = co.pull_request.calculated_review_status() if old_calculated_status != calculated_status: diff --git a/rhodecode/lib/audit_logger.py b/rhodecode/lib/audit_logger.py --- a/rhodecode/lib/audit_logger.py +++ b/rhodecode/lib/audit_logger.py @@ -28,7 +28,7 @@ from rhodecode.model.db import User, Use log = logging.getLogger(__name__) # action as key, and expected action_data as value -ACTIONS = { +ACTIONS_V1 = { 'user.login.success': {'user_agent': ''}, 'user.login.failure': {'user_agent': ''}, 'user.logout': {'user_agent': ''}, @@ -64,11 +64,28 @@ ACTIONS = { 'repo.commit.strip': {}, 'repo.archive.download': {}, + 'repo.pull_request.create': '', + 'repo.pull_request.edit': '', + 'repo.pull_request.delete': '', + 'repo.pull_request.close': '', + 'repo.pull_request.merge': '', + 'repo.pull_request.vote': '', + 'repo.pull_request.comment.create': '', + 'repo.pull_request.comment.delete': '', + + 'repo.pull_request.reviewer.add': '', + 'repo.pull_request.reviewer.delete': '', + + 'repo.commit.comment.create': '', + 'repo.commit.comment.delete': '', + 'repo.commit.vote': '', + 'repo_group.create': {'data': {}}, 'repo_group.edit': {'old_data': {}}, 'repo_group.edit.permissions': {}, 'repo_group.delete': {'old_data': {}}, } +ACTIONS = ACTIONS_V1 SOURCE_WEB = 'source_web' SOURCE_API = 'source_api' diff --git a/rhodecode/lib/utils.py b/rhodecode/lib/utils.py --- a/rhodecode/lib/utils.py +++ b/rhodecode/lib/utils.py @@ -139,68 +139,6 @@ def get_user_group_slug(request): return _group -def action_logger(user, action, repo, ipaddr='', sa=None, commit=False): - """ - Action logger for various actions made by users - - :param user: user that made this action, can be a unique username string or - object containing user_id attribute - :param action: action to log, should be on of predefined unique actions for - easy translations - :param repo: string name of repository or object containing repo_id, - that action was made on - :param ipaddr: optional ip address from what the action was made - :param sa: optional sqlalchemy session - - """ - - if not sa: - sa = meta.Session() - # if we don't get explicit IP address try to get one from registered user - # in tmpl context var - if not ipaddr: - ipaddr = getattr(get_current_rhodecode_user(), 'ip_addr', '') - - try: - if getattr(user, 'user_id', None): - user_obj = User.get(user.user_id) - elif isinstance(user, basestring): - user_obj = User.get_by_username(user) - else: - raise Exception('You have to provide a user object or a username') - - if getattr(repo, 'repo_id', None): - repo_obj = Repository.get(repo.repo_id) - repo_name = repo_obj.repo_name - elif isinstance(repo, basestring): - repo_name = repo.lstrip('/') - repo_obj = Repository.get_by_repo_name(repo_name) - else: - repo_obj = None - repo_name = '' - - user_log = UserLog() - user_log.user_id = user_obj.user_id - user_log.username = user_obj.username - action = safe_unicode(action) - user_log.action = action[:1200000] - - user_log.repository = repo_obj - user_log.repository_name = repo_name - - user_log.action_date = datetime.datetime.now() - user_log.user_ip = ipaddr - sa.add(user_log) - - log.info('Logging action:`%s` on repo:`%s` by user:%s ip:%s', - action, safe_unicode(repo), user_obj, ipaddr) - if commit: - sa.commit() - except Exception: - log.error(traceback.format_exc()) - raise - - def get_filesystem_repos(path, recursive=False, skip_removed_repos=True): """ Scans given path for repos and return (name,(type,path)) tuple diff --git a/rhodecode/model/comment.py b/rhodecode/model/comment.py --- a/rhodecode/model/comment.py +++ b/rhodecode/model/comment.py @@ -34,8 +34,8 @@ from sqlalchemy.sql.expression import nu from sqlalchemy.sql.functions import coalesce from rhodecode.lib import helpers as h, diffs +from rhodecode.lib import audit_logger from rhodecode.lib.channelstream import channelstream_request -from rhodecode.lib.utils import action_logger from rhodecode.lib.utils2 import extract_mentioned_users, safe_str from rhodecode.model import BaseModel from rhodecode.model.db import ( @@ -163,6 +163,13 @@ class CommentsModel(BaseModel): return todos + def _log_audit_action(self, action, action_data, user, comment): + audit_logger.store( + action=action, + action_data=action_data, + user=user, + repo=comment.repo) + def create(self, text, repo, user, commit_id=None, pull_request=None, f_path=None, line_no=None, status_change=None, status_change_type=None, comment_type=None, @@ -337,13 +344,15 @@ class CommentsModel(BaseModel): email_kwargs=kwargs, ) - action = ( - 'user_commented_pull_request:{}'.format( - comment.pull_request.pull_request_id) - if comment.pull_request - else 'user_commented_revision:{}'.format(comment.revision) - ) - action_logger(user, action, comment.repo) + Session().flush() + if comment.pull_request: + action = 'repo.pull_request.comment.create' + else: + action = 'repo.commit.comment.create' + + comment_data = comment.get_api_data() + self._log_audit_action( + action, {'data': comment_data}, user, comment) registry = get_current_registry() rhodecode_plugins = getattr(registry, 'rhodecode_plugins', {}) @@ -385,15 +394,22 @@ class CommentsModel(BaseModel): return comment - def delete(self, comment): + def delete(self, comment, user): """ Deletes given comment - - :param comment_id: """ comment = self.__get_commit_comment(comment) + old_data = comment.get_api_data() Session().delete(comment) + if comment.pull_request: + action = 'repo.pull_request.comment.delete' + else: + action = 'repo.commit.comment.delete' + + self._log_audit_action( + action, {'old_data': old_data}, user, comment) + return comment def get_all_comments(self, repo_id, revision=None, pull_request=None): diff --git a/rhodecode/model/db.py b/rhodecode/model/db.py --- a/rhodecode/model/db.py +++ b/rhodecode/model/db.py @@ -3122,6 +3122,25 @@ class ChangesetComment(Base, BaseModel): else: return '' % id(self) + def get_api_data(self): + comment = self + data = { + 'comment_id': comment.comment_id, + 'comment_type': comment.comment_type, + 'comment_text': comment.text, + 'comment_status': comment.status_change, + 'comment_f_path': comment.f_path, + 'comment_lineno': comment.line_no, + 'comment_author': comment.author, + 'comment_created_on': comment.created_on + } + return data + + def __json__(self): + data = dict() + data.update(self.get_api_data()) + return data + class ChangesetStatus(Base, BaseModel): __tablename__ = 'changeset_statuses' @@ -3173,6 +3192,19 @@ class ChangesetStatus(Base, BaseModel): def status_lbl(self): return ChangesetStatus.get_status_lbl(self.status) + def get_api_data(self): + status = self + data = { + 'status_id': status.changeset_status_id, + 'status': status.status, + } + return data + + def __json__(self): + data = dict() + data.update(self.get_api_data()) + return data + class _PullRequestBase(BaseModel): """ @@ -3304,15 +3336,19 @@ class _PullRequestBase(BaseModel): else: return None - def get_api_data(self): - from pylons import url + def get_api_data(self, with_merge_state=True): from rhodecode.model.pull_request import PullRequestModel + pull_request = self - merge_status = PullRequestModel().merge_status(pull_request) - - pull_request_url = url( - 'pullrequest_show', repo_name=self.target_repo.repo_name, - pull_request_id=self.pull_request_id, qualified=True) + if with_merge_state: + merge_status = PullRequestModel().merge_status(pull_request) + merge_state = { + 'status': merge_status[0], + 'message': safe_unicode(merge_status[1]), + } + else: + merge_state = {'status': 'not_available', + 'message': 'not_available'} merge_data = { 'clone_url': PullRequestModel().get_shadow_clone_url(pull_request), @@ -3323,7 +3359,7 @@ class _PullRequestBase(BaseModel): data = { 'pull_request_id': pull_request.pull_request_id, - 'url': pull_request_url, + 'url': PullRequestModel().get_url(pull_request), 'title': pull_request.title, 'description': pull_request.description, 'status': pull_request.status, @@ -3331,10 +3367,7 @@ class _PullRequestBase(BaseModel): 'updated_on': pull_request.updated_on, 'commit_ids': pull_request.revisions, 'review_status': pull_request.calculated_review_status(), - 'mergeable': { - 'status': merge_status[0], - 'message': unicode(merge_status[1]), - }, + 'mergeable': merge_state, 'source': { 'clone_url': pull_request.source_repo.clone_url(), 'repository': pull_request.source_repo.repo_name, @@ -3389,7 +3422,8 @@ class PullRequest(Base, _PullRequestBase reviewers = relationship('PullRequestReviewers', cascade="all, delete, delete-orphan") - statuses = relationship('ChangesetStatus') + statuses = relationship('ChangesetStatus', + cascade="all, delete, delete-orphan") comments = relationship('ChangesetComment', cascade="all, delete, delete-orphan") versions = relationship('PullRequestVersion', diff --git a/rhodecode/model/pull_request.py b/rhodecode/model/pull_request.py --- a/rhodecode/model/pull_request.py +++ b/rhodecode/model/pull_request.py @@ -36,11 +36,11 @@ from sqlalchemy import or_ from rhodecode import events from rhodecode.lib import helpers as h, hooks_utils, diffs +from rhodecode.lib import audit_logger from rhodecode.lib.compat import OrderedDict from rhodecode.lib.hooks_daemon import prepare_callback_daemon from rhodecode.lib.markup_renderer import ( DEFAULT_COMMENTS_RENDERER, RstTemplateRenderer) -from rhodecode.lib.utils import action_logger from rhodecode.lib.utils2 import safe_unicode, safe_str, md5_safe from rhodecode.lib.vcs.backends.base import ( Reference, MergeResponse, MergeFailureReason, UpdateFailureReason) @@ -470,6 +470,11 @@ class PullRequestModel(BaseModel): self._trigger_pull_request_hook( pull_request, created_by_user, 'create') + creation_data = pull_request.get_api_data(with_merge_state=False) + self._log_audit_action( + 'repo.pull_request.create', {'data': creation_data}, + created_by_user, pull_request) + return pull_request def _trigger_pull_request_hook(self, pull_request, user, action): @@ -520,7 +525,12 @@ class PullRequestModel(BaseModel): log.debug( "Merge was successful, updating the pull request comments.") self._comment_and_close_pr(pull_request, user, merge_state) - self._log_action('user_merged_pull_request', user, pull_request) + + self._log_audit_action( + 'repo.pull_request.merge', + {'merge_state': merge_state.__dict__}, + user, pull_request) + else: log.warn("Merge failed, not updating the pull request.") return merge_state @@ -899,8 +909,9 @@ class PullRequestModel(BaseModel): renderer = RstTemplateRenderer() return renderer.render('pull_request_update.mako', **params) - def edit(self, pull_request, title, description): + def edit(self, pull_request, title, description, user): pull_request = self.__get_pull_request(pull_request) + old_data = pull_request.get_api_data(with_merge_state=False) if pull_request.is_closed(): raise ValueError('This pull request is closed') if title: @@ -908,8 +919,11 @@ class PullRequestModel(BaseModel): pull_request.description = description pull_request.updated_on = datetime.datetime.now() Session().add(pull_request) + self._log_audit_action( + 'repo.pull_request.edit', {'old_data': old_data}, + user, pull_request) - def update_reviewers(self, pull_request, reviewer_data): + def update_reviewers(self, pull_request, reviewer_data, user): """ Update the reviewers in the pull request @@ -946,8 +960,11 @@ class PullRequestModel(BaseModel): reviewer.pull_request = pull_request reviewer.reasons = reviewers[uid]['reasons'] # NOTE(marcink): mandatory shouldn't be changed now - #reviewer.mandatory = reviewers[uid]['reasons'] + # reviewer.mandatory = reviewers[uid]['reasons'] Session().add(reviewer) + self._log_audit_action( + 'repo.pull_request.reviewer.add', {'data': reviewer.get_dict()}, + user, pull_request) for uid in ids_to_remove: changed = True @@ -958,7 +975,11 @@ class PullRequestModel(BaseModel): # use .all() in case we accidentally added the same person twice # this CAN happen due to the lack of DB checks for obj in reviewers: + old_data = obj.get_dict() Session().delete(obj) + self._log_audit_action( + 'repo.pull_request.reviewer.delete', + {'old_data': old_data}, user, pull_request) if changed: pull_request.updated_on = datetime.datetime.now() @@ -1054,9 +1075,13 @@ class PullRequestModel(BaseModel): email_kwargs=kwargs, ) - def delete(self, pull_request): + def delete(self, pull_request, user): pull_request = self.__get_pull_request(pull_request) + old_data = pull_request.get_api_data(with_merge_state=False) self._cleanup_merge_workspace(pull_request) + self._log_audit_action( + 'repo.pull_request.delete', {'old_data': old_data}, + user, pull_request) Session().delete(pull_request) def close_pull_request(self, pull_request, user): @@ -1067,7 +1092,8 @@ class PullRequestModel(BaseModel): Session().add(pull_request) self._trigger_pull_request_hook( pull_request, pull_request.author, 'close') - self._log_action('user_closed_pull_request', user, pull_request) + self._log_audit_action( + 'repo.pull_request.close', {}, user, pull_request) def close_pull_request_with_comment( self, pull_request, user, repo, message=None): @@ -1402,12 +1428,12 @@ class PullRequestModel(BaseModel): settings = settings_model.get_general_settings() return settings.get('rhodecode_hg_use_rebase_for_merging', False) - def _log_action(self, action, user, pull_request): - action_logger( - user, - '{action}:{pr_id}'.format( - action=action, pr_id=pull_request.pull_request_id), - pull_request.target_repo) + def _log_audit_action(self, action, action_data, user, pull_request): + audit_logger.store( + action=action, + action_data=action_data, + user=user, + repo=pull_request.target_repo) def get_reviewer_functions(self): """ diff --git a/rhodecode/tests/functional/test_admin_permissions.py b/rhodecode/tests/functional/test_admin_permissions.py --- a/rhodecode/tests/functional/test_admin_permissions.py +++ b/rhodecode/tests/functional/test_admin_permissions.py @@ -199,6 +199,9 @@ class TestAdminPermissionsController(Tes url('edit_user_ips', user_id=default_user_id), params={'_method': 'delete', 'del_ip_id': del_ip_id, 'csrf_token': self.csrf_token}) + + assert_session_flash(response, 'Removed ip address from user whitelist') + clear_all_caches() response = self.app.get(url('admin_permissions_ips')) response.mustcontain('All IP addresses are allowed') diff --git a/rhodecode/tests/functional/test_pullrequests.py b/rhodecode/tests/functional/test_pullrequests.py --- a/rhodecode/tests/functional/test_pullrequests.py +++ b/rhodecode/tests/functional/test_pullrequests.py @@ -27,7 +27,7 @@ from rhodecode.lib.vcs.nodes import File from rhodecode.lib import helpers as h from rhodecode.model.changeset_status import ChangesetStatusModel from rhodecode.model.db import ( - PullRequest, ChangesetStatus, UserLog, Notification) + PullRequest, ChangesetStatus, UserLog, Notification, ChangesetComment) from rhodecode.model.meta import Session from rhodecode.model.pull_request import PullRequestModel from rhodecode.model.user import UserModel @@ -256,29 +256,32 @@ class TestPullrequestsController(object) 'csrf_token': csrf_token}, extra_environ=xhr_header,) - 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)\ + .filter(UserLog.repository_id == repo) \ + .order_by('user_log_id') \ .all() - assert len(journal) == 1 + assert journal[-1].action == 'repo.pull_request.close' 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 - assert pull_request.comments[-1].text == 'Closing a PR' + comments = ChangesetComment().query() \ + .filter(ChangesetComment.pull_request == pull_request) \ + .order_by(ChangesetComment.comment_id.asc())\ + .all() + assert comments[-1].text == 'Closing a PR' def test_comment_force_close_pull_request_rejected( self, pr_util, csrf_token, xhr_header): pull_request = pr_util.create_pull_request() pull_request_id = pull_request.pull_request_id PullRequestModel().update_reviewers( - pull_request_id, [(1, ['reason'], False), (2, ['reason2'], False)]) + pull_request_id, [(1, ['reason'], False), (2, ['reason2'], False)], + pull_request.author) author = pull_request.user_id repo = pull_request.target_repo.repo_id @@ -294,12 +297,11 @@ class TestPullrequestsController(object) 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 + journal = UserLog.query()\ + .filter(UserLog.user_id == author, UserLog.repository_id == repo) \ + .order_by('user_log_id') \ + .all() + assert journal[-1].action == 'repo.pull_request.close' # check only the latest status, not the review status status = ChangesetStatusModel().get_status( @@ -449,7 +451,8 @@ class TestPullrequestsController(object) # Change reviewers and check that a notification was made PullRequestModel().update_reviewers( - pull_request.pull_request_id, [(1, [], False)]) + pull_request.pull_request_id, [(1, [], False)], + pull_request.author) assert len(notifications.all()) == 2 def test_create_pull_request_stores_ancestor_commit_id(self, backend, @@ -541,25 +544,20 @@ class TestPullrequestsController(object) 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(3) + user_logs = UserLog.query().order_by('-user_log_id').limit(3) actions = [log.action for log in user_logs] pr_commit_ids = PullRequestModel()._get_commit_ids(pull_request) expected_actions = [ - u'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'repo.pull_request.close', + u'repo.pull_request.merge', + u'repo.pull_request.comment.create' ] 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 + user_logs = UserLog.query().order_by('-user_log_id').limit(4) + actions = [log for log in user_logs] + assert actions[-1].action == 'user.push' + assert actions[-1].action_data['commit_ids'] == pr_commit_ids # Check post_push rcextension was really executed push_calls = rhodecode.EXTENSIONS.calls['post_push'] diff --git a/rhodecode/tests/models/test_pullrequest.py b/rhodecode/tests/models/test_pullrequest.py --- a/rhodecode/tests/models/test_pullrequest.py +++ b/rhodecode/tests/models/test_pullrequest.py @@ -50,7 +50,9 @@ class TestPullRequestModel(object): A pull request combined with multiples patches. """ BackendClass = get_backend(backend.alias) - self.merge_patcher = mock.patch.object(BackendClass, 'merge') + self.merge_patcher = mock.patch.object( + BackendClass, 'merge', return_value=MergeResponse( + False, False, None, MergeFailureReason.UNKNOWN)) self.workspace_remove_patcher = mock.patch.object( BackendClass, 'cleanup_merge_workspace') @@ -117,7 +119,8 @@ class TestPullRequestModel(object): def test_get_awaiting_my_review(self, pull_request): PullRequestModel().update_reviewers( - pull_request, [(pull_request.author, ['author'], False)]) + pull_request, [(pull_request.author, ['author'], False)], + pull_request.author) prs = PullRequestModel().get_awaiting_my_review( pull_request.target_repo, user_id=pull_request.author.user_id) assert isinstance(prs, list) @@ -125,13 +128,14 @@ class TestPullRequestModel(object): def test_count_awaiting_my_review(self, pull_request): PullRequestModel().update_reviewers( - pull_request, [(pull_request.author, ['author'], False)]) + pull_request, [(pull_request.author, ['author'], False)], + pull_request.author) pr_count = PullRequestModel().count_awaiting_my_review( pull_request.target_repo, user_id=pull_request.author.user_id) assert pr_count == 1 def test_delete_calls_cleanup_merge(self, pull_request): - PullRequestModel().delete(pull_request) + PullRequestModel().delete(pull_request, pull_request.author) self.workspace_remove_mock.assert_called_once_with( self.workspace_id) diff --git a/rhodecode/tests/plugin.py b/rhodecode/tests/plugin.py --- a/rhodecode/tests/plugin.py +++ b/rhodecode/tests/plugin.py @@ -892,7 +892,7 @@ class RepoServer(object): @pytest.fixture -def pr_util(backend, request): +def pr_util(backend, request, config_stub): """ Utility for tests of models and for functional tests around pull requests. @@ -1085,7 +1085,7 @@ class PRTestUtility(object): # request will already be deleted. pull_request = PullRequest().get(self.pull_request_id) if pull_request: - PullRequestModel().delete(pull_request) + PullRequestModel().delete(pull_request, pull_request.author) Session().commit() if self.notification_patcher: @@ -1648,14 +1648,6 @@ def no_notifications(request): request.addfinalizer(notification_patcher.stop) -@pytest.fixture -def silence_action_logger(request): - notification_patcher = mock.patch( - 'rhodecode.lib.utils.action_logger') - notification_patcher.start() - request.addfinalizer(notification_patcher.stop) - - @pytest.fixture(scope='session') def repeat(request): """