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

rhodecode/apps/repository/views/repo_pull_requests.py

@@ -29,7 +29,6 b' 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
@@ -1176,6 +1175,7 b' 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()
 
@@ -1186,11 +1186,19 b' 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(
@@ -1269,12 +1277,16 b' 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.
@@ -1324,15 +1336,17 b' 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()
 
@@ -1392,8 +1406,9 b' 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',

rhodecode/events/pullrequest.py

@@ -100,11 +100,15 b' 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):
     """

rhodecode/lib/hooks_utils.py

@@ -64,7 +64,7 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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

rhodecode/model/pull_request.py

@@ -539,7 +539,7 b' 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)
@@ -549,7 +549,7 b' 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':
@@ -562,6 +562,12 b' 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
 
@@ -569,7 +575,8 b' 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):
         """
@@ -669,7 +676,7 b' 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
@@ -839,8 +846,7 b' 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,
@@ -1174,7 +1180,7 b' 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)
@@ -1228,8 +1234,9 b' 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(

rhodecode/tests/models/test_pullrequest.py

@@ -69,7 +69,7 b' 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(