Merge lp:~cjwatson/launchpad/git-activity-model into lp:launchpad

[Colin Watson]

Add GitActivity model, currently logging additions, changes, and removals of rules and grants.

See https://docs.google.com/document/d/1JW_D_Tgo4X2-vPMZtShSbi3cm1iOsGcNIzeOpa5E_wA for the design and https://code.launchpad.net/~cjwatson/launchpad/db-git-activity/+merge/354398 for schema changes.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2018-09-12 16:26:15 +0000
+++ lib/lp/code/configure.zcml	2018-10-12 16:44:41 +0000
@@ -948,6 +948,19 @@
       for="lp.code.interfaces.gitrule.IGitRuleGrant zope.lifecycleevent.interfaces.IObjectModifiedEvent"
       handler="lp.code.model.gitrule.git_rule_grant_modified"/>
 
+  <!-- GitActivity -->
+
+  <class class="lp.code.model.gitactivity.GitActivity">
+    <require
+        permission="launchpad.View"
+        interface="lp.code.interfaces.gitactivity.IGitActivity" />
+  </class>
+  <securedutility
+      class="lp.code.model.gitactivity.GitActivitySet"
+      provides="lp.code.interfaces.gitactivity.IGitActivitySet">
+    <allow interface="lp.code.interfaces.gitactivity.IGitActivitySet" />
+  </securedutility>
+
   <!-- GitCollection -->
 
   <class class="lp.code.model.gitcollection.GenericGitCollection">

=== modified file 'lib/lp/code/enums.py'
--- lib/lp/code/enums.py	2018-09-03 16:07:56 +0000
+++ lib/lp/code/enums.py	2018-10-12 16:44:41 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Enumerations used in the lp/code modules."""
@@ -21,6 +21,7 @@
     'CodeImportReviewStatus',
     'CodeReviewNotificationLevel',
     'CodeReviewVote',
+    'GitActivityType',
     'GitGranteeType',
     'GitObjectType',
     'GitRepositoryType',
@@ -196,6 +197,27 @@
         """)
 
 
+class GitActivityType(DBEnumeratedType):
+    """Git Activity Type
+
+    Various kinds of change can be recorded for a Git repository.
+    """
+
+    RULE_ADDED = DBItem(1, "Added access rule")
+
+    RULE_CHANGED = DBItem(2, "Changed access rule")
+
+    RULE_REMOVED = DBItem(3, "Removed access rule")
+
+    RULE_MOVED = DBItem(4, "Moved access rule")
+
+    GRANT_ADDED = DBItem(11, "Added access grant")
+
+    GRANT_CHANGED = DBItem(12, "Changed access grant")
+
+    GRANT_REMOVED = DBItem(13, "Removed access grant")
+
+
 class BranchLifecycleStatusFilter(EnumeratedType):
     """Branch Lifecycle Status Filter
 

=== added file 'lib/lp/code/interfaces/gitactivity.py'
--- lib/lp/code/interfaces/gitactivity.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/interfaces/gitactivity.py	2018-10-12 16:44:41 +0000
@@ -0,0 +1,135 @@
+# Copyright 2018 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Git repository activity logs."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'IGitActivity',
+    'IGitActivitySet',
+    ]
+
+from lazr.restful.fields import Reference
+from zope.interface import Interface
+from zope.schema import (
+    Choice,
+    Datetime,
+    Dict,
+    Int,
+    Text,
+    TextLine,
+    )
+
+from lp import _
+from lp.code.enums import GitActivityType
+from lp.code.interfaces.gitrepository import IGitRepository
+from lp.services.fields import (
+    PersonChoice,
+    PublicPersonChoice,
+    )
+
+
+class IGitActivity(Interface):
+    """An activity log entry for a Git repository."""
+
+    id = Int(title=_("ID"), readonly=True, required=True)
+
+    repository = Reference(
+        title=_("Repository"), required=True, readonly=True,
+        schema=IGitRepository,
+        description=_("The repository that this log entry is for."))
+
+    date_changed = Datetime(
+        title=_("Date changed"), required=True, readonly=True,
+        description=_("The time when this change happened."))
+
+    changer = PublicPersonChoice(
+        title=_("Changer"), required=True, readonly=True,
+        vocabulary="ValidPerson",
+        description=_("The user who made this change."))
+
+    changee = PersonChoice(
+        title=_("Changee"), required=False, readonly=True,
+        vocabulary="ValidPersonOrTeam",
+        description=_("The person or team that this change was applied to."))
+
+    what_changed = Choice(
+        title=_("What changed"), required=True, readonly=True,
+        vocabulary=GitActivityType,
+        description=_("The property of the repository that changed."))
+
+    old_value = Dict(
+        title=_("Old value"), required=False, readonly=True,
+        description=_("Representation of the value before the change."),
+        key_type=TextLine(), value_type=Text())
+
+    new_value = Dict(
+        title=_("New value"), required=False, readonly=True,
+        description=_("Representation of the value after the change."),
+        key_type=TextLine(), value_type=Text())
+
+
+class IGitActivitySet(Interface):
+    """Utilities for managing Git repository activity log entries."""
+
+    def logRuleAdded(rule, user):
+        """Log that an access rule was added.
+
+        :param rule: The `IGitRule` that was added.
+        :param user: The `IPerson` who added it.
+        :return: The new `IGitActivity`.
+        """
+
+    def logRuleChanged(old_rule, new_rule, user):
+        """Log that an access rule was changed.
+
+        :param old_rule: The `IGitRule` before the change.
+        :param new_rule: The `IGitRule` after the change.
+        :param user: The `IPerson` who made the change.
+        :return: The new `IGitActivity`.
+        """
+
+    def logRuleRemoved(rule, user):
+        """Log that an access rule was removed.
+
+        :param rule: The `IGitRule` that was removed.
+        :param user: The `IPerson` who removed it.
+        :return: The new `IGitActivity`.
+        """
+
+    def logRuleMoved(rule, old_position, new_position, user):
+        """Log that an access rule was moved to a different position.
+
+        :param rule: The `IGitRule` that was moved.
+        :param old_position: The old position in its repository's order.
+        :param new_position: The new position in its repository's order.
+        :param user: The `IPerson` who moved it.
+        :return: The new `IGitActivity`.
+        """
+
+    def logGrantAdded(grant, user):
+        """Log that an access grant was added.
+
+        :param grant: The `IGitRuleGrant` that was added.
+        :param user: The `IPerson` who added it.
+        :return: The new `IGitActivity`.
+        """
+
+    def logGrantChanged(old_grant, new_grant, user):
+        """Log that an access grant was changed.
+
+        :param old_grant: The `IGitRuleGrant` before the change.
+        :param new_grant: The `IGitRuleGrant` after the change.
+        :param user: The `IPerson` who made the change.
+        :return: The new `IGitActivity`.
+        """
+
+    def logGrantRemoved(grant, user):
+        """Log that an access grant was removed.
+
+        :param grant: The `IGitRuleGrant` that was removed.
+        :param user: The `IPerson` who removed it.
+        :return: The new `IGitActivity`.
+        """

=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py	2018-10-03 13:19:08 +0000
+++ lib/lp/code/interfaces/gitrepository.py	2018-10-12 16:44:41 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Git repository interfaces."""
@@ -622,6 +622,12 @@
         :return: The diff as a binary string.
         """
 
+    def getActivity():
+        """Get activity log entries for this repository.
+
+        :return: A `ResultSet` of `IGitActivity`.
+        """
+
 
 class IGitRepositoryModerateAttributes(Interface):
     """IGitRepository attributes that can be edited by more than one community.
@@ -729,7 +735,7 @@
             None to append it.
         """
 
-    def moveRule(rule, position):
+    def moveRule(rule, position, user):
         """Move a rule to a new position in its repository's rule order.
 
         :param rule: The `IGitRule` to move.
@@ -737,6 +743,7 @@
             the start, while `len(repository.rules)` puts the rule at the
             end.  If the new position is before the end of the list, then
             other rules are shifted to later positions to make room.
+        :param user: The `IPerson` who is moving the rule.
         """
 
     @export_read_operation()

=== modified file 'lib/lp/code/interfaces/gitrule.py'
--- lib/lp/code/interfaces/gitrule.py	2018-10-03 13:18:27 +0000
+++ lib/lp/code/interfaces/gitrule.py	2018-10-12 16:44:41 +0000
@@ -100,8 +100,11 @@
             matching this rule.
         """
 
-    def destroySelf():
-        """Delete this rule."""
+    def destroySelf(user):
+        """Delete this rule.
+
+        :param user: The `IPerson` doing the deletion.
+        """
 
 
 class IGitRule(IGitRuleView, IGitRuleEditableAttributes, IGitRuleEdit):
@@ -169,8 +172,12 @@
 class IGitRuleGrantEdit(Interface):
     """`IGitRuleGrant` attributes that require launchpad.Edit."""
 
-    def destroySelf():
-        """Delete this access grant."""
+    def destroySelf(user=None):
+        """Delete this access grant.
+
+        :param user: The `IPerson` doing the deletion, or None if the
+            deletion should not be logged.
+        """
 
 
 class IGitRuleGrant(IGitRuleGrantView, IGitRuleGrantEditableAttributes,

=== added file 'lib/lp/code/model/gitactivity.py'
--- lib/lp/code/model/gitactivity.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/model/gitactivity.py	2018-10-12 16:44:41 +0000
@@ -0,0 +1,145 @@
+# Copyright 2018 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Git repository activity logs."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'GitActivity',
+    ]
+
+import pytz
+from storm.locals import (
+    DateTime,
+    Int,
+    JSON,
+    Reference,
+    )
+from zope.interface import implementer
+
+from lp.code.enums import GitActivityType
+from lp.code.interfaces.gitactivity import (
+    IGitActivity,
+    IGitActivitySet,
+    )
+from lp.registry.interfaces.person import (
+    validate_person,
+    validate_public_person,
+    )
+from lp.services.database.constants import DEFAULT
+from lp.services.database.enumcol import EnumCol
+from lp.services.database.stormbase import StormBase
+
+
+@implementer(IGitActivity)
+class GitActivity(StormBase):
+    """See IGitActivity`."""
+
+    __storm_table__ = 'GitActivity'
+
+    id = Int(primary=True)
+
+    repository_id = Int(name='repository', allow_none=False)
+    repository = Reference(repository_id, 'GitRepository.id')
+
+    date_changed = DateTime(
+        name='date_changed', tzinfo=pytz.UTC, allow_none=False)
+
+    changer_id = Int(
+        name='changer', allow_none=False, validator=validate_public_person)
+    changer = Reference(changer_id, 'Person.id')
+
+    changee_id = Int(
+        name='changee', allow_none=True, validator=validate_person)
+    changee = Reference(changee_id, 'Person.id')
+
+    what_changed = EnumCol(
+        dbName='what_changed', enum=GitActivityType, notNull=True)
+
+    old_value = JSON(name='old_value', allow_none=True)
+    new_value = JSON(name='new_value', allow_none=True)
+
+    def __init__(self, repository, changer, what_changed, changee=None,
+                 old_value=None, new_value=None, date_changed=DEFAULT):
+        super(GitActivity, self).__init__()
+        self.repository = repository
+        self.date_changed = date_changed
+        self.changer = changer
+        self.changee = changee
+        self.what_changed = what_changed
+        self.old_value = old_value
+        self.new_value = new_value
+
+
+def _make_rule_value(rule, position=None):
+    return {
+        "ref_pattern": rule.ref_pattern,
+        "position": position if position is not None else rule.position,
+        }
+
+
+def _make_grant_value(grant):
+    return {
+        # If the grantee is a person, then we store that in
+        # GitActivity.changee; this makes it visible to person merges and so
+        # on.
+        "changee_type": grant.grantee_type.title,
+        "ref_pattern": grant.rule.ref_pattern,
+        "can_create": grant.can_create,
+        "can_push": grant.can_push,
+        "can_force_push": grant.can_force_push,
+        }
+
+
+@implementer(IGitActivitySet)
+class GitActivitySet:
+
+    def logRuleAdded(self, rule, user):
+        """See `IGitActivitySet`."""
+        return GitActivity(
+            rule.repository, user, GitActivityType.RULE_ADDED,
+            new_value=_make_rule_value(rule))
+
+    def logRuleChanged(self, old_rule, new_rule, user):
+        """See `IGitActivitySet`."""
+        return GitActivity(
+            new_rule.repository, user, GitActivityType.RULE_CHANGED,
+            old_value=_make_rule_value(old_rule),
+            new_value=_make_rule_value(new_rule))
+
+    def logRuleRemoved(self, rule, user):
+        """See `IGitActivitySet`."""
+        return GitActivity(
+            rule.repository, user, GitActivityType.RULE_REMOVED,
+            old_value=_make_rule_value(rule))
+
+    def logRuleMoved(self, rule, old_position, new_position, user):
+        """See `IGitActivitySet`."""
+        return GitActivity(
+            rule.repository, user, GitActivityType.RULE_MOVED,
+            old_value=_make_rule_value(rule, position=old_position),
+            new_value=_make_rule_value(rule, position=new_position))
+
+    def logGrantAdded(self, grant, user):
+        """See `IGitActivitySet`."""
+        return GitActivity(
+            grant.repository, user, GitActivityType.GRANT_ADDED,
+            changee=grant.grantee,
+            new_value=_make_grant_value(grant))
+
+    def logGrantChanged(self, old_grant, new_grant, user):
+        """See `IGitActivitySet`."""
+        return GitActivity(
+            new_grant.repository, user, GitActivityType.GRANT_CHANGED,
+            changee=new_grant.grantee,
+            old_value=_make_grant_value(old_grant),
+            new_value=_make_grant_value(new_grant))
+
+    def logGrantRemoved(self, grant, user):
+        """See `IGitActivitySet`."""
+        return GitActivity(
+            grant.repository, user, GitActivityType.GRANT_REMOVED,
+            changee=grant.grantee,
+            old_value=_make_grant_value(grant))

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2018-10-05 15:35:34 +0000
+++ lib/lp/code/model/gitrepository.py	2018-10-12 16:44:41 +0000
@@ -88,6 +88,7 @@
     notify_modified,
     )
 from lp.code.interfaces.codeimport import ICodeImportSet
+from lp.code.interfaces.gitactivity import IGitActivitySet
 from lp.code.interfaces.gitcollection import (
     IAllGitRepositories,
     IGitCollection,
@@ -108,6 +109,7 @@
 from lp.code.interfaces.revision import IRevisionSet
 from lp.code.mail.branch import send_git_repository_modified_notifications
 from lp.code.model.branchmergeproposal import BranchMergeProposal
+from lp.code.model.gitactivity import GitActivity
 from lp.code.model.gitref import (
     GitRef,
     GitRefDefault,
@@ -1166,19 +1168,24 @@
         else:
             rules.insert(position, rule)
         self._syncRulePositions(rules)
+        getUtility(IGitActivitySet).logRuleAdded(rule, creator)
         return rule
 
-    def moveRule(self, rule, position):
+    def moveRule(self, rule, position, user):
         """See `IGitRepository`."""
         if rule.repository != self:
             raise ValueError("%r does not belong to %r" % (rule, self))
         if position < 0:
             raise ValueError("Negative positions are not supported")
-        if position != rule.position:
+        current_position = rule.position
+        if position != current_position:
             rules = list(self.rules)
             rules.remove(rule)
             rules.insert(position, rule)
             self._syncRulePositions(rules)
+            if rule.position != current_position:
+                getUtility(IGitActivitySet).logRuleMoved(
+                    rule, current_position, rule.position, user)
 
     @property
     def grants(self):
@@ -1186,6 +1193,12 @@
         return Store.of(self).find(
             GitRuleGrant, GitRuleGrant.repository_id == self.id)
 
+    def getActivity(self):
+        """See `IGitRepository`."""
+        clauses = [GitActivity.repository_id == self.id]
+        return Store.of(self).find(GitActivity, *clauses).order_by(
+            Desc(GitActivity.date_changed), Desc(GitActivity.id))
+
     def canBeDeleted(self):
         """See `IGitRepository`."""
         # Can't delete if the repository is associated with anything.
@@ -1317,6 +1330,7 @@
         self._deleteRepositorySubscriptions()
         self._deleteJobs()
         getUtility(IWebhookSet).delete(self.webhooks)
+        self.getActivity().remove()
         # We intentionally skip the usual destructors; the only other useful
         # thing they do is to log the removal activity, and we remove the
         # activity logs for removed repositories anyway.

=== modified file 'lib/lp/code/model/gitrule.py'
--- lib/lp/code/model/gitrule.py	2018-10-05 14:48:42 +0000
+++ lib/lp/code/model/gitrule.py	2018-10-12 16:44:41 +0000
@@ -21,15 +21,18 @@
     Store,
     Unicode,
     )
+from zope.component import getUtility
 from zope.interface import implementer
 from zope.security.proxy import removeSecurityProxy
 
 from lp.code.enums import GitGranteeType
+from lp.code.interfaces.gitactivity import IGitActivitySet
 from lp.code.interfaces.gitrule import (
     IGitRule,
     IGitRuleGrant,
     )
 from lp.registry.interfaces.person import (
+    IPerson,
     validate_person,
     validate_public_person,
     )
@@ -48,6 +51,9 @@
     events on Git repository rules.
     """
     if event.edited_fields:
+        user = IPerson(event.user)
+        getUtility(IGitActivitySet).logRuleChanged(
+            event.object_before_modification, rule, user)
         removeSecurityProxy(rule).date_last_modified = UTC_NOW
 
 
@@ -105,13 +111,16 @@
     def addGrant(self, grantee, grantor, can_create=False, can_push=False,
                  can_force_push=False):
         """See `IGitRule`."""
-        return GitRuleGrant(
+        grant = GitRuleGrant(
             rule=self, grantee=grantee, can_create=can_create,
             can_push=can_push, can_force_push=can_force_push, grantor=grantor,
             date_created=DEFAULT)
+        getUtility(IGitActivitySet).logGrantAdded(grant, grantor)
+        return grant
 
-    def destroySelf(self):
+    def destroySelf(self, user):
         """See `IGitRule`."""
+        getUtility(IGitActivitySet).logRuleRemoved(self, user)
         for grant in self.grants:
             grant.destroySelf()
         rules = list(self.repository.rules)
@@ -127,6 +136,9 @@
     events on Git repository grants.
     """
     if event.edited_fields:
+        user = IPerson(event.user)
+        getUtility(IGitActivitySet).logGrantChanged(
+            event.object_before_modification, grant, user)
         removeSecurityProxy(grant).date_last_modified = UTC_NOW
 
 
@@ -203,6 +215,8 @@
             ", ".join(permissions), grantee_name, self.repository.unique_name,
             self.rule.ref_pattern)
 
-    def destroySelf(self):
+    def destroySelf(self, user=None):
         """See `IGitRuleGrant`."""
+        if user is not None:
+            getUtility(IGitActivitySet).logGrantRemoved(self, user)
         Store.of(self).remove(self)

=== added file 'lib/lp/code/model/tests/test_gitactivity.py'
--- lib/lp/code/model/tests/test_gitactivity.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/model/tests/test_gitactivity.py	2018-10-12 16:44:41 +0000
@@ -0,0 +1,48 @@
+# Copyright 2018 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for Git repository activity logs."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+from storm.store import Store
+from testtools.matchers import MatchesStructure
+
+from lp.code.enums import GitActivityType
+from lp.code.interfaces.gitactivity import IGitActivity
+from lp.code.model.gitactivity import GitActivity
+from lp.services.database.sqlbase import get_transaction_timestamp
+from lp.testing import (
+    TestCaseWithFactory,
+    verifyObject,
+    )
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class TestGitActivity(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_implements_IGitActivity(self):
+        repository = self.factory.makeGitRepository()
+        activity = GitActivity(
+            repository, repository.owner, GitActivityType.RULE_ADDED)
+        verifyObject(IGitActivity, activity)
+
+    def test_properties(self):
+        repository = self.factory.makeGitRepository()
+        changee = self.factory.makePerson()
+        activity = GitActivity(
+            repository, repository.owner, GitActivityType.RULE_ADDED,
+            changee=changee, old_value={"old": None}, new_value={"new": None})
+        now = get_transaction_timestamp(Store.of(activity))
+        self.assertThat(activity, MatchesStructure.byEquality(
+            repository=repository,
+            date_changed=now,
+            changer=repository.owner,
+            changee=changee,
+            what_changed=GitActivityType.RULE_ADDED,
+            old_value={"old": None},
+            new_value={"new": None}))

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2018-10-05 15:35:34 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2018-10-12 16:44:41 +0000
@@ -87,6 +87,7 @@
     UpdatePreviewDiffJob,
     )
 from lp.code.model.codereviewcomment import CodeReviewComment
+from lp.code.model.gitactivity import GitActivity
 from lp.code.model.gitjob import (
     GitJob,
     GitJobType,
@@ -542,10 +543,18 @@
     def test_related_rules_and_grants_deleted(self):
         rule = self.factory.makeGitRule(repository=self.repository)
         grant = self.factory.makeGitRuleGrant(rule=rule)
+        store = Store.of(self.repository)
+        repository_id = self.repository.id
+        activities = store.find(
+            GitActivity, GitActivity.repository_id == repository_id)
+        self.assertNotEqual([], list(activities))
         self.repository.destroySelf()
         transaction.commit()
         self.assertRaises(LostObjectError, getattr, grant, 'rule')
         self.assertRaises(LostObjectError, getattr, rule, 'repository')
+        activities = store.find(
+            GitActivity, GitActivity.repository_id == repository_id)
+        self.assertEqual([], list(activities))
 
 
 class TestGitRepositoryDeletionConsequences(TestCaseWithFactory):
@@ -2273,11 +2282,11 @@
             for _ in range(5)]
         with person_logged_in(repository.owner):
             self.assertEqual(rules, list(repository.rules))
-            repository.moveRule(rules[0], 4)
+            repository.moveRule(rules[0], 4, repository.owner)
             self.assertEqual(rules[1:] + [rules[0]], list(repository.rules))
-            repository.moveRule(rules[0], 0)
+            repository.moveRule(rules[0], 0, repository.owner)
             self.assertEqual(rules, list(repository.rules))
-            repository.moveRule(rules[2], 1)
+            repository.moveRule(rules[2], 1, repository.owner)
             self.assertEqual(
                 [rules[0], rules[2], rules[1], rules[3], rules[4]],
                 list(repository.rules))
@@ -2285,7 +2294,9 @@
     def test_moveRule_non_negative(self):
         rule = self.factory.makeGitRule()
         with person_logged_in(rule.repository.owner):
-            self.assertRaises(ValueError, rule.repository.moveRule, rule, -1)
+            self.assertRaises(
+                ValueError, rule.repository.moveRule,
+                rule, -1, rule.repository.owner)
 
     def test_grants(self):
         repository = self.factory.makeGitRepository()

=== modified file 'lib/lp/code/model/tests/test_gitrule.py'
--- lib/lp/code/model/tests/test_gitrule.py	2018-10-03 17:06:11 +0000
+++ lib/lp/code/model/tests/test_gitrule.py	2018-10-12 16:44:41 +0000
@@ -7,15 +7,23 @@
 
 __metaclass__ = type
 
+from lazr.lifecycle.event import ObjectModifiedEvent
+from lazr.lifecycle.snapshot import Snapshot
 from storm.store import Store
 from testtools.matchers import (
     Equals,
     Is,
+    MatchesDict,
     MatchesSetwise,
     MatchesStructure,
     )
+from zope.event import notify
+from zope.interface import providedBy
 
-from lp.code.enums import GitGranteeType
+from lp.code.enums import (
+    GitActivityType,
+    GitGranteeType,
+    )
 from lp.code.interfaces.gitrule import (
     IGitRule,
     IGitRuleGrant,
@@ -114,6 +122,93 @@
                 can_push=Is(False),
                 can_force_push=Is(True))))
 
+    def test_activity_rule_added(self):
+        owner = self.factory.makeTeam()
+        member = self.factory.makePerson(member_of=[owner])
+        repository = self.factory.makeGitRepository(owner=owner)
+        self.factory.makeGitRule(repository=repository, creator=member)
+        self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/stable/*",
+            creator=member)
+        self.assertThat(repository.getActivity().first(), MatchesStructure(
+            repository=Equals(repository),
+            changer=Equals(member),
+            changee=Is(None),
+            what_changed=Equals(GitActivityType.RULE_ADDED),
+            old_value=Is(None),
+            new_value=MatchesDict({
+                "ref_pattern": Equals("refs/heads/stable/*"),
+                "position": Equals(1),
+                })))
+
+    def test_activity_rule_changed(self):
+        owner = self.factory.makeTeam()
+        member = self.factory.makePerson(member_of=[owner])
+        repository = self.factory.makeGitRepository(owner=owner)
+        rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/*")
+        rule_before_modification = Snapshot(rule, providing=providedBy(rule))
+        with person_logged_in(member):
+            rule.ref_pattern = "refs/heads/other/*"
+            notify(ObjectModifiedEvent(
+                rule, rule_before_modification, ["ref_pattern"]))
+        self.assertThat(repository.getActivity().first(), MatchesStructure(
+            repository=Equals(repository),
+            changer=Equals(member),
+            changee=Is(None),
+            what_changed=Equals(GitActivityType.RULE_CHANGED),
+            old_value=MatchesDict({
+                "ref_pattern": Equals("refs/heads/*"),
+                "position": Equals(0),
+                }),
+            new_value=MatchesDict({
+                "ref_pattern": Equals("refs/heads/other/*"),
+                "position": Equals(0),
+                })))
+
+    def test_activity_rule_removed(self):
+        owner = self.factory.makeTeam()
+        member = self.factory.makePerson(member_of=[owner])
+        repository = self.factory.makeGitRepository(owner=owner)
+        rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/*")
+        with person_logged_in(member):
+            rule.destroySelf(member)
+        self.assertThat(repository.getActivity().first(), MatchesStructure(
+            repository=Equals(repository),
+            changer=Equals(member),
+            changee=Is(None),
+            what_changed=Equals(GitActivityType.RULE_REMOVED),
+            old_value=MatchesDict({
+                "ref_pattern": Equals("refs/heads/*"),
+                "position": Equals(0),
+                }),
+            new_value=Is(None)))
+
+    def test_activity_rule_moved(self):
+        owner = self.factory.makeTeam()
+        member = self.factory.makePerson(member_of=[owner])
+        repository = self.factory.makeGitRepository(owner=owner)
+        self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/*")
+        rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/stable/*")
+        with person_logged_in(member):
+            repository.moveRule(rule, 0, member)
+        self.assertThat(repository.getActivity().first(), MatchesStructure(
+            repository=Equals(repository),
+            changer=Equals(member),
+            changee=Is(None),
+            what_changed=Equals(GitActivityType.RULE_MOVED),
+            old_value=MatchesDict({
+                "ref_pattern": Equals("refs/heads/stable/*"),
+                "position": Equals(1),
+                }),
+            new_value=MatchesDict({
+                "ref_pattern": Equals("refs/heads/stable/*"),
+                "position": Equals(0),
+                })))
+
     def test_destroySelf(self):
         repository = self.factory.makeGitRepository()
         rules = [
@@ -125,7 +220,7 @@
         self.assertEqual([0, 1, 2, 3], [rule.position for rule in rules])
         self.assertEqual(rules, list(repository.rules))
         with person_logged_in(repository.owner):
-            rules[1].destroySelf()
+            rules[1].destroySelf(repository.owner)
         del rules[1]
         self.assertEqual([0, 1, 2], [rule.position for rule in rules])
         self.assertEqual(rules, list(repository.rules))
@@ -136,7 +231,7 @@
         grant = self.factory.makeGitRuleGrant(rule=rule)
         self.assertEqual([grant], list(repository.grants))
         with person_logged_in(repository.owner):
-            rule.destroySelf()
+            rule.destroySelf(repository.owner)
         self.assertEqual([], list(repository.grants))
 
 
@@ -208,6 +303,84 @@
                 grantee.name, rule.repository.unique_name, rule.ref_pattern),
             repr(grant))
 
+    def test_activity_grant_added(self):
+        owner = self.factory.makeTeam()
+        member = self.factory.makePerson(member_of=[owner])
+        repository = self.factory.makeGitRepository(owner=owner)
+        grant = self.factory.makeGitRuleGrant(
+            repository=repository, grantor=member, can_push=True)
+        self.assertThat(repository.getActivity().first(), MatchesStructure(
+            repository=Equals(repository),
+            changer=Equals(member),
+            changee=Equals(grant.grantee),
+            what_changed=Equals(GitActivityType.GRANT_ADDED),
+            old_value=Is(None),
+            new_value=MatchesDict({
+                "changee_type": Equals("Person"),
+                "ref_pattern": Equals("refs/heads/*"),
+                "can_create": Is(False),
+                "can_push": Is(True),
+                "can_force_push": Is(False),
+                })))
+
+    def test_activity_grant_changed(self):
+        owner = self.factory.makeTeam()
+        member = self.factory.makePerson(member_of=[owner])
+        repository = self.factory.makeGitRepository(owner=owner)
+        grant = self.factory.makeGitRuleGrant(
+            repository=repository, grantee=GitGranteeType.REPOSITORY_OWNER,
+            can_create=True)
+        grant_before_modification = Snapshot(
+            grant, providing=providedBy(grant))
+        with person_logged_in(member):
+            grant.can_create = False
+            grant.can_force_push = True
+            notify(ObjectModifiedEvent(
+                grant, grant_before_modification,
+                ["can_create", "can_force_push"]))
+        self.assertThat(repository.getActivity().first(), MatchesStructure(
+            repository=Equals(repository),
+            changer=Equals(member),
+            changee=Is(None),
+            what_changed=Equals(GitActivityType.GRANT_CHANGED),
+            old_value=MatchesDict({
+                "changee_type": Equals("Repository owner"),
+                "ref_pattern": Equals("refs/heads/*"),
+                "can_create": Is(True),
+                "can_push": Is(False),
+                "can_force_push": Is(False),
+                }),
+            new_value=MatchesDict({
+                "changee_type": Equals("Repository owner"),
+                "ref_pattern": Equals("refs/heads/*"),
+                "can_create": Is(False),
+                "can_push": Is(False),
+                "can_force_push": Is(True),
+                })))
+
+    def test_activity_grant_removed(self):
+        owner = self.factory.makeTeam()
+        member = self.factory.makePerson(member_of=[owner])
+        repository = self.factory.makeGitRepository(owner=owner)
+        grant = self.factory.makeGitRuleGrant(
+            repository=repository, can_create=True, can_push=True)
+        grantee = grant.grantee
+        with person_logged_in(member):
+            grant.destroySelf(member)
+        self.assertThat(repository.getActivity().first(), MatchesStructure(
+            repository=Equals(repository),
+            changer=Equals(member),
+            changee=Equals(grantee),
+            what_changed=Equals(GitActivityType.GRANT_REMOVED),
+            old_value=MatchesDict({
+                "changee_type": Equals("Person"),
+                "ref_pattern": Equals("refs/heads/*"),
+                "can_create": Is(True),
+                "can_push": Is(True),
+                "can_force_push": Is(False),
+                }),
+            new_value=Is(None)))
+
     def test_destroySelf(self):
         rule = self.factory.makeGitRule()
         grants = [
@@ -217,5 +390,5 @@
             self.factory.makeGitRuleGrant(rule=rule, can_push=True),
             ]
         with person_logged_in(rule.repository.owner):
-            grants[1].destroySelf()
+            grants[1].destroySelf(rule.repository.owner)
         self.assertThat(rule.grants, MatchesSetwise(Equals(grants[0])))

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2018-10-05 15:35:34 +0000
+++ lib/lp/security.py	2018-10-12 16:44:41 +0000
@@ -81,6 +81,7 @@
     )
 from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
 from lp.code.interfaces.diff import IPreviewDiff
+from lp.code.interfaces.gitactivity import IGitActivity
 from lp.code.interfaces.gitcollection import IGitCollection
 from lp.code.interfaces.gitref import IGitRef
 from lp.code.interfaces.gitrepository import (
@@ -2383,6 +2384,15 @@
         super(EditGitRuleGrant, self).__init__(obj, obj.repository)
 
 
+class ViewGitActivity(DelegatedAuthorization):
+    """Anyone who can see a Git repository can see its activity logs."""
+    permission = 'launchpad.View'
+    usedfor = IGitActivity
+
+    def __init__(self, obj):
+        super(ViewGitActivity, self).__init__(obj, obj.repository)
+
+
 class AdminDistroSeriesTranslations(AuthorizationBase):
     permission = 'launchpad.TranslationsAdmin'
     usedfor = IDistroSeries

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2018-10-05 15:35:34 +0000
+++ lib/lp/testing/factory.py	2018-10-12 16:44:41 +0000
@@ -1840,10 +1840,10 @@
 
     def makeGitRuleGrant(self, rule=None, grantee=None, grantor=None,
                          can_create=False, can_push=False,
-                         can_force_push=False):
+                         can_force_push=False, **rule_kwargs):
         """Create a Git repository access grant."""
         if rule is None:
-            rule = self.makeGitRule()
+            rule = self.makeGitRule(**rule_kwargs)
         if grantee is None:
             grantee = self.makePerson()
         if grantor is None: