reviewers: store reviewer reasons to database, fixes #4238
This commit is contained in:
parent
5290e2381e
commit
b3e97c1715
17 changed files with 247 additions and 109 deletions
|
|
@ -95,9 +95,10 @@ class TestGetPullRequest(object):
|
|||
{
|
||||
'user': reviewer.get_api_data(include_secrets=False,
|
||||
details='basic'),
|
||||
'reasons': reasons,
|
||||
'review_status': st[0][1].status if st else 'not_reviewed',
|
||||
}
|
||||
for reviewer, st in pull_request.reviewers_statuses()
|
||||
for reviewer, reasons, st in pull_request.reviewers_statuses()
|
||||
]
|
||||
}
|
||||
assert_ok(id_, expected, response.body)
|
||||
|
|
|
|||
|
|
@ -463,7 +463,12 @@ def create_pull_request(
|
|||
:type description: Optional(str)
|
||||
:param reviewers: Set the new pull request reviewers list.
|
||||
:type reviewers: Optional(list)
|
||||
Accepts username strings or objects of the format:
|
||||
{
|
||||
'username': 'nick', 'reasons': ['original author']
|
||||
}
|
||||
"""
|
||||
|
||||
source = get_repo_or_error(source_repo)
|
||||
target = get_repo_or_error(target_repo)
|
||||
if not has_superadmin_permission(apiuser):
|
||||
|
|
@ -490,12 +495,21 @@ def create_pull_request(
|
|||
if not ancestor:
|
||||
raise JSONRPCError('no common ancestor found')
|
||||
|
||||
reviewer_names = Optional.extract(reviewers) or []
|
||||
if not isinstance(reviewer_names, list):
|
||||
reviewer_objects = Optional.extract(reviewers) or []
|
||||
if not isinstance(reviewer_objects, list):
|
||||
raise JSONRPCError('reviewers should be specified as a list')
|
||||
|
||||
reviewer_users = [get_user_or_error(n) for n in reviewer_names]
|
||||
reviewer_ids = [u.user_id for u in reviewer_users]
|
||||
reviewers_reasons = []
|
||||
for reviewer_object in reviewer_objects:
|
||||
reviewer_reasons = []
|
||||
if isinstance(reviewer_object, (basestring, int)):
|
||||
reviewer_username = reviewer_object
|
||||
else:
|
||||
reviewer_username = reviewer_object['username']
|
||||
reviewer_reasons = reviewer_object.get('reasons', [])
|
||||
|
||||
user = get_user_or_error(reviewer_username)
|
||||
reviewers_reasons.append((user.user_id, reviewer_reasons))
|
||||
|
||||
pull_request_model = PullRequestModel()
|
||||
pull_request = pull_request_model.create(
|
||||
|
|
@ -506,7 +520,7 @@ def create_pull_request(
|
|||
target_ref=full_target_ref,
|
||||
revisions=reversed(
|
||||
[commit.raw_id for commit in reversed(commit_ranges)]),
|
||||
reviewers=reviewer_ids,
|
||||
reviewers=reviewers_reasons,
|
||||
title=title,
|
||||
description=Optional.extract(description)
|
||||
)
|
||||
|
|
@ -585,12 +599,23 @@ def update_pull_request(
|
|||
'pull request `%s` update failed, pull request is closed' % (
|
||||
pullrequestid,))
|
||||
|
||||
reviewer_names = Optional.extract(reviewers) or []
|
||||
if not isinstance(reviewer_names, list):
|
||||
reviewer_objects = Optional.extract(reviewers) or []
|
||||
if not isinstance(reviewer_objects, list):
|
||||
raise JSONRPCError('reviewers should be specified as a list')
|
||||
|
||||
reviewer_users = [get_user_or_error(n) for n in reviewer_names]
|
||||
reviewer_ids = [u.user_id for u in reviewer_users]
|
||||
reviewers_reasons = []
|
||||
reviewer_ids = set()
|
||||
for reviewer_object in reviewer_objects:
|
||||
reviewer_reasons = []
|
||||
if isinstance(reviewer_object, (int, basestring)):
|
||||
reviewer_username = reviewer_object
|
||||
else:
|
||||
reviewer_username = reviewer_object['username']
|
||||
reviewer_reasons = reviewer_object.get('reasons', [])
|
||||
|
||||
user = get_user_or_error(reviewer_username)
|
||||
reviewer_ids.add(user.user_id)
|
||||
reviewers_reasons.append((user.user_id, reviewer_reasons))
|
||||
|
||||
title = Optional.extract(title)
|
||||
description = Optional.extract(description)
|
||||
|
|
@ -611,7 +636,7 @@ def update_pull_request(
|
|||
reviewers_changes = {"added": [], "removed": []}
|
||||
if reviewer_ids:
|
||||
added_reviewers, removed_reviewers = \
|
||||
PullRequestModel().update_reviewers(pull_request, reviewer_ids)
|
||||
PullRequestModel().update_reviewers(pull_request, reviewers_reasons)
|
||||
|
||||
reviewers_changes['added'] = sorted(
|
||||
[get_user_or_error(n).username for n in added_reviewers])
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@
|
|||
pull requests controller for rhodecode for initializing pull requests
|
||||
"""
|
||||
|
||||
import peppercorn
|
||||
import formencode
|
||||
import logging
|
||||
|
||||
|
|
@ -399,8 +400,10 @@ class PullrequestsController(BaseRepoController):
|
|||
if not repo:
|
||||
raise HTTPNotFound
|
||||
|
||||
controls = peppercorn.parse(request.POST.items())
|
||||
|
||||
try:
|
||||
_form = PullRequestForm(repo.repo_id)().to_python(request.POST)
|
||||
_form = PullRequestForm(repo.repo_id)().to_python(controls)
|
||||
except formencode.Invalid as errors:
|
||||
if errors.error_dict.get('revisions'):
|
||||
msg = 'Revisions: %s' % errors.error_dict['revisions']
|
||||
|
|
@ -419,7 +422,8 @@ class PullrequestsController(BaseRepoController):
|
|||
target_repo = _form['target_repo']
|
||||
target_ref = _form['target_ref']
|
||||
commit_ids = _form['revisions'][::-1]
|
||||
reviewers = _form['review_members']
|
||||
reviewers = [
|
||||
(r['user_id'], r['reasons']) for r in _form['review_members']]
|
||||
|
||||
# find the ancestor for this pr
|
||||
source_db_repo = Repository.get_by_repo_name(_form['source_repo'])
|
||||
|
|
@ -478,8 +482,11 @@ class PullrequestsController(BaseRepoController):
|
|||
allowed_to_update = PullRequestModel().check_user_update(
|
||||
pull_request, c.rhodecode_user)
|
||||
if allowed_to_update:
|
||||
if 'reviewers_ids' in request.POST:
|
||||
self._update_reviewers(pull_request_id)
|
||||
controls = peppercorn.parse(request.POST.items())
|
||||
|
||||
if 'review_members' in controls:
|
||||
self._update_reviewers(
|
||||
pull_request_id, controls['review_members'])
|
||||
elif str2bool(request.POST.get('update_commits', 'false')):
|
||||
self._update_commits(pull_request)
|
||||
elif str2bool(request.POST.get('close_pull_request', 'false')):
|
||||
|
|
@ -631,11 +638,10 @@ class PullrequestsController(BaseRepoController):
|
|||
merge_resp.failure_reason)
|
||||
h.flash(msg, category='error')
|
||||
|
||||
def _update_reviewers(self, pull_request_id):
|
||||
reviewers_ids = map(int, filter(
|
||||
lambda v: v not in [None, ''],
|
||||
request.POST.get('reviewers_ids', '').split(',')))
|
||||
PullRequestModel().update_reviewers(pull_request_id, reviewers_ids)
|
||||
def _update_reviewers(self, pull_request_id, review_members):
|
||||
reviewers = [
|
||||
(int(r['user_id']), r['reasons']) for r in review_members]
|
||||
PullRequestModel().update_reviewers(pull_request_id, reviewers)
|
||||
Session().commit()
|
||||
|
||||
def _reject_close(self, pull_request):
|
||||
|
|
|
|||
|
|
@ -59,7 +59,7 @@ from rhodecode.lib.utils2 import (
|
|||
str2bool, safe_str, get_commit_safe, safe_unicode, remove_prefix, md5_safe,
|
||||
time_to_datetime, aslist, Optional, safe_int, get_clone_url, AttributeDict,
|
||||
glob2re)
|
||||
from rhodecode.lib.jsonalchemy import MutationObj, JsonType, JSONDict
|
||||
from rhodecode.lib.jsonalchemy import MutationObj, MutationList, JsonType, JSONDict
|
||||
from rhodecode.lib.ext_json import json
|
||||
from rhodecode.lib.caching_query import FromCache
|
||||
from rhodecode.lib.encrypt import AESCipher
|
||||
|
|
@ -3149,9 +3149,10 @@ class PullRequest(Base, _PullRequestBase):
|
|||
{
|
||||
'user': reviewer.get_api_data(include_secrets=False,
|
||||
details='basic'),
|
||||
'reasons': reasons,
|
||||
'review_status': st[0][1].status if st else 'not_reviewed',
|
||||
}
|
||||
for reviewer, st in pull_request.reviewers_statuses()
|
||||
for reviewer, reasons, st in pull_request.reviewers_statuses()
|
||||
]
|
||||
}
|
||||
|
||||
|
|
@ -3203,9 +3204,23 @@ class PullRequestReviewers(Base, BaseModel):
|
|||
'mysql_charset': 'utf8', 'sqlite_autoincrement': True},
|
||||
)
|
||||
|
||||
def __init__(self, user=None, pull_request=None):
|
||||
def __init__(self, user=None, pull_request=None, reasons=None):
|
||||
self.user = user
|
||||
self.pull_request = pull_request
|
||||
self.reasons = reasons or []
|
||||
|
||||
@hybrid_property
|
||||
def reasons(self):
|
||||
if not self._reasons:
|
||||
return []
|
||||
return self._reasons
|
||||
|
||||
@reasons.setter
|
||||
def reasons(self, val):
|
||||
val = val or []
|
||||
if any(not isinstance(x, basestring) for x in val):
|
||||
raise Exception('invalid reasons type, must be list of strings')
|
||||
self._reasons = val
|
||||
|
||||
pull_requests_reviewers_id = Column(
|
||||
'pull_requests_reviewers_id', Integer(), nullable=False,
|
||||
|
|
@ -3215,8 +3230,9 @@ class PullRequestReviewers(Base, BaseModel):
|
|||
ForeignKey('pull_requests.pull_request_id'), nullable=False)
|
||||
user_id = Column(
|
||||
"user_id", Integer(), ForeignKey('users.user_id'), nullable=True)
|
||||
reason = Column('reason',
|
||||
UnicodeText().with_variant(UnicodeText(255), 'mysql'), nullable=True)
|
||||
_reasons = Column(
|
||||
'reason', MutationList.as_mutable(
|
||||
JsonType('list', dialect_map=dict(mysql=UnicodeText(16384)))))
|
||||
|
||||
user = relationship('User')
|
||||
pull_request = relationship('PullRequest')
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ def upgrade(migrate_engine):
|
|||
_reset_base(migrate_engine)
|
||||
from rhodecode.lib.dbmigrate.schema import db_4_5_0_0
|
||||
|
||||
db_4_5_0_0.PullRequestReviewers.reason.create(
|
||||
db_4_5_0_0.PullRequestReviewers.reasons.create(
|
||||
table=db_4_5_0_0.PullRequestReviewers.__table__)
|
||||
|
||||
def downgrade(migrate_engine):
|
||||
|
|
|
|||
|
|
@ -80,7 +80,7 @@ class ChangesetStatusModel(BaseModel):
|
|||
"""
|
||||
votes = defaultdict(int)
|
||||
reviewers_number = len(statuses_by_reviewers)
|
||||
for user, statuses in statuses_by_reviewers:
|
||||
for user, reasons, statuses in statuses_by_reviewers:
|
||||
if statuses:
|
||||
ver, latest = statuses[0]
|
||||
votes[latest.status] += 1
|
||||
|
|
@ -254,7 +254,7 @@ class ChangesetStatusModel(BaseModel):
|
|||
for x, y in (itertools.groupby(sorted(st, key=version),
|
||||
version))]
|
||||
|
||||
pull_request_reviewers.append([o.user, st])
|
||||
pull_request_reviewers.append((o.user, o.reasons, st))
|
||||
return pull_request_reviewers
|
||||
|
||||
def calculated_review_status(self, pull_request, reviewers_statuses=None):
|
||||
|
|
|
|||
|
|
@ -59,7 +59,7 @@ from rhodecode.lib.utils2 import (
|
|||
str2bool, safe_str, get_commit_safe, safe_unicode, remove_prefix, md5_safe,
|
||||
time_to_datetime, aslist, Optional, safe_int, get_clone_url, AttributeDict,
|
||||
glob2re)
|
||||
from rhodecode.lib.jsonalchemy import MutationObj, JsonType, JSONDict
|
||||
from rhodecode.lib.jsonalchemy import MutationObj, MutationList, JsonType, JSONDict
|
||||
from rhodecode.lib.ext_json import json
|
||||
from rhodecode.lib.caching_query import FromCache
|
||||
from rhodecode.lib.encrypt import AESCipher
|
||||
|
|
@ -3149,9 +3149,10 @@ class PullRequest(Base, _PullRequestBase):
|
|||
{
|
||||
'user': reviewer.get_api_data(include_secrets=False,
|
||||
details='basic'),
|
||||
'reasons': reasons,
|
||||
'review_status': st[0][1].status if st else 'not_reviewed',
|
||||
}
|
||||
for reviewer, st in pull_request.reviewers_statuses()
|
||||
for reviewer, reasons, st in pull_request.reviewers_statuses()
|
||||
]
|
||||
}
|
||||
|
||||
|
|
@ -3203,9 +3204,23 @@ class PullRequestReviewers(Base, BaseModel):
|
|||
'mysql_charset': 'utf8', 'sqlite_autoincrement': True},
|
||||
)
|
||||
|
||||
def __init__(self, user=None, pull_request=None):
|
||||
def __init__(self, user=None, pull_request=None, reasons=None):
|
||||
self.user = user
|
||||
self.pull_request = pull_request
|
||||
self.reasons = reasons or []
|
||||
|
||||
@hybrid_property
|
||||
def reasons(self):
|
||||
if not self._reasons:
|
||||
return []
|
||||
return self._reasons
|
||||
|
||||
@reasons.setter
|
||||
def reasons(self, val):
|
||||
val = val or []
|
||||
if any(not isinstance(x, basestring) for x in val):
|
||||
raise Exception('invalid reasons type, must be list of strings')
|
||||
self._reasons = val
|
||||
|
||||
pull_requests_reviewers_id = Column(
|
||||
'pull_requests_reviewers_id', Integer(), nullable=False,
|
||||
|
|
@ -3215,8 +3230,9 @@ class PullRequestReviewers(Base, BaseModel):
|
|||
ForeignKey('pull_requests.pull_request_id'), nullable=False)
|
||||
user_id = Column(
|
||||
"user_id", Integer(), ForeignKey('users.user_id'), nullable=True)
|
||||
reason = Column('reason',
|
||||
UnicodeText().with_variant(UnicodeText(255), 'mysql'), nullable=True)
|
||||
_reasons = Column(
|
||||
'reason', MutationList.as_mutable(
|
||||
JsonType('list', dialect_map=dict(mysql=UnicodeText(16384)))))
|
||||
|
||||
user = relationship('User')
|
||||
pull_request = relationship('PullRequest')
|
||||
|
|
|
|||
|
|
@ -519,7 +519,12 @@ def UserExtraIpForm():
|
|||
return _UserExtraIpForm
|
||||
|
||||
|
||||
|
||||
def PullRequestForm(repo_id):
|
||||
class ReviewerForm(formencode.Schema):
|
||||
user_id = v.Int(not_empty=True)
|
||||
reasons = All()
|
||||
|
||||
class _PullRequestForm(formencode.Schema):
|
||||
allow_extra_fields = True
|
||||
filter_extra_fields = True
|
||||
|
|
@ -531,8 +536,7 @@ def PullRequestForm(repo_id):
|
|||
target_ref = v.UnicodeString(strip=True, required=True)
|
||||
revisions = All(#v.NotReviewedRevisions(repo_id)(),
|
||||
v.UniqueList()(not_empty=True))
|
||||
review_members = v.UniqueList(convert=int)(not_empty=True)
|
||||
|
||||
review_members = formencode.ForEach(ReviewerForm())
|
||||
pullrequest_title = v.UnicodeString(strip=True, required=True)
|
||||
pullrequest_desc = v.UnicodeString(strip=True, required=False)
|
||||
|
||||
|
|
|
|||
|
|
@ -333,10 +333,18 @@ class PullRequestModel(BaseModel):
|
|||
Session().add(pull_request)
|
||||
Session().flush()
|
||||
|
||||
reviewer_ids = set()
|
||||
# members / reviewers
|
||||
for user_id in set(reviewers):
|
||||
for reviewer_object in reviewers:
|
||||
if isinstance(reviewer_object, tuple):
|
||||
user_id, reasons = reviewer_object
|
||||
else:
|
||||
user_id, reasons = reviewer_object, []
|
||||
|
||||
user = self._get_user(user_id)
|
||||
reviewer = PullRequestReviewers(user, pull_request)
|
||||
reviewer_ids.add(user.user_id)
|
||||
|
||||
reviewer = PullRequestReviewers(user, pull_request, reasons)
|
||||
Session().add(reviewer)
|
||||
|
||||
# Set approval status to "Under Review" for all commits which are
|
||||
|
|
@ -348,7 +356,7 @@ class PullRequestModel(BaseModel):
|
|||
pull_request=pull_request
|
||||
)
|
||||
|
||||
self.notify_reviewers(pull_request, reviewers)
|
||||
self.notify_reviewers(pull_request, reviewer_ids)
|
||||
self._trigger_pull_request_hook(
|
||||
pull_request, created_by_user, 'create')
|
||||
|
||||
|
|
@ -570,6 +578,7 @@ class PullRequestModel(BaseModel):
|
|||
Session().commit()
|
||||
self._trigger_pull_request_hook(pull_request, pull_request.author,
|
||||
'update')
|
||||
|
||||
return (pull_request_version, changes)
|
||||
|
||||
def _create_version_from_snapshot(self, pull_request):
|
||||
|
|
@ -711,8 +720,21 @@ class PullRequestModel(BaseModel):
|
|||
pull_request.updated_on = datetime.datetime.now()
|
||||
Session().add(pull_request)
|
||||
|
||||
def update_reviewers(self, pull_request, reviewers_ids):
|
||||
reviewers_ids = set(reviewers_ids)
|
||||
def update_reviewers(self, pull_request, reviewer_data):
|
||||
"""
|
||||
Update the reviewers in the pull request
|
||||
|
||||
:param pull_request: the pr to update
|
||||
:param reviewer_data: list of tuples [(user, ['reason1', 'reason2'])]
|
||||
"""
|
||||
|
||||
reviewers_reasons = {}
|
||||
for user_id, reasons in reviewer_data:
|
||||
if isinstance(user_id, (int, basestring)):
|
||||
user_id = self._get_user(user_id).user_id
|
||||
reviewers_reasons[user_id] = reasons
|
||||
|
||||
reviewers_ids = set(reviewers_reasons.keys())
|
||||
pull_request = self.__get_pull_request(pull_request)
|
||||
current_reviewers = PullRequestReviewers.query()\
|
||||
.filter(PullRequestReviewers.pull_request ==
|
||||
|
|
@ -728,7 +750,8 @@ class PullRequestModel(BaseModel):
|
|||
for uid in ids_to_add:
|
||||
changed = True
|
||||
_usr = self._get_user(uid)
|
||||
reviewer = PullRequestReviewers(_usr, pull_request)
|
||||
reasons = reviewers_reasons[uid]
|
||||
reviewer = PullRequestReviewers(_usr, pull_request, reasons)
|
||||
Session().add(reviewer)
|
||||
|
||||
self.notify_reviewers(pull_request, ids_to_add)
|
||||
|
|
|
|||
|
|
@ -145,7 +145,7 @@ input.inline[type="file"] {
|
|||
.alert {
|
||||
margin: @padding 0;
|
||||
}
|
||||
|
||||
|
||||
.error-branding {
|
||||
font-family: @text-semibold;
|
||||
color: @grey4;
|
||||
|
|
@ -1282,6 +1282,9 @@ table.integrations {
|
|||
width: 100%;
|
||||
overflow: auto;
|
||||
}
|
||||
.reviewer_reason {
|
||||
padding-left: 20px;
|
||||
}
|
||||
.reviewer_status {
|
||||
display: inline-block;
|
||||
vertical-align: top;
|
||||
|
|
|
|||
|
|
@ -32,7 +32,7 @@ var removeReviewMember = function(reviewer_id, mark_delete){
|
|||
var obj = $('#reviewer_{0}_name'.format(reviewer_id));
|
||||
obj.addClass('to-delete');
|
||||
// now delete the input
|
||||
$('#reviewer_{0}_input'.format(reviewer_id)).remove();
|
||||
$('#reviewer_{0} input'.format(reviewer_id)).remove();
|
||||
}
|
||||
}
|
||||
else{
|
||||
|
|
@ -43,29 +43,36 @@ var removeReviewMember = function(reviewer_id, mark_delete){
|
|||
var addReviewMember = function(id, fname, lname, nname, gravatar_link, reasons) {
|
||||
var members = $('#review_members').get(0);
|
||||
var reasons_html = '';
|
||||
var reasons_inputs = '';
|
||||
var reasons = reasons || [];
|
||||
if (reasons) {
|
||||
for (var i = 0; i < reasons.length; i++) {
|
||||
reasons_html += '<div class="reviewer_reason">- {0}</div>'.format(
|
||||
reasons[i]
|
||||
);
|
||||
reasons_html += '<div class="reviewer_reason">- {0}</div>'.format(reasons[i]);
|
||||
reasons_inputs += '<input type="hidden" name="reason" value="' + escapeHtml(reasons[i]) + '">';
|
||||
}
|
||||
}
|
||||
var tmpl = '<li id="reviewer_{2}">'+
|
||||
'<input type="hidden" name="__start__" value="reviewer:mapping">'+
|
||||
'<div class="reviewer_status">'+
|
||||
'<div class="flag_status not_reviewed pull-left reviewer_member_status"></div>'+
|
||||
'</div>'+
|
||||
'<img alt="gravatar" class="gravatar" src="{0}"/>'+
|
||||
'<span class="reviewer_name user">{1}</span>'+
|
||||
reasons_html +
|
||||
'<input type="hidden" value="{2}" name="review_members" />'+
|
||||
'<input type="hidden" name="user_id" value="{2}">'+
|
||||
'<input type="hidden" name="__start__" value="reasons:sequence">'+
|
||||
'{3}'+
|
||||
'<input type="hidden" name="__end__" value="reasons:sequence">'+
|
||||
'<div class="reviewer_member_remove action_button" onclick="removeReviewMember({2})">' +
|
||||
'<i class="icon-remove-sign"></i>'+
|
||||
'</div>'+
|
||||
'</div>'+
|
||||
'<input type="hidden" name="__end__" value="reviewer:mapping">'+
|
||||
'</li>' ;
|
||||
|
||||
var displayname = "{0} ({1} {2})".format(
|
||||
nname, escapeHtml(fname), escapeHtml(lname));
|
||||
var element = tmpl.format(gravatar_link,displayname,id);
|
||||
var element = tmpl.format(gravatar_link,displayname,id,reasons_inputs);
|
||||
// check if we don't have this ID already in
|
||||
var ids = [];
|
||||
var _els = $('#review_members li').toArray();
|
||||
|
|
@ -83,7 +90,11 @@ var _updatePullRequest = function(repo_name, pull_request_id, postData) {
|
|||
var url = pyroutes.url(
|
||||
'pullrequest_update',
|
||||
{"repo_name": repo_name, "pull_request_id": pull_request_id});
|
||||
postData.csrf_token = CSRF_TOKEN;
|
||||
if (typeof postData === 'string' ) {
|
||||
postData += '&csrf_token=' + CSRF_TOKEN;
|
||||
} else {
|
||||
postData.csrf_token = CSRF_TOKEN;
|
||||
}
|
||||
var success = function(o) {
|
||||
window.location.reload();
|
||||
};
|
||||
|
|
@ -92,17 +103,9 @@ var _updatePullRequest = function(repo_name, pull_request_id, postData) {
|
|||
|
||||
var updateReviewers = function(reviewers_ids, repo_name, pull_request_id){
|
||||
if (reviewers_ids === undefined){
|
||||
var reviewers_ids = [];
|
||||
var ids = $('#review_members input').toArray();
|
||||
for(var i=0; i<ids.length;i++){
|
||||
var id = ids[i].value
|
||||
reviewers_ids.push(id);
|
||||
}
|
||||
var postData = '_method=put&' + $('#reviewers input').serialize();
|
||||
_updatePullRequest(repo_name, pull_request_id, postData);
|
||||
}
|
||||
var postData = {
|
||||
'_method':'put',
|
||||
'reviewers_ids': reviewers_ids};
|
||||
_updatePullRequest(repo_name, pull_request_id, postData);
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -20,6 +20,9 @@
|
|||
* turns objects into GET query string
|
||||
*/
|
||||
var toQueryString = function(o) {
|
||||
if(typeof o === 'string') {
|
||||
return o;
|
||||
}
|
||||
if(typeof o !== 'object') {
|
||||
return false;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -111,7 +111,9 @@
|
|||
</div>
|
||||
<div id="reviewers" class="block-right pr-details-content reviewers">
|
||||
## members goes here, filled via JS based on initial selection !
|
||||
<input type="hidden" name="__start__" value="review_members:sequence">
|
||||
<ul id="review_members" class="group_members"></ul>
|
||||
<input type="hidden" name="__end__" value="review_members:sequence">
|
||||
<div id="add_reviewer_input" class='ac'>
|
||||
<div class="reviewer_ac">
|
||||
${h.text('user', class_='ac-input', placeholder=_('Add reviewer'))}
|
||||
|
|
|
|||
|
|
@ -188,17 +188,27 @@
|
|||
</div>
|
||||
<div id="reviewers" class="block-right pr-details-content reviewers">
|
||||
## members goes here !
|
||||
<input type="hidden" name="__start__" value="review_members:sequence">
|
||||
<ul id="review_members" class="group_members">
|
||||
%for member,status in c.pull_request_reviewers:
|
||||
%for member,reasons,status in c.pull_request_reviewers:
|
||||
<li id="reviewer_${member.user_id}">
|
||||
<div class="reviewers_member">
|
||||
<div class="reviewer_status tooltip" title="${h.tooltip(h.commit_status_lbl(status[0][1].status if status else 'not_reviewed'))}">
|
||||
<div class="${'flag_status %s' % (status[0][1].status if status else 'not_reviewed')} pull-left reviewer_member_status"></div>
|
||||
</div>
|
||||
<div id="reviewer_${member.user_id}_name" class="reviewer_name">
|
||||
${self.gravatar_with_user(member.email, 16)} <div class="reviewer">(${_('owner') if c.pull_request.user_id == member.user_id else _('reviewer')})</div>
|
||||
${self.gravatar_with_user(member.email, 16)}
|
||||
</div>
|
||||
<input id="reviewer_${member.user_id}_input" type="hidden" value="${member.user_id}" name="review_members" />
|
||||
<input type="hidden" name="__start__" value="reviewer:mapping">
|
||||
<input type="hidden" name="__start__" value="reasons:sequence">
|
||||
%for reason in reasons:
|
||||
<div class="reviewer_reason">- ${reason}</div>
|
||||
<input type="hidden" name="reason" value="${reason}">
|
||||
|
||||
%endfor
|
||||
<input type="hidden" name="__end__" value="reasons:sequence">
|
||||
<input id="reviewer_${member.user_id}_input" type="hidden" value="${member.user_id}" name="user_id" />
|
||||
<input type="hidden" name="__end__" value="reviewer:mapping">
|
||||
%if c.allowed_to_update:
|
||||
<div class="reviewer_member_remove action_button" onclick="removeReviewMember(${member.user_id}, true)" style="visibility: hidden;">
|
||||
<i class="icon-remove-sign" ></i>
|
||||
|
|
@ -208,6 +218,7 @@
|
|||
</li>
|
||||
%endfor
|
||||
</ul>
|
||||
<input type="hidden" name="__end__" value="review_members:sequence">
|
||||
%if not c.pull_request.is_closed():
|
||||
<div id="add_reviewer_input" class='ac' style="display: none;">
|
||||
%if c.allowed_to_update:
|
||||
|
|
@ -569,7 +580,7 @@
|
|||
$('.show-inline-comments').on('click', function(e){
|
||||
var boxid = $(this).attr('data-comment-id');
|
||||
var button = $(this);
|
||||
|
||||
|
||||
if(button.hasClass("comments-visible")) {
|
||||
$('#{0} .inline-comments'.format(boxid)).each(function(index){
|
||||
$(this).hide();
|
||||
|
|
|
|||
|
|
@ -256,8 +256,8 @@ class TestPullrequestsController:
|
|||
def test_comment_force_close_pull_request(self, pr_util, csrf_token):
|
||||
pull_request = pr_util.create_pull_request()
|
||||
pull_request_id = pull_request.pull_request_id
|
||||
reviewers_ids = [1, 2]
|
||||
PullRequestModel().update_reviewers(pull_request_id, reviewers_ids)
|
||||
reviewers_data = [(1, ['reason']), (2, ['reason2'])]
|
||||
PullRequestModel().update_reviewers(pull_request_id, reviewers_data)
|
||||
author = pull_request.user_id
|
||||
repo = pull_request.target_repo.repo_id
|
||||
self.app.post(
|
||||
|
|
@ -297,19 +297,27 @@ class TestPullrequestsController:
|
|||
url(
|
||||
controller='pullrequests',
|
||||
action='create',
|
||||
repo_name=source.repo_name),
|
||||
params={
|
||||
'source_repo': source.repo_name,
|
||||
'source_ref': 'branch:default:' + commit_ids['change'],
|
||||
'target_repo': target.repo_name,
|
||||
'target_ref': 'branch:default:' + commit_ids['ancestor'],
|
||||
'pullrequest_desc': 'Description',
|
||||
'pullrequest_title': 'Title',
|
||||
'review_members': '1',
|
||||
'revisions': commit_ids['change'],
|
||||
'user': '',
|
||||
'csrf_token': csrf_token,
|
||||
},
|
||||
repo_name=source.repo_name
|
||||
),
|
||||
[
|
||||
('source_repo', source.repo_name),
|
||||
('source_ref', 'branch:default:' + commit_ids['change']),
|
||||
('target_repo', target.repo_name),
|
||||
('target_ref', 'branch:default:' + commit_ids['ancestor']),
|
||||
('pullrequest_desc', 'Description'),
|
||||
('pullrequest_title', 'Title'),
|
||||
('__start__', 'review_members:sequence'),
|
||||
('__start__', 'reviewer:mapping'),
|
||||
('user_id', '1'),
|
||||
('__start__', 'reasons:sequence'),
|
||||
('reason', 'Some reason'),
|
||||
('__end__', 'reasons:sequence'),
|
||||
('__end__', 'reviewer:mapping'),
|
||||
('__end__', 'review_members:sequence'),
|
||||
('revisions', commit_ids['change']),
|
||||
('user', ''),
|
||||
('csrf_token', csrf_token),
|
||||
],
|
||||
status=302)
|
||||
|
||||
location = response.headers['Location']
|
||||
|
|
@ -344,19 +352,27 @@ class TestPullrequestsController:
|
|||
url(
|
||||
controller='pullrequests',
|
||||
action='create',
|
||||
repo_name=source.repo_name),
|
||||
params={
|
||||
'source_repo': source.repo_name,
|
||||
'source_ref': 'branch:default:' + commit_ids['change'],
|
||||
'target_repo': target.repo_name,
|
||||
'target_ref': 'branch:default:' + commit_ids['ancestor-child'],
|
||||
'pullrequest_desc': 'Description',
|
||||
'pullrequest_title': 'Title',
|
||||
'review_members': '2',
|
||||
'revisions': commit_ids['change'],
|
||||
'user': '',
|
||||
'csrf_token': csrf_token,
|
||||
},
|
||||
repo_name=source.repo_name
|
||||
),
|
||||
[
|
||||
('source_repo', source.repo_name),
|
||||
('source_ref', 'branch:default:' + commit_ids['change']),
|
||||
('target_repo', target.repo_name),
|
||||
('target_ref', 'branch:default:' + commit_ids['ancestor-child']),
|
||||
('pullrequest_desc', 'Description'),
|
||||
('pullrequest_title', 'Title'),
|
||||
('__start__', 'review_members:sequence'),
|
||||
('__start__', 'reviewer:mapping'),
|
||||
('user_id', '2'),
|
||||
('__start__', 'reasons:sequence'),
|
||||
('reason', 'Some reason'),
|
||||
('__end__', 'reasons:sequence'),
|
||||
('__end__', 'reviewer:mapping'),
|
||||
('__end__', 'review_members:sequence'),
|
||||
('revisions', commit_ids['change']),
|
||||
('user', ''),
|
||||
('csrf_token', csrf_token),
|
||||
],
|
||||
status=302)
|
||||
|
||||
location = response.headers['Location']
|
||||
|
|
@ -373,7 +389,8 @@ class TestPullrequestsController:
|
|||
assert len(notifications.all()) == 1
|
||||
|
||||
# Change reviewers and check that a notification was made
|
||||
PullRequestModel().update_reviewers(pull_request.pull_request_id, [1])
|
||||
PullRequestModel().update_reviewers(
|
||||
pull_request.pull_request_id, [(1, [])])
|
||||
assert len(notifications.all()) == 2
|
||||
|
||||
def test_create_pull_request_stores_ancestor_commit_id(self, backend,
|
||||
|
|
@ -397,19 +414,27 @@ class TestPullrequestsController:
|
|||
url(
|
||||
controller='pullrequests',
|
||||
action='create',
|
||||
repo_name=source.repo_name),
|
||||
params={
|
||||
'source_repo': source.repo_name,
|
||||
'source_ref': 'branch:default:' + commit_ids['change'],
|
||||
'target_repo': target.repo_name,
|
||||
'target_ref': 'branch:default:' + commit_ids['ancestor-child'],
|
||||
'pullrequest_desc': 'Description',
|
||||
'pullrequest_title': 'Title',
|
||||
'review_members': '1',
|
||||
'revisions': commit_ids['change'],
|
||||
'user': '',
|
||||
'csrf_token': csrf_token,
|
||||
},
|
||||
repo_name=source.repo_name
|
||||
),
|
||||
[
|
||||
('source_repo', source.repo_name),
|
||||
('source_ref', 'branch:default:' + commit_ids['change']),
|
||||
('target_repo', target.repo_name),
|
||||
('target_ref', 'branch:default:' + commit_ids['ancestor-child']),
|
||||
('pullrequest_desc', 'Description'),
|
||||
('pullrequest_title', 'Title'),
|
||||
('__start__', 'review_members:sequence'),
|
||||
('__start__', 'reviewer:mapping'),
|
||||
('user_id', '1'),
|
||||
('__start__', 'reasons:sequence'),
|
||||
('reason', 'Some reason'),
|
||||
('__end__', 'reasons:sequence'),
|
||||
('__end__', 'reviewer:mapping'),
|
||||
('__end__', 'review_members:sequence'),
|
||||
('revisions', commit_ids['change']),
|
||||
('user', ''),
|
||||
('csrf_token', csrf_token),
|
||||
],
|
||||
status=302)
|
||||
|
||||
location = response.headers['Location']
|
||||
|
|
|
|||
|
|
@ -80,7 +80,7 @@ def test_pull_request_stays_if_update_without_change(pr_util, voted_status):
|
|||
voted_status, *pull_request.reviewers)
|
||||
|
||||
# Update, without change
|
||||
PullRequestModel().update_commits(pull_request)
|
||||
version, changes = PullRequestModel().update_commits(pull_request)
|
||||
|
||||
# Expect that review status is the voted_status
|
||||
expected_review_status = voted_status
|
||||
|
|
@ -99,7 +99,7 @@ def test_pull_request_under_review_if_update(pr_util, voted_status):
|
|||
|
||||
# Update, with change
|
||||
pr_util.update_source_repository()
|
||||
PullRequestModel().update_commits(pull_request)
|
||||
version, changes = PullRequestModel().update_commits(pull_request)
|
||||
|
||||
# Expect that review status is the voted_status
|
||||
expected_review_status = db.ChangesetStatus.STATUS_UNDER_REVIEW
|
||||
|
|
|
|||
|
|
@ -115,7 +115,7 @@ class TestPullRequestModel:
|
|||
|
||||
def test_get_awaiting_my_review(self, pull_request):
|
||||
PullRequestModel().update_reviewers(
|
||||
pull_request, [pull_request.author])
|
||||
pull_request, [(pull_request.author, ['author'])])
|
||||
prs = PullRequestModel().get_awaiting_my_review(
|
||||
pull_request.target_repo, user_id=pull_request.author.user_id)
|
||||
assert isinstance(prs, list)
|
||||
|
|
@ -123,7 +123,7 @@ class TestPullRequestModel:
|
|||
|
||||
def test_count_awaiting_my_review(self, pull_request):
|
||||
PullRequestModel().update_reviewers(
|
||||
pull_request, [pull_request.author])
|
||||
pull_request, [(pull_request.author, ['author'])])
|
||||
pr_count = PullRequestModel().count_awaiting_my_review(
|
||||
pull_request.target_repo, user_id=pull_request.author.user_id)
|
||||
assert pr_count == 1
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue