Mercurial > kallithea

changeset 6222:8ad40ef0ea80

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
db: add some PullRequest.query() shortcuts This makes database query code more explicit and increases readability. E.g. the function name get_pullrequest_cnt_for_user was bad, because the concept of "pullrequest for user" is incredibly vague, and could refer to any kind of association between PRs and users. (Quiz time! Does it mean that the user is the PR owner, that the user is reviewing, or that the user has commented on the PR and thus is receiving notifications?) A descriptive name could be "get_open_pull_request_count_for_reviewer", because the function is indeed only concerned with reviewers and only with open pull requests. But at this point, we might as well say PullRequest.query(reviewer_id=user, include_closed=False).count() which is only slightly longer, and doesn't require us to write dozens of little wrapper functions (including, any moment now, a separate function for listing the PRs instead of counting them). Note that we're not actually going down an abstraction level by doing this. We're still operating on the concepts of "pull request", "open" and "reviewer", and are not leaking database implementation details. The query() shortcuts are designed so they default to not altering the query. Any processing requires explicit opt-in by the caller.
author Søren Løvborg <sorenl@unity3d.com>
date Thu, 15 Sep 2016 13:36:05 +0200
parents 7c0b55fb3a85
children 873a3839865d
files kallithea/controllers/pullrequests.py kallithea/lib/base.py kallithea/model/db.py kallithea/model/pull_request.py
diffstat 4 files changed, 41 insertions(+), 40 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/controllers/pullrequests.py	Mon Sep 05 18:08:14 2016 +0200
+++ b/kallithea/controllers/pullrequests.py	Thu Sep 15 13:36:05 2016 +0200
@@ -195,9 +195,15 @@
     def show_all(self, repo_name):
         c.from_ = request.GET.get('from_') or ''
         c.closed = request.GET.get('closed') or ''
-        c.pull_requests = PullRequestModel().get_all(repo_name, from_=c.from_, closed=c.closed)
         p = safe_int(request.GET.get('page'), 1)
 
+        q = PullRequest.query(include_closed=c.closed, sorted=True)
+        if c.from_:
+            q = q.filter_by(org_repo=c.db_repo)
+        else:
+            q = q.filter_by(other_repo=c.db_repo)
+        c.pull_requests = q.all()
+
         c.pullrequests_pager = Page(c.pull_requests, page=p, items_per_page=100)
 
         return render('/pullrequests/pullrequest_show_all.html')
@@ -207,25 +213,19 @@
     def show_my(self):
         c.closed = request.GET.get('closed') or ''
 
-        def _filter(pr):
-            s = sorted(pr, key=lambda o: o.created_on, reverse=True)
-            if not c.closed:
-                s = filter(lambda p: p.status != PullRequest.STATUS_CLOSED, s)
-            return s
-
-        c.my_pull_requests = _filter(PullRequest.query() \
-                                .filter(PullRequest.user_id ==
-                                        self.authuser.user_id) \
-                                .all())
+        c.my_pull_requests = PullRequest.query(
+            include_closed=c.closed,
+            sorted=True,
+        ).filter_by(user_id=self.authuser.user_id).all()
 
         c.participate_in_pull_requests = []
         c.participate_in_pull_requests_todo = []
         done_status = set([ChangesetStatus.STATUS_APPROVED, ChangesetStatus.STATUS_REJECTED])
-        for pr in _filter(PullRequest.query()
-                                .join(PullRequestReviewers)
-                                .filter(PullRequestReviewers.user_id ==
-                                        self.authuser.user_id)
-                         ):
+        for pr in PullRequest.query(
+            include_closed=c.closed,
+            reviewer_id=self.authuser.user_id,
+            sorted=True,
+        ):
             status = pr.user_review_status(c.authuser.user_id) # very inefficient!!!
             if status in done_status:
                 c.participate_in_pull_requests.append(pr)
--- a/kallithea/lib/base.py	Mon Sep 05 18:08:14 2016 +0200
+++ b/kallithea/lib/base.py	Thu Sep 15 13:36:05 2016 +0200
@@ -55,10 +55,9 @@
 from kallithea.lib.vcs.exceptions import RepositoryError, EmptyRepositoryError, ChangesetDoesNotExistError
 from kallithea.model import meta
 
-from kallithea.model.db import Repository, Ui, User, Setting
+from kallithea.model.db import PullRequest, Repository, Ui, User, Setting
 from kallithea.model.notification import NotificationModel
 from kallithea.model.scm import ScmModel
-from kallithea.model.pull_request import PullRequestModel
 
 log = logging.getLogger(__name__)
 
@@ -359,7 +358,7 @@
 
         self.cut_off_limit = safe_int(config.get('cut_off_limit'))
 
-        c.my_pr_count = PullRequestModel().get_pullrequest_cnt_for_user(c.authuser.user_id)
+        c.my_pr_count = PullRequest.query(reviewer_id=c.authuser.user_id, include_closed=False).count()
 
         self.sa = meta.Session
         self.scm_model = ScmModel(self.sa)
--- a/kallithea/model/db.py	Mon Sep 05 18:08:14 2016 +0200
+++ b/kallithea/model/db.py	Thu Sep 15 13:36:05 2016 +0200
@@ -2340,6 +2340,29 @@
     comments = relationship('ChangesetComment', order_by='ChangesetComment.comment_id',
                              cascade="all, delete-orphan")
 
+    @classmethod
+    def query(cls, reviewer_id=None, include_closed=True, sorted=False):
+        """Add PullRequest-specific helpers for common query constructs.
+
+        reviewer_id: only PRs with the specified user added as reviewer.
+
+        include_closed: if False, do not include closed PRs.
+
+        sorted: if True, apply the default ordering (newest first).
+        """
+        q = super(PullRequest, cls).query()
+
+        if reviewer_id is not None:
+            q = q.join(PullRequestReviewers).filter(PullRequestReviewers.user_id == reviewer_id)
+
+        if not include_closed:
+            q = q.filter(PullRequest.status != PullRequest.STATUS_CLOSED)
+
+        if sorted:
+            q = q.order_by(PullRequest.created_on.desc())
+
+        return q
+
     def get_reviewer_users(self):
         """Like .reviewers, but actually returning the users"""
         return User.query() \
--- a/kallithea/model/pull_request.py	Mon Sep 05 18:08:14 2016 +0200
+++ b/kallithea/model/pull_request.py	Thu Sep 15 13:36:05 2016 +0200
@@ -47,27 +47,6 @@
 
 class PullRequestModel(BaseModel):
 
-    def get_pullrequest_cnt_for_user(self, user):
-        return PullRequest.query() \
-                                .join(PullRequestReviewers) \
-                                .filter(PullRequestReviewers.user_id == user) \
-                                .filter(PullRequest.status != PullRequest.STATUS_CLOSED) \
-                                .count()
-
-    def get_all(self, repo_name, from_=False, closed=False):
-        """Get all PRs for repo.
-        Default is all PRs to the repo, PRs from the repo if from_.
-        Closed PRs are only included if closed is true."""
-        repo = self._get_repo(repo_name)
-        q = PullRequest.query()
-        if from_:
-            q = q.filter(PullRequest.org_repo == repo)
-        else:
-            q = q.filter(PullRequest.other_repo == repo)
-        if not closed:
-            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