##// END OF EJS Templates
reviewers: store reviewer reasons to database, fixes #4238
dan -
r873:930d1a1f default
parent child Browse files
Show More
@@ -95,9 +95,10 b' class TestGetPullRequest(object):'
95 {
95 {
96 'user': reviewer.get_api_data(include_secrets=False,
96 'user': reviewer.get_api_data(include_secrets=False,
97 details='basic'),
97 details='basic'),
98 'reasons': reasons,
98 'review_status': st[0][1].status if st else 'not_reviewed',
99 'review_status': st[0][1].status if st else 'not_reviewed',
99 }
100 }
100 for reviewer, st in pull_request.reviewers_statuses()
101 for reviewer, reasons, st in pull_request.reviewers_statuses()
101 ]
102 ]
102 }
103 }
103 assert_ok(id_, expected, response.body)
104 assert_ok(id_, expected, response.body)
@@ -463,7 +463,12 b' def create_pull_request('
463 :type description: Optional(str)
463 :type description: Optional(str)
464 :param reviewers: Set the new pull request reviewers list.
464 :param reviewers: Set the new pull request reviewers list.
465 :type reviewers: Optional(list)
465 :type reviewers: Optional(list)
466 Accepts username strings or objects of the format:
467 {
468 'username': 'nick', 'reasons': ['original author']
469 }
466 """
470 """
471
467 source = get_repo_or_error(source_repo)
472 source = get_repo_or_error(source_repo)
468 target = get_repo_or_error(target_repo)
473 target = get_repo_or_error(target_repo)
469 if not has_superadmin_permission(apiuser):
474 if not has_superadmin_permission(apiuser):
@@ -490,12 +495,21 b' def create_pull_request('
490 if not ancestor:
495 if not ancestor:
491 raise JSONRPCError('no common ancestor found')
496 raise JSONRPCError('no common ancestor found')
492
497
493 reviewer_names = Optional.extract(reviewers) or []
498 reviewer_objects = Optional.extract(reviewers) or []
494 if not isinstance(reviewer_names, list):
499 if not isinstance(reviewer_objects, list):
495 raise JSONRPCError('reviewers should be specified as a list')
500 raise JSONRPCError('reviewers should be specified as a list')
496
501
497 reviewer_users = [get_user_or_error(n) for n in reviewer_names]
502 reviewers_reasons = []
498 reviewer_ids = [u.user_id for u in reviewer_users]
503 for reviewer_object in reviewer_objects:
504 reviewer_reasons = []
505 if isinstance(reviewer_object, (basestring, int)):
506 reviewer_username = reviewer_object
507 else:
508 reviewer_username = reviewer_object['username']
509 reviewer_reasons = reviewer_object.get('reasons', [])
510
511 user = get_user_or_error(reviewer_username)
512 reviewers_reasons.append((user.user_id, reviewer_reasons))
499
513
500 pull_request_model = PullRequestModel()
514 pull_request_model = PullRequestModel()
501 pull_request = pull_request_model.create(
515 pull_request = pull_request_model.create(
@@ -506,7 +520,7 b' def create_pull_request('
506 target_ref=full_target_ref,
520 target_ref=full_target_ref,
507 revisions=reversed(
521 revisions=reversed(
508 [commit.raw_id for commit in reversed(commit_ranges)]),
522 [commit.raw_id for commit in reversed(commit_ranges)]),
509 reviewers=reviewer_ids,
523 reviewers=reviewers_reasons,
510 title=title,
524 title=title,
511 description=Optional.extract(description)
525 description=Optional.extract(description)
512 )
526 )
@@ -585,12 +599,23 b' def update_pull_request('
585 'pull request `%s` update failed, pull request is closed' % (
599 'pull request `%s` update failed, pull request is closed' % (
586 pullrequestid,))
600 pullrequestid,))
587
601
588 reviewer_names = Optional.extract(reviewers) or []
602 reviewer_objects = Optional.extract(reviewers) or []
589 if not isinstance(reviewer_names, list):
603 if not isinstance(reviewer_objects, list):
590 raise JSONRPCError('reviewers should be specified as a list')
604 raise JSONRPCError('reviewers should be specified as a list')
591
605
592 reviewer_users = [get_user_or_error(n) for n in reviewer_names]
606 reviewers_reasons = []
593 reviewer_ids = [u.user_id for u in reviewer_users]
607 reviewer_ids = set()
608 for reviewer_object in reviewer_objects:
609 reviewer_reasons = []
610 if isinstance(reviewer_object, (int, basestring)):
611 reviewer_username = reviewer_object
612 else:
613 reviewer_username = reviewer_object['username']
614 reviewer_reasons = reviewer_object.get('reasons', [])
615
616 user = get_user_or_error(reviewer_username)
617 reviewer_ids.add(user.user_id)
618 reviewers_reasons.append((user.user_id, reviewer_reasons))
594
619
595 title = Optional.extract(title)
620 title = Optional.extract(title)
596 description = Optional.extract(description)
621 description = Optional.extract(description)
@@ -611,7 +636,7 b' def update_pull_request('
611 reviewers_changes = {"added": [], "removed": []}
636 reviewers_changes = {"added": [], "removed": []}
612 if reviewer_ids:
637 if reviewer_ids:
613 added_reviewers, removed_reviewers = \
638 added_reviewers, removed_reviewers = \
614 PullRequestModel().update_reviewers(pull_request, reviewer_ids)
639 PullRequestModel().update_reviewers(pull_request, reviewers_reasons)
615
640
616 reviewers_changes['added'] = sorted(
641 reviewers_changes['added'] = sorted(
617 [get_user_or_error(n).username for n in added_reviewers])
642 [get_user_or_error(n).username for n in added_reviewers])
@@ -22,6 +22,7 b''
22 pull requests controller for rhodecode for initializing pull requests
22 pull requests controller for rhodecode for initializing pull requests
23 """
23 """
24
24
25 import peppercorn
25 import formencode
26 import formencode
26 import logging
27 import logging
27
28
@@ -399,8 +400,10 b' class PullrequestsController(BaseRepoCon'
399 if not repo:
400 if not repo:
400 raise HTTPNotFound
401 raise HTTPNotFound
401
402
403 controls = peppercorn.parse(request.POST.items())
404
402 try:
405 try:
403 _form = PullRequestForm(repo.repo_id)().to_python(request.POST)
406 _form = PullRequestForm(repo.repo_id)().to_python(controls)
404 except formencode.Invalid as errors:
407 except formencode.Invalid as errors:
405 if errors.error_dict.get('revisions'):
408 if errors.error_dict.get('revisions'):
406 msg = 'Revisions: %s' % errors.error_dict['revisions']
409 msg = 'Revisions: %s' % errors.error_dict['revisions']
@@ -419,7 +422,8 b' class PullrequestsController(BaseRepoCon'
419 target_repo = _form['target_repo']
422 target_repo = _form['target_repo']
420 target_ref = _form['target_ref']
423 target_ref = _form['target_ref']
421 commit_ids = _form['revisions'][::-1]
424 commit_ids = _form['revisions'][::-1]
422 reviewers = _form['review_members']
425 reviewers = [
426 (r['user_id'], r['reasons']) for r in _form['review_members']]
423
427
424 # find the ancestor for this pr
428 # find the ancestor for this pr
425 source_db_repo = Repository.get_by_repo_name(_form['source_repo'])
429 source_db_repo = Repository.get_by_repo_name(_form['source_repo'])
@@ -478,8 +482,11 b' class PullrequestsController(BaseRepoCon'
478 allowed_to_update = PullRequestModel().check_user_update(
482 allowed_to_update = PullRequestModel().check_user_update(
479 pull_request, c.rhodecode_user)
483 pull_request, c.rhodecode_user)
480 if allowed_to_update:
484 if allowed_to_update:
481 if 'reviewers_ids' in request.POST:
485 controls = peppercorn.parse(request.POST.items())
482 self._update_reviewers(pull_request_id)
486
487 if 'review_members' in controls:
488 self._update_reviewers(
489 pull_request_id, controls['review_members'])
483 elif str2bool(request.POST.get('update_commits', 'false')):
490 elif str2bool(request.POST.get('update_commits', 'false')):
484 self._update_commits(pull_request)
491 self._update_commits(pull_request)
485 elif str2bool(request.POST.get('close_pull_request', 'false')):
492 elif str2bool(request.POST.get('close_pull_request', 'false')):
@@ -631,11 +638,10 b' class PullrequestsController(BaseRepoCon'
631 merge_resp.failure_reason)
638 merge_resp.failure_reason)
632 h.flash(msg, category='error')
639 h.flash(msg, category='error')
633
640
634 def _update_reviewers(self, pull_request_id):
641 def _update_reviewers(self, pull_request_id, review_members):
635 reviewers_ids = map(int, filter(
642 reviewers = [
636 lambda v: v not in [None, ''],
643 (int(r['user_id']), r['reasons']) for r in review_members]
637 request.POST.get('reviewers_ids', '').split(',')))
644 PullRequestModel().update_reviewers(pull_request_id, reviewers)
638 PullRequestModel().update_reviewers(pull_request_id, reviewers_ids)
639 Session().commit()
645 Session().commit()
640
646
641 def _reject_close(self, pull_request):
647 def _reject_close(self, pull_request):
@@ -59,7 +59,7 b' from rhodecode.lib.utils2 import ('
59 str2bool, safe_str, get_commit_safe, safe_unicode, remove_prefix, md5_safe,
59 str2bool, safe_str, get_commit_safe, safe_unicode, remove_prefix, md5_safe,
60 time_to_datetime, aslist, Optional, safe_int, get_clone_url, AttributeDict,
60 time_to_datetime, aslist, Optional, safe_int, get_clone_url, AttributeDict,
61 glob2re)
61 glob2re)
62 from rhodecode.lib.jsonalchemy import MutationObj, JsonType, JSONDict
62 from rhodecode.lib.jsonalchemy import MutationObj, MutationList, JsonType, JSONDict
63 from rhodecode.lib.ext_json import json
63 from rhodecode.lib.ext_json import json
64 from rhodecode.lib.caching_query import FromCache
64 from rhodecode.lib.caching_query import FromCache
65 from rhodecode.lib.encrypt import AESCipher
65 from rhodecode.lib.encrypt import AESCipher
@@ -3149,9 +3149,10 b' class PullRequest(Base, _PullRequestBase'
3149 {
3149 {
3150 'user': reviewer.get_api_data(include_secrets=False,
3150 'user': reviewer.get_api_data(include_secrets=False,
3151 details='basic'),
3151 details='basic'),
3152 'reasons': reasons,
3152 'review_status': st[0][1].status if st else 'not_reviewed',
3153 'review_status': st[0][1].status if st else 'not_reviewed',
3153 }
3154 }
3154 for reviewer, st in pull_request.reviewers_statuses()
3155 for reviewer, reasons, st in pull_request.reviewers_statuses()
3155 ]
3156 ]
3156 }
3157 }
3157
3158
@@ -3203,9 +3204,23 b' class PullRequestReviewers(Base, BaseMod'
3203 'mysql_charset': 'utf8', 'sqlite_autoincrement': True},
3204 'mysql_charset': 'utf8', 'sqlite_autoincrement': True},
3204 )
3205 )
3205
3206
3206 def __init__(self, user=None, pull_request=None):
3207 def __init__(self, user=None, pull_request=None, reasons=None):
3207 self.user = user
3208 self.user = user
3208 self.pull_request = pull_request
3209 self.pull_request = pull_request
3210 self.reasons = reasons or []
3211
3212 @hybrid_property
3213 def reasons(self):
3214 if not self._reasons:
3215 return []
3216 return self._reasons
3217
3218 @reasons.setter
3219 def reasons(self, val):
3220 val = val or []
3221 if any(not isinstance(x, basestring) for x in val):
3222 raise Exception('invalid reasons type, must be list of strings')
3223 self._reasons = val
3209
3224
3210 pull_requests_reviewers_id = Column(
3225 pull_requests_reviewers_id = Column(
3211 'pull_requests_reviewers_id', Integer(), nullable=False,
3226 'pull_requests_reviewers_id', Integer(), nullable=False,
@@ -3215,8 +3230,9 b' class PullRequestReviewers(Base, BaseMod'
3215 ForeignKey('pull_requests.pull_request_id'), nullable=False)
3230 ForeignKey('pull_requests.pull_request_id'), nullable=False)
3216 user_id = Column(
3231 user_id = Column(
3217 "user_id", Integer(), ForeignKey('users.user_id'), nullable=True)
3232 "user_id", Integer(), ForeignKey('users.user_id'), nullable=True)
3218 reason = Column('reason',
3233 _reasons = Column(
3219 UnicodeText().with_variant(UnicodeText(255), 'mysql'), nullable=True)
3234 'reason', MutationList.as_mutable(
3235 JsonType('list', dialect_map=dict(mysql=UnicodeText(16384)))))
3220
3236
3221 user = relationship('User')
3237 user = relationship('User')
3222 pull_request = relationship('PullRequest')
3238 pull_request = relationship('PullRequest')
@@ -26,7 +26,7 b' def upgrade(migrate_engine):'
26 _reset_base(migrate_engine)
26 _reset_base(migrate_engine)
27 from rhodecode.lib.dbmigrate.schema import db_4_5_0_0
27 from rhodecode.lib.dbmigrate.schema import db_4_5_0_0
28
28
29 db_4_5_0_0.PullRequestReviewers.reason.create(
29 db_4_5_0_0.PullRequestReviewers.reasons.create(
30 table=db_4_5_0_0.PullRequestReviewers.__table__)
30 table=db_4_5_0_0.PullRequestReviewers.__table__)
31
31
32 def downgrade(migrate_engine):
32 def downgrade(migrate_engine):
@@ -80,7 +80,7 b' class ChangesetStatusModel(BaseModel):'
80 """
80 """
81 votes = defaultdict(int)
81 votes = defaultdict(int)
82 reviewers_number = len(statuses_by_reviewers)
82 reviewers_number = len(statuses_by_reviewers)
83 for user, statuses in statuses_by_reviewers:
83 for user, reasons, statuses in statuses_by_reviewers:
84 if statuses:
84 if statuses:
85 ver, latest = statuses[0]
85 ver, latest = statuses[0]
86 votes[latest.status] += 1
86 votes[latest.status] += 1
@@ -254,7 +254,7 b' class ChangesetStatusModel(BaseModel):'
254 for x, y in (itertools.groupby(sorted(st, key=version),
254 for x, y in (itertools.groupby(sorted(st, key=version),
255 version))]
255 version))]
256
256
257 pull_request_reviewers.append([o.user, st])
257 pull_request_reviewers.append((o.user, o.reasons, st))
258 return pull_request_reviewers
258 return pull_request_reviewers
259
259
260 def calculated_review_status(self, pull_request, reviewers_statuses=None):
260 def calculated_review_status(self, pull_request, reviewers_statuses=None):
@@ -59,7 +59,7 b' from rhodecode.lib.utils2 import ('
59 str2bool, safe_str, get_commit_safe, safe_unicode, remove_prefix, md5_safe,
59 str2bool, safe_str, get_commit_safe, safe_unicode, remove_prefix, md5_safe,
60 time_to_datetime, aslist, Optional, safe_int, get_clone_url, AttributeDict,
60 time_to_datetime, aslist, Optional, safe_int, get_clone_url, AttributeDict,
61 glob2re)
61 glob2re)
62 from rhodecode.lib.jsonalchemy import MutationObj, JsonType, JSONDict
62 from rhodecode.lib.jsonalchemy import MutationObj, MutationList, JsonType, JSONDict
63 from rhodecode.lib.ext_json import json
63 from rhodecode.lib.ext_json import json
64 from rhodecode.lib.caching_query import FromCache
64 from rhodecode.lib.caching_query import FromCache
65 from rhodecode.lib.encrypt import AESCipher
65 from rhodecode.lib.encrypt import AESCipher
@@ -3149,9 +3149,10 b' class PullRequest(Base, _PullRequestBase'
3149 {
3149 {
3150 'user': reviewer.get_api_data(include_secrets=False,
3150 'user': reviewer.get_api_data(include_secrets=False,
3151 details='basic'),
3151 details='basic'),
3152 'reasons': reasons,
3152 'review_status': st[0][1].status if st else 'not_reviewed',
3153 'review_status': st[0][1].status if st else 'not_reviewed',
3153 }
3154 }
3154 for reviewer, st in pull_request.reviewers_statuses()
3155 for reviewer, reasons, st in pull_request.reviewers_statuses()
3155 ]
3156 ]
3156 }
3157 }
3157
3158
@@ -3203,9 +3204,23 b' class PullRequestReviewers(Base, BaseMod'
3203 'mysql_charset': 'utf8', 'sqlite_autoincrement': True},
3204 'mysql_charset': 'utf8', 'sqlite_autoincrement': True},
3204 )
3205 )
3205
3206
3206 def __init__(self, user=None, pull_request=None):
3207 def __init__(self, user=None, pull_request=None, reasons=None):
3207 self.user = user
3208 self.user = user
3208 self.pull_request = pull_request
3209 self.pull_request = pull_request
3210 self.reasons = reasons or []
3211
3212 @hybrid_property
3213 def reasons(self):
3214 if not self._reasons:
3215 return []
3216 return self._reasons
3217
3218 @reasons.setter
3219 def reasons(self, val):
3220 val = val or []
3221 if any(not isinstance(x, basestring) for x in val):
3222 raise Exception('invalid reasons type, must be list of strings')
3223 self._reasons = val
3209
3224
3210 pull_requests_reviewers_id = Column(
3225 pull_requests_reviewers_id = Column(
3211 'pull_requests_reviewers_id', Integer(), nullable=False,
3226 'pull_requests_reviewers_id', Integer(), nullable=False,
@@ -3215,8 +3230,9 b' class PullRequestReviewers(Base, BaseMod'
3215 ForeignKey('pull_requests.pull_request_id'), nullable=False)
3230 ForeignKey('pull_requests.pull_request_id'), nullable=False)
3216 user_id = Column(
3231 user_id = Column(
3217 "user_id", Integer(), ForeignKey('users.user_id'), nullable=True)
3232 "user_id", Integer(), ForeignKey('users.user_id'), nullable=True)
3218 reason = Column('reason',
3233 _reasons = Column(
3219 UnicodeText().with_variant(UnicodeText(255), 'mysql'), nullable=True)
3234 'reason', MutationList.as_mutable(
3235 JsonType('list', dialect_map=dict(mysql=UnicodeText(16384)))))
3220
3236
3221 user = relationship('User')
3237 user = relationship('User')
3222 pull_request = relationship('PullRequest')
3238 pull_request = relationship('PullRequest')
@@ -519,7 +519,12 b' def UserExtraIpForm():'
519 return _UserExtraIpForm
519 return _UserExtraIpForm
520
520
521
521
522
522 def PullRequestForm(repo_id):
523 def PullRequestForm(repo_id):
524 class ReviewerForm(formencode.Schema):
525 user_id = v.Int(not_empty=True)
526 reasons = All()
527
523 class _PullRequestForm(formencode.Schema):
528 class _PullRequestForm(formencode.Schema):
524 allow_extra_fields = True
529 allow_extra_fields = True
525 filter_extra_fields = True
530 filter_extra_fields = True
@@ -531,8 +536,7 b' def PullRequestForm(repo_id):'
531 target_ref = v.UnicodeString(strip=True, required=True)
536 target_ref = v.UnicodeString(strip=True, required=True)
532 revisions = All(#v.NotReviewedRevisions(repo_id)(),
537 revisions = All(#v.NotReviewedRevisions(repo_id)(),
533 v.UniqueList()(not_empty=True))
538 v.UniqueList()(not_empty=True))
534 review_members = v.UniqueList(convert=int)(not_empty=True)
539 review_members = formencode.ForEach(ReviewerForm())
535
536 pullrequest_title = v.UnicodeString(strip=True, required=True)
540 pullrequest_title = v.UnicodeString(strip=True, required=True)
537 pullrequest_desc = v.UnicodeString(strip=True, required=False)
541 pullrequest_desc = v.UnicodeString(strip=True, required=False)
538
542
@@ -333,10 +333,18 b' class PullRequestModel(BaseModel):'
333 Session().add(pull_request)
333 Session().add(pull_request)
334 Session().flush()
334 Session().flush()
335
335
336 reviewer_ids = set()
336 # members / reviewers
337 # members / reviewers
337 for user_id in set(reviewers):
338 for reviewer_object in reviewers:
339 if isinstance(reviewer_object, tuple):
340 user_id, reasons = reviewer_object
341 else:
342 user_id, reasons = reviewer_object, []
343
338 user = self._get_user(user_id)
344 user = self._get_user(user_id)
339 reviewer = PullRequestReviewers(user, pull_request)
345 reviewer_ids.add(user.user_id)
346
347 reviewer = PullRequestReviewers(user, pull_request, reasons)
340 Session().add(reviewer)
348 Session().add(reviewer)
341
349
342 # Set approval status to "Under Review" for all commits which are
350 # Set approval status to "Under Review" for all commits which are
@@ -348,7 +356,7 b' class PullRequestModel(BaseModel):'
348 pull_request=pull_request
356 pull_request=pull_request
349 )
357 )
350
358
351 self.notify_reviewers(pull_request, reviewers)
359 self.notify_reviewers(pull_request, reviewer_ids)
352 self._trigger_pull_request_hook(
360 self._trigger_pull_request_hook(
353 pull_request, created_by_user, 'create')
361 pull_request, created_by_user, 'create')
354
362
@@ -570,6 +578,7 b' class PullRequestModel(BaseModel):'
570 Session().commit()
578 Session().commit()
571 self._trigger_pull_request_hook(pull_request, pull_request.author,
579 self._trigger_pull_request_hook(pull_request, pull_request.author,
572 'update')
580 'update')
581
573 return (pull_request_version, changes)
582 return (pull_request_version, changes)
574
583
575 def _create_version_from_snapshot(self, pull_request):
584 def _create_version_from_snapshot(self, pull_request):
@@ -711,8 +720,21 b' class PullRequestModel(BaseModel):'
711 pull_request.updated_on = datetime.datetime.now()
720 pull_request.updated_on = datetime.datetime.now()
712 Session().add(pull_request)
721 Session().add(pull_request)
713
722
714 def update_reviewers(self, pull_request, reviewers_ids):
723 def update_reviewers(self, pull_request, reviewer_data):
715 reviewers_ids = set(reviewers_ids)
724 """
725 Update the reviewers in the pull request
726
727 :param pull_request: the pr to update
728 :param reviewer_data: list of tuples [(user, ['reason1', 'reason2'])]
729 """
730
731 reviewers_reasons = {}
732 for user_id, reasons in reviewer_data:
733 if isinstance(user_id, (int, basestring)):
734 user_id = self._get_user(user_id).user_id
735 reviewers_reasons[user_id] = reasons
736
737 reviewers_ids = set(reviewers_reasons.keys())
716 pull_request = self.__get_pull_request(pull_request)
738 pull_request = self.__get_pull_request(pull_request)
717 current_reviewers = PullRequestReviewers.query()\
739 current_reviewers = PullRequestReviewers.query()\
718 .filter(PullRequestReviewers.pull_request ==
740 .filter(PullRequestReviewers.pull_request ==
@@ -728,7 +750,8 b' class PullRequestModel(BaseModel):'
728 for uid in ids_to_add:
750 for uid in ids_to_add:
729 changed = True
751 changed = True
730 _usr = self._get_user(uid)
752 _usr = self._get_user(uid)
731 reviewer = PullRequestReviewers(_usr, pull_request)
753 reasons = reviewers_reasons[uid]
754 reviewer = PullRequestReviewers(_usr, pull_request, reasons)
732 Session().add(reviewer)
755 Session().add(reviewer)
733
756
734 self.notify_reviewers(pull_request, ids_to_add)
757 self.notify_reviewers(pull_request, ids_to_add)
@@ -1282,6 +1282,9 b' table.integrations {'
1282 width: 100%;
1282 width: 100%;
1283 overflow: auto;
1283 overflow: auto;
1284 }
1284 }
1285 .reviewer_reason {
1286 padding-left: 20px;
1287 }
1285 .reviewer_status {
1288 .reviewer_status {
1286 display: inline-block;
1289 display: inline-block;
1287 vertical-align: top;
1290 vertical-align: top;
@@ -32,7 +32,7 b' var removeReviewMember = function(review'
32 var obj = $('#reviewer_{0}_name'.format(reviewer_id));
32 var obj = $('#reviewer_{0}_name'.format(reviewer_id));
33 obj.addClass('to-delete');
33 obj.addClass('to-delete');
34 // now delete the input
34 // now delete the input
35 $('#reviewer_{0}_input'.format(reviewer_id)).remove();
35 $('#reviewer_{0} input'.format(reviewer_id)).remove();
36 }
36 }
37 }
37 }
38 else{
38 else{
@@ -43,29 +43,36 b' var removeReviewMember = function(review'
43 var addReviewMember = function(id, fname, lname, nname, gravatar_link, reasons) {
43 var addReviewMember = function(id, fname, lname, nname, gravatar_link, reasons) {
44 var members = $('#review_members').get(0);
44 var members = $('#review_members').get(0);
45 var reasons_html = '';
45 var reasons_html = '';
46 var reasons_inputs = '';
47 var reasons = reasons || [];
46 if (reasons) {
48 if (reasons) {
47 for (var i = 0; i < reasons.length; i++) {
49 for (var i = 0; i < reasons.length; i++) {
48 reasons_html += '<div class="reviewer_reason">- {0}</div>'.format(
50 reasons_html += '<div class="reviewer_reason">- {0}</div>'.format(reasons[i]);
49 reasons[i]
51 reasons_inputs += '<input type="hidden" name="reason" value="' + escapeHtml(reasons[i]) + '">';
50 );
51 }
52 }
52 }
53 }
53 var tmpl = '<li id="reviewer_{2}">'+
54 var tmpl = '<li id="reviewer_{2}">'+
55 '<input type="hidden" name="__start__" value="reviewer:mapping">'+
54 '<div class="reviewer_status">'+
56 '<div class="reviewer_status">'+
55 '<div class="flag_status not_reviewed pull-left reviewer_member_status"></div>'+
57 '<div class="flag_status not_reviewed pull-left reviewer_member_status"></div>'+
56 '</div>'+
58 '</div>'+
57 '<img alt="gravatar" class="gravatar" src="{0}"/>'+
59 '<img alt="gravatar" class="gravatar" src="{0}"/>'+
58 '<span class="reviewer_name user">{1}</span>'+
60 '<span class="reviewer_name user">{1}</span>'+
59 reasons_html +
61 reasons_html +
60 '<input type="hidden" value="{2}" name="review_members" />'+
62 '<input type="hidden" name="user_id" value="{2}">'+
63 '<input type="hidden" name="__start__" value="reasons:sequence">'+
64 '{3}'+
65 '<input type="hidden" name="__end__" value="reasons:sequence">'+
61 '<div class="reviewer_member_remove action_button" onclick="removeReviewMember({2})">' +
66 '<div class="reviewer_member_remove action_button" onclick="removeReviewMember({2})">' +
62 '<i class="icon-remove-sign"></i>'+
67 '<i class="icon-remove-sign"></i>'+
63 '</div>'+
68 '</div>'+
64 '</div>'+
69 '</div>'+
70 '<input type="hidden" name="__end__" value="reviewer:mapping">'+
65 '</li>' ;
71 '</li>' ;
72
66 var displayname = "{0} ({1} {2})".format(
73 var displayname = "{0} ({1} {2})".format(
67 nname, escapeHtml(fname), escapeHtml(lname));
74 nname, escapeHtml(fname), escapeHtml(lname));
68 var element = tmpl.format(gravatar_link,displayname,id);
75 var element = tmpl.format(gravatar_link,displayname,id,reasons_inputs);
69 // check if we don't have this ID already in
76 // check if we don't have this ID already in
70 var ids = [];
77 var ids = [];
71 var _els = $('#review_members li').toArray();
78 var _els = $('#review_members li').toArray();
@@ -83,7 +90,11 b' var _updatePullRequest = function(repo_n'
83 var url = pyroutes.url(
90 var url = pyroutes.url(
84 'pullrequest_update',
91 'pullrequest_update',
85 {"repo_name": repo_name, "pull_request_id": pull_request_id});
92 {"repo_name": repo_name, "pull_request_id": pull_request_id});
93 if (typeof postData === 'string' ) {
94 postData += '&csrf_token=' + CSRF_TOKEN;
95 } else {
86 postData.csrf_token = CSRF_TOKEN;
96 postData.csrf_token = CSRF_TOKEN;
97 }
87 var success = function(o) {
98 var success = function(o) {
88 window.location.reload();
99 window.location.reload();
89 };
100 };
@@ -92,17 +103,9 b' var _updatePullRequest = function(repo_n'
92
103
93 var updateReviewers = function(reviewers_ids, repo_name, pull_request_id){
104 var updateReviewers = function(reviewers_ids, repo_name, pull_request_id){
94 if (reviewers_ids === undefined){
105 if (reviewers_ids === undefined){
95 var reviewers_ids = [];
106 var postData = '_method=put&' + $('#reviewers input').serialize();
96 var ids = $('#review_members input').toArray();
107 _updatePullRequest(repo_name, pull_request_id, postData);
97 for(var i=0; i<ids.length;i++){
98 var id = ids[i].value
99 reviewers_ids.push(id);
100 }
108 }
101 }
102 var postData = {
103 '_method':'put',
104 'reviewers_ids': reviewers_ids};
105 _updatePullRequest(repo_name, pull_request_id, postData);
106 };
109 };
107
110
108 /**
111 /**
@@ -20,6 +20,9 b''
20 * turns objects into GET query string
20 * turns objects into GET query string
21 */
21 */
22 var toQueryString = function(o) {
22 var toQueryString = function(o) {
23 if(typeof o === 'string') {
24 return o;
25 }
23 if(typeof o !== 'object') {
26 if(typeof o !== 'object') {
24 return false;
27 return false;
25 }
28 }
@@ -111,7 +111,9 b''
111 </div>
111 </div>
112 <div id="reviewers" class="block-right pr-details-content reviewers">
112 <div id="reviewers" class="block-right pr-details-content reviewers">
113 ## members goes here, filled via JS based on initial selection !
113 ## members goes here, filled via JS based on initial selection !
114 <input type="hidden" name="__start__" value="review_members:sequence">
114 <ul id="review_members" class="group_members"></ul>
115 <ul id="review_members" class="group_members"></ul>
116 <input type="hidden" name="__end__" value="review_members:sequence">
115 <div id="add_reviewer_input" class='ac'>
117 <div id="add_reviewer_input" class='ac'>
116 <div class="reviewer_ac">
118 <div class="reviewer_ac">
117 ${h.text('user', class_='ac-input', placeholder=_('Add reviewer'))}
119 ${h.text('user', class_='ac-input', placeholder=_('Add reviewer'))}
@@ -188,17 +188,27 b''
188 </div>
188 </div>
189 <div id="reviewers" class="block-right pr-details-content reviewers">
189 <div id="reviewers" class="block-right pr-details-content reviewers">
190 ## members goes here !
190 ## members goes here !
191 <input type="hidden" name="__start__" value="review_members:sequence">
191 <ul id="review_members" class="group_members">
192 <ul id="review_members" class="group_members">
192 %for member,status in c.pull_request_reviewers:
193 %for member,reasons,status in c.pull_request_reviewers:
193 <li id="reviewer_${member.user_id}">
194 <li id="reviewer_${member.user_id}">
194 <div class="reviewers_member">
195 <div class="reviewers_member">
195 <div class="reviewer_status tooltip" title="${h.tooltip(h.commit_status_lbl(status[0][1].status if status else 'not_reviewed'))}">
196 <div class="reviewer_status tooltip" title="${h.tooltip(h.commit_status_lbl(status[0][1].status if status else 'not_reviewed'))}">
196 <div class="${'flag_status %s' % (status[0][1].status if status else 'not_reviewed')} pull-left reviewer_member_status"></div>
197 <div class="${'flag_status %s' % (status[0][1].status if status else 'not_reviewed')} pull-left reviewer_member_status"></div>
197 </div>
198 </div>
198 <div id="reviewer_${member.user_id}_name" class="reviewer_name">
199 <div id="reviewer_${member.user_id}_name" class="reviewer_name">
199 ${self.gravatar_with_user(member.email, 16)} <div class="reviewer">(${_('owner') if c.pull_request.user_id == member.user_id else _('reviewer')})</div>
200 ${self.gravatar_with_user(member.email, 16)}
200 </div>
201 </div>
201 <input id="reviewer_${member.user_id}_input" type="hidden" value="${member.user_id}" name="review_members" />
202 <input type="hidden" name="__start__" value="reviewer:mapping">
203 <input type="hidden" name="__start__" value="reasons:sequence">
204 %for reason in reasons:
205 <div class="reviewer_reason">- ${reason}</div>
206 <input type="hidden" name="reason" value="${reason}">
207
208 %endfor
209 <input type="hidden" name="__end__" value="reasons:sequence">
210 <input id="reviewer_${member.user_id}_input" type="hidden" value="${member.user_id}" name="user_id" />
211 <input type="hidden" name="__end__" value="reviewer:mapping">
202 %if c.allowed_to_update:
212 %if c.allowed_to_update:
203 <div class="reviewer_member_remove action_button" onclick="removeReviewMember(${member.user_id}, true)" style="visibility: hidden;">
213 <div class="reviewer_member_remove action_button" onclick="removeReviewMember(${member.user_id}, true)" style="visibility: hidden;">
204 <i class="icon-remove-sign" ></i>
214 <i class="icon-remove-sign" ></i>
@@ -208,6 +218,7 b''
208 </li>
218 </li>
209 %endfor
219 %endfor
210 </ul>
220 </ul>
221 <input type="hidden" name="__end__" value="review_members:sequence">
211 %if not c.pull_request.is_closed():
222 %if not c.pull_request.is_closed():
212 <div id="add_reviewer_input" class='ac' style="display: none;">
223 <div id="add_reviewer_input" class='ac' style="display: none;">
213 %if c.allowed_to_update:
224 %if c.allowed_to_update:
@@ -256,8 +256,8 b' class TestPullrequestsController:'
256 def test_comment_force_close_pull_request(self, pr_util, csrf_token):
256 def test_comment_force_close_pull_request(self, pr_util, csrf_token):
257 pull_request = pr_util.create_pull_request()
257 pull_request = pr_util.create_pull_request()
258 pull_request_id = pull_request.pull_request_id
258 pull_request_id = pull_request.pull_request_id
259 reviewers_ids = [1, 2]
259 reviewers_data = [(1, ['reason']), (2, ['reason2'])]
260 PullRequestModel().update_reviewers(pull_request_id, reviewers_ids)
260 PullRequestModel().update_reviewers(pull_request_id, reviewers_data)
261 author = pull_request.user_id
261 author = pull_request.user_id
262 repo = pull_request.target_repo.repo_id
262 repo = pull_request.target_repo.repo_id
263 self.app.post(
263 self.app.post(
@@ -297,19 +297,27 b' class TestPullrequestsController:'
297 url(
297 url(
298 controller='pullrequests',
298 controller='pullrequests',
299 action='create',
299 action='create',
300 repo_name=source.repo_name),
300 repo_name=source.repo_name
301 params={
301 ),
302 'source_repo': source.repo_name,
302 [
303 'source_ref': 'branch:default:' + commit_ids['change'],
303 ('source_repo', source.repo_name),
304 'target_repo': target.repo_name,
304 ('source_ref', 'branch:default:' + commit_ids['change']),
305 'target_ref': 'branch:default:' + commit_ids['ancestor'],
305 ('target_repo', target.repo_name),
306 'pullrequest_desc': 'Description',
306 ('target_ref', 'branch:default:' + commit_ids['ancestor']),
307 'pullrequest_title': 'Title',
307 ('pullrequest_desc', 'Description'),
308 'review_members': '1',
308 ('pullrequest_title', 'Title'),
309 'revisions': commit_ids['change'],
309 ('__start__', 'review_members:sequence'),
310 'user': '',
310 ('__start__', 'reviewer:mapping'),
311 'csrf_token': csrf_token,
311 ('user_id', '1'),
312 },
312 ('__start__', 'reasons:sequence'),
313 ('reason', 'Some reason'),
314 ('__end__', 'reasons:sequence'),
315 ('__end__', 'reviewer:mapping'),
316 ('__end__', 'review_members:sequence'),
317 ('revisions', commit_ids['change']),
318 ('user', ''),
319 ('csrf_token', csrf_token),
320 ],
313 status=302)
321 status=302)
314
322
315 location = response.headers['Location']
323 location = response.headers['Location']
@@ -344,19 +352,27 b' class TestPullrequestsController:'
344 url(
352 url(
345 controller='pullrequests',
353 controller='pullrequests',
346 action='create',
354 action='create',
347 repo_name=source.repo_name),
355 repo_name=source.repo_name
348 params={
356 ),
349 'source_repo': source.repo_name,
357 [
350 'source_ref': 'branch:default:' + commit_ids['change'],
358 ('source_repo', source.repo_name),
351 'target_repo': target.repo_name,
359 ('source_ref', 'branch:default:' + commit_ids['change']),
352 'target_ref': 'branch:default:' + commit_ids['ancestor-child'],
360 ('target_repo', target.repo_name),
353 'pullrequest_desc': 'Description',
361 ('target_ref', 'branch:default:' + commit_ids['ancestor-child']),
354 'pullrequest_title': 'Title',
362 ('pullrequest_desc', 'Description'),
355 'review_members': '2',
363 ('pullrequest_title', 'Title'),
356 'revisions': commit_ids['change'],
364 ('__start__', 'review_members:sequence'),
357 'user': '',
365 ('__start__', 'reviewer:mapping'),
358 'csrf_token': csrf_token,
366 ('user_id', '2'),
359 },
367 ('__start__', 'reasons:sequence'),
368 ('reason', 'Some reason'),
369 ('__end__', 'reasons:sequence'),
370 ('__end__', 'reviewer:mapping'),
371 ('__end__', 'review_members:sequence'),
372 ('revisions', commit_ids['change']),
373 ('user', ''),
374 ('csrf_token', csrf_token),
375 ],
360 status=302)
376 status=302)
361
377
362 location = response.headers['Location']
378 location = response.headers['Location']
@@ -373,7 +389,8 b' class TestPullrequestsController:'
373 assert len(notifications.all()) == 1
389 assert len(notifications.all()) == 1
374
390
375 # Change reviewers and check that a notification was made
391 # Change reviewers and check that a notification was made
376 PullRequestModel().update_reviewers(pull_request.pull_request_id, [1])
392 PullRequestModel().update_reviewers(
393 pull_request.pull_request_id, [(1, [])])
377 assert len(notifications.all()) == 2
394 assert len(notifications.all()) == 2
378
395
379 def test_create_pull_request_stores_ancestor_commit_id(self, backend,
396 def test_create_pull_request_stores_ancestor_commit_id(self, backend,
@@ -397,19 +414,27 b' class TestPullrequestsController:'
397 url(
414 url(
398 controller='pullrequests',
415 controller='pullrequests',
399 action='create',
416 action='create',
400 repo_name=source.repo_name),
417 repo_name=source.repo_name
401 params={
418 ),
402 'source_repo': source.repo_name,
419 [
403 'source_ref': 'branch:default:' + commit_ids['change'],
420 ('source_repo', source.repo_name),
404 'target_repo': target.repo_name,
421 ('source_ref', 'branch:default:' + commit_ids['change']),
405 'target_ref': 'branch:default:' + commit_ids['ancestor-child'],
422 ('target_repo', target.repo_name),
406 'pullrequest_desc': 'Description',
423 ('target_ref', 'branch:default:' + commit_ids['ancestor-child']),
407 'pullrequest_title': 'Title',
424 ('pullrequest_desc', 'Description'),
408 'review_members': '1',
425 ('pullrequest_title', 'Title'),
409 'revisions': commit_ids['change'],
426 ('__start__', 'review_members:sequence'),
410 'user': '',
427 ('__start__', 'reviewer:mapping'),
411 'csrf_token': csrf_token,
428 ('user_id', '1'),
412 },
429 ('__start__', 'reasons:sequence'),
430 ('reason', 'Some reason'),
431 ('__end__', 'reasons:sequence'),
432 ('__end__', 'reviewer:mapping'),
433 ('__end__', 'review_members:sequence'),
434 ('revisions', commit_ids['change']),
435 ('user', ''),
436 ('csrf_token', csrf_token),
437 ],
413 status=302)
438 status=302)
414
439
415 location = response.headers['Location']
440 location = response.headers['Location']
@@ -80,7 +80,7 b' def test_pull_request_stays_if_update_wi'
80 voted_status, *pull_request.reviewers)
80 voted_status, *pull_request.reviewers)
81
81
82 # Update, without change
82 # Update, without change
83 PullRequestModel().update_commits(pull_request)
83 version, changes = PullRequestModel().update_commits(pull_request)
84
84
85 # Expect that review status is the voted_status
85 # Expect that review status is the voted_status
86 expected_review_status = voted_status
86 expected_review_status = voted_status
@@ -99,7 +99,7 b' def test_pull_request_under_review_if_up'
99
99
100 # Update, with change
100 # Update, with change
101 pr_util.update_source_repository()
101 pr_util.update_source_repository()
102 PullRequestModel().update_commits(pull_request)
102 version, changes = PullRequestModel().update_commits(pull_request)
103
103
104 # Expect that review status is the voted_status
104 # Expect that review status is the voted_status
105 expected_review_status = db.ChangesetStatus.STATUS_UNDER_REVIEW
105 expected_review_status = db.ChangesetStatus.STATUS_UNDER_REVIEW
@@ -115,7 +115,7 b' class TestPullRequestModel:'
115
115
116 def test_get_awaiting_my_review(self, pull_request):
116 def test_get_awaiting_my_review(self, pull_request):
117 PullRequestModel().update_reviewers(
117 PullRequestModel().update_reviewers(
118 pull_request, [pull_request.author])
118 pull_request, [(pull_request.author, ['author'])])
119 prs = PullRequestModel().get_awaiting_my_review(
119 prs = PullRequestModel().get_awaiting_my_review(
120 pull_request.target_repo, user_id=pull_request.author.user_id)
120 pull_request.target_repo, user_id=pull_request.author.user_id)
121 assert isinstance(prs, list)
121 assert isinstance(prs, list)
@@ -123,7 +123,7 b' class TestPullRequestModel:'
123
123
124 def test_count_awaiting_my_review(self, pull_request):
124 def test_count_awaiting_my_review(self, pull_request):
125 PullRequestModel().update_reviewers(
125 PullRequestModel().update_reviewers(
126 pull_request, [pull_request.author])
126 pull_request, [(pull_request.author, ['author'])])
127 pr_count = PullRequestModel().count_awaiting_my_review(
127 pr_count = PullRequestModel().count_awaiting_my_review(
128 pull_request.target_repo, user_id=pull_request.author.user_id)
128 pull_request.target_repo, user_id=pull_request.author.user_id)
129 assert pr_count == 1
129 assert pr_count == 1
General Comments 0
You need to be logged in to leave comments. Login now