kallithea Changeset - aa0560cfca9b

Changeset - aa0560cfca9b

Parent rev.

Child rev.

[Not reviewed]

default

0 3 0

Mads Kiilerich - 9 years ago 2016-10-24 15:18:51
madski@unity3d.com

pullrequests: when updating a PR, only add and remove the reviewers that actually were added/removed

3 files changed with 28 insertions and 39 deletions:

kallithea/controllers/pullrequests.py

kallithea/model/pull_request.py

kallithea/tests/functional/test_pullrequests.py

0 comments (0 inline, 0 general)

kallithea/controllers/pullrequests.py

➞

Show inline comments

@@ @@ -529,9 +529,12 @@ class PullrequestsController(BaseRepoCon @@
         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()

kallithea/model/pull_request.py

➞

Show inline comments

@@ @@ -98,19 +98,18 @@ class PullRequestModel(BaseModel): @@
         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 @@ class PullRequestModel(BaseModel): @@
         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 @@ class PullRequestModel(BaseModel): @@
                                        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 @@ class PullRequestModel(BaseModel): @@
                               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)

kallithea/tests/functional/test_pullrequests.py

➞

Show inline comments

@@ @@ -90,9 +90,9 @@ class TestPullrequestsController(TestCon @@
         # 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

0 comments (0 inline, 0 general)