Mercurial > kallithea
changeset 6525:bcc67f909d9f
pullrequests: pass around reviewer User objects, not IDs
This moves reviewer user validation into the controller and eliminates
the UserInvalidException.
| author | Søren Løvborg <sorenl@unity3d.com> |
|---|---|
| date | Mon, 27 Feb 2017 15:38:20 +0100 |
| parents | d32a2218b525 |
| children | 33a44dd50a87 |
| files | kallithea/controllers/pullrequests.py kallithea/lib/exceptions.py kallithea/model/pull_request.py |
| diffstat | 3 files changed, 53 insertions(+), 54 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/controllers/pullrequests.py Mon Feb 27 15:44:49 2017 +0100 +++ b/kallithea/controllers/pullrequests.py Mon Feb 27 15:38:20 2017 +0100 @@ -42,7 +42,6 @@ from kallithea.lib.base import BaseRepoController, render, jsonify from kallithea.lib.compat import json, OrderedDict from kallithea.lib.diffs import LimitedDiffContainer -from kallithea.lib.exceptions import UserInvalidException from kallithea.lib.page import Page from kallithea.lib.utils import action_logger from kallithea.lib.vcs.exceptions import EmptyRepositoryError, ChangesetDoesNotExistError @@ -65,6 +64,20 @@ log = logging.getLogger(__name__) +def _get_reviewer(user_id): + """Look up user by ID and validate it as a potential reviewer.""" + try: + user = User.get(int(user_id)) + except ValueError: + user = None + + if user is None or user.is_default_user: + h.flash(_('Invalid reviewer "%s" specified') % user_id, category='error') + raise HTTPBadRequest() + + return user + + class PullrequestsController(BaseRepoController): def _get_repo_refs(self, repo, rev=None, branch=None, branch_rev=None): @@ -363,7 +376,7 @@ # requested destination and have the exact ancestor other_ref = '%s:%s:%s' % (other_ref_type, other_ref_name, ancestor_rev) - reviewer_ids = [] + reviewers = [] title = _form['pullrequest_title'] if not title: @@ -377,13 +390,10 @@ created_by = User.get(request.authuser.user_id) pull_request = PullRequestModel().create( created_by, org_repo, org_ref, other_repo, other_ref, revisions, - title, description, reviewer_ids) + title, description, reviewers) Session().commit() h.flash(_('Successfully opened new pull request'), category='success') - except UserInvalidException as u: - h.flash(_('Invalid reviewer "%s" specified') % u, category='error') - raise HTTPBadRequest() except Exception: h.flash(_('Error occurred while creating pull request'), category='error') @@ -392,7 +402,7 @@ raise HTTPFound(location=pull_request.url()) - def create_new_iteration(self, old_pull_request, new_rev, title, description, reviewer_ids): + def create_new_iteration(self, old_pull_request, new_rev, title, description, reviewers): org_repo = old_pull_request.org_repo org_ref_type, org_ref_name, org_rev = old_pull_request.org_ref.split(':') new_org_rev = self._get_ref_rev(org_repo, 'rev', new_rev) @@ -478,10 +488,7 @@ created_by = User.get(request.authuser.user_id) pull_request = PullRequestModel().create( created_by, org_repo, new_org_ref, other_repo, new_other_ref, revisions, - title, description, reviewer_ids) - except UserInvalidException as u: - h.flash(_('Invalid reviewer "%s" specified') % u, category='error') - raise HTTPBadRequest() + title, description, reviewers) except Exception: h.flash(_('Error occurred while creating pull request'), category='error') @@ -518,12 +525,14 @@ raise HTTPForbidden() _form = PullRequestPostForm()().to_python(request.POST) - reviewer_ids = set(int(s) for s in _form['review_members']) - org_reviewer_ids = set(int(s) for s in _form['org_review_members']) - current_reviewer_ids = set(prr.user_id for prr in pull_request.reviewers) - other_added = [User.get(u) for u in current_reviewer_ids - org_reviewer_ids] - other_removed = [User.get(u) for u in org_reviewer_ids - current_reviewer_ids] + cur_reviewers = set(pull_request.get_reviewer_users()) + new_reviewers = set(_get_reviewer(s) for s in _form['review_members']) + old_reviewers = set(_get_reviewer(s) for s in _form['org_review_members']) + + other_added = cur_reviewers - old_reviewers + other_removed = old_reviewers - cur_reviewers + if other_added: h.flash(_('Meanwhile, the following reviewers have been added: %s') % (', '.join(u.username for u in other_added)), @@ -538,22 +547,20 @@ _form['updaterev'], _form['pullrequest_title'], _form['pullrequest_desc'], - reviewer_ids) + new_reviewers) + + added_reviewers = new_reviewers - old_reviewers - cur_reviewers + removed_reviewers = (old_reviewers - new_reviewers) & cur_reviewers old_description = pull_request.description pull_request.title = _form['pullrequest_title'] pull_request.description = _form['pullrequest_desc'].strip() or _('No description') pull_request.owner = User.get_by_username(_form['owner']) user = User.get(request.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().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() + + PullRequestModel().mention_from_description(user, pull_request, old_description) + PullRequestModel().add_reviewers(user, pull_request, added_reviewers) + PullRequestModel().remove_reviewers(user, pull_request, removed_reviewers) Session().commit() h.flash(_('Pull request updated'), category='success')
--- a/kallithea/lib/exceptions.py Mon Feb 27 15:44:49 2017 +0100 +++ b/kallithea/lib/exceptions.py Mon Feb 27 15:38:20 2017 +0100 @@ -99,10 +99,6 @@ pass -class UserInvalidException(Exception): - pass - - class RepositoryCreationError(Exception): pass
--- a/kallithea/model/pull_request.py Mon Feb 27 15:44:49 2017 +0100 +++ b/kallithea/model/pull_request.py Mon Feb 27 15:38:20 2017 +0100 @@ -34,9 +34,8 @@ from kallithea.model.meta import Session from kallithea.lib import helpers as h -from kallithea.lib.exceptions import UserInvalidException from kallithea.model.db import PullRequest, PullRequestReviewer, Notification, \ - ChangesetStatus, User + ChangesetStatus from kallithea.model.notification import NotificationModel from kallithea.lib.utils2 import extract_mentioned_users, safe_unicode @@ -44,21 +43,18 @@ log = logging.getLogger(__name__) -class PullRequestModel(object): +def _assert_valid_reviewers(seq): + """Sanity check: elements are actual User objects, and not the default user.""" + assert not any(user.is_default_user for user in seq) - def _get_valid_reviewers(self, seq): - """ Generate User objects from a sequence of user IDs, usernames or - User objects. Raises UserInvalidException if the DEFAULT user is - specified, or if a given ID or username does not match any user. - """ - for user_spec in seq: - user = User.guess_instance(user_spec) - if user is None or user.is_default_user: - raise UserInvalidException(user_spec) - yield user + +class PullRequestModel(object): def create(self, created_by, org_repo, org_ref, other_repo, other_ref, revisions, title, description, reviewers): + reviewers = set(reviewers) + _assert_valid_reviewers(reviewers) + pr = PullRequest() pr.org_repo = org_repo pr.org_ref = org_ref @@ -90,7 +86,6 @@ pull_request=pr, ) - reviewers = set(self._get_valid_reviewers(reviewers)) mention_recipients = extract_mentioned_users(description) self.add_reviewers(created_by, pr, reviewers, mention_recipients) @@ -99,9 +94,14 @@ 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)) + reviewers = set(reviewers) + _assert_valid_reviewers(reviewers) + if mention_recipients is not None: + mention_recipients = set(mention_recipients) - reviewers + _assert_valid_reviewers(mention_recipients) + #members - for reviewer in reviewer_users: + for reviewer in reviewers: prr = PullRequestReviewer(reviewer, pr) Session().add(prr) @@ -151,16 +151,12 @@ type_=Notification.TYPE_PULL_REQUEST, email_kwargs=email_kwargs) - mention_recipient_users = set() 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_recipient_users, + recipients=mention_recipients, type_=Notification.TYPE_PULL_REQUEST, email_kwargs=email_kwargs) @@ -171,12 +167,12 @@ log.debug("Mentioning %s", mention_recipients) self.add_reviewers(user, pr, set(), mention_recipients) - def remove_reviewers(self, user, pull_request, reviewer_ids): - """Remove users in the given user_id list from being reviewers of the PR.""" + def remove_reviewers(self, user, pull_request, reviewers): + """Remove specified users from being reviewers of the PR.""" PullRequestReviewer.query() \ .filter_by(pull_request=pull_request) \ - .filter(PullRequestReviewer.user_id.in_(reviewer_ids)) \ + .filter(PullRequestReviewer.user_id.in_(r.user_id for r in reviewers)) \ .delete(synchronize_session='fetch') # the default of 'evaluate' is not available def delete(self, pull_request):