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
This commit is contained in:
parent
4631b879c2
commit
efc39f31eb
5 changed files with 58 additions and 25 deletions
|
|
@ -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, DataGridAppView):
|
|||
|
||||
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, DataGridAppView):
|
|||
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, DataGridAppView):
|
|||
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, DataGridAppView):
|
|||
# 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, DataGridAppView):
|
|||
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',
|
||||
|
|
|
|||
|
|
@ -100,11 +100,15 @@ class PullRequestUpdateEvent(PullRequestEvent):
|
|||
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):
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -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(username, repo_name, repo_alias,
|
|||
: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(username, repo_name, repo_alias,
|
|||
|
||||
|
||||
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(username, repo_name, repo_alias,
|
|||
: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(username, repo_name, repo_alias,
|
|||
|
||||
|
||||
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(username, repo_name, repo_alias,
|
|||
: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(username, repo_name, repo_alias,
|
|||
|
||||
|
||||
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(username, repo_name, repo_alias,
|
|||
: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(username, repo_name, repo_alias,
|
|||
: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
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue