Changeset - bcc67f909d9f
Parent rev.
Child rev.
[Not reviewed]
default
0 3 0
Søren Løvborg - 9 years ago 2017-02-27 15:38:20
sorenl@unity3d.com
pullrequests: pass around reviewer User objects, not IDs

This moves reviewer user validation into the controller and eliminates
the UserInvalidException.
3 files changed with 52 insertions and 53 deletions:
kallithea/controllers/pullrequests.py
32
25
kallithea/lib/exceptions.py
4
kallithea/model/pull_request.py
20
24
0 comments (0 inline, 0 general)
kallithea/controllers/pullrequests.py
➞
Show inline comments
 
@@ -39,13 +39,12 @@ from kallithea.lib import helpers as h
 
from kallithea.lib import diffs
 
from kallithea.lib.auth import LoginRequired, HasRepoPermissionLevelDecorator, \
 
    NotAnonymous
 
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
 
from kallithea.lib.vcs.utils import safe_str
 
from kallithea.lib.vcs.utils.hgcompat import unionrepo
 
from kallithea.model.db import PullRequest, ChangesetStatus, ChangesetComment, \
 
@@ -62,12 +61,26 @@ from kallithea.controllers.changeset imp
 
from kallithea.controllers.compare import CompareController
 
from kallithea.lib.graphmod import graph_data
 

			




	
	
	
		
 
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):

			




	
	
	
		
 
        """return a structure with repo's interesting changesets, suitable for

			




	
	
	
		
 
        the selectors in pullrequest.html

			




	
	
	
		
 

			




	
	
		
 
@@ -360,13 +373,13 @@ class PullrequestsController(BaseRepoCon

			




	
	
	
		
 
        revisions = [cs_.raw_id for cs_ in cs_ranges]

			




	
	
	
		
 

			




	
	
	
		
 
        # hack: ancestor_rev is not an other_rev but we want to show the

			




	
	
	
		
 
        # 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:

			




	
	
	
		
 
            if org_repo == other_repo:

			




	
	
	
		
 
                title = '%s to %s' % (org_display, other_display)

			




	
	
	
		
 
            else:

			




	
	
		
 
@@ -374,28 +387,25 @@ class PullrequestsController(BaseRepoCon

			




	
	
	
		
 
                                            other_repo.repo_name, other_display)

			




	
	
	
		
 
        description = _form['pullrequest_desc'].strip() or _('No description')

			




	
	
	
		
 
        try:

			




	
	
	
		
 
            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')

			




	
	
	
		
 
            log.error(traceback.format_exc())

			




	
	
	
		
 
            raise HTTPFound(location=url('pullrequest_home', repo_name=repo_name))

			




	
	
	
		
 

			




	
	
	
		
 
        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)

			




	
	
	
		
 

			




	
	
	
		
 
        other_repo = old_pull_request.other_repo

			




	
	
	
		
 
        other_ref_type, other_ref_name, other_rev = old_pull_request.other_ref.split(':') # other_rev is ancestor

			




	
	
		
 
@@ -475,16 +485,13 @@ class PullrequestsController(BaseRepoCon

			




	
	
	
		
 
            description += '\n\n' + descriptions[1].strip()

			




	
	
	
		
 

			




	
	
	
		
 
        try:

			




	
	
	
		
 
            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')

			




	
	
	
		
 
            log.error(traceback.format_exc())

			




	
	
	
		
 
            raise HTTPFound(location=old_pull_request.url())

			




	
	
	
		
 

			




	
	
		
 
@@ -515,18 +522,20 @@ class PullrequestsController(BaseRepoCon

			




	
	
	
		
 
        owner = pull_request.owner_id == request.authuser.user_id

			




	
	
	
		
 
        repo_admin = h.HasRepoPermissionLevel('admin')(c.repo_name)

			




	
	
	
		
 
        if not (h.HasPermissionAny('hg.admin')() or repo_admin or owner):

			




	
	
	
		
 
            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)),

			




	
	
	
		
 
                    category='warning')

			




	
	
	
		
 
        if other_removed:

			




	
	
	
		
 
            h.flash(_('Meanwhile, the following reviewers have been removed: %s') %

			




	
	
		
 
@@ -535,28 +544,26 @@ class PullrequestsController(BaseRepoCon

			




	
	
	
		
 

			




	
	
	
		
 
        if _form['updaterev']:

			




	
	
	
		
 
            return self.create_new_iteration(pull_request,

			




	
	
	
		
 
                                      _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().add_reviewers(user, pull_request, added_reviewers)

			




	
	
	
		
 
        PullRequestModel().remove_reviewers(user, pull_request, removed_reviewers)

			




	
	
	
		
 

			




	
	
	
		
 
        Session().commit()

			




	
	
	
		
 
        h.flash(_('Pull request updated'), category='success')

			




	
	
	
		
 

			




	
	
	
		
 
        raise HTTPFound(location=pull_request.url())

			




	
	
	
		
 

			




        

    


    
    

    

        

                

                    kallithea/lib/exceptions.py
                

                

                  
                      
                        

                      ➞

                      
                        
                  

                  
                      
                  
                      
                  
                      
                  
                      
                  
                  
                

                

                    Show inline comments
                    
                

        

        

            



	
	
		
 
@@ -96,16 +96,12 @@ class IMCCommitError(Exception):

			




	
	
	
		
 

			




	
	
	
		
 

			




	
	
	
		
 
class UserCreationError(Exception):

			




	
	
	
		
 
    pass

			




	
	
	
		
 

			




	
	
	
		
 

			




	
	
	
		
 
class UserInvalidException(Exception):

			




	
	
	
		
 
    pass

			




	
	
	
		
 

			




	
	
	
		
 

			




	
	
	
		
 
class RepositoryCreationError(Exception):

			




	
	
	
		
 
    pass

			




	
	
	
		
 

			




	
	
	
		
 

			




	
	
	
		
 
class HgsubversionImportError(Exception):

			




	
	
	
		
 
    pass

			




        

    


    
    

    

        

                

                    kallithea/model/pull_request.py
                

                

                  
                      
                        

                      ➞

                      
                        
                  

                  
                      
                  
                      
                  
                      
                  
                      
                  
                  
                

                

                    Show inline comments
                    
                

        

        

            



	
	
		
 
@@ -31,37 +31,33 @@ import datetime

			




	
	
	
		
 
from pylons.i18n.translation import _

			




	
	
	
		
 

			




	
	
	
		
 
from sqlalchemy.orm import joinedload

			




	
	
	
		
 

			




	
	
	
		
 
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

			




	
	
	
		
 

			




	
	
	
		
 

			




	
	
	
		
 
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

			




	
	
	
		
 
        pr.other_repo = other_repo

			




	
	
	
		
 
        pr.other_ref = other_ref

			




	
	
	
		
 
        pr.revisions = revisions

			




	
	
		
 
@@ -87,24 +83,28 @@ class PullRequestModel(object):

			




	
	
	
		
 
            ChangesetStatus.STATUS_UNDER_REVIEW,

			




	
	
	
		
 
            created_by,

			




	
	
	
		
 
            comment,

			




	
	
	
		
 
            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)

			




	
	
	
		
 

			




	
	
	
		
 
        return pr

			




	
	
	
		
 

			




	
	
	
		
 
    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)

			




	
	
	
		
 

			




	
	
	
		
 
        #notification to reviewers

			




	
	
	
		
 
        pr_url = pr.url(canonical=True)

			




	
	
	
		
 
        threading = ['%s-pr-%s@%s' % (pr.other_repo.repo_name,

			




	
	
		
 
@@ -148,38 +148,34 @@ class PullRequestModel(object):

			




	
	
	
		
 
        if reviewers:

			




	
	
	
		
 
            NotificationModel().create(created_by=user, subject=subject, body=body,

			




	
	
	
		
 
                                       recipients=reviewers,

			




	
	
	
		
 
                                       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)

			




	
	
	
		
 

			




	
	
	
		
 
    def mention_from_description(self, user, pr, old_description=''):

			




	
	
	
		
 
        mention_recipients = (extract_mentioned_users(pr.description) -

			




	
	
	
		
 
                              extract_mentioned_users(old_description))

			




	
	
	
		
 

			




	
	
	
		
 
        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):

			




	
	
	
		
 
        pull_request = PullRequest.guess_instance(pull_request)

			




	
	
	
		
 
        Session().delete(pull_request)

			




	
	
	
		
 

			




        

    



    


    



    



      

      





    
    0 comments (0 inline, 0 general)
    





    


  

  







  


    




    








    
        
    
    
        This site is powered by
            Kallithea 0.7.0,
        which is
        © 2010–2025 by various authors & licensed under GPLv3.