Merge ~ines-almeida/launchpad:project-tokens/add-new-access-token-tables-to-view into launchpad:master

[Ines Almeida]

Add 'project tokens' table to git repo access tokens view

This is a small update but I think it will improve the UX for these tokens, since we will be able to have a better view of the tokens that grant access to a repository within that repository "+access-tokens" view.

Regarding the CSS change, it is quite small and in line with Vanilla. It improves slightly the tables look. You can see a diff in the Jira ticket: https://warthogs.atlassian.net/browse/LP-1315. This bit is in a separate commit - happy to move it into a another new MP if you prefer.

[Colin Watson (cjwatson)]

review: Approve

[Peter Makowski (petermakowski)]

Nice. Appreciate you trying to follow vanilla styles as closely as possible.

[Ines Almeida (ines-almeida)]

Thank you for the review! I addressed your comments

[Peter Makowski (petermakowski)]

No problem, this looks great! Just one minor thing.

[Peter Makowski (petermakowski)]

review: Approve

b1b04f8... by Ines Almeida on 2023-10-05

Add parent tokens table to access tokens view

Original table was moved into a reusable macro, and an 'owner' column was added

a14cea1... by Ines Almeida on 2023-10-05

Update style of access tokens tables

[Ines Almeida (ines-almeida)]

Thank you for the tips Colin!

diff --git a/lib/canonical/launchpad/icing/css/base.scss b/lib/canonical/launchpad/icing/css/base.scss
index f86bbf0..ea976cc 100644
--- a/lib/canonical/launchpad/icing/css/base.scss
+++ b/lib/canonical/launchpad/icing/css/base.scss
@@ -366,9 +366,15 @@ body {
   table {
     &.listing {
       margin: 0;
+      margin-bottom: 1.5rem;
       width: 100%;
       border-bottom: 1px solid #d2d2d2;
 
+      form {
+        /* Remove margin from forms when they are embedded within a table */
+        margin-bottom: 0;
+      }
+
       tbody, td.end-of-section {
         border-bottom: 1px solid #d2d2d2;
       }
@@ -440,7 +446,8 @@ body {
       }
 
       th, td {
-        padding: 0.25em;
+        padding: 0.5rem;
+        padding-top: calc(.5rem - 1px);
       }
 
       table {
diff --git a/lib/canonical/launchpad/icing/css/components/_index.scss b/lib/canonical/launchpad/icing/css/components/_index.scss
index e1623c8..3facac8 100644
--- a/lib/canonical/launchpad/icing/css/components/_index.scss
+++ b/lib/canonical/launchpad/icing/css/components/_index.scss
@@ -1,4 +1,5 @@
-@import 'batch_navigation',
+@import 'access_tokens',
+        'batch_navigation',
         'bug_picker',
         'profiling_info',
         'sidebar_portlets',
diff --git a/lib/canonical/launchpad/icing/css/components/access_tokens.css b/lib/canonical/launchpad/icing/css/components/access_tokens.css
new file mode 100644
index 0000000..351ae1c
--- /dev/null
+++ b/lib/canonical/launchpad/icing/css/components/access_tokens.css
@@ -0,0 +1,3 @@
+.access-tokens-table {
+    max-width: 80rem;
+}
diff --git a/lib/lp/services/auth/browser.py b/lib/lp/services/auth/browser.py
index b10d64b..21ccfcb 100644
--- a/lib/lp/services/auth/browser.py
+++ b/lib/lp/services/auth/browser.py
@@ -16,7 +16,13 @@ from lp.app.browser.launchpadform import LaunchpadFormView, action
 from lp.app.errors import UnexpectedFormData
 from lp.app.widgets.date import DateTimeWidget
 from lp.app.widgets.itemswidgets import LabeledMultiCheckBoxWidget
-from lp.services.auth.interfaces import IAccessToken, IAccessTokenSet
+from lp.code.interfaces.gitrepository import IGitRepository
+from lp.registry.interfaces.product import IProduct
+from lp.services.auth.interfaces import (
+    IAccessToken,
+    IAccessTokenSet,
+    IAccessTokenTarget,
+)
 from lp.services.propertycache import cachedproperty
 from lp.services.webapp.publisher import canonical_url
 
@@ -49,6 +55,14 @@ class AccessTokensView(LaunchpadFormView):
 
     page_title = "Personal access tokens"
 
+    @property
+    def context_type(self):
+        if IGitRepository.providedBy(self.context):
+            return "Git repository"
+        elif IProduct.providedBy(self.context):
+            return "project"
+        return "target"
+
     @cachedproperty
     def access_tokens(self):
         return list(
@@ -57,6 +71,18 @@ class AccessTokensView(LaunchpadFormView):
             )
         )
 
+    @cachedproperty
+    def parent_tokens(self):
+        if IGitRepository.providedBy(
+            self.context
+        ) and IAccessTokenTarget.providedBy(self.context.target):
+            return list(
+                getUtility(IAccessTokenSet).findByTarget(
+                    self.context.target, visible_by_user=self.user
+                )
+            )
+        return []
+
     @action("Revoke", name="revoke")
     def revoke_action(self, action, data):
         form = self.request.form
diff --git a/lib/lp/services/auth/javascript/tokens.js b/lib/lp/services/auth/javascript/tokens.js
index 81b4d4b..ebc6039 100644
--- a/lib/lp/services/auth/javascript/tokens.js
+++ b/lib/lp/services/auth/javascript/tokens.js
@@ -103,6 +103,7 @@ YUI.add('lp.services.auth.tokens', function(Y) {
                                     ? Y.lp.ui.CSS_ODD : Y.lp.ui.CSS_EVEN)
                             .append(Y.Node.create('<td />')
                                 .set('text', description))
+                            .append(Y.Node.create('<td />'))
                             .append(Y.Node.create('<td />')
                                 .set('text', scopes.join(', ')))
                             .append(Y.Node.create('<td />')
diff --git a/lib/lp/services/auth/templates/accesstokentarget-access-tokens.pt b/lib/lp/services/auth/templates/accesstokentarget-access-tokens.pt
index 6d85df0..d6e3563 100644
--- a/lib/lp/services/auth/templates/accesstokentarget-access-tokens.pt
+++ b/lib/lp/services/auth/templates/accesstokentarget-access-tokens.pt
@@ -32,6 +32,60 @@
   </script>
 </metal:block>
 
+
+<metal:macros fill-slot="bogus">
+  <metal:access-tokens-table define-macro="access-tokens-table">
+    <tal:comment condition="nothing">
+      This macro requires a string value for 'access_tokens_attribute', which
+      will be the name of the view function used to fetch the access tokens.
+    </tal:comment>
+    <table tal:attributes="id access_tokens_attribute" 
+           class="listing access-tokens-table">
+      <thead>
+        <tr>
+          <th>Description</th>
+          <th>Owner</th>
+          <th>Scopes</th>
+          <th>Created</th>
+          <th>Last used</th>
+          <th>Expires</th>
+          <th><tal:comment condition="nothing">Revoke button</tal:comment></th>
+        </tr>
+      </thead>
+      <tbody id="access-tokens-tbody">
+        <tr tal:repeat="token python:getattr(view, access_tokens_attribute)"
+            tal:attributes="
+              class python: 'yui3-lazr-even' if repeat['token'].even()
+                            else 'yui3-lazr-odd';
+              token-id token/id">
+          <td tal:content="token/description" />
+          <td tal:content="structure token/owner/fmt:link" />
+          <td tal:content="
+            python: ', '.join(scope.title for scope in token.scopes)" />
+          <td tal:content="
+            structure token/date_created/fmt:approximatedatetitle" />
+          <td tal:content="
+            structure token/date_last_used/fmt:approximatedatetitle" />
+          <td>
+            <tal:expires
+              condition="token/date_expires"
+              replace="
+                structure token/date_expires/fmt:approximatedatetitle" />
+            <span tal:condition="not: token/date_expires">Never</span>
+          </td>
+          <td>
+            <form method="post" tal:attributes="name string:revoke-${token/id}">
+              <input type="hidden" name="token_id"
+                    tal:attributes="value token/id" />
+              <input type="submit" name="field.actions.revoke" value="Revoke" />
+            </form>
+          </td>
+        </tr>
+      </tbody>
+    </table>
+  </metal:access-tokens-table>
+</metal:macros>
+
 <div metal:fill-slot="main">
   <p>Personal access tokens allow using certain parts of the Launchpad API.</p>
 
@@ -74,48 +128,24 @@
   </div>
 
   <h2>Active tokens</h2>
-  <table class="listing access-tokens-table"
-         style="max-width: 80em;">
-    <thead>
-      <tr>
-        <th>Description</th>
-        <th>Scopes</th>
-        <th>Created</th>
-        <th>Last used</th>
-        <th>Expires</th>
-        <th><tal:comment condition="nothing">Revoke button</tal:comment></th>
-      </tr>
-    </thead>
-    <tbody id="access-tokens-tbody">
-      <tr tal:repeat="token view/access_tokens"
-          tal:attributes="
-            class python: 'yui3-lazr-even' if repeat['token'].even()
-                          else 'yui3-lazr-odd';
-            token-id token/id">
-        <td tal:content="token/description" />
-        <td tal:content="
-          python: ', '.join(scope.title for scope in token.scopes)" />
-        <td tal:content="
-          structure token/date_created/fmt:approximatedatetitle" />
-        <td tal:content="
-          structure token/date_last_used/fmt:approximatedatetitle" />
-        <td>
-          <tal:expires
-            condition="token/date_expires"
-            replace="
-              structure token/date_expires/fmt:approximatedatetitle" />
-          <span tal:condition="not: token/date_expires">Never</span>
-        </td>
-        <td>
-          <form method="post" tal:attributes="name string:revoke-${token/id}">
-            <input type="hidden" name="token_id"
-                   tal:attributes="value token/id" />
-            <input type="submit" name="field.actions.revoke" value="Revoke" />
-          </form>
-        </td>
-      </tr>
-    </tbody>
-  </table>
+  <p>Active personal access tokens for
+    <a tal:replace="structure context/fmt:link" />
+    <span tal:content="view/context_type" />.
+  </p>
+  <tal:access-tokens-table define="access_tokens_attribute string:access_tokens">
+    <metal:access-tokens-table use-macro="template/macros/access-tokens-table" />
+  </tal:access-tokens-table>
+  <div tal:condition="view/parent_tokens">
+    <h2>Project tokens</h2>
+    <p>Your project access tokens for this Git repository's project:
+      <a tal:replace="structure context/target/fmt:link" />.
+    </p>
+    <p>Project tokens grant access to all the project's Git repositories
+    accessible to a user.</p>
+    <tal:access-tokens-table define="access_tokens_attribute string:parent_tokens">
+      <metal:access-tokens-table use-macro="template/macros/access-tokens-table" />
+    </tal:access-tokens-table>
+  </div>
 </div>
 
 </body>
diff --git a/lib/lp/services/auth/tests/test_browser.py b/lib/lp/services/auth/tests/test_browser.py
index 76e1221..721ef8b 100644
--- a/lib/lp/services/auth/tests/test_browser.py
+++ b/lib/lp/services/auth/tests/test_browser.py
@@ -17,6 +17,7 @@ from testtools.matchers import (
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from lp.code.interfaces.gitrepository import IGitRepository
 from lp.services.webapp.interfaces import IPlacelessAuthUtility
 from lp.services.webapp.publisher import canonical_url
 from lp.testing import TestCaseWithFactory, login_person
@@ -33,7 +34,13 @@ token_listing_constants = soupmatchers.HTMLContains(
     soupmatchers.Within(breadcrumbs_tag, tokens_page_crumb_tag)
 )
 token_listing_tag = soupmatchers.Tag(
-    "tokens table", "table", attrs={"class": "listing"}
+    "tokens table", "table", attrs={"id": "access_tokens"}
+)
+
+project_token_listing_tag = soupmatchers.Tag(
+    "project tokens table",
+    "table",
+    attrs={"id": "parent_tokens"},
 )
 
 
@@ -82,9 +89,11 @@ class TestAccessTokenViewBase:
         tokens_link = browser.getLink("Manage access tokens")
         self.assertEqual(expected_tokens_url, tokens_link.url)
 
-    def makeTokensAndMatchers(self, count):
+    def makeTokensAndMatchers(self, count, target=None):
+        if target is None:
+            target = self.target
         tokens = [
-            self.factory.makeAccessToken(target=self.target)[1]
+            self.factory.makeAccessToken(target=target)[1]
             for _ in range(count)
         ]
         # There is a row for each token.
@@ -124,6 +133,23 @@ class TestAccessTokenViewBase:
             MatchesAll(*self.getPageContent(token_matchers)),
         )
 
+    def test_extra_tokens(self):
+        if not IGitRepository.providedBy(self.target):
+            self.skipTest("Test only relevant for Git Repository tests.")
+
+        project = self.factory.makeProduct(owner=self.owner)
+        self.target.setTarget(target=project, user=self.owner)
+        token_matchers = self.makeTokensAndMatchers(5)
+        extra_matchers = self.makeTokensAndMatchers(5, target=project)
+        self.assertThat(
+            self.makeView("+access-tokens")(),
+            MatchesAll(
+                *self.getPageContent(
+                    token_matchers, extra_matchers=extra_matchers
+                )
+            ),
+        )
+
     def test_revoke(self):
         tokens = [
             self.factory.makeAccessToken(target=self.target)[1]
@@ -192,12 +218,20 @@ class TestAccessTokenViewGitRepository(
     def getTraversalStack(self, obj):
         return [obj.target, obj]
 
-    def getPageContent(self, token_matchers):
-        return [
+    def getPageContent(self, token_matchers, extra_matchers=None):
+        contents = [
             token_listing_constants,
             soupmatchers.HTMLContains(token_listing_tag, *token_matchers),
         ]
 
+        if extra_matchers is not None:
+            contents.append(
+                soupmatchers.HTMLContains(
+                    project_token_listing_tag, *extra_matchers
+                )
+            )
+        return contents
+
 
 class TestAccessTokenViewProject(TestAccessTokenViewBase, TestCaseWithFactory):
     rootsite = None