Mercurial > kallithea
changeset 6268:aa0560cfca9b
pullrequests: when updating a PR, only add and remove the reviewers that actually were added/removed
| author | Mads Kiilerich <madski@unity3d.com> |
|---|---|
| date | Mon, 24 Oct 2016 15:18:51 +0200 |
| parents | 69ee6a249f55 |
| children | c073c723e264 |
| files | kallithea/controllers/pullrequests.py kallithea/model/pull_request.py kallithea/tests/functional/test_pullrequests.py |
| diffstat | 3 files changed, 28 insertions(+), 39 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/controllers/pullrequests.py Mon Oct 24 15:18:51 2016 +0200 +++ b/kallithea/controllers/pullrequests.py Mon Oct 24 15:18:51 2016 +0200 @@ -529,9 +529,12 @@ pull_request.description = _form['pullrequest_desc'].strip() or _('No description') pull_request.owner = User.get_by_username(_form['owner']) user = User.get(c.authuser.user_id) + add_reviewer_ids = reviewer_ids - org_reviewer_ids - current_reviewer_ids + remove_reviewer_ids = (org_reviewer_ids - reviewer_ids) & current_reviewer_ids try: PullRequestModel().mention_from_description(user, pull_request, old_description) - PullRequestModel().update_reviewers(user, pull_request_id, reviewer_ids) + PullRequestModel().add_reviewers(user, pull_request, add_reviewer_ids) + PullRequestModel().remove_reviewers(user, pull_request, remove_reviewer_ids) except UserInvalidException as u: h.flash(_('Invalid reviewer "%s" specified') % u, category='error') raise HTTPBadRequest()
--- a/kallithea/model/pull_request.py Mon Oct 24 15:18:51 2016 +0200 +++ b/kallithea/model/pull_request.py Mon Oct 24 15:18:51 2016 +0200 @@ -98,19 +98,18 @@ reviewers = set(self._get_valid_reviewers(reviewers)) mention_recipients = extract_mentioned_users(new.description) - self.__add_reviewers(created_by_user, new, reviewers, mention_recipients) + self.add_reviewers(created_by_user, new, reviewers, mention_recipients) return new - def __add_reviewers(self, user, pr, reviewers, mention_recipients): - # reviewers and mention_recipients should be sets of User objects. + def add_reviewers(self, user, pr, reviewers, mention_recipients=None): + """Add reviewer and send notification to them. + """ + reviewer_users = set(self._get_valid_reviewers(reviewers)) #members - for reviewer in reviewers: - reviewer = PullRequestReviewers(reviewer, pr) - Session().add(reviewer) - - revision_data = [(x.raw_id, x.message) - for x in map(pr.org_repo.get_changeset, pr.revisions)] + for reviewer in reviewer_users: + prr = PullRequestReviewers(reviewer, pr) + Session().add(prr) #notification to reviewers pr_url = pr.url(canonical=True) @@ -128,6 +127,8 @@ body = pr.description _org_ref_type, org_ref_name, _org_rev = pr.org_ref.split(':') _other_ref_type, other_ref_name, _other_rev = pr.other_ref.split(':') + revision_data = [(x.raw_id, x.message) + for x in map(pr.org_repo.get_changeset, pr.revisions)] email_kwargs = { 'pr_title': pr.title, 'pr_title_short': h.shorter(pr.title, 50), @@ -156,14 +157,16 @@ type_=Notification.TYPE_PULL_REQUEST, email_kwargs=email_kwargs) + mention_recipient_users = set() if mention_recipients: - mention_recipients.difference_update(reviewers) - if mention_recipients: + mention_recipient_users = set(self._get_valid_reviewers(mention_recipients)) + mention_recipient_users.difference_update(reviewers) + if mention_recipient_users: email_kwargs['is_mention'] = True subject = _('[Mention]') + ' ' + subject # FIXME: this subject is wrong and unused! NotificationModel().create(created_by=user, subject=subject, body=body, - recipients=mention_recipients, + recipients=mention_recipient_users, type_=Notification.TYPE_PULL_REQUEST, email_kwargs=email_kwargs) @@ -172,32 +175,15 @@ extract_mentioned_users(old_description)) log.debug("Mentioning %s", mention_recipients) - self.__add_reviewers(user, pr, set(), mention_recipients) + self.add_reviewers(user, pr, set(), mention_recipients) - def update_reviewers(self, user, pull_request, reviewers): - """Set PR reviewers to exactly the given list of users""" - pull_request = PullRequest.guess_instance(pull_request) - current_reviewers = PullRequestReviewers.query() \ - .options(joinedload('user')) \ - .filter_by(pull_request=pull_request) \ - .all() - current_reviewer_users = set(x.user for x in current_reviewers) - - new_reviewer_users = set(self._get_valid_reviewers(reviewers)) + def remove_reviewers(self, user, pull_request, reviewer_ids): + """Remove users in the given user_id list from being reviewers of the PR.""" - to_add = new_reviewer_users - current_reviewer_users - to_remove = current_reviewer_users - new_reviewer_users - - if not to_add and not to_remove: - return # all done - - log.debug("Adding %s reviewers", to_add) - self.__add_reviewers(user, pull_request, to_add, set()) - - log.debug("Removing %s reviewers", to_remove) - for prr in current_reviewers: - if prr.user in to_remove: - Session().delete(prr) + PullRequestReviewers.query() \ + .filter_by(pull_request=pull_request) \ + .filter(PullRequestReviewers.user_id.in_(reviewer_ids)) \ + .delete(synchronize_session='fetch') # the default of 'evaluate' is not available def delete(self, pull_request): pull_request = PullRequest.guess_instance(pull_request)
--- a/kallithea/tests/functional/test_pullrequests.py Mon Oct 24 15:18:51 2016 +0200 +++ b/kallithea/tests/functional/test_pullrequests.py Mon Oct 24 15:18:51 2016 +0200 @@ -90,9 +90,9 @@ # verify reviewers were added / removed response.mustcontain('Meanwhile, the following reviewers have been added: test_regular') response.mustcontain('Meanwhile, the following reviewers have been removed: test_admin') - response.mustcontain(no='<input type="hidden" value="%s" name="review_members" />' % regular_user.user_id) + response.mustcontain('<input type="hidden" value="%s" name="review_members" />' % regular_user.user_id) response.mustcontain('<input type="hidden" value="%s" name="review_members" />' % regular_user2.user_id) - response.mustcontain('<input type="hidden" value="%s" name="review_members" />' % admin_user.user_id) + response.mustcontain(no='<input type="hidden" value="%s" name="review_members" />' % admin_user.user_id) def test_update_with_invalid_reviewer(self): invalid_user_id = 99999