launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging lp:~twom/launchpad/git-permissions-activity-view into lp:launchpad.

Commit message:
Add log page for GitActivity

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/git-permissions-activity-view/+merge/358375

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
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~twom/launchpad/git-permissions-activity-view into lp:launchpad.

=== 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-06 11:54:49 +0000
@@ -877,6 +877,12 @@
         template="../templates/gitrepository-delete.pt"/>
     <browser:page
         for="lp.code.interfaces.gitrepository.IGitRepository"
+        class="lp.code.browser.gitrepository.GitRepositoryPermissionsActivityView"
+        permission="launchpad.Edit"
+        name="+permissionsactivity"
+        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-06 11:54:49 +0000
@@ -16,6 +16,7 @@
     'GitRepositoryEditReviewerView',
     'GitRepositoryEditView',
     'GitRepositoryNavigation',
+    'GitRepositoryPermissionsActivityView',
     'GitRepositoryURL',
     'GitRepositoryView',
     ]
@@ -210,7 +211,7 @@
     usedfor = IGitRepository
     facet = "branches"
     title = "Edit Git repository"
-    links = ["edit", "reviewer", "webhooks", "delete"]
+    links = ["edit", "reviewer", "webhooks", "permissions_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 permissions_activity(self):
+        text = "View permissions activity"
+        return Link("+permissionsactivity", text, icon="edit")
+
+    @enabled_with_permission("launchpad.Edit")
     def delete(self):
         text = "Delete repository"
         return Link("+delete", text, icon="trash-icon")
@@ -771,3 +777,14 @@
     @property
     def cancel_url(self):
         return canonical_url(self.context)
+
+
+class GitRepositoryPermissionsActivityView(LaunchpadView):
+
+    @property
+    def page_title(self):
+        return "Permissions Activity"
+
+    @property
+    def activity(self):
+        return self.context.getPrecachedActivity()

=== 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-06 11:54:49 +0000
@@ -1154,3 +1154,53 @@
         self.assertEqual(
             ["Repository %s deleted." % name],
             get_feedback_messages(browser.contents))
+
+
+class TestGitRepositoryPermissionsActivityView(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, "+permissionsactivity", rootsite="code",
+            user=repository.owner)
+        self.assertIsNotNone(
+            find_tag_by_id(browser.contents, 'activity-listing'))
+
+    def test_activity_query_count(self):
+        requester = self.factory.makePerson()
+        repository = 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,
+                "+permissionsactivity",
+                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-06 11:54:49 +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/gitactivity.py'
--- lib/lp/code/model/gitactivity.py	2018-09-11 19:43:53 +0000
+++ lib/lp/code/model/gitactivity.py	2018-11-06 11:54:49 +0000
@@ -17,6 +17,7 @@
     JSON,
     Reference,
     )
+from zope.component import getUtility
 from zope.interface import implementer
 
 from lp.code.enums import GitActivityType
@@ -25,6 +26,7 @@
     IGitActivitySet,
     )
 from lp.registry.interfaces.person import (
+    IPersonSet,
     validate_person,
     validate_public_person,
     )
@@ -83,6 +85,17 @@
         else:
             return None
 
+    @staticmethod
+    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
+
 
 def _make_rule_value(rule, position=None):
     return {

=== 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-06 11:54:49 +0000
@@ -1287,6 +1287,11 @@
         return Store.of(self).find(GitActivity, *clauses).order_by(
             Desc(GitActivity.date_changed), Desc(GitActivity.id))
 
+    def getPrecachedActivity(self, changed_after=None):
+        results = self.getActivity(changed_after)
+        loader = partial(GitActivity.preloadDataForActivities)
+        return DecoratedResultSet(results, pre_iter_hook=loader)
+
     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-06 11:54:49 +0000
@@ -0,0 +1,74 @@
+<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><b>Ref Pattern:</b> <span tal:content="value/ref_pattern" /></li>
+          <tal:comment condition="nothing">
+            Grant changes
+          </tal:comment>
+          <span tal:condition="python:'can_create' in value"><b>create: </b><span tal:content="value/can_create" />, </span>
+          <span tal:condition="python:'can_push' in value"><b>push: </b><span tal:content="value/can_push" />, </span>
+          <span tal:condition="python:'can_force_push' in value"><b>force-push: </b><span tal:content="value/can_force_push" /></span>
+          <tal:comment condition="nothing">
+            Rule changes
+          </tal:comment>
+          <li tal:condition="python: 'position' in value">Rule position: <span tal:content="value/position" /></li>
+        </ul>
+    </metal:macro>
+</metal:macros>
+
+<div metal:fill-slot="main">
+    <div class="yui-g">
+      <div class="top-portlet">
+        <h1>Activity log for repository <span tal:replace="context/display_name" /></h1>
+        <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 view/activity">
+              <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" />
+                </tal:activity>
+              </td>
+              <td>
+                <tal:activity define="value log/new_value">
+                    <metal:logs use-macro="template/macros/activity-value" />
+                </tal:activity>
+              </td>
+            </tr>
+          </tbody>
+        </table>
+      </div>
+    </div>
+</div>
+
+</body>
+</html>