launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-unify-findRuleGrantsByGrantee into lp:launchpad.

Commit message:
Unify GitRepository.findRuleGrants* into one method.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-unify-findRuleGrantsByGrantee/+merge/357616

This follows a pattern in some other places (e.g. GitRule.addGrant) of taking a "combined" grantee in higher-level contexts.

I'd like to find a more consistent way to describe and process combined grantees (and maybe a better name), but this will do for now.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-unify-findRuleGrantsByGrantee into lp:launchpad.

=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py	2018-10-18 16:06:38 +0000
+++ lib/lp/code/interfaces/gitrepository.py	2018-10-22 11:19:34 +0000
@@ -751,11 +751,9 @@
     def findRuleGrantsByGrantee(grantee):
         """Find the grants for a grantee applied to this repository.
 
-        :param grantee: The Person affected
-        """
-
-    def findRuleGrantsForRepositoryOwner():
-        """Find the grants of type REPOSITORY_OWNER applied to this repository.
+        :param grantee: The `IPerson` to search for, or an item of
+            `GitGranteeType` other than `GitGranteeType.PERSON` to search
+            for some other kind of entity.
         """
 
     @export_read_operation()

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2018-10-18 16:06:38 +0000
+++ lib/lp/code/model/gitrepository.py	2018-10-22 11:19:34 +0000
@@ -20,6 +20,7 @@
 from urllib import quote_plus
 
 from bzrlib import urlutils
+from lazr.enum import DBItem
 import pytz
 from storm.databases.postgres import Returning
 from storm.expr import (
@@ -1196,18 +1197,20 @@
 
     def findRuleGrantsByGrantee(self, grantee):
         """See `IGitRepository`."""
-        clauses = [
-            GitRuleGrant.grantee_type == GitGranteeType.PERSON,
-            TeamParticipation.person == grantee,
-            GitRuleGrant.grantee == TeamParticipation.teamID
-            ]
+        if isinstance(grantee, DBItem) and grantee.enum == GitGranteeType:
+            if grantee == GitGranteeType.PERSON:
+                raise ValueError(
+                    "grantee may not be GitGranteeType.PERSON; pass a person "
+                    "object instead")
+            clauses = [GitRuleGrant.grantee_type == grantee]
+        else:
+            clauses = [
+                GitRuleGrant.grantee_type == GitGranteeType.PERSON,
+                TeamParticipation.person == grantee,
+                GitRuleGrant.grantee == TeamParticipation.teamID
+                ]
         return self.grants.find(*clauses).config(distinct=True)
 
-    def findRuleGrantsForRepositoryOwner(self):
-        """See `IGitRepository`."""
-        return self.grants.find(
-            GitRuleGrant.grantee_type == GitGranteeType.REPOSITORY_OWNER)
-
     def getActivity(self, changed_after=None):
         """See `IGitRepository`."""
         clauses = [GitActivity.repository_id == self.id]

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2018-10-16 10:10:10 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2018-10-22 11:19:34 +0000
@@ -317,8 +317,10 @@
             grantee=GitGranteeType.REPOSITORY_OWNER,
             can_push=True,
             can_create=True)
+        self.factory.makeGitRuleGrant(rule=rule)
 
-        results = repository.findRuleGrantsForRepositoryOwner()
+        results = repository.findRuleGrantsByGrantee(
+            GitGranteeType.REPOSITORY_OWNER)
         self.assertEqual([grant], list(results))
 
     def test_findRuleGrantsByGrantee_owner_and_other(self):

=== modified file 'lib/lp/code/xmlrpc/git.py'
--- lib/lp/code/xmlrpc/git.py	2018-10-19 23:10:41 +0000
+++ lib/lp/code/xmlrpc/git.py	2018-10-22 11:19:34 +0000
@@ -368,7 +368,8 @@
         grants_for_user = defaultdict(list)
         grants = repository.findRuleGrantsByGrantee(requester)
         if is_owner:
-            owner_grants = repository.findRuleGrantsForRepositoryOwner()
+            owner_grants = repository.findRuleGrantsByGrantee(
+                GitGranteeType.REPOSITORY_OWNER)
             grants = grants.union(owner_grants)
         for grant in grants:
             grants_for_user[grant.rule].append(grant)