Merge lp:~twom/launchpad/git-permissions-activity-view into lp:launchpad

[Tom Wardill]

Add log page for GitActivity

Add a page for viewing the GitActivity records for a repository.
* Add link to sidebar of GitRepository index
* Precache the granter/grantee details on loading the page
* Add tests for rendering and precaching
* Format the raw output to something more human friendly

[Colin Watson (cjwatson)]

review: Needs Fixing

[Colin Watson (cjwatson)]

review: Approve

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2018-08-29 15:38:17 +0000
+++ lib/lp/code/browser/configure.zcml	2018-11-08 16:01:42 +0000
@@ -877,6 +877,12 @@
         template="../templates/gitrepository-delete.pt"/>
     <browser:page
         for="lp.code.interfaces.gitrepository.IGitRepository"
+        class="lp.code.browser.gitrepository.GitRepositoryActivityView"
+        permission="launchpad.Edit"
+        name="+activity"
+        template="../templates/gitrepository-activity.pt" />
+    <browser:page
+        for="lp.code.interfaces.gitrepository.IGitRepository"
         class="lp.code.browser.gitsubscription.GitSubscriptionAddView"
         permission="launchpad.AnyPerson"
         name="+subscribe"

=== modified file 'lib/lp/code/browser/gitrepository.py'
--- lib/lp/code/browser/gitrepository.py	2018-08-31 13:36:38 +0000
+++ lib/lp/code/browser/gitrepository.py	2018-11-08 16:01:42 +0000
@@ -8,6 +8,7 @@
 __all__ = [
     'GitRefBatchNavigator',
     'GitRepositoriesBreadcrumb',
+    'GitRepositoryActivityView',
     'GitRepositoryBreadcrumb',
     'GitRepositoryContextMenu',
     'GitRepositoryDeletionView',
@@ -210,7 +211,7 @@
     usedfor = IGitRepository
     facet = "branches"
     title = "Edit Git repository"
-    links = ["edit", "reviewer", "webhooks", "delete"]
+    links = ["edit", "reviewer", "webhooks", "activity", "delete"]
 
     @enabled_with_permission("launchpad.Edit")
     def edit(self):
@@ -230,6 +231,11 @@
             enabled=bool(getFeatureFlag('webhooks.new.enabled')))
 
     @enabled_with_permission("launchpad.Edit")
+    def activity(self):
+        text = "View activity"
+        return Link("+activity", text, icon="info")
+
+    @enabled_with_permission("launchpad.Edit")
     def delete(self):
         text = "Delete repository"
         return Link("+delete", text, icon="trash-icon")
@@ -771,3 +777,23 @@
     @property
     def cancel_url(self):
         return canonical_url(self.context)
+
+
+class GitRepositoryActivityView(LaunchpadView):
+
+    page_title = "Activity"
+
+    @property
+    def label(self):
+        return "Activity log for %s" % self.context.display_name
+
+    def displayPermissions(self, values):
+        """Assemble a human readable list from the permissions changes."""
+        permissions = []
+        if values.get('can_create'):
+            permissions.append('create')
+        if values.get('can_push'):
+            permissions.append('push')
+        if values.get('can_force_push'):
+            permissions.append('force-push')
+        return ', '.join(permissions)

=== modified file 'lib/lp/code/browser/tests/test_gitrepository.py'
--- lib/lp/code/browser/tests/test_gitrepository.py	2018-09-11 15:04:43 +0000
+++ lib/lp/code/browser/tests/test_gitrepository.py	2018-11-08 16:01:42 +0000
@@ -1154,3 +1154,77 @@
         self.assertEqual(
             ["Repository %s deleted." % name],
             get_feedback_messages(browser.contents))
+
+
+class TestGitRepositoryActivityView(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_render(self):
+        requester = self.factory.makePerson()
+        repository = removeSecurityProxy(
+            self.factory.makeGitRepository(owner=requester))
+
+        rule = self.factory.makeGitRule(repository)
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=requester, can_push=True, can_create=True)
+
+        browser = self.getViewBrowser(
+            repository, "+activity", rootsite="code",
+            user=repository.owner)
+
+        table = find_tag_by_id(browser.contents, "activity-listing")
+        date = repository.getActivity()[0].date_changed.strftime(
+            '%Y-%m-%d %T')
+        person_name = repository.owner.display_name
+        self.assertThat(extract_text(table), DocTestMatches(dedent("""
+            Date
+            Author
+            Target
+            What changed
+            Old value
+            New value
+            {date}
+            {person_name}
+            {person_name}
+            Added access grant
+            Pattern: refs/heads/*
+            Permissions:
+            create, push
+            {date}
+            {person_name}
+            Added access rule
+            Pattern: refs/heads/*
+            Rule position: 0
+            """.format(date=date, person_name=person_name)),
+            flags=doctest.NORMALIZE_WHITESPACE))
+
+    def test_activity_query_count(self):
+        requester = self.factory.makePerson()
+        repository = removeSecurityProxy(
+            self.factory.makeGitRepository(owner=requester))
+        rule = self.factory.makeGitRule(repository)
+
+        grantees = iter([self.factory.makePerson() for _ in range(4)])
+
+        def login_and_view():
+            browser = self.getViewBrowser(
+                repository,
+                "+activity",
+                rootsite="code",
+                user=repository.owner)
+            self.assertIsNotNone(
+                find_tag_by_id(browser.contents, 'activity-listing'))
+
+        def create_activity():
+            self.factory.makeGitRuleGrant(
+                rule=rule,
+                grantee=next(grantees),
+                can_push=True,
+                can_create=True)
+
+        recorder1, recorder2 = record_two_runs(
+            login_and_view,
+            create_activity,
+            2)
+        self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))

=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py	2018-10-23 16:15:37 +0000
+++ lib/lp/code/interfaces/gitrepository.py	2018-11-08 16:01:42 +0000
@@ -639,6 +639,14 @@
         :return: A `ResultSet` of `IGitActivity`.
         """
 
+    def getPrecachedActivity(changed_after=None):
+        """Activity log entries are preloaded.
+
+        :param changed_after: If supplied, only return entries for changes
+            made after this date.
+        :return: A `ResultSet` of `IGitActivity`.
+        """
+
 
 class IGitRepositoryModerateAttributes(Interface):
     """IGitRepository attributes that can be edited by more than one community.

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2018-10-22 16:24:46 +0000
+++ lib/lp/code/model/gitrepository.py	2018-11-08 16:01:42 +0000
@@ -137,7 +137,10 @@
 from lp.registry.interfaces.distributionsourcepackage import (
     IDistributionSourcePackage,
     )
-from lp.registry.interfaces.person import IPerson
+from lp.registry.interfaces.person import (
+    IPerson,
+    IPersonSet,
+    )
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.role import IHasOwner
 from lp.registry.interfaces.sharingjob import (
@@ -1287,6 +1290,22 @@
         return Store.of(self).find(GitActivity, *clauses).order_by(
             Desc(GitActivity.date_changed), Desc(GitActivity.id))
 
+    def getPrecachedActivity(self, **kwargs):
+
+        def preloadDataForActivities(activities):
+            # Utility to load related data for a list of GitActivity
+            person_ids = set()
+            for activity in activities:
+                person_ids.add(activity.changer_id)
+                person_ids.add(activity.changee_id)
+            list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
+                person_ids, need_validity=True))
+            return activities
+
+        results = self.getActivity(**kwargs)
+        return DecoratedResultSet(
+            results, pre_iter_hook=preloadDataForActivities)
+
     def canBeDeleted(self):
         """See `IGitRepository`."""
         # Can't delete if the repository is associated with anything.

=== added file 'lib/lp/code/templates/gitrepository-activity.pt'
--- lib/lp/code/templates/gitrepository-activity.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/gitrepository-activity.pt	2018-11-08 16:01:42 +0000
@@ -0,0 +1,79 @@
+<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:macros fill-slot="bogus">
+    <metal:macro define-macro="activity-value">
+      <ul tal:condition="value">
+        <li><strong>Pattern:</strong> <span tal:content="value/ref_pattern" /></li>
+        <tal:comment condition="nothing">
+          Rule changes
+        </tal:comment>
+        <li tal:condition="python: 'position' in value"><strong>Rule position:</strong> <span tal:content="value/position" /></li>
+        <tal:comment condition="nothing">
+          Grant changes
+        </tal:comment>
+        <li tal:condition="python: 'position' not in value">
+          <strong>Permissions:</strong>
+          <span metal:define-slot="permissions"></span>
+        </li>
+      </ul>
+    </metal:macro>
+  </metal:macros>
+
+  <div metal:fill-slot="main">
+    <div class="yui-g">
+      <div class="top-portlet">
+        <table id="activity-listing" class="listing">
+          <thead>
+            <tr>
+              <th>Date</th>
+              <th>Author</th>
+              <th>Target</th>
+              <th>What changed</th>
+              <th>Old value</th>
+              <th>New value</th>
+            </tr>
+          </thead>
+          <tbody>
+            <tr tal:repeat="log context/getPrecachedActivity">
+              <tal:comment condition="nothing">
+                XXX: Gavin Panella 2009-08-12 bug=412963: Using strftime()
+                here because fmt:datetime changes timezone, even though we
+                always want to show only UTC.
+              </tal:comment>
+              <td tal:content="python:log.date_changed.strftime('%Y-%m-%d %T')">
+                2004-09-24 12:04:43
+              </td>
+              <td tal:content="structure log/changer/fmt:link">Changer</td>
+              <td tal:content="structure log/changee/fmt:link">Changee</td>
+              <td tal:content="log/what_changed/title">description</td>
+              <td>
+                <tal:activity define="value log/old_value">
+                  <metal:logs use-macro="template/macros/activity-value">
+                    <span metal:fill-slot="permissions" tal:content="python:view.displayPermissions(log.old_value)"></span>
+                  </metal:logs>
+                </tal:activity>
+              </td>
+              <td>
+                <tal:activity define="value log/new_value">
+                  <metal:logs use-macro="template/macros/activity-value">
+                    <span metal:fill-slot="permissions" tal:content="python:view.displayPermissions(log.new_value)"></span>
+                  </metal:logs>
+                </tal:activity>
+              </td>
+            </tr>
+          </tbody>
+        </table>
+      </div>
+    </div>
+  </div>
+
+</body>
+
+</html>