Merge lp:~cjwatson/launchpad/git-permissions-vocabulary into lp:launchpad

[Colin Watson]

Add a vocabulary for typical combinations of Git access permissions.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/code/interfaces/gitrule.py'
--- lib/lp/code/interfaces/gitrule.py	2018-10-19 23:10:41 +0000
+++ lib/lp/code/interfaces/gitrule.py	2018-10-22 00:40:44 +0000
@@ -7,6 +7,7 @@
 
 __metaclass__ = type
 __all__ = [
+    'describe_git_permissions',
     'IGitNascentRuleGrant',
     'IGitRule',
     'IGitRuleGrant',
@@ -218,3 +219,20 @@
 
     can_force_push = copy_field(
         IGitRuleGrant["can_force_push"], required=False, default=False)
+
+
+def describe_git_permissions(permissions, verbose=False):
+    """Return human-readable descriptions of a set of Git access permissions.
+
+    :param permissions: A collection of `GitPermissionType`.
+    :return: A list of human-readable descriptions of the input permissions,
+        in a conventional order.
+    """
+    names = []
+    if GitPermissionType.CAN_CREATE in permissions:
+        names.append("create")
+    if GitPermissionType.CAN_PUSH in permissions:
+        names.append("push")
+    if GitPermissionType.CAN_FORCE_PUSH in permissions:
+        names.append("force-push")
+    return names

=== modified file 'lib/lp/code/model/gitjob.py'
--- lib/lp/code/model/gitjob.py	2018-10-16 10:04:18 +0000
+++ lib/lp/code/model/gitjob.py	2018-10-22 00:40:44 +0000
@@ -33,7 +33,10 @@
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.errors import NotFoundError
-from lp.code.enums import GitActivityType
+from lp.code.enums import (
+    GitActivityType,
+    GitPermissionType,
+    )
 from lp.code.interfaces.githosting import IGitHostingClient
 from lp.code.interfaces.gitjob import (
     IGitJob,
@@ -44,6 +47,7 @@
     IReclaimGitRepositorySpaceJob,
     IReclaimGitRepositorySpaceJobSource,
     )
+from lp.code.interfaces.gitrule import describe_git_permissions
 from lp.code.mail.branch import BranchMailer
 from lp.registry.interfaces.person import IPersonSet
 from lp.services.config import config
@@ -319,15 +323,20 @@
     }
 
 
+_activity_permissions = {
+    "can_create": GitPermissionType.CAN_CREATE,
+    "can_push": GitPermissionType.CAN_PUSH,
+    "can_force_push": GitPermissionType.CAN_FORCE_PUSH,
+    }
+
+
 def describe_grants(activity_value):
-    output = []
     if activity_value is not None:
-        if activity_value.get("can_create"):
-            output.append("create")
-        if activity_value.get("can_push"):
-            output.append("push")
-        if activity_value.get("can_force_push"):
-            output.append("force-push")
+        output = describe_git_permissions({
+            permission for attr, permission in _activity_permissions.items()
+            if activity_value.get(attr)})
+    else:
+        output = []
     return english_list(output)
 
 

=== modified file 'lib/lp/code/model/gitrule.py'
--- lib/lp/code/model/gitrule.py	2018-10-19 23:10:41 +0000
+++ lib/lp/code/model/gitrule.py	2018-10-22 00:40:44 +0000
@@ -48,6 +48,7 @@
     )
 from lp.code.interfaces.gitactivity import IGitActivitySet
 from lp.code.interfaces.gitrule import (
+    describe_git_permissions,
     IGitNascentRuleGrant,
     IGitRule,
     IGitRuleGrant,
@@ -283,20 +284,13 @@
         self.date_last_modified = date_created
 
     def __repr__(self):
-        permissions = []
-        if self.can_create:
-            permissions.append("create")
-        if self.can_push:
-            permissions.append("push")
-        if self.can_force_push:
-            permissions.append("force-push")
         if self.grantee_type == GitGranteeType.PERSON:
             grantee_name = "~%s" % self.grantee.name
         else:
             grantee_name = self.grantee_type.title.lower()
         return "<GitRuleGrant [%s] to %s for %s:%s>" % (
-            ", ".join(permissions), grantee_name, self.repository.unique_name,
-            self.rule.ref_pattern)
+            ", ".join(describe_git_permissions(self.permissions)),
+            grantee_name, self.repository.unique_name, self.rule.ref_pattern)
 
     @property
     def permissions(self):

=== modified file 'lib/lp/code/vocabularies/configure.zcml'
--- lib/lp/code/vocabularies/configure.zcml	2015-06-24 21:14:20 +0000
+++ lib/lp/code/vocabularies/configure.zcml	2018-10-22 00:40:44 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2010-2018 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -77,4 +77,15 @@
     <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary"/>
   </class>
 
+  <securedutility
+    name="GitPermissions"
+    component=".gitrule.GitPermissionsVocabulary"
+    provides="zope.schema.interfaces.IVocabularyFactory">
+    <allow interface="zope.schema.interfaces.IVocabularyFactory"/>
+  </securedutility>
+
+  <class class=".gitrule.GitPermissionsVocabulary">
+    <allow interface="zope.schema.interfaces.IVocabularyTokenized"/>
+  </class>
+
 </configure>

=== added file 'lib/lp/code/vocabularies/gitrule.py'
--- lib/lp/code/vocabularies/gitrule.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/vocabularies/gitrule.py	2018-10-22 00:40:44 +0000
@@ -0,0 +1,108 @@
+# Copyright 2018 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Vocabularies related to Git access rules."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'GitPermissionsVocabulary',
+    ]
+
+from zope.schema.vocabulary import (
+    SimpleTerm,
+    SimpleVocabulary,
+    )
+
+from lp import _
+from lp.code.enums import GitPermissionType
+from lp.code.interfaces.gitref import IGitRef
+from lp.code.interfaces.gitrule import (
+    describe_git_permissions,
+    IGitRule,
+    IGitRuleGrant,
+    )
+
+
+branch_permissions = [
+    SimpleTerm(
+        frozenset(),
+        "cannot_push", _("Cannot push")),
+    SimpleTerm(
+        frozenset({
+            GitPermissionType.CAN_PUSH,
+            }),
+        "can_push_existing", _("Can push if the branch already exists")),
+    SimpleTerm(
+        frozenset({
+            GitPermissionType.CAN_CREATE,
+            GitPermissionType.CAN_PUSH,
+            }),
+        "can_push", _("Can push")),
+    SimpleTerm(
+        frozenset({
+            GitPermissionType.CAN_CREATE,
+            GitPermissionType.CAN_PUSH,
+            GitPermissionType.CAN_FORCE_PUSH,
+            }),
+        "can_force_push", _("Can force-push")),
+    ]
+
+
+tag_permissions = [
+    SimpleTerm(
+        frozenset(),
+        "cannot_create", _("Cannot create")),
+    SimpleTerm(
+        frozenset({
+            GitPermissionType.CAN_CREATE,
+            }),
+        "can_create", _("Can create")),
+    SimpleTerm(
+        frozenset({
+            GitPermissionType.CAN_CREATE,
+            GitPermissionType.CAN_PUSH,
+            GitPermissionType.CAN_FORCE_PUSH,
+            }),
+        "can_move", _("Can move")),
+    ]
+
+
+class GitPermissionsVocabulary(SimpleVocabulary):
+    """A vocabulary for typical combinations of Git access permissions.
+
+    The terms of this vocabulary are combinations of permissions that make
+    sense in the UI without being too overwhelming, depending on the rule's
+    reference pattern (different combinations make sense for branches vs.
+    tags).
+    """
+
+    def __init__(self, context):
+        if IGitRef.providedBy(context):
+            path = context.path
+        elif IGitRule.providedBy(context):
+            path = context.ref_pattern
+        elif IGitRuleGrant.providedBy(context):
+            path = context.rule.ref_pattern
+        else:
+            raise AssertionError("GitPermissionsVocabulary needs a context.")
+        if path.startswith("refs/tags/"):
+            terms = list(tag_permissions)
+        else:
+            # We could restrict this to just refs/heads/*, but it's helpful
+            # to be able to offer *some* choices in the UI if somebody tries
+            # to create grants for e.g. refs/*, and the choices we offer for
+            # branches are probably more useful there than those we offer
+            # for tags.
+            terms = list(branch_permissions)
+        if IGitRuleGrant.providedBy(context):
+            grant_permissions = context.permissions
+            if grant_permissions not in (term.value for term in terms):
+                # Supplement the vocabulary with any atypical permissions
+                # used by this grant.
+                names = describe_git_permissions(grant_permissions)
+                terms.append(SimpleTerm(
+                    grant_permissions,
+                    "custom", "Custom permissions: %s" % ", ".join(names)))
+        super(GitPermissionsVocabulary, self).__init__(terms)

=== added file 'lib/lp/code/vocabularies/tests/test_gitrule_vocabularies.py'
--- lib/lp/code/vocabularies/tests/test_gitrule_vocabularies.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/vocabularies/tests/test_gitrule_vocabularies.py	2018-10-22 00:40:44 +0000
@@ -0,0 +1,103 @@
+# Copyright 2018 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test vocabularies related to Git access rules."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+from lp.code.enums import GitPermissionType
+from lp.code.vocabularies.gitrule import GitPermissionsVocabulary
+from lp.testing import TestCaseWithFactory
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class TestGitPermissionsVocabulary(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    expected_branch_values = [
+        set(),
+        {GitPermissionType.CAN_PUSH},
+        {GitPermissionType.CAN_CREATE, GitPermissionType.CAN_PUSH},
+        {GitPermissionType.CAN_CREATE, GitPermissionType.CAN_PUSH,
+         GitPermissionType.CAN_FORCE_PUSH},
+        ]
+    expected_branch_tokens = [
+        "cannot_push", "can_push_existing", "can_push", "can_force_push",
+        ]
+    expected_tag_values = [
+        set(),
+        {GitPermissionType.CAN_CREATE},
+        {GitPermissionType.CAN_CREATE, GitPermissionType.CAN_PUSH,
+         GitPermissionType.CAN_FORCE_PUSH},
+        ]
+    expected_tag_tokens = ["cannot_create", "can_create", "can_move"]
+
+    def assertVocabularyHasTerms(self, context, expected_values,
+                                 expected_tokens):
+        vocabulary = GitPermissionsVocabulary(context)
+        terms = list(vocabulary)
+        self.assertEqual(expected_values, [term.value for term in terms])
+        self.assertEqual(expected_tokens, [term.token for term in terms])
+        return terms
+
+    def test_ref_branch(self):
+        [ref] = self.factory.makeGitRefs(paths=["refs/heads/master"])
+        self.assertVocabularyHasTerms(
+            ref, self.expected_branch_values, self.expected_branch_tokens)
+
+    def test_ref_tag(self):
+        [ref] = self.factory.makeGitRefs(paths=["refs/tags/1.0"])
+        self.assertVocabularyHasTerms(
+            ref, self.expected_tag_values, self.expected_tag_tokens)
+
+    def test_ref_other(self):
+        [ref] = self.factory.makeGitRefs(paths=["refs/other"])
+        self.assertVocabularyHasTerms(
+            ref, self.expected_branch_values, self.expected_branch_tokens)
+
+    def test_rule_branch(self):
+        rule = self.factory.makeGitRule(ref_pattern="refs/heads/*")
+        self.assertVocabularyHasTerms(
+            rule, self.expected_branch_values, self.expected_branch_tokens)
+
+    def test_rule_tag(self):
+        rule = self.factory.makeGitRule(ref_pattern="refs/tags/*")
+        self.assertVocabularyHasTerms(
+            rule, self.expected_tag_values, self.expected_tag_tokens)
+
+    def test_rule_other(self):
+        rule = self.factory.makeGitRule(ref_pattern="refs/*")
+        self.assertVocabularyHasTerms(
+            rule, self.expected_branch_values, self.expected_branch_tokens)
+
+    def test_rule_grant_branch(self):
+        grant = self.factory.makeGitRuleGrant(
+            ref_pattern="refs/heads/*", can_create=True, can_push=True)
+        self.assertVocabularyHasTerms(
+            grant, self.expected_branch_values, self.expected_branch_tokens)
+
+    def test_rule_grant_branch_with_custom(self):
+        grant = self.factory.makeGitRuleGrant(
+            ref_pattern="refs/heads/*", can_push=True, can_force_push=True)
+        expected_values = (
+            self.expected_branch_values +
+            [{GitPermissionType.CAN_PUSH, GitPermissionType.CAN_FORCE_PUSH}])
+        expected_tokens = self.expected_branch_tokens + ["custom"]
+        terms = self.assertVocabularyHasTerms(
+            grant, expected_values, expected_tokens)
+        self.assertEqual(
+            "Custom permissions: push, force-push", terms[-1].title)
+
+    def test_rule_grant_tag(self):
+        grant = self.factory.makeGitRuleGrant(
+            ref_pattern="refs/tags/*", can_create=True)
+        self.assertVocabularyHasTerms(
+            grant, self.expected_tag_values, self.expected_tag_tokens)
+
+    def test_rule_grant_other(self):
+        grant = self.factory.makeGitRuleGrant(ref_pattern="refs/*")
+        self.assertVocabularyHasTerms(
+            grant, self.expected_branch_values, self.expected_branch_tokens)