Mercurial > kallithea
changeset 5878:1658beb26ff9
pull requests: prevent adding DEFAULT user as reviewer
Add a helper method to resolve reviewers, with an added check to prevent
the adding of the DEFAULT user as reviewer.
The __add_reviewers method, although internal, had a troubling interface
where the method was responsible for resolving reviewers, but the caller
was responsible for resolving mention_recipients. The method is changed
to always take two sets of database User objects, leaving the resolution
to the caller in both cases.
author | Søren Løvborg <sorenl@unity3d.com> |
---|---|
date | Wed, 06 Apr 2016 21:47:53 +0200 |
parents | ba5fee3879c8 |
children | 278a742731cd |
files | kallithea/model/pull_request.py |
diffstat | 1 files changed, 19 insertions(+), 9 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/model/pull_request.py Wed Apr 06 14:50:47 2016 +0200 +++ b/kallithea/model/pull_request.py Wed Apr 06 21:47:53 2016 +0200 @@ -35,7 +35,7 @@ from kallithea.lib.exceptions import UserInvalidException from kallithea.model import BaseModel from kallithea.model.db import PullRequest, PullRequestReviewers, Notification, \ - ChangesetStatus + ChangesetStatus, User from kallithea.model.notification import NotificationModel from kallithea.lib.utils2 import extract_mentioned_users, safe_unicode @@ -71,6 +71,17 @@ q = q.filter(PullRequest.status != PullRequest.STATUS_CLOSED) return q.order_by(PullRequest.created_on.desc()).all() + 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 = self._get_user(user_spec) + if user is None or user.username == User.DEFAULT_USER: + raise UserInvalidException(user_spec) + yield user + def create(self, created_by, org_repo, org_ref, other_repo, other_ref, revisions, reviewers, title, description=None): from kallithea.model.changeset_status import ChangesetStatusModel @@ -109,18 +120,17 @@ pull_request=new ) + reviewers = set(self._get_valid_reviewers(reviewers)) mention_recipients = extract_mentioned_users(new.description) self.__add_reviewers(created_by_user, new, reviewers, mention_recipients) return new - def __add_reviewers(self, user, pr, reviewers, mention_recipients=None): + def __add_reviewers(self, user, pr, reviewers, mention_recipients): + # reviewers and mention_recipients should be sets of User objects. #members - for member in set(reviewers): - _usr = self._get_user(member) - if _usr is None: - raise UserInvalidException(member) - reviewer = PullRequestReviewers(_usr, pr) + for reviewer in reviewers: + reviewer = PullRequestReviewers(reviewer, pr) Session().add(reviewer) revision_data = [(x.raw_id, x.message) @@ -176,7 +186,7 @@ extract_mentioned_users(old_description)) log.debug("Mentioning %s", mention_recipients) - self.__add_reviewers(user, pr, [], mention_recipients) + self.__add_reviewers(user, pr, set(), mention_recipients) def update_reviewers(self, user, pull_request, reviewers_ids): reviewers_ids = set(reviewers_ids) @@ -191,7 +201,7 @@ to_remove = current_reviewers_ids.difference(reviewers_ids) log.debug("Adding %s reviewers", to_add) - self.__add_reviewers(user, pull_request, to_add) + self.__add_reviewers(user, pull_request, set(self._get_valid_reviewers(to_add)), set()) log.debug("Removing %s reviewers", to_remove) for uid in to_remove: