observers: code cleanups and fixed tests.
This commit is contained in:
Marcin Kuzminski
parent
dd344bb96b
commit
c3f059ebc6
15 changed files with 206 additions and 108 deletions
|
|
@ -30,9 +30,11 @@ from rhodecode.lib import channelstream
|
|||
from rhodecode.lib.auth import (HasRepoPermissionAnyApi)
|
||||
from rhodecode.lib.base import vcs_operation_context
|
||||
from rhodecode.lib.utils2 import str2bool
|
||||
from rhodecode.lib.vcs.backends.base import unicode_to_reference
|
||||
from rhodecode.model.changeset_status import ChangesetStatusModel
|
||||
from rhodecode.model.comment import CommentsModel
|
||||
from rhodecode.model.db import Session, ChangesetStatus, ChangesetComment, PullRequest
|
||||
from rhodecode.model.db import (
|
||||
Session, ChangesetStatus, ChangesetComment, PullRequest, PullRequestReviewers)
|
||||
from rhodecode.model.pull_request import PullRequestModel, MergeCheck
|
||||
from rhodecode.model.settings import SettingsModel
|
||||
from rhodecode.model.validation_schema import Invalid
|
||||
|
|
@ -600,7 +602,7 @@ def comment_pull_request(
|
|||
extra_recipients=extra_recipients,
|
||||
send_email=send_email
|
||||
)
|
||||
is_inline = bool(comment.f_path and comment.line_no)
|
||||
is_inline = comment.is_inline
|
||||
|
||||
if allowed_to_change_status and status:
|
||||
old_calculated_status = pull_request.calculated_review_status()
|
||||
|
|
@ -646,12 +648,26 @@ def comment_pull_request(
|
|||
|
||||
return data
|
||||
|
||||
def _reviewers_validation(obj_list):
|
||||
schema = ReviewerListSchema()
|
||||
try:
|
||||
reviewer_objects = schema.deserialize(obj_list)
|
||||
except Invalid as err:
|
||||
raise JSONRPCValidationError(colander_exc=err)
|
||||
|
||||
# validate users
|
||||
for reviewer_object in reviewer_objects:
|
||||
user = get_user_or_error(reviewer_object['username'])
|
||||
reviewer_object['user_id'] = user.user_id
|
||||
return reviewer_objects
|
||||
|
||||
|
||||
@jsonrpc_method()
|
||||
def create_pull_request(
|
||||
request, apiuser, source_repo, target_repo, source_ref, target_ref,
|
||||
owner=Optional(OAttr('apiuser')), title=Optional(''), description=Optional(''),
|
||||
description_renderer=Optional(''), reviewers=Optional(None)):
|
||||
description_renderer=Optional(''),
|
||||
reviewers=Optional(None), observers=Optional(None)):
|
||||
"""
|
||||
Creates a new pull request.
|
||||
|
||||
|
|
@ -689,6 +705,13 @@ def create_pull_request(
|
|||
Accepts username strings or objects of the format:
|
||||
|
||||
[{'username': 'nick', 'reasons': ['original author'], 'mandatory': <bool>}]
|
||||
:param observers: Set the new pull request observers list.
|
||||
Reviewer defined by review rules will be added automatically to the
|
||||
defined list. This feature is only available in RhodeCode EE
|
||||
:type observers: Optional(list)
|
||||
Accepts username strings or objects of the format:
|
||||
|
||||
[{'username': 'nick', 'reasons': ['original author']}]
|
||||
"""
|
||||
|
||||
source_db_repo = get_repo_or_error(source_repo)
|
||||
|
|
@ -702,38 +725,39 @@ def create_pull_request(
|
|||
full_source_ref = resolve_ref_or_error(source_ref, source_db_repo)
|
||||
full_target_ref = resolve_ref_or_error(target_ref, target_db_repo)
|
||||
|
||||
source_commit = get_commit_or_error(full_source_ref, source_db_repo)
|
||||
target_commit = get_commit_or_error(full_target_ref, target_db_repo)
|
||||
get_commit_or_error(full_source_ref, source_db_repo)
|
||||
get_commit_or_error(full_target_ref, target_db_repo)
|
||||
|
||||
reviewer_objects = Optional.extract(reviewers) or []
|
||||
observer_objects = Optional.extract(observers) or []
|
||||
|
||||
# serialize and validate passed in given reviewers
|
||||
if reviewer_objects:
|
||||
schema = ReviewerListSchema()
|
||||
try:
|
||||
reviewer_objects = schema.deserialize(reviewer_objects)
|
||||
except Invalid as err:
|
||||
raise JSONRPCValidationError(colander_exc=err)
|
||||
reviewer_objects = _reviewers_validation(reviewer_objects)
|
||||
|
||||
# validate users
|
||||
for reviewer_object in reviewer_objects:
|
||||
user = get_user_or_error(reviewer_object['username'])
|
||||
reviewer_object['user_id'] = user.user_id
|
||||
if observer_objects:
|
||||
observer_objects = _reviewers_validation(reviewer_objects)
|
||||
|
||||
get_default_reviewers_data, validate_default_reviewers, validate_observers = \
|
||||
PullRequestModel().get_reviewer_functions()
|
||||
|
||||
source_ref_obj = unicode_to_reference(full_source_ref)
|
||||
target_ref_obj = unicode_to_reference(full_target_ref)
|
||||
|
||||
# recalculate reviewers logic, to make sure we can validate this
|
||||
default_reviewers_data = get_default_reviewers_data(
|
||||
owner,
|
||||
source_repo,
|
||||
Reference(source_type, source_name, source_commit_id),
|
||||
target_repo,
|
||||
Reference(target_type, target_name, target_commit_id)
|
||||
source_db_repo,
|
||||
source_ref_obj,
|
||||
target_db_repo,
|
||||
target_ref_obj,
|
||||
)
|
||||
|
||||
# now MERGE our given with the calculated
|
||||
reviewer_objects = default_reviewers_data['reviewers'] + reviewer_objects
|
||||
# now MERGE our given with the calculated from the default rules
|
||||
just_reviewers = [
|
||||
x for x in default_reviewers_data['reviewers']
|
||||
if x['role'] == PullRequestReviewers.ROLE_REVIEWER]
|
||||
reviewer_objects = just_reviewers + reviewer_objects
|
||||
|
||||
try:
|
||||
reviewers = validate_default_reviewers(
|
||||
|
|
@ -741,9 +765,21 @@ def create_pull_request(
|
|||
except ValueError as e:
|
||||
raise JSONRPCError('Reviewers Validation: {}'.format(e))
|
||||
|
||||
# now MERGE our given with the calculated from the default rules
|
||||
just_observers = [
|
||||
x for x in default_reviewers_data['reviewers']
|
||||
if x['role'] == PullRequestReviewers.ROLE_OBSERVER]
|
||||
observer_objects = just_observers + observer_objects
|
||||
|
||||
try:
|
||||
observers = validate_observers(
|
||||
observer_objects, default_reviewers_data)
|
||||
except ValueError as e:
|
||||
raise JSONRPCError('Observer Validation: {}'.format(e))
|
||||
|
||||
title = Optional.extract(title)
|
||||
if not title:
|
||||
title_source_ref = source_ref.split(':', 2)[1]
|
||||
title_source_ref = source_ref_obj.name
|
||||
title = PullRequestModel().generate_pullrequest_title(
|
||||
source=source_repo,
|
||||
source_ref=title_source_ref,
|
||||
|
|
@ -752,20 +788,17 @@ def create_pull_request(
|
|||
|
||||
diff_info = default_reviewers_data['diff_info']
|
||||
common_ancestor_id = diff_info['ancestor']
|
||||
commits = diff_info['commits']
|
||||
# NOTE(marcink): reversed is consistent with how we open it in the WEB interface
|
||||
commits = [commit['commit_id'] for commit in reversed(diff_info['commits'])]
|
||||
|
||||
if not common_ancestor_id:
|
||||
raise JSONRPCError('no common ancestor found')
|
||||
raise JSONRPCError('no common ancestor found between specified references')
|
||||
|
||||
if not commits:
|
||||
raise JSONRPCError('no commits found')
|
||||
|
||||
# NOTE(marcink): reversed is consistent with how we open it in the WEB interface
|
||||
revisions = [commit.raw_id for commit in reversed(commits)]
|
||||
raise JSONRPCError('no commits found for merge between specified references')
|
||||
|
||||
# recalculate target ref based on ancestor
|
||||
target_ref_type, target_ref_name, __ = full_target_ref.split(':')
|
||||
full_target_ref = ':'.join((target_ref_type, target_ref_name, common_ancestor_id))
|
||||
full_target_ref = ':'.join((target_ref_obj.type, target_ref_obj.name, common_ancestor_id))
|
||||
|
||||
# fetch renderer, if set fallback to plain in case of PR
|
||||
rc_config = SettingsModel().get_all_settings()
|
||||
|
|
@ -780,8 +813,9 @@ def create_pull_request(
|
|||
target_repo=target_repo,
|
||||
target_ref=full_target_ref,
|
||||
common_ancestor_id=common_ancestor_id,
|
||||
revisions=revisions,
|
||||
revisions=commits,
|
||||
reviewers=reviewers,
|
||||
observers=observers,
|
||||
title=title,
|
||||
description=description,
|
||||
description_renderer=description_renderer,
|
||||
|
|
@ -801,7 +835,7 @@ def create_pull_request(
|
|||
def update_pull_request(
|
||||
request, apiuser, pullrequestid, repoid=Optional(None),
|
||||
title=Optional(''), description=Optional(''), description_renderer=Optional(''),
|
||||
reviewers=Optional(None), update_commits=Optional(None)):
|
||||
reviewers=Optional(None), observers=Optional(None), update_commits=Optional(None)):
|
||||
"""
|
||||
Updates a pull request.
|
||||
|
||||
|
|
@ -823,7 +857,11 @@ def update_pull_request(
|
|||
Accepts username strings or objects of the format:
|
||||
|
||||
[{'username': 'nick', 'reasons': ['original author'], 'mandatory': <bool>}]
|
||||
:param observers: Update pull request observers list with new value.
|
||||
:type observers: Optional(list)
|
||||
Accepts username strings or objects of the format:
|
||||
|
||||
[{'username': 'nick', 'reasons': ['should be aware about this PR']}]
|
||||
:param update_commits: Trigger update of commits for this pull request
|
||||
:type: update_commits: Optional(bool)
|
||||
|
||||
|
|
@ -841,6 +879,12 @@ def update_pull_request(
|
|||
],
|
||||
"removed": []
|
||||
},
|
||||
"updated_observers": {
|
||||
"added": [
|
||||
"username"
|
||||
],
|
||||
"removed": []
|
||||
},
|
||||
"updated_commits": {
|
||||
"added": [
|
||||
"<sha1_hash>"
|
||||
|
|
@ -872,30 +916,7 @@ def update_pull_request(
|
|||
pullrequestid,))
|
||||
|
||||
reviewer_objects = Optional.extract(reviewers) or []
|
||||
|
||||
if reviewer_objects:
|
||||
schema = ReviewerListSchema()
|
||||
try:
|
||||
reviewer_objects = schema.deserialize(reviewer_objects)
|
||||
except Invalid as err:
|
||||
raise JSONRPCValidationError(colander_exc=err)
|
||||
|
||||
# validate users
|
||||
for reviewer_object in reviewer_objects:
|
||||
user = get_user_or_error(reviewer_object['username'])
|
||||
reviewer_object['user_id'] = user.user_id
|
||||
|
||||
get_default_reviewers_data, get_validated_reviewers, validate_observers = \
|
||||
PullRequestModel().get_reviewer_functions()
|
||||
|
||||
# re-use stored rules
|
||||
reviewer_rules = pull_request.reviewer_data
|
||||
try:
|
||||
reviewers = get_validated_reviewers(reviewer_objects, reviewer_rules)
|
||||
except ValueError as e:
|
||||
raise JSONRPCError('Reviewers Validation: {}'.format(e))
|
||||
else:
|
||||
reviewers = []
|
||||
observer_objects = Optional.extract(observers) or []
|
||||
|
||||
title = Optional.extract(title)
|
||||
description = Optional.extract(description)
|
||||
|
|
@ -935,12 +956,41 @@ def update_pull_request(
|
|||
commits_changed = True
|
||||
|
||||
# Update reviewers
|
||||
# serialize and validate passed in given reviewers
|
||||
if reviewer_objects:
|
||||
reviewer_objects = _reviewers_validation(reviewer_objects)
|
||||
|
||||
if observer_objects:
|
||||
observer_objects = _reviewers_validation(reviewer_objects)
|
||||
|
||||
# re-use stored rules
|
||||
default_reviewers_data = pull_request.reviewer_data
|
||||
|
||||
__, validate_default_reviewers, validate_observers = \
|
||||
PullRequestModel().get_reviewer_functions()
|
||||
|
||||
if reviewer_objects:
|
||||
try:
|
||||
reviewers = validate_default_reviewers(reviewer_objects, default_reviewers_data)
|
||||
except ValueError as e:
|
||||
raise JSONRPCError('Reviewers Validation: {}'.format(e))
|
||||
else:
|
||||
reviewers = []
|
||||
|
||||
if observer_objects:
|
||||
try:
|
||||
observers = validate_default_reviewers(reviewer_objects, default_reviewers_data)
|
||||
except ValueError as e:
|
||||
raise JSONRPCError('Observer Validation: {}'.format(e))
|
||||
else:
|
||||
observers = []
|
||||
|
||||
reviewers_changed = False
|
||||
reviewers_changes = {"added": [], "removed": []}
|
||||
if reviewers:
|
||||
old_calculated_status = pull_request.calculated_review_status()
|
||||
added_reviewers, removed_reviewers = \
|
||||
PullRequestModel().update_reviewers(pull_request, reviewers, apiuser)
|
||||
PullRequestModel().update_reviewers(pull_request, reviewers, apiuser.get_instance())
|
||||
|
||||
reviewers_changes['added'] = sorted(
|
||||
[get_user_or_error(n).username for n in added_reviewers])
|
||||
|
|
@ -957,6 +1007,18 @@ def update_pull_request(
|
|||
reviewers_changed = True
|
||||
|
||||
observers_changed = False
|
||||
observers_changes = {"added": [], "removed": []}
|
||||
if observers:
|
||||
added_observers, removed_observers = \
|
||||
PullRequestModel().update_observers(pull_request, observers, apiuser.get_instance())
|
||||
|
||||
observers_changes['added'] = sorted(
|
||||
[get_user_or_error(n).username for n in added_observers])
|
||||
observers_changes['removed'] = sorted(
|
||||
[get_user_or_error(n).username for n in removed_observers])
|
||||
Session().commit()
|
||||
|
||||
reviewers_changed = True
|
||||
|
||||
# push changed to channelstream
|
||||
if commits_changed or reviewers_changed or observers_changed:
|
||||
|
|
@ -966,11 +1028,11 @@ def update_pull_request(
|
|||
request, pr_broadcast_channel, apiuser, msg)
|
||||
|
||||
data = {
|
||||
'msg': 'Updated pull request `{}`'.format(
|
||||
pull_request.pull_request_id),
|
||||
'msg': 'Updated pull request `{}`'.format(pull_request.pull_request_id),
|
||||
'pull_request': pull_request.get_api_data(),
|
||||
'updated_commits': commit_changes,
|
||||
'updated_reviewers': reviewers_changes
|
||||
'updated_reviewers': reviewers_changes,
|
||||
'updated_observers': observers_changes,
|
||||
}
|
||||
|
||||
return data
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue