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 53 insertions and 54 deletions:
kallithea/controllers/pullrequests.py
33
26
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
 
@@ -42,7 +42,6 @@ from kallithea.lib.auth import LoginRequ
 
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 @@ from kallithea.lib.graphmod import graph
 
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 @@ class PullrequestsController(BaseRepoCon

			




	
	
	
		
 
        # 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 @@ class PullrequestsController(BaseRepoCon

			




	
	
	
		
 
            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 @@ class PullrequestsController(BaseRepoCon

			




	
	
	
		
 

			




	
	
	
		
 
        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 @@ class PullrequestsController(BaseRepoCon

			




	
	
	
		
 
            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 @@ class PullrequestsController(BaseRepoCon

			




	
	
	
		
 
            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 @@ class PullrequestsController(BaseRepoCon

			




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

			




        

    


    
    

    

        

                

                    kallithea/lib/exceptions.py
                

                

                  
                      
                        

                      ➞

                      
                        
                  

                  
                      
                  
                      
                  
                      
                  
                      
                  
                  
                

                

                    Show inline comments
                    
                

        

        

            



	
	
		
 
@@ -99,10 +99,6 @@ class UserCreationError(Exception):

			




	
	
	
		
 
    pass

			




	
	
	
		
 

			




	
	
	
		
 

			




	
	
	
		
 
class UserInvalidException(Exception):

			




	
	
	
		
 
    pass

			




	
	
	
		
 

			




	
	
	
		
 

			




	
	
	
		
 
class RepositoryCreationError(Exception):

			




	
	
	
		
 
    pass

			




	
	
	
		
 

			




        

    


    
    

    

        

                

                    kallithea/model/pull_request.py
                

                

                  
                      
                        

                      ➞

                      
                        
                  

                  
                      
                  
                      
                  
                      
                  
                      
                  
                  
                

                

                    Show inline comments
                    
                

        

        

            



	
	
		
 
@@ -34,9 +34,8 @@ 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

			




	
	
	
		
 

			




	
	
		
 
@@ -44,21 +43,18 @@ from kallithea.lib.utils2 import extract

			




	
	
	
		
 
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 @@ class PullRequestModel(object):

			




	
	
	
		
 
            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 @@ class PullRequestModel(object):

			




	
	
	
		
 
    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 @@ class PullRequestModel(object):

			




	
	
	
		
 
                                       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 @@ class PullRequestModel(object):

			




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

			




        

    



    


    



    



      

      





    
    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.