Merge lp:~cjwatson/launchpad/git-permissions-ui-edit into lp:launchpad

[Colin Watson]

Add a Git repository permissions view.

This was very substantially complicated by not wanting to invest the time in a JS-based interface just yet (and wanting to have a non-JS fallback, in any case). Given that, we need to be able to cram everything into a single form which can be submitted in one go and specify the new state of the entire permissions structure. The UI-level separation between protected branches and protected tags also makes things hard, particularly when it comes to lining up columns consistently. This is the best I was able to do given those constraints; some bits are quite ugly (especially rule positions), but I think it's tolerable, and it should allow for future JS-based enhancement.

Example screenshot: https://people.canonical.com/~cjwatson/tmp/lp-git-repository-permissions.png

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2018-11-08 11:40:25 +0000
+++ lib/lp/code/browser/configure.zcml	2019-01-10 10:50:57 +0000
@@ -871,6 +871,12 @@
     </class>
     <browser:page
         for="lp.code.interfaces.gitrepository.IGitRepository"
+        class="lp.code.browser.gitrepository.GitRepositoryPermissionsView"
+        name="+permissions"
+        permission="launchpad.Edit"
+        template="../templates/gitrepository-permissions.pt"/>
+    <browser:page
+        for="lp.code.interfaces.gitrepository.IGitRepository"
         class="lp.code.browser.gitrepository.GitRepositoryDeletionView"
         permission="launchpad.Edit"
         name="+delete"

=== modified file 'lib/lp/code/browser/gitrepository.py'
--- lib/lp/code/browser/gitrepository.py	2018-11-08 15:53:56 +0000
+++ lib/lp/code/browser/gitrepository.py	2019-01-10 10:50:57 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Git repository views."""
@@ -17,10 +17,14 @@
     'GitRepositoryEditReviewerView',
     'GitRepositoryEditView',
     'GitRepositoryNavigation',
+    'GitRepositoryPermissionsView',
     'GitRepositoryURL',
     'GitRepositoryView',
     ]
 
+import base64
+from collections import defaultdict
+
 from lazr.lifecycle.event import ObjectModifiedEvent
 from lazr.lifecycle.snapshot import Snapshot
 from lazr.restful.interface import (
@@ -34,14 +38,21 @@
 from zope.component import getUtility
 from zope.event import notify
 from zope.formlib import form
+from zope.formlib.textwidgets import IntWidget
+from zope.formlib.widget import CustomWidgetFactory
 from zope.interface import (
     implementer,
     Interface,
     providedBy,
     )
 from zope.publisher.interfaces.browser import IBrowserPublisher
-from zope.schema import Choice
+from zope.schema import (
+    Bool,
+    Choice,
+    Int,
+    )
 from zope.schema.vocabulary import (
+    getVocabularyRegistry,
     SimpleTerm,
     SimpleVocabulary,
     )
@@ -53,7 +64,10 @@
     LaunchpadEditFormView,
     LaunchpadFormView,
     )
-from lp.app.errors import NotFoundError
+from lp.app.errors import (
+    NotFoundError,
+    UnexpectedFormData,
+    )
 from lp.app.vocabularies import InformationTypeVocabulary
 from lp.app.widgets.itemswidgets import LaunchpadRadioWidgetWithDescription
 from lp.code.browser.branch import CodeEditOwnerMixin
@@ -62,11 +76,19 @@
     )
 from lp.code.browser.codeimport import CodeImportTargetMixin
 from lp.code.browser.sourcepackagerecipelisting import HasRecipesMenuMixin
+from lp.code.browser.widgets.gitgrantee import (
+    GitGranteeDisplayWidget,
+    GitGranteeField,
+    GitGranteeWidget,
+    )
 from lp.code.browser.widgets.gitrepositorytarget import (
     GitRepositoryTargetDisplayWidget,
     GitRepositoryTargetWidget,
     )
-from lp.code.enums import GitRepositoryType
+from lp.code.enums import (
+    GitGranteeType,
+    GitRepositoryType,
+    )
 from lp.code.errors import (
     GitDefaultConflict,
     GitRepositoryCreationForbidden,
@@ -76,6 +98,7 @@
 from lp.code.interfaces.gitnamespace import get_git_namespace
 from lp.code.interfaces.gitref import IGitRefBatchNavigator
 from lp.code.interfaces.gitrepository import IGitRepository
+from lp.code.vocabularies.gitrule import GitPermissionsVocabulary
 from lp.registry.interfaces.person import (
     IPerson,
     IPersonSet,
@@ -84,6 +107,7 @@
 from lp.services.config import config
 from lp.services.database.constants import UTC_NOW
 from lp.services.features import getFeatureFlag
+from lp.services.fields import UniqueField
 from lp.services.propertycache import cachedproperty
 from lp.services.webapp import (
     canonical_url,
@@ -105,6 +129,7 @@
 from lp.services.webapp.escaping import structured
 from lp.services.webapp.interfaces import ICanonicalUrlData
 from lp.services.webapp.publisher import DataDownloadView
+from lp.services.webapp.snapshot import notify_modified
 from lp.services.webhooks.browser import WebhookTargetNavigationMixin
 from lp.snappy.browser.hassnaps import HasSnapsViewMixin
 
@@ -211,7 +236,14 @@
     usedfor = IGitRepository
     facet = "branches"
     title = "Edit Git repository"
-    links = ["edit", "reviewer", "webhooks", "activity", "delete"]
+    links = [
+        "edit",
+        "reviewer",
+        "permissions",
+        "activity",
+        "webhooks",
+        "delete",
+        ]
 
     @enabled_with_permission("launchpad.Edit")
     def edit(self):
@@ -224,6 +256,11 @@
         return Link("+reviewer", text, icon="edit")
 
     @enabled_with_permission("launchpad.Edit")
+    def permissions(self):
+        text = "Manage permissions"
+        return Link("+permissions", text, icon="edit")
+
+    @enabled_with_permission("launchpad.Edit")
     def webhooks(self):
         text = "Manage webhooks"
         return Link(
@@ -709,6 +746,489 @@
         return self, ()
 
 
+def encode_form_field_id(value):
+    """Encode text for use in form field names.
+
+    We use a modified version of base32 which fits into CSS identifiers and
+    so doesn't cause FormattersAPI.zope_css_id to do unhelpful things.
+    """
+    return base64.b32encode(
+        value.encode("UTF-8")).decode("UTF-8").replace("=", "_")
+
+
+def decode_form_field_id(encoded):
+    """Inverse of `encode_form_field_id`."""
+    return base64.b32decode(
+        encoded.replace("_", "=").encode("UTF-8")).decode("UTF-8")
+
+
+class GitRulePatternField(UniqueField):
+
+    errormessage = _("%s is already in use by another rule")
+    attribute = "ref_pattern"
+    _content_iface = IGitRepository
+
+    def __init__(self, ref_prefix, rule=None, *args, **kwargs):
+        self.ref_prefix = ref_prefix
+        self.rule = rule
+        super(GitRulePatternField, self).__init__(*args, **kwargs)
+
+    def _getByAttribute(self, ref_pattern):
+        """See `UniqueField`."""
+        if self._content_iface.providedBy(self.context):
+            return self.context.getRule(self.ref_prefix + ref_pattern)
+        else:
+            return None
+
+    def unchanged(self, input):
+        """See `UniqueField`."""
+        return (
+            self.rule is not None and
+            self.ref_prefix + input == self.rule.ref_pattern)
+
+    def set(self, object, value):
+        """See `IField`."""
+        if value is not None:
+            value = value.strip()
+        super(GitRulePatternField, self).set(object, value)
+
+
+class GitRepositoryPermissionsView(LaunchpadFormView):
+    """A view to manage repository permissions."""
+
+    heads_prefix = u"refs/heads/"
+    tags_prefix = u"refs/tags/"
+
+    @property
+    def label(self):
+        return "Manage permissions for %s" % self.context.identity
+
+    page_title = "Manage permissions"
+
+    @property
+    def repository(self):
+        return self.context
+
+    @cachedproperty
+    def rules(self):
+        return self.repository.getRules()
+
+    @property
+    def branch_rules(self):
+        return [
+            rule for rule in self.rules
+            if rule.ref_pattern.startswith(self.heads_prefix)]
+
+    @property
+    def tag_rules(self):
+        return [
+            rule for rule in self.rules
+            if rule.ref_pattern.startswith(self.tags_prefix)]
+
+    @property
+    def other_rules(self):
+        return [
+            rule for rule in self.rules
+            if not rule.ref_pattern.startswith(self.heads_prefix) and
+               not rule.ref_pattern.startswith(self.tags_prefix)]
+
+    def _getRuleGrants(self, rule):
+        def grantee_key(grant):
+            if grant.grantee is not None:
+                return (grant.grantee_type, grant.grantee.name)
+            else:
+                return (grant.grantee_type,)
+
+        return sorted(rule.grants, key=grantee_key)
+
+    def _parseRefPattern(self, ref_pattern):
+        """Parse a pattern into a prefix and the displayed portion."""
+        for prefix in (self.heads_prefix, self.tags_prefix):
+            if ref_pattern.startswith(prefix):
+                return prefix, ref_pattern[len(prefix):]
+        return u"", ref_pattern
+
+    def _getFieldName(self, name, ref_pattern, grantee=None):
+        """Get the combined field name for a ref pattern and optional grantee.
+
+        In order to be able to render a permissions table, we encode the ref
+        pattern and the grantee in the form field name.
+        """
+        suffix = "." + encode_form_field_id(ref_pattern)
+        if grantee is not None:
+            if IPerson.providedBy(grantee):
+                suffix += "." + str(grantee.id)
+            else:
+                suffix += "._" + grantee.name.lower()
+        return name + suffix
+
+    def _parseFieldName(self, field_name):
+        """Parse a combined field name as described in `_getFieldName`.
+
+        :raises UnexpectedFormData: if the field name cannot be parsed or
+            the grantee cannot be found.
+        """
+        field_bits = field_name.split(".")
+        if len(field_bits) < 2:
+            raise UnexpectedFormData(
+                "Cannot parse field name: %s" % field_name)
+        field_type = field_bits[0]
+        try:
+            ref_pattern = decode_form_field_id(field_bits[1])
+        except TypeError:
+            raise UnexpectedFormData(
+                "Cannot parse field name: %s" % field_name)
+        if len(field_bits) > 2:
+            grantee_id = field_bits[2]
+            if grantee_id.startswith("_"):
+                grantee_id = grantee_id[1:]
+                try:
+                    grantee = GitGranteeType.getTermByToken(grantee_id).value
+                except LookupError:
+                    grantee = None
+            else:
+                try:
+                    grantee_id = int(grantee_id)
+                except ValueError:
+                    grantee = None
+                else:
+                    grantee = getUtility(IPersonSet).get(grantee_id)
+            if grantee is None or grantee == GitGranteeType.PERSON:
+                raise UnexpectedFormData("No such grantee: %s" % grantee_id)
+        else:
+            grantee = None
+        return field_type, ref_pattern, grantee
+
+    def _getPermissionsTerm(self, grant):
+        """Return a term from `GitPermissionsVocabulary` for this grant."""
+        vocabulary = getVocabularyRegistry().get(grant, "GitPermissions")
+        try:
+            return vocabulary.getTerm(grant.permissions)
+        except LookupError:
+            # This should never happen, because GitPermissionsVocabulary
+            # adds a custom term for the context grant if necessary.
+            raise AssertionError(
+                "Could not find GitPermissions term for %r" % grant)
+
+    def setUpFields(self):
+        """See `LaunchpadFormView`."""
+        position_fields = []
+        pattern_fields = []
+        delete_fields = []
+        readonly_grantee_fields = []
+        grantee_fields = []
+        permissions_fields = []
+
+        default_permissions_by_prefix = {
+            self.heads_prefix: "can_push",
+            self.tags_prefix: "can_create",
+            "": "can_push",
+            }
+
+        for rule_index, rule in enumerate(self.rules):
+            # Remove the usual branch/tag prefixes from patterns.  The full
+            # pattern goes into form field names, so no data is lost here.
+            ref_pattern = rule.ref_pattern
+            ref_prefix, short_pattern = self._parseRefPattern(ref_pattern)
+            position_fields.append(
+                Int(
+                    __name__=self._getFieldName("position", ref_pattern),
+                    required=True, readonly=False, default=rule_index + 1))
+            pattern_fields.append(
+                GitRulePatternField(
+                    __name__=self._getFieldName("pattern", ref_pattern),
+                    required=True, readonly=False, ref_prefix=ref_prefix,
+                    rule=rule, default=short_pattern))
+            delete_fields.append(
+                Bool(
+                    __name__=self._getFieldName("delete", ref_pattern),
+                    readonly=False, default=False))
+            for grant in self._getRuleGrants(rule):
+                grantee = grant.combined_grantee
+                readonly_grantee_fields.append(
+                    GitGranteeField(
+                        __name__=self._getFieldName(
+                            "grantee", ref_pattern, grantee),
+                        required=False, readonly=True, default=grantee,
+                        rule=rule))
+                permissions_fields.append(
+                    Choice(
+                        __name__=self._getFieldName(
+                            "permissions", ref_pattern, grantee),
+                        source=GitPermissionsVocabulary(grant),
+                        readonly=False,
+                        default=self._getPermissionsTerm(grant).value))
+                delete_fields.append(
+                    Bool(
+                        __name__=self._getFieldName(
+                            "delete", ref_pattern, grantee),
+                        readonly=False, default=False))
+            grantee_fields.append(
+                GitGranteeField(
+                    __name__=self._getFieldName("grantee", ref_pattern),
+                    required=False, readonly=False, rule=rule))
+            permissions_vocabulary = GitPermissionsVocabulary(rule)
+            permissions_fields.append(
+                Choice(
+                    __name__=self._getFieldName(
+                        "permissions", ref_pattern),
+                    source=permissions_vocabulary, readonly=False,
+                    default=permissions_vocabulary.getTermByToken(
+                        default_permissions_by_prefix[ref_prefix]).value))
+        for ref_prefix in (self.heads_prefix, self.tags_prefix):
+            position_fields.append(
+                Int(
+                    __name__=self._getFieldName("new-position", ref_prefix),
+                    required=False, readonly=True))
+            pattern_fields.append(
+                GitRulePatternField(
+                    __name__=self._getFieldName("new-pattern", ref_prefix),
+                    required=False, readonly=False, ref_prefix=ref_prefix))
+
+        self.form_fields = (
+            form.FormFields(
+                *position_fields,
+                custom_widget=CustomWidgetFactory(IntWidget, displayWidth=2)) +
+            form.FormFields(*pattern_fields) +
+            form.FormFields(*delete_fields) +
+            form.FormFields(
+                *readonly_grantee_fields,
+                custom_widget=CustomWidgetFactory(GitGranteeDisplayWidget)) +
+            form.FormFields(
+                *grantee_fields,
+                custom_widget=CustomWidgetFactory(GitGranteeWidget)) +
+            form.FormFields(*permissions_fields))
+
+    def setUpWidgets(self, context=None):
+        """See `LaunchpadFormView`."""
+        super(GitRepositoryPermissionsView, self).setUpWidgets(
+            context=context)
+        for widget in self.widgets:
+            widget.display_label = False
+            widget.hint = None
+
+    @property
+    def cancel_url(self):
+        return canonical_url(self.context)
+
+    def getRuleWidgets(self, rule):
+        widgets_by_name = {widget.name: widget for widget in self.widgets}
+        ref_pattern = rule.ref_pattern
+        position_field_name = (
+            "field." + self._getFieldName("position", ref_pattern))
+        pattern_field_name = (
+            "field." + self._getFieldName("pattern", ref_pattern))
+        delete_field_name = (
+            "field." + self._getFieldName("delete", ref_pattern))
+        grant_widgets = []
+        for grant in self._getRuleGrants(rule):
+            grantee = grant.combined_grantee
+            grantee_field_name = (
+                "field." + self._getFieldName("grantee", ref_pattern, grantee))
+            permissions_field_name = (
+                "field." +
+                self._getFieldName("permissions", ref_pattern, grantee))
+            delete_grant_field_name = (
+                "field." + self._getFieldName("delete", ref_pattern, grantee))
+            grant_widgets.append({
+                "grantee": widgets_by_name[grantee_field_name],
+                "permissions": widgets_by_name[permissions_field_name],
+                "delete": widgets_by_name[delete_grant_field_name],
+                })
+        new_grantee_field_name = (
+            "field." + self._getFieldName("grantee", ref_pattern))
+        new_permissions_field_name = (
+            "field." + self._getFieldName("permissions", ref_pattern))
+        new_grant_widgets = {
+            "grantee": widgets_by_name[new_grantee_field_name],
+            "permissions": widgets_by_name[new_permissions_field_name],
+            }
+        return {
+            "position": widgets_by_name[position_field_name],
+            "pattern": widgets_by_name[pattern_field_name],
+            "delete": widgets_by_name.get(delete_field_name),
+            "grants": grant_widgets,
+            "new_grant": new_grant_widgets,
+            }
+
+    def getNewRuleWidgets(self, ref_prefix):
+        widgets_by_name = {widget.name: widget for widget in self.widgets}
+        new_position_field_name = (
+            "field." + self._getFieldName("new-position", ref_prefix))
+        new_pattern_field_name = (
+            "field." + self._getFieldName("new-pattern", ref_prefix))
+        return {
+            "position": widgets_by_name[new_position_field_name],
+            "pattern": widgets_by_name[new_pattern_field_name],
+            }
+
+    def parseData(self, data):
+        """Rearrange form data to make it easier to process."""
+        parsed_data = {
+            "rules": {},
+            "grants": defaultdict(list),
+            }
+
+        for field_name in sorted(
+                name for name in data if name.split(".")[0] == "pattern"):
+            _, ref_pattern, _ = self._parseFieldName(field_name)
+            prefix, _ = self._parseRefPattern(ref_pattern)
+            position_field_name = self._getFieldName("position", ref_pattern)
+            delete_field_name = self._getFieldName("delete", ref_pattern)
+            if data.get(delete_field_name):
+                position = None
+                action = "delete"
+            else:
+                position = max(0, data[position_field_name] - 1)
+                action = "change"
+            parsed_data["rules"].setdefault(ref_pattern, {
+                "position": position,
+                "pattern": ref_pattern,
+                "new_pattern": prefix + data[field_name],
+                "action": action,
+                })
+
+        for field_name in sorted(
+                name for name in data if name.split(".")[0] == "new-pattern"):
+            _, prefix, _ = self._parseFieldName(field_name)
+            position_field_name = self._getFieldName("position", prefix)
+            if not data[field_name]:
+                continue
+            if position_field_name in data:
+                position = max(0, data[position_field_name] - 1)
+            else:
+                position = None
+            parsed_data["rules"].setdefault(prefix, {
+                "position": position,
+                "pattern": prefix + data[field_name],
+                "action": "add",
+                })
+
+        for field_name in sorted(
+                name for name in data if name.split(".")[0] == "permissions"):
+            _, ref_pattern, grantee = self._parseFieldName(field_name)
+            grantee_field_name = self._getFieldName("grantee", ref_pattern)
+            delete_field_name = self._getFieldName(
+                "delete", ref_pattern, grantee)
+            if grantee is None:
+                grantee = data.get(grantee_field_name)
+                if grantee is None:
+                    continue
+                action = "add"
+            elif data.get(delete_field_name):
+                action = "delete"
+            else:
+                action = "change"
+            parsed_data["grants"][ref_pattern].append({
+                "grantee": grantee,
+                "permissions": data[field_name],
+                "action": action,
+                })
+
+        return parsed_data
+
+    def updateRepositoryFromData(self, repository, parsed_data):
+        # Fetch rules before making any changes, since their ref_patterns
+        # may change as a result of this update.
+        rule_map = {rule.ref_pattern: rule for rule in self.repository.rules}
+        grant_map = {
+            (grant.rule.ref_pattern, grant.combined_grantee): grant
+            for grant in self.repository.grants}
+
+        # Patterns must be processed in rule order so that position changes
+        # work in a reasonably natural way.  Process new rules last.
+        ordered_rules = []
+        for ref_pattern, parsed_rule in parsed_data["rules"].items():
+            rule = rule_map.get(ref_pattern)
+            if parsed_rule["action"] == "add":
+                ordered_rules.append((ref_pattern, parsed_rule, -1))
+            elif rule is not None:
+                # Ignore attempts to change or delete rules that have
+                # already been deleted by somebody else.
+                ordered_rules.append((ref_pattern, parsed_rule, rule.position))
+        ordered_rules.sort(
+            key=lambda item: (item[1]["action"] != "add", item[2]))
+
+        for ref_pattern, parsed_rule, position in ordered_rules:
+            rule = rule_map.get(parsed_rule["pattern"])
+            action = parsed_rule["action"]
+            if action not in ("add", "change", "delete"):
+                raise AssertionError(
+                    "unknown action: %s" % parsed_rule["action"])
+
+            if action == "add" and rule is None:
+                rule = repository.addRule(
+                    parsed_rule["pattern"], self.user,
+                    position=parsed_rule["position"])
+                if ref_pattern == self.tags_prefix:
+                    # Tags are a special case: on creation, they
+                    # automatically get a grant of create permissions to
+                    # the repository owner (suppressing the normal
+                    # ability of the repository owner to push protected
+                    # references).
+                    rule.addGrant(
+                        GitGranteeType.REPOSITORY_OWNER, self.user,
+                        can_create=True)
+            elif action == "change" and rule is not None:
+                self.repository.moveRule(
+                    rule, parsed_rule["position"], self.user)
+                if parsed_rule["new_pattern"] != rule.ref_pattern:
+                    with notify_modified(rule, ["ref_pattern"]):
+                        rule.ref_pattern = parsed_rule["new_pattern"]
+            elif action == "delete" and rule is not None:
+                rule.destroySelf(self.user)
+                rule_map[parsed_rule["pattern"]] = None
+            else:
+                raise AssertionError(
+                    "unknown action: %s" % parsed_rule["action"])
+
+        for ref_pattern, parsed_grants in sorted(
+                parsed_data["grants"].items()):
+            if ref_pattern not in rule_map:
+                self.addError(structured(
+                    "Cannot edit grants for nonexistent rule %s", ref_pattern))
+                return
+            rule = rule_map.get(ref_pattern)
+            if rule is None:
+                # Already deleted.
+                continue
+
+            for parsed_grant in parsed_grants:
+                grant = grant_map.get((ref_pattern, parsed_grant["grantee"]))
+                action = parsed_grant["action"]
+                if action not in ("add", "change", "delete"):
+                    raise AssertionError(
+                        "unknown action: %s" % parsed_rule["action"])
+
+                if action == "add" and grant is None:
+                    rule.addGrant(
+                        parsed_grant["grantee"], self.user,
+                        permissions=parsed_grant["permissions"])
+                elif (action in ("add", "change") and grant is not None and
+                      parsed_grant["permissions"] != grant.permissions):
+                    # Make the requested changes.  This can happen in the
+                    # add case if somebody else added the grant since the
+                    # form was last rendered, in which case updating it with
+                    # the permissions from this request seems best.
+                    with notify_modified(
+                            grant,
+                            ["can_create", "can_push", "can_force_push"]):
+                        grant.permissions = parsed_grant["permissions"]
+                elif action == "delete" and grant is not None:
+                    grant.destroySelf(self.user)
+                    grant_map[(ref_pattern, parsed_grant["grantee"])] = None
+
+    @action("Save", name="save")
+    def save_action(self, action, data):
+        with notify_modified(self.repository, []):
+            parsed_data = self.parseData(data)
+            self.updateRepositoryFromData(self.repository, parsed_data)
+
+        self.request.response.addNotification(
+            "Saved permissions for %s" % self.context.identity)
+        self.next_url = canonical_url(self.context, view_name="+permissions")
+
+
 class GitRepositoryDeletionView(LaunchpadFormView):
 
     schema = IGitRepository

=== modified file 'lib/lp/code/browser/tests/test_gitrepository.py'
--- lib/lp/code/browser/tests/test_gitrepository.py	2018-11-08 15:33:03 +0000
+++ lib/lp/code/browser/tests/test_gitrepository.py	2019-01-10 10:50:57 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for GitRepositoryView."""
@@ -7,16 +7,27 @@
 
 __metaclass__ = type
 
+import base64
 from datetime import datetime
 import doctest
+from itertools import chain
+from operator import attrgetter
+import re
 from textwrap import dedent
 
 from fixtures import FakeLogger
 import pytz
+import soupmatchers
 from storm.store import Store
 from testtools.matchers import (
+    AfterPreprocessing,
     DocTestMatches,
     Equals,
+    Is,
+    MatchesDict,
+    MatchesListwise,
+    MatchesSetwise,
+    MatchesStructure,
     )
 import transaction
 from zope.component import getUtility
@@ -26,11 +37,16 @@
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import InformationType
+from lp.app.errors import UnexpectedFormData
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.app.interfaces.services import IService
+from lp.code.browser.gitrepository import encode_form_field_id
 from lp.code.enums import (
     BranchMergeProposalStatus,
     CodeReviewVote,
+    GitActivityType,
+    GitGranteeType,
+    GitPermissionType,
     GitRepositoryType,
     )
 from lp.code.interfaces.revision import IRevisionSet
@@ -40,7 +56,10 @@
     VCSType,
     )
 from lp.registry.interfaces.accesspolicy import IAccessPolicySource
-from lp.registry.interfaces.person import PersonVisibility
+from lp.registry.interfaces.person import (
+    IPerson,
+    PersonVisibility,
+    )
 from lp.services.beautifulsoup import BeautifulSoup
 from lp.services.database.constants import UTC_NOW
 from lp.services.features.testing import FeatureFixture
@@ -67,6 +86,7 @@
 from lp.testing.pages import (
     extract_text,
     find_tag_by_id,
+    find_tags_by_class,
     get_feedback_messages,
     setupBrowser,
     setupBrowserForUser,
@@ -1097,6 +1117,625 @@
             browser.headers["Content-Disposition"])
 
 
+class TestGitRepositoryPermissionsView(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_rules_properties(self):
+        repository = self.factory.makeGitRepository()
+        heads_rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/*")
+        tags_rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/tags/*")
+        catch_all_rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="*")
+        login_person(repository.owner)
+        view = create_initialized_view(repository, name="+permissions")
+        self.assertEqual([heads_rule], view.branch_rules)
+        self.assertEqual([tags_rule], view.tag_rules)
+        self.assertEqual([catch_all_rule], view.other_rules)
+
+    def test__getRuleGrants(self):
+        rule = self.factory.makeGitRule()
+        grantees = sorted(
+            [self.factory.makePerson() for _ in range(3)],
+            key=attrgetter("name"))
+        for grantee in (grantees[1], grantees[0], grantees[2]):
+            self.factory.makeGitRuleGrant(rule=rule, grantee=grantee)
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=GitGranteeType.REPOSITORY_OWNER)
+        login_person(rule.repository.owner)
+        view = create_initialized_view(rule.repository, name="+permissions")
+        self.assertThat(view._getRuleGrants(rule), MatchesListwise([
+            MatchesStructure.byEquality(
+                grantee_type=GitGranteeType.REPOSITORY_OWNER),
+            MatchesStructure.byEquality(grantee=grantees[0]),
+            MatchesStructure.byEquality(grantee=grantees[1]),
+            MatchesStructure.byEquality(grantee=grantees[2]),
+            ]))
+
+    def test__parseRefPattern(self):
+        repository = self.factory.makeGitRepository()
+        login_person(repository.owner)
+        view = create_initialized_view(repository, name="+permissions")
+        self.assertEqual(
+            ("refs/heads/", "stable/*"),
+            view._parseRefPattern("refs/heads/stable/*"))
+        self.assertEqual(
+            ("refs/tags/", "1.0"), view._parseRefPattern("refs/tags/1.0"))
+        self.assertEqual(
+            ("", "refs/other/*"), view._parseRefPattern("refs/other/*"))
+        self.assertEqual(("", "*"), view._parseRefPattern("*"))
+
+    def test__getFieldName_no_grantee(self):
+        repository = self.factory.makeGitRepository()
+        login_person(repository.owner)
+        view = create_initialized_view(repository, name="+permissions")
+        encoded_ref_pattern = base64.b32encode(
+            b"refs/heads/*").replace("=", "_").decode("UTF-8")
+        self.assertEqual(
+            "field.%s" % encoded_ref_pattern,
+            view._getFieldName("field", "refs/heads/*"))
+
+    def test__getFieldName_grantee_repository_owner(self):
+        repository = self.factory.makeGitRepository()
+        login_person(repository.owner)
+        view = create_initialized_view(repository, name="+permissions")
+        encoded_ref_pattern = base64.b32encode(
+            b"refs/tags/*").replace("=", "_").decode("UTF-8")
+        self.assertEqual(
+            "field.%s._repository_owner" % encoded_ref_pattern,
+            view._getFieldName(
+                "field", "refs/tags/*",
+                grantee=GitGranteeType.REPOSITORY_OWNER))
+
+    def test__getFieldName_grantee_person(self):
+        repository = self.factory.makeGitRepository()
+        grantee = self.factory.makePerson()
+        login_person(repository.owner)
+        view = create_initialized_view(repository, name="+permissions")
+        encoded_ref_pattern = base64.b32encode(
+            b"refs/*").replace("=", "_").decode("UTF-8")
+        self.assertEqual(
+            "field.%s.%s" % (encoded_ref_pattern, grantee.id),
+            view._getFieldName("field", "refs/*", grantee=grantee))
+
+    def test__parseFieldName_too_few_components(self):
+        repository = self.factory.makeGitRepository()
+        login_person(repository.owner)
+        view = create_initialized_view(repository, name="+permissions")
+        self.assertRaises(UnexpectedFormData, view._parseFieldName, "field")
+
+    def test__parseFieldName_bad_ref_pattern(self):
+        repository = self.factory.makeGitRepository()
+        login_person(repository.owner)
+        view = create_initialized_view(repository, name="+permissions")
+        self.assertRaises(
+            UnexpectedFormData, view._parseFieldName, "field.nonsense")
+
+    def test__parseFieldName_no_grantee(self):
+        repository = self.factory.makeGitRepository()
+        login_person(repository.owner)
+        view = create_initialized_view(repository, name="+permissions")
+        encoded_ref_pattern = base64.b32encode(
+            b"refs/heads/*").replace("=", "_").decode("UTF-8")
+        self.assertEqual(
+            ("permissions", "refs/heads/*", None),
+            view._parseFieldName("permissions.%s" % encoded_ref_pattern))
+
+    def test__parseFieldName_grantee_unknown_type(self):
+        repository = self.factory.makeGitRepository()
+        login_person(repository.owner)
+        view = create_initialized_view(repository, name="+permissions")
+        encoded_ref_pattern = base64.b32encode(
+            b"refs/tags/*").replace("=", "_").decode("UTF-8")
+        self.assertRaises(
+            UnexpectedFormData, view._parseFieldName,
+            "field.%s._nonsense" % encoded_ref_pattern)
+        self.assertRaises(
+            UnexpectedFormData, view._parseFieldName,
+            "field.%s._person" % encoded_ref_pattern)
+
+    def test__parseFieldName_grantee_repository_owner(self):
+        repository = self.factory.makeGitRepository()
+        login_person(repository.owner)
+        view = create_initialized_view(repository, name="+permissions")
+        encoded_ref_pattern = base64.b32encode(
+            b"refs/tags/*").replace("=", "_").decode("UTF-8")
+        self.assertEqual(
+            ("pattern", "refs/tags/*", GitGranteeType.REPOSITORY_OWNER),
+            view._parseFieldName(
+                "pattern.%s._repository_owner" % encoded_ref_pattern))
+
+    def test__parseFieldName_grantee_unknown_person(self):
+        repository = self.factory.makeGitRepository()
+        grantee = self.factory.makePerson()
+        login_person(repository.owner)
+        view = create_initialized_view(repository, name="+permissions")
+        encoded_ref_pattern = base64.b32encode(
+            b"refs/*").replace("=", "_").decode("UTF-8")
+        self.assertRaises(
+            UnexpectedFormData, view._parseFieldName,
+            "delete.%s.%s" % (encoded_ref_pattern, grantee.id * 2))
+
+    def test__parseFieldName_grantee_person(self):
+        repository = self.factory.makeGitRepository()
+        grantee = self.factory.makePerson()
+        login_person(repository.owner)
+        view = create_initialized_view(repository, name="+permissions")
+        encoded_ref_pattern = base64.b32encode(
+            b"refs/*").replace("=", "_").decode("UTF-8")
+        self.assertEqual(
+            ("delete", "refs/*", grantee),
+            view._parseFieldName(
+                "delete.%s.%s" % (encoded_ref_pattern, grantee.id)))
+
+    def test__getPermissionsTerm_standard(self):
+        grant = self.factory.makeGitRuleGrant(
+            ref_pattern="refs/heads/*", can_create=True, can_push=True)
+        login_person(grant.repository.owner)
+        view = create_initialized_view(grant.repository, name="+permissions")
+        self.assertThat(
+            view._getPermissionsTerm(grant), MatchesStructure.byEquality(
+                value={
+                    GitPermissionType.CAN_CREATE, GitPermissionType.CAN_PUSH},
+                token="can_push",
+                title="Can push"))
+
+    def test__getPermissionsTerm_custom(self):
+        grant = self.factory.makeGitRuleGrant(
+            ref_pattern="refs/heads/*", can_force_push=True)
+        login_person(grant.repository.owner)
+        view = create_initialized_view(grant.repository, name="+permissions")
+        self.assertThat(
+            view._getPermissionsTerm(grant), MatchesStructure.byEquality(
+                value={GitPermissionType.CAN_FORCE_PUSH},
+                token="custom",
+                title="Custom permissions: force-push"))
+
+    def _matchesCells(self, row_tag, cell_matchers):
+        return AfterPreprocessing(
+            str, soupmatchers.HTMLContains(*(
+                soupmatchers.Within(row_tag, cell_matcher)
+                for cell_matcher in cell_matchers)))
+
+    def _matchesRule(self, position, pattern, short_pattern):
+        rule_tag = soupmatchers.Tag(
+            "rule row", "tr", attrs={"class": "git-rule"})
+        suffix = "." + encode_form_field_id(pattern)
+        position_field_name = "field.position" + suffix
+        pattern_field_name = "field.pattern" + suffix
+        delete_field_name = "field.delete" + suffix
+        return self._matchesCells(rule_tag, [
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "position cell", "td",
+                    attrs={"class": "git-rule-position"}),
+                soupmatchers.Tag(
+                    "position widget", "input",
+                    attrs={"name": position_field_name, "value": position})),
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "pattern cell", "td", attrs={"class": "git-rule-pattern"}),
+                soupmatchers.Tag(
+                    "pattern widget", "input",
+                    attrs={
+                        "name": pattern_field_name,
+                        "value": short_pattern,
+                        })),
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "delete cell", "td", attrs={"class": "git-rule-delete"}),
+                soupmatchers.Tag(
+                    "delete widget", "input",
+                    attrs={"name": delete_field_name})),
+            ])
+
+    def _matchesNewRule(self, ref_prefix):
+        new_rule_tag = soupmatchers.Tag(
+            "new rule row", "tr", attrs={"class": "git-new-rule"})
+        suffix = "." + encode_form_field_id(ref_prefix)
+        new_position_field_name = "field.new-position" + suffix
+        new_pattern_field_name = "field.new-pattern" + suffix
+        return self._matchesCells(new_rule_tag, [
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "position cell", "td",
+                    attrs={"class": "git-rule-position"}),
+                soupmatchers.Tag(
+                    "position widget", "input",
+                    attrs={"name": new_position_field_name, "value": ""})),
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "pattern cell", "td", attrs={"class": "git-rule-pattern"}),
+                soupmatchers.Tag(
+                    "pattern widget", "input",
+                    attrs={"name": new_pattern_field_name, "value": ""})),
+            ])
+
+    def _matchesRuleGrant(self, pattern, grantee, permissions_token,
+                          permissions_title):
+        rule_grant_tag = soupmatchers.Tag(
+            "rule grant row", "tr", attrs={"class": "git-rule-grant"})
+        suffix = "." + encode_form_field_id(pattern)
+        if IPerson.providedBy(grantee):
+            suffix += "." + str(grantee.id)
+            grantee_url = canonical_url(grantee, path_only_if_possible=True)
+            grantee_widget_matcher = soupmatchers.Tag(
+                "grantee widget", "a", attrs={"href": grantee_url},
+                text=grantee.display_name)
+        else:
+            suffix += "._" + grantee.name.lower()
+            grantee_widget_matcher = soupmatchers.Tag(
+                "grantee widget", "label",
+                text=re.compile(re.escape(grantee.title)))
+        permissions_field_name = "field.permissions" + suffix
+        delete_field_name = "field.delete" + suffix
+        return self._matchesCells(rule_grant_tag, [
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "grantee cell", "td",
+                    attrs={"class": "git-rule-grant-grantee"}),
+                grantee_widget_matcher),
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "permissions cell", "td",
+                    attrs={"class": "git-rule-grant-permissions"}),
+                soupmatchers.Within(
+                    soupmatchers.Tag(
+                        "permissions widget", "select",
+                        attrs={"name": permissions_field_name}),
+                    soupmatchers.Tag(
+                        "selected permissions option", "option",
+                        attrs={
+                            "selected": "selected",
+                            "value": permissions_token,
+                            },
+                        text=permissions_title))),
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "delete cell", "td", attrs={"class": "git-rule-delete"}),
+                soupmatchers.Tag(
+                    "delete widget", "input",
+                    attrs={"name": delete_field_name})),
+            ])
+
+    def _matchesNewRuleGrant(self, pattern, permissions_token):
+        rule_grant_tag = soupmatchers.Tag(
+            "rule grant row", "tr", attrs={"class": "git-new-rule-grant"})
+        suffix = "." + encode_form_field_id(pattern)
+        grantee_field_name = "field.grantee" + suffix
+        permissions_field_name = "field.permissions" + suffix
+        return self._matchesCells(rule_grant_tag, [
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "grantee cell", "td",
+                    attrs={"class": "git-rule-grant-grantee"}),
+                soupmatchers.Tag(
+                    "grantee widget", "input",
+                    attrs={"name": grantee_field_name})),
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "permissions cell", "td",
+                    attrs={"class": "git-rule-grant-permissions"}),
+                soupmatchers.Within(
+                    soupmatchers.Tag(
+                        "permissions widget", "select",
+                        attrs={"name": permissions_field_name}),
+                    soupmatchers.Tag(
+                        "selected permissions option", "option",
+                        attrs={
+                            "selected": "selected",
+                            "value": permissions_token,
+                            }))),
+            ])
+
+    def test_rules_tables(self):
+        repository = self.factory.makeGitRepository()
+        heads_rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/stable/*")
+        heads_grantee_1 = self.factory.makePerson(
+            name=self.factory.getUniqueString("person-name-a"))
+        heads_grantee_2 = self.factory.makePerson(
+            name=self.factory.getUniqueString("person-name-b"))
+        self.factory.makeGitRuleGrant(
+            rule=heads_rule, grantee=heads_grantee_1, can_push=True)
+        self.factory.makeGitRuleGrant(
+            rule=heads_rule, grantee=heads_grantee_2, can_force_push=True)
+        tags_rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/tags/*")
+        self.factory.makeGitRuleGrant(
+            rule=tags_rule, grantee=GitGranteeType.REPOSITORY_OWNER)
+        login_person(repository.owner)
+        view = create_initialized_view(
+            repository, name="+permissions", principal=repository.owner)
+        rules_tables = find_tags_by_class(view(), "git-rules-table")
+        rows = list(chain.from_iterable([
+            rules_table.findAll("tr", {"class": True})
+            for rules_table in rules_tables]))
+        self.assertThat(rows, MatchesListwise([
+            self._matchesRule("1", "refs/heads/stable/*", "stable/*"),
+            self._matchesRuleGrant(
+                "refs/heads/stable/*", heads_grantee_1, "can_push_existing",
+                "Can push if the branch already exists"),
+            self._matchesRuleGrant(
+                "refs/heads/stable/*", heads_grantee_2, "custom",
+                "Custom permissions: force-push"),
+            self._matchesNewRuleGrant("refs/heads/stable/*", "can_push"),
+            self._matchesNewRule("refs/heads/"),
+            self._matchesRule("2", "refs/tags/*", "*"),
+            self._matchesRuleGrant(
+                "refs/tags/*", GitGranteeType.REPOSITORY_OWNER,
+                "cannot_create", "Cannot create"),
+            self._matchesNewRuleGrant("refs/tags/*", "can_create"),
+            self._matchesNewRule("refs/tags/"),
+            ]))
+
+    def assertHasRules(self, repository, ref_patterns):
+        self.assertThat(list(repository.rules), MatchesListwise([
+            MatchesStructure.byEquality(ref_pattern=ref_pattern)
+            for ref_pattern in ref_patterns
+            ]))
+
+    def assertHasSavedNotification(self, view, repository):
+        self.assertThat(view.request.response.notifications, MatchesListwise([
+            MatchesStructure.byEquality(
+                message="Saved permissions for %s" % repository.identity),
+            ]))
+
+    def test_save_add_rules(self):
+        repository = self.factory.makeGitRepository()
+        self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/stable/*")
+        removeSecurityProxy(repository.getActivity()).remove()
+        login_person(repository.owner)
+        encoded_heads_prefix = encode_form_field_id("refs/heads/")
+        encoded_tags_prefix = encode_form_field_id("refs/tags/")
+        form = {
+            "field.new-pattern." + encoded_heads_prefix: "*",
+            "field.new-pattern." + encoded_tags_prefix: "1.0",
+            "field.actions.save": "Save",
+            }
+        view = create_initialized_view(
+            repository, name="+permissions", form=form,
+            principal=repository.owner)
+        self.assertHasRules(
+            repository,
+            ["refs/tags/1.0", "refs/heads/stable/*", "refs/heads/*"])
+        self.assertThat(list(repository.getActivity()), MatchesListwise([
+            # Adding a tag rule automatically adds a repository owner grant.
+            MatchesStructure(
+                changer=Equals(repository.owner),
+                changee=Is(None),
+                what_changed=Equals(GitActivityType.GRANT_ADDED),
+                new_value=MatchesDict({
+                    "changee_type": Equals("Repository owner"),
+                    "ref_pattern": Equals("refs/tags/1.0"),
+                    "can_create": Is(True),
+                    "can_push": Is(False),
+                    "can_force_push": Is(False),
+                    })),
+            MatchesStructure(
+                changer=Equals(repository.owner),
+                what_changed=Equals(GitActivityType.RULE_ADDED),
+                new_value=MatchesDict({
+                    "ref_pattern": Equals("refs/tags/1.0"),
+                    "position": Equals(0),
+                    })),
+            MatchesStructure(
+                changer=Equals(repository.owner),
+                what_changed=Equals(GitActivityType.RULE_ADDED),
+                new_value=MatchesDict({
+                    "ref_pattern": Equals("refs/heads/*"),
+                    # Initially inserted at 1, although refs/tags/1.0 was
+                    # later inserted before it.
+                    "position": Equals(1),
+                    })),
+            ]))
+        self.assertHasSavedNotification(view, repository)
+
+    def test_save_add_duplicate_rule(self):
+        repository = self.factory.makeGitRepository()
+        self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/stable/*")
+        transaction.commit()
+        login_person(repository.owner)
+        encoded_heads_prefix = encode_form_field_id("refs/heads/")
+        form = {
+            "field.new-pattern." + encoded_heads_prefix: "stable/*",
+            "field.actions.save": "Save",
+            }
+        view = create_initialized_view(
+            repository, name="+permissions", form=form,
+            principal=repository.owner)
+        self.assertThat(view.errors, MatchesListwise([
+            MatchesStructure(
+                field_name=Equals("new-pattern." + encoded_heads_prefix),
+                errors=MatchesStructure.byEquality(
+                    args=("stable/* is already in use by another rule",))),
+            ]))
+        self.assertHasRules(repository, ["refs/heads/stable/*"])
+
+    def test_save_move_rule(self):
+        repository = self.factory.makeGitRepository()
+        self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/stable/*")
+        self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/*/next")
+        encoded_patterns = [
+            encode_form_field_id(rule.ref_pattern)
+            for rule in repository.rules]
+        removeSecurityProxy(repository.getActivity()).remove()
+        login_person(repository.owner)
+        # Positions are 1-based in the UI.
+        form = {
+            "field.position." + encoded_patterns[0]: "2",
+            "field.pattern." + encoded_patterns[0]: "stable/*",
+            "field.position." + encoded_patterns[1]: "1",
+            "field.pattern." + encoded_patterns[1]: "*/more-next",
+            "field.actions.save": "Save",
+            }
+        view = create_initialized_view(
+            repository, name="+permissions", form=form,
+            principal=repository.owner)
+        self.assertHasRules(
+            repository, ["refs/heads/*/more-next", "refs/heads/stable/*"])
+        self.assertThat(list(repository.getActivity()), MatchesListwise([
+            MatchesStructure(
+                changer=Equals(repository.owner),
+                what_changed=Equals(GitActivityType.RULE_CHANGED),
+                old_value=MatchesDict({
+                    "ref_pattern": Equals("refs/heads/*/next"),
+                    "position": Equals(0),
+                    }),
+                new_value=MatchesDict({
+                    "ref_pattern": Equals("refs/heads/*/more-next"),
+                    "position": Equals(0),
+                    })),
+            # Only one rule is recorded as moving; the other is already in
+            # its new position by the time it's processed.
+            MatchesStructure(
+                changer=Equals(repository.owner),
+                what_changed=Equals(GitActivityType.RULE_MOVED),
+                old_value=MatchesDict({
+                    "ref_pattern": Equals("refs/heads/stable/*"),
+                    "position": Equals(0),
+                    }),
+                new_value=MatchesDict({
+                    "ref_pattern": Equals("refs/heads/stable/*"),
+                    "position": Equals(1),
+                    })),
+            ]))
+        self.assertHasSavedNotification(view, repository)
+
+    def test_save_change_grants(self):
+        repository = self.factory.makeGitRepository()
+        stable_rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/stable/*")
+        next_rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/*/next")
+        grantees = [self.factory.makePerson() for _ in range(3)]
+        self.factory.makeGitRuleGrant(
+            rule=stable_rule, grantee=GitGranteeType.REPOSITORY_OWNER,
+            can_create=True)
+        self.factory.makeGitRuleGrant(
+            rule=stable_rule,
+            grantee=grantees[0], can_create=True, can_push=True)
+        self.factory.makeGitRuleGrant(
+            rule=next_rule, grantee=grantees[1],
+            can_create=True, can_push=True, can_force_push=True)
+        encoded_patterns = [
+            encode_form_field_id(rule.ref_pattern)
+            for rule in repository.rules]
+        removeSecurityProxy(repository.getActivity()).remove()
+        login_person(repository.owner)
+        form = {
+            "field.permissions.%s._repository_owner" % encoded_patterns[0]: (
+                "can_push"),
+            "field.permissions.%s.%s" % (
+                encoded_patterns[0], grantees[0].id): "can_push",
+            "field.delete.%s.%s" % (encoded_patterns[0], grantees[0].id): "on",
+            "field.grantee.%s" % encoded_patterns[1]: "person",
+            "field.grantee.%s.person" % encoded_patterns[1]: grantees[2].name,
+            "field.permissions.%s" % encoded_patterns[1]: "can_push_existing",
+            "field.actions.save": "Save",
+            }
+        view = create_initialized_view(
+            repository, name="+permissions", form=form,
+            principal=repository.owner)
+        self.assertHasRules(
+            repository, ["refs/heads/stable/*", "refs/heads/*/next"])
+        self.assertThat(stable_rule.grants, MatchesSetwise(
+            MatchesStructure.byEquality(
+                grantee_type=GitGranteeType.REPOSITORY_OWNER,
+                can_create=True, can_push=True, can_force_push=False)))
+        self.assertThat(next_rule.grants, MatchesSetwise(
+            MatchesStructure.byEquality(
+                grantee=grantees[1],
+                can_create=True, can_push=True, can_force_push=True),
+            MatchesStructure.byEquality(
+                grantee=grantees[2],
+                can_create=False, can_push=True, can_force_push=False)))
+        self.assertThat(repository.getActivity(), MatchesSetwise(
+            MatchesStructure(
+                changer=Equals(repository.owner),
+                changee=Is(None),
+                what_changed=Equals(GitActivityType.GRANT_CHANGED),
+                old_value=Equals({
+                    "changee_type": "Repository owner",
+                    "ref_pattern": "refs/heads/stable/*",
+                    "can_create": True,
+                    "can_push": False,
+                    "can_force_push": False,
+                    }),
+                new_value=Equals({
+                    "changee_type": "Repository owner",
+                    "ref_pattern": "refs/heads/stable/*",
+                    "can_create": True,
+                    "can_push": True,
+                    "can_force_push": False,
+                    })),
+            MatchesStructure(
+                changer=Equals(repository.owner),
+                changee=Equals(grantees[0]),
+                what_changed=Equals(GitActivityType.GRANT_REMOVED),
+                old_value=Equals({
+                    "changee_type": "Person",
+                    "ref_pattern": "refs/heads/stable/*",
+                    "can_create": True,
+                    "can_push": True,
+                    "can_force_push": False,
+                    })),
+            MatchesStructure(
+                changer=Equals(repository.owner),
+                changee=Equals(grantees[2]),
+                what_changed=Equals(GitActivityType.GRANT_ADDED),
+                new_value=Equals({
+                    "changee_type": "Person",
+                    "ref_pattern": "refs/heads/*/next",
+                    "can_create": False,
+                    "can_push": True,
+                    "can_force_push": False,
+                    }))))
+        self.assertHasSavedNotification(view, repository)
+
+    def test_save_delete_rule(self):
+        repository = self.factory.makeGitRepository()
+        self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/stable/*")
+        heads_rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/*")
+        self.factory.makeGitRuleGrant(
+            rule=heads_rule, grantee=GitGranteeType.REPOSITORY_OWNER,
+            can_push=True)
+        removeSecurityProxy(repository.getActivity()).remove()
+        login_person(repository.owner)
+        encoded_pattern = encode_form_field_id("refs/heads/*")
+        form = {
+            "field.pattern." + encoded_pattern: "*",
+            "field.delete." + encoded_pattern: "on",
+            # Form data entries relating to grants for the deleted rule are
+            # ignored.
+            "field.permissions.%s._repository_owner" % encoded_pattern: (
+                "can_push"),
+            "field.actions.save": "Save",
+            }
+        view = create_initialized_view(
+            repository, name="+permissions", form=form,
+            principal=repository.owner)
+        self.assertHasRules(repository, ["refs/heads/stable/*"])
+        self.assertThat(list(repository.getActivity()), MatchesListwise([
+            MatchesStructure(
+                changer=Equals(repository.owner),
+                what_changed=Equals(GitActivityType.RULE_REMOVED),
+                old_value=MatchesDict({
+                    "ref_pattern": Equals("refs/heads/*"),
+                    "position": Equals(1),
+                    })),
+            ]))
+        self.assertHasSavedNotification(view, repository)
+
+
 class TestGitRepositoryDeletionView(BrowserTestCase):
 
     layer = DatabaseFunctionalLayer

=== modified file 'lib/lp/code/interfaces/gitrule.py'
--- lib/lp/code/interfaces/gitrule.py	2018-10-23 16:17:39 +0000
+++ lib/lp/code/interfaces/gitrule.py	2019-01-10 10:50:57 +0000
@@ -158,6 +158,10 @@
         vocabulary="ValidPersonOrTeam",
         description=_("The person being granted access."))
 
+    combined_grantee = Attribute(
+        "The overall grantee of this grant: either a `GitGranteeType` (other "
+        "than `PERSON`) or an `IPerson`.")
+
     date_created = Datetime(
         title=_("Date created"), required=True, readonly=True,
         description=_("The time when this grant was created."))

=== modified file 'lib/lp/code/model/gitrule.py'
--- lib/lp/code/model/gitrule.py	2018-10-29 14:27:36 +0000
+++ lib/lp/code/model/gitrule.py	2019-01-10 10:50:57 +0000
@@ -310,6 +310,13 @@
         self.date_created = date_created
         self.date_last_modified = date_created
 
+    @property
+    def combined_grantee(self):
+        if self.grantee_type == GitGranteeType.PERSON:
+            return self.grantee
+        else:
+            return self.grantee_type
+
     def __repr__(self):
         if self.grantee_type == GitGranteeType.PERSON:
             grantee_name = "~%s" % self.grantee.name

=== modified file 'lib/lp/code/model/tests/test_gitrule.py'
--- lib/lp/code/model/tests/test_gitrule.py	2018-10-21 17:38:05 +0000
+++ lib/lp/code/model/tests/test_gitrule.py	2019-01-10 10:50:57 +0000
@@ -594,6 +594,7 @@
             rule=Equals(rule),
             grantee_type=Equals(GitGranteeType.REPOSITORY_OWNER),
             grantee=Is(None),
+            combined_grantee=Equals(GitGranteeType.REPOSITORY_OWNER),
             can_create=Is(True),
             can_push=Is(False),
             can_force_push=Is(True),
@@ -614,6 +615,7 @@
             rule=Equals(rule),
             grantee_type=Equals(GitGranteeType.PERSON),
             grantee=Equals(grantee),
+            combined_grantee=Equals(grantee),
             can_create=Is(False),
             can_push=Is(True),
             can_force_push=Is(False),

=== added file 'lib/lp/code/templates/gitrepository-permissions.pt'
--- lib/lp/code/templates/gitrepository-permissions.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/gitrepository-permissions.pt	2019-01-10 10:50:57 +0000
@@ -0,0 +1,232 @@
+<html
+  xmlns="http://www.w3.org/1999/xhtml"
+  xmlns:tal="http://xml.zope.org/namespaces/tal"
+  xmlns:metal="http://xml.zope.org/namespaces/metal"
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+  metal:use-macro="view/macro:page/main_only"
+  i18n:domain="launchpad">
+<body>
+
+  <metal:block fill-slot="head_epilogue">
+    <style type="text/css">
+      table.git-rules-table tr.even {
+        background-color: #eee;
+      }
+      tr.git-rule td, tr.git-new-rule td {
+        padding-top: 1em;
+      }
+      tr.git-new-rule-grant td, tr.git-new-rule td {
+        padding-bottom: 1em;
+      }
+      /* Position, pattern, and delete add up to 100%. */
+      tr .git-rule-position {
+        width: 10%;
+      }
+      tr .git-rule-pattern {
+        width: 85%;
+      }
+      tr .git-rule-delete {
+        width: 5%;
+      }
+      /* Grantee and permissions add up to pattern (85%). */
+      tr .git-rule-grant-grantee {
+        width: 40%;
+      }
+      tr .git-rule-grant-permissions {
+        width: 45%;
+      }
+    </style>
+  </metal:block>
+
+  <metal:macros fill-slot="bogus">
+    <metal:macro define-macro="rule-rows">
+      <tal:rule repeat="rule rules">
+        <tal:rule_widgets
+            define="rule_widgets python:view.getRuleWidgets(rule);
+                    parity python:'even' if repeat['rule'].even() else 'odd'">
+          <tr tal:attributes="class string:git-rule ${parity}">
+            <td class="git-rule-position"
+                tal:define="widget nocall:rule_widgets/position">
+              <metal:block use-macro="context/@@launchpad_form/widget_div" />
+            </td>
+            <td class="git-rule-pattern"
+                tal:define="widget nocall:rule_widgets/pattern" colspan="2">
+              <metal:block use-macro="context/@@launchpad_form/widget_div" />
+            </td>
+            <td class="git-rule-delete"
+                tal:define="widget nocall:rule_widgets/delete">
+              <metal:block use-macro="context/@@launchpad_form/widget_div" />
+            </td>
+          </tr>
+          <tr tal:attributes="class string:git-rule-grant ${parity}"
+              tal:repeat="grant_widgets rule_widgets/grants">
+            <td></td>
+            <td class="git-rule-grant-grantee"
+                tal:define="widget nocall:grant_widgets/grantee">
+              <metal:block use-macro="context/@@launchpad_form/widget_div" />
+            </td>
+            <td class="git-rule-grant-permissions"
+                tal:define="widget nocall:grant_widgets/permissions">
+              <metal:block use-macro="context/@@launchpad_form/widget_div" />
+            </td>
+            <td class="git-rule-delete"
+                tal:define="widget nocall:grant_widgets/delete">
+              <metal:block use-macro="context/@@launchpad_form/widget_div" />
+            </td>
+          </tr>
+          <tr tal:attributes="class string:git-new-rule-grant ${parity}"
+              tal:define="new_grant_widgets rule_widgets/new_grant">
+            <td></td>
+            <td class="git-rule-grant-grantee"
+                tal:define="widget nocall:new_grant_widgets/grantee">
+              <metal:block use-macro="context/@@launchpad_form/widget_div" />
+            </td>
+            <td class="git-rule-grant-permissions"
+                tal:define="widget nocall:new_grant_widgets/permissions">
+              <metal:block use-macro="context/@@launchpad_form/widget_div" />
+            </td>
+            <td></td>
+          </tr>
+        </tal:rule_widgets>
+      </tal:rule>
+      <tal:allows-new-rule
+          condition="ref_prefix"
+          define="parity python:'odd' if len(rules) % 2 else 'even'">
+        <tr tal:attributes="class string:git-new-rule ${parity}"
+            tal:define="new_rule_widgets python:view.getNewRuleWidgets(ref_prefix)">
+          <td class="git-rule-position"
+              tal:define="widget nocall:new_rule_widgets/position">
+            <metal:block use-macro="context/@@launchpad_form/widget_div" />
+          </td>
+          <td class="git-rule-pattern"
+              tal:define="widget nocall:new_rule_widgets/pattern" colspan="2">
+            <metal:block use-macro="context/@@launchpad_form/widget_div" />
+          </td>
+          <td></td>
+        </tr>
+      </tal:allows-new-rule>
+    </metal:macro>
+
+    <metal:macro define-macro="rules-table">
+      <table class="listing git-rules-table"
+             style="max-width: 60em; margin-bottom: 1em;">
+        <thead>
+          <tr>
+            <th class="git-rule-position">Position</th>
+            <th class="git-rule-pattern" colspan="2">Rule</th>
+            <th class="git-rule-delete">Delete?</th>
+          </tr>
+        </thead>
+        <tbody>
+          <metal:rules use-macro="template/macros/rule-rows" />
+        </tbody>
+      </table>
+    </metal:macro>
+  </metal:macros>
+
+  <div metal:fill-slot="main">
+    <p>
+      By default, repository owners may create, push, force-push, or delete
+      any branch or tag in their repositories, and nobody else may modify
+      them in any way.
+    </p>
+    <p>
+      If any of the rules below matches a branch or tag, then it is
+      <em>protected</em>.  By default, protecting a branch implicitly
+      prevents repository owners from force-pushing to it or deleting it,
+      while protecting a tag prevents repository owners from moving it.
+      Protecting a branch or tag also allows you to grant other permissions.
+    </p>
+    <p>
+      You may create rules that match a single branch or tag, or wildcard
+      rules that match a pattern: for example, <code>*</code> matches
+      everything, while <code>stable/*</code> matches
+      <code>stable/1.0</code> but not <code>master</code>.
+    </p>
+
+    <metal:grants-form use-macro="context/@@launchpad_form/form">
+      <div class="form" metal:fill-slot="widgets">
+        <h3>Protected branches (under <code>refs/heads/</code>)</h3>
+        <tal:branches define="rules view/branch_rules;
+                              ref_prefix string:refs/heads/">
+          <metal:table use-macro="template/macros/rules-table" />
+        </tal:branches>
+
+        <h3>Protected tags (under <code>refs/tags/</code>)</h3>
+        <tal:tags define="rules view/tag_rules;
+                          ref_prefix string:refs/tags/">
+          <metal:table use-macro="template/macros/rules-table" />
+        </tal:tags>
+
+        <tal:has-other condition="view/other_rules">
+          <h3>Other protected references</h3>
+          <tal:other define="rules view/other_rules; ref_prefix nothing">
+            <metal:table use-macro="template/macros/rules-table" />
+          </tal:other>
+        </tal:has-other>
+
+        <p class="actions">
+          <input tal:replace="structure view/save_action/render" />
+          or <a tal:attributes="href view/cancel_url">Cancel</a>
+        </p>
+      </div>
+
+      <metal:buttons fill-slot="buttons" />
+    </metal:grants-form>
+
+    <h2>Wildcards</h2>
+    <p>The special characters used in wildcard rules are:</p>
+    <table class="listing narrow-listing">
+      <thead>
+        <tr>
+          <th>Pattern</th>
+          <th>Meaning</th>
+        </tr>
+      </thead>
+      <tbody>
+        <tr>
+          <td><code>*</code></td>
+          <td>matches zero or more characters</td>
+        </tr>
+        <tr>
+          <td><code>?</code></td>
+          <td>matches any single character</td>
+        </tr>
+        <tr>
+          <td><code>[chars]</code></td>
+          <td>matches any character in <em>chars</em></td>
+        </tr>
+        <tr>
+          <td><code>[!chars]</code></td>
+          <td>matches any character not in <em>chars</em></td>
+        </tr>
+      </tbody>
+    </table>
+
+    <h2>Effective permissions</h2>
+    <p>
+      Launchpad works out the effective permissions that a user has on a
+      protected branch or tag as follows:
+    </p>
+    <ol>
+      <li>Take all the rules that match the branch or tag.</li>
+      <li>
+        For each matching rule, select any grants whose grantee matches the
+        user, as long as the same grantee has not already been seen in an
+        earlier matching rule.  (A user can be matched by more than one
+        grantee: for example, they might be in multiple teams.)
+      </li>
+      <li>
+        If the user is an owner of the repository and there was no previous
+        “Repository owner” grant, then add an implicit grant allowing them
+        to create or push.
+      </li>
+      <li>
+        The effective permission set is the union of the permissions granted
+        by all the selected grants.
+      </li>
+    </ol>
+  </div>
+
+</body>
+</html>