kallithea Changeset - 533ec10dfd59

Changeset - 533ec10dfd59

Parent rev.

Child rev.

[Not reviewed]

default

0 2 0

Søren Løvborg - 9 years ago 2016-10-10 23:15:05
sorenl@unity3d.com

pullrequests: refactor PullRequestModel().create

The database lookups of the user and repositories are moved outside,
into the callers, which have already looked up the relevant objects,
thus saving two database lookups when creating a PR.

2 files changed with 26 insertions and 33 deletions:

kallithea/controllers/pullrequests.py

kallithea/model/pull_request.py

0 comments (0 inline, 0 general)

kallithea/controllers/pullrequests.py

➞

Show inline comments

@@ @@ -360,52 +360,52 @@ class PullrequestsController(BaseRepoCon @@
                     ) % (other_ref_name, org_ref_name)
         if ancestor_rev is None:
             h.flash(msg, category='error')
             log.error(msg)
             raise HTTPNotFound
         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 = []
         title = _form['pullrequest_title']
         if not title:
             if org_repo_name == other_repo_name:
                 title = '%s to %s' % (h.short_ref(org_ref_type, org_ref_name),
                                       h.short_ref(other_ref_type, other_ref_name))
             else:
                 title = '%s#%s to %s#%s' % (org_repo_name, h.short_ref(org_ref_type, org_ref_name),
                                             other_repo_name, h.short_ref(other_ref_type, other_ref_name))
         description = _form['pullrequest_desc'].strip() or _('No description')
         try:
             created_by = User.get(self.authuser.user_id)
             pull_request = PullRequestModel().create(
                 self.authuser.user_id, org_repo_name, org_ref, other_repo_name,
                 other_ref, revisions, reviewer_ids, title, description
+            )
                 created_by, org_repo, org_ref, other_repo, other_ref, revisions,
                 title, description, reviewer_ids)
             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):
         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
         #assert other_ref_type == 'branch', other_ref_type # TODO: what if not?
         new_other_rev = self._get_ref_rev(other_repo, other_ref_type, other_ref_name)
@@ @@ -462,54 +462,52 @@ class PullrequestsController(BaseRepoCon @@
                 infos.append(_('This iteration is based on another %s revision and there is no simple diff.') % other_ref_name)
         else:
            infos.append(_('No changes found on %s %s since previous iteration.') % (org_ref_type, org_ref_name))
            # TODO: fail?
         # hack: ancestor_rev is not an other_ref but we want to show the
         # requested destination and have the exact ancestor
         new_other_ref = '%s:%s:%s' % (other_ref_type, other_ref_name, ancestor_rev)
         new_org_ref = '%s:%s:%s' % (org_ref_type, org_ref_name, new_org_rev)
         try:
             title, old_v = re.match(r'(.*)\(v(\d+)\)\s*$', title).groups()
             v = int(old_v) + 1
         except (AttributeError, ValueError):
             v = 2
         title = '%s (v%s)' % (title.strip(), v)
         # using a mail-like separator, insert new iteration info in description with latest first
         descriptions = description.replace('\r\n', '\n').split('\n-- \n', 1)
         description = descriptions[0].strip() + '\n\n-- \n' + '\n'.join(infos)
         if len(descriptions) > 1:
             description += '\n\n' + descriptions[1].strip()
         try:
             created_by = User.get(self.authuser.user_id)
             pull_request = PullRequestModel().create(
                 self.authuser.user_id,
                 old_pull_request.org_repo.repo_name, new_org_ref,
                 old_pull_request.other_repo.repo_name, new_other_ref,
                 revisions, reviewer_ids, title, description
+            )
                 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()
         except Exception:
             h.flash(_('Error occurred while creating pull request'),
                     category='error')
             log.error(traceback.format_exc())
             raise HTTPFound(location=old_pull_request.url())
         ChangesetCommentsModel().create(
             text=_('Closed, next iteration: %s .') % pull_request.url(canonical=True),
             repo=old_pull_request.other_repo_id,
             author=c.authuser.user_id,
             pull_request=old_pull_request.pull_request_id,
             closing_pr=True)
         PullRequestModel().close_pull_request(old_pull_request.pull_request_id)
         Session().commit()
         h.flash(_('New pull request iteration created'),
                 category='success')
         raise HTTPFound(location=pull_request.url())
     # pullrequest_post for PR editing

kallithea/model/pull_request.py

➞

Show inline comments

@@ @@ -38,90 +38,85 @@ from kallithea.lib.exceptions import Use @@
 from kallithea.model.base import BaseModel
 from kallithea.model.db import PullRequest, PullRequestReviewer, Notification, \
     ChangesetStatus, User
 from kallithea.model.notification import NotificationModel
 from kallithea.lib.utils2 import extract_mentioned_users, safe_unicode
 log = logging.getLogger(__name__)
 class PullRequestModel(BaseModel):
     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
         created_by_user = self._get_user(created_by)
         org_repo = self._get_repo(org_repo)
         other_repo = self._get_repo(other_repo)
         new = PullRequest()
         new.org_repo = org_repo
         new.org_ref = org_ref
         new.other_repo = other_repo
         new.other_ref = other_ref
         new.revisions = revisions
         new.title = title
         new.description = description
         new.owner = created_by_user
         Session().add(new)
         Session().flush()
                revisions, title, description, 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
         pr.title = title
         pr.description = description
         pr.owner = created_by
         Session().add(pr)
         Session().flush() # make database assign pull_request_id
         #reset state to under-review
         from kallithea.model.changeset_status import ChangesetStatusModel
         from kallithea.model.comment import ChangesetCommentsModel
         comment = ChangesetCommentsModel().create(
             text=u'',
             repo=org_repo,
             author=new.owner,
             pull_request=new,
             author=created_by,
             pull_request=pr,
             send_email=False,
             status_change=ChangesetStatus.STATUS_UNDER_REVIEW,
+        )
         ChangesetStatusModel().set_status(
             org_repo,
             ChangesetStatus.STATUS_UNDER_REVIEW,
-            new.owner,
+            created_by,
             comment,
-            pull_request=new
+            pull_request=pr,
+        )
         reviewers = set(self._get_valid_reviewers(reviewers))
         mention_recipients = extract_mentioned_users(new.description)
         self.add_reviewers(created_by_user, new, reviewers, mention_recipients)
         mention_recipients = extract_mentioned_users(description)
         self.add_reviewers(created_by, pr, reviewers, mention_recipients)
-        return new
+        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))
         #members
         for reviewer in reviewer_users:
             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,
                                       pr.pull_request_id,
                                       h.canonical_hostname())]
         subject = safe_unicode(
             h.link_to(
               _('%(user)s wants you to review pull request %(pr_nice_id)s: %(pr_title)s') % \
                 {'user': user.username,
                  'pr_title': pr.title,
                  'pr_nice_id': pr.nice_id()},
                 pr_url)
+            )
         body = pr.description

0 comments (0 inline, 0 general)