# HG changeset patch # User Marcin Kuzminski # Date 2019-01-18 12:16:34 # Node ID e49bff61d5381560b6aa27d07c4918e1f043a49f # Parent a70c3099df14b8fd7fff1499b12adba6cb02c353 events: trigger 'review_status_change' when reviewers are updated. - small code refactoring - extended commenting event with comment - extended review_status_change event with new status diff --git a/rhodecode/apps/repository/views/repo_pull_requests.py b/rhodecode/apps/repository/views/repo_pull_requests.py --- a/rhodecode/apps/repository/views/repo_pull_requests.py +++ b/rhodecode/apps/repository/views/repo_pull_requests.py @@ -29,7 +29,6 @@ from pyramid.httpexceptions import ( from pyramid.view import view_config from pyramid.renderers import render -from rhodecode import events from rhodecode.apps._base import RepoAppView, DataGridAppView from rhodecode.lib import helpers as h, diffs, codeblocks, channelstream @@ -1231,6 +1230,7 @@ class RepoPullRequestsView(RepoAppView, def _update_reviewers(self, pull_request, review_members, reviewer_rules): _ = self.request.translate + get_default_reviewers_data, validate_default_reviewers = \ PullRequestModel().get_reviewer_functions() @@ -1241,11 +1241,19 @@ class RepoPullRequestsView(RepoAppView, h.flash(e, category='error') return + old_calculated_status = pull_request.calculated_review_status() PullRequestModel().update_reviewers( pull_request, reviewers, self._rhodecode_user) h.flash(_('Pull request reviewers updated.'), category='success') Session().commit() + # trigger status changed if change in reviewers changes the status + calculated_status = pull_request.calculated_review_status() + if old_calculated_status != calculated_status: + PullRequestModel().trigger_pull_request_hook( + pull_request, self._rhodecode_user, 'review_status_change', + data={'status': calculated_status}) + @LoginRequired() @NotAnonymous() @HasRepoPermissionAnyDecorator( @@ -1324,12 +1332,16 @@ class RepoPullRequestsView(RepoAppView, log.debug('comment: forbidden because not allowed to close ' 'pull request %s', pull_request_id) raise HTTPForbidden() + + # This also triggers `review_status_change` comment, status = PullRequestModel().close_pull_request_with_comment( pull_request, self._rhodecode_user, self.db_repo, message=text, auth_user=self._rhodecode_user) Session().flush() - events.trigger( - events.PullRequestCommentEvent(pull_request, comment)) + + PullRequestModel().trigger_pull_request_hook( + pull_request, self._rhodecode_user, 'comment', + data={'comment': comment}) else: # regular comment case, could be inline, or one with status. @@ -1379,15 +1391,17 @@ class RepoPullRequestsView(RepoAppView, # loaded on comment Session().refresh(comment) - events.trigger( - events.PullRequestCommentEvent(pull_request, comment)) + PullRequestModel().trigger_pull_request_hook( + pull_request, self._rhodecode_user, 'comment', + data={'comment': comment}) # we now calculate the status of pull request, and based on that # calculation we set the commits status calculated_status = pull_request.calculated_review_status() if old_calculated_status != calculated_status: - PullRequestModel()._trigger_pull_request_hook( - pull_request, self._rhodecode_user, 'review_status_change') + PullRequestModel().trigger_pull_request_hook( + pull_request, self._rhodecode_user, 'review_status_change', + data={'status': calculated_status}) Session().commit() @@ -1447,8 +1461,9 @@ class RepoPullRequestsView(RepoAppView, Session().commit() calculated_status = comment.pull_request.calculated_review_status() if old_calculated_status != calculated_status: - PullRequestModel()._trigger_pull_request_hook( - comment.pull_request, self._rhodecode_user, 'review_status_change') + PullRequestModel().trigger_pull_request_hook( + comment.pull_request, self._rhodecode_user, 'review_status_change', + data={'status': calculated_status}) return True else: log.warning('No permissions for user %s to delete comment_id: %s', diff --git a/rhodecode/events/pullrequest.py b/rhodecode/events/pullrequest.py --- a/rhodecode/events/pullrequest.py +++ b/rhodecode/events/pullrequest.py @@ -100,11 +100,15 @@ class PullRequestUpdateEvent(PullRequest class PullRequestReviewEvent(PullRequestEvent): """ An instance of this class is emitted as an :term:`event` after a pull - request review has changed. + request review has changed. A status defines new status of review. """ name = 'pullrequest-review' display_name = lazy_ugettext('pullrequest review changed') + def __init__(self, pullrequest, status): + super(PullRequestReviewEvent, self).__init__(pullrequest) + self.status = status + class PullRequestMergeEvent(PullRequestEvent): """ diff --git a/rhodecode/lib/hooks_utils.py b/rhodecode/lib/hooks_utils.py --- a/rhodecode/lib/hooks_utils.py +++ b/rhodecode/lib/hooks_utils.py @@ -64,7 +64,7 @@ def trigger_post_push_hook( def trigger_log_create_pull_request_hook(username, repo_name, repo_alias, - pull_request): + pull_request, data=None): """ Triggers create pull request action hooks @@ -72,6 +72,7 @@ def trigger_log_create_pull_request_hook :param repo_name: name of target repo :param repo_alias: the type of SCM target repo :param pull_request: the pull request that was created + :param data: extra data for specific events e.g {'comment': comment_obj} """ if repo_alias not in ('hg', 'git'): return @@ -84,7 +85,7 @@ def trigger_log_create_pull_request_hook def trigger_log_merge_pull_request_hook(username, repo_name, repo_alias, - pull_request): + pull_request, data=None): """ Triggers merge pull request action hooks @@ -92,6 +93,7 @@ def trigger_log_merge_pull_request_hook( :param repo_name: name of target repo :param repo_alias: the type of SCM target repo :param pull_request: the pull request that was merged + :param data: extra data for specific events e.g {'comment': comment_obj} """ if repo_alias not in ('hg', 'git'): return @@ -104,7 +106,7 @@ def trigger_log_merge_pull_request_hook( def trigger_log_close_pull_request_hook(username, repo_name, repo_alias, - pull_request): + pull_request, data=None): """ Triggers close pull request action hooks @@ -112,6 +114,7 @@ def trigger_log_close_pull_request_hook( :param repo_name: name of target repo :param repo_alias: the type of SCM target repo :param pull_request: the pull request that was closed + :param data: extra data for specific events e.g {'comment': comment_obj} """ if repo_alias not in ('hg', 'git'): return @@ -124,7 +127,7 @@ def trigger_log_close_pull_request_hook( def trigger_log_review_pull_request_hook(username, repo_name, repo_alias, - pull_request): + pull_request, data=None): """ Triggers review status change pull request action hooks @@ -132,19 +135,21 @@ def trigger_log_review_pull_request_hook :param repo_name: name of target repo :param repo_alias: the type of SCM target repo :param pull_request: the pull request that review status changed + :param data: extra data for specific events e.g {'comment': comment_obj} """ if repo_alias not in ('hg', 'git'): return extras = _get_rc_scm_extras(username, repo_name, repo_alias, 'review_pull_request') - events.trigger(events.PullRequestReviewEvent(pull_request)) + status = data.get('status') + events.trigger(events.PullRequestReviewEvent(pull_request, status)) extras.update(pull_request.get_api_data()) hooks_base.log_review_pull_request(**extras) def trigger_log_update_pull_request_hook(username, repo_name, repo_alias, - pull_request): + pull_request, data=None): """ Triggers update pull request action hooks @@ -152,6 +157,7 @@ def trigger_log_update_pull_request_hook :param repo_name: name of target repo :param repo_alias: the type of SCM target repo :param pull_request: the pull request that was updated + :param data: extra data for specific events e.g {'comment': comment_obj} """ if repo_alias not in ('hg', 'git'): return 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 @@ -513,7 +513,7 @@ class PullRequestModel(BaseModel): pull_request, auth_user=auth_user, translator=translator) self.notify_reviewers(pull_request, reviewer_ids) - self._trigger_pull_request_hook( + self.trigger_pull_request_hook( pull_request, created_by_user, 'create') creation_data = pull_request.get_api_data(with_merge_state=False) @@ -523,7 +523,7 @@ class PullRequestModel(BaseModel): return pull_request - def _trigger_pull_request_hook(self, pull_request, user, action): + def trigger_pull_request_hook(self, pull_request, user, action, data=None): pull_request = self.__get_pull_request(pull_request) target_scm = pull_request.target_repo.scm_instance() if action == 'create': @@ -536,6 +536,12 @@ class PullRequestModel(BaseModel): trigger_hook = hooks_utils.trigger_log_review_pull_request_hook elif action == 'update': trigger_hook = hooks_utils.trigger_log_update_pull_request_hook + elif action == 'comment': + # dummy hook ! for comment. We want this function to handle all cases + def trigger_hook(*args, **kwargs): + pass + comment = data['comment'] + events.trigger(events.PullRequestCommentEvent(pull_request, comment)) else: return @@ -543,7 +549,8 @@ class PullRequestModel(BaseModel): username=user.username, repo_name=pull_request.target_repo.repo_name, repo_alias=target_scm.alias, - pull_request=pull_request) + pull_request=pull_request, + data=data) def _get_commit_ids(self, pull_request): """ @@ -642,7 +649,7 @@ class PullRequestModel(BaseModel): # TODO: paris: replace invalidation with less radical solution ScmModel().mark_for_invalidation( pull_request.target_repo.repo_name) - self._trigger_pull_request_hook(pull_request, user, 'merge') + self.trigger_pull_request_hook(pull_request, user, 'merge') def has_valid_update_type(self, pull_request): source_ref_type = pull_request.source_ref_parts.type @@ -811,7 +818,7 @@ class PullRequestModel(BaseModel): pull_request.source_ref_parts.commit_id, pull_request_version.pull_request_version_id) Session().commit() - self._trigger_pull_request_hook(pull_request, pull_request.author, 'update') + self.trigger_pull_request_hook(pull_request, pull_request.author, 'update') return UpdateResponse( executed=True, reason=UpdateFailureReason.NONE, @@ -1146,7 +1153,7 @@ class PullRequestModel(BaseModel): pull_request.status = PullRequest.STATUS_CLOSED pull_request.updated_on = datetime.datetime.now() Session().add(pull_request) - self._trigger_pull_request_hook( + self.trigger_pull_request_hook( pull_request, pull_request.author, 'close') pr_data = pull_request.get_api_data(with_merge_state=False) @@ -1200,8 +1207,9 @@ class PullRequestModel(BaseModel): # change the status, while if he's a reviewer this might change it. calculated_status = pull_request.calculated_review_status() if old_calculated_status != calculated_status: - self._trigger_pull_request_hook( - pull_request, user, 'review_status_change') + self.trigger_pull_request_hook( + pull_request, user, 'review_status_change', + data={'status': calculated_status}) # finally close the PR PullRequestModel().close_pull_request( 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 @@ -71,7 +71,7 @@ class TestPullRequestModel(object): self.helper_patcher.start() self.hook_patcher = mock.patch.object(PullRequestModel, - '_trigger_pull_request_hook') + 'trigger_pull_request_hook') self.hook_mock = self.hook_patcher.start() self.invalidation_patcher = mock.patch(