Page MenuHomeSoftware Heritage

Add a button to the deposit admin UI to show deposit metadata
Needs RevisionPublic

Authored by vlorentz on Mon, Nov 15, 6:32 PM.

Details

Summary

as a modal

I can't figure how to make the button look like the other ones.

I also can't find how to adapt the modal's width to its content,
as it defaults to 500px, and I can't find an easy way to ignore this.

@anlambert Can you take this diff from here?

Test Plan

I also can't find how to test the Python side :/

I'll write Cypress tests if you're fine with the design

Diff Detail

Repository
rDWAPPS Web applications
Branch
deposit-metadata
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 25022
Build 39101: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 39100: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D6640 (id=24138)

Rebasing onto e2659cfed5...

Current branch diff-target is up to date.
Changes applied before test
commit 0b57f734b52bf2cafa04009abc29f909760c00e4
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Nov 15 18:30:36 2021 +0100

    Add a button to the deposit admin UI to show deposit metadata
    
    as a modal

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1232/ for more details.

@anlambert Can you take this diff from here?

I will take a closer look once I am done with T3724.

assets/src/bundles/admin/deposit.js
92

Use class="btn btn-default metadata" to match other buttons design.

assets/src/bundles/admin/deposit.js
92

Already tried it, they look like this:

assets/src/bundles/admin/deposit.js
92

nvm, it works this time for some reason...

add button style, remove debug prints, hide button if no metadata

Build is green

Patch application report for D6640 (id=24142)

Rebasing onto e2659cfed5...

Current branch diff-target is up to date.
Changes applied before test
commit 85589a75d747782f94bbd2910a545f6b6ba9efbd
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Nov 15 18:30:36 2021 +0100

    Add a button to the deposit admin UI to show deposit metadata
    
    as a modal

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1236/ for more details.

@vlorentz What's the status of this diff in the end?

Looks good but some cosmetic improvements can be added, notably:

  • use a dedicated column in the table to display the "Show metadata" button
  • highlight metadata xml code
  • restrain the height of of the popover dialog to avoid overflow when metadata file is large

You can obtain the following result with the diff below:


14:48 $ git diff
diff --git a/assets/src/bundles/admin/deposit.js b/assets/src/bundles/admin/deposit.js
index 4f5324ad..198bcf43 100644
--- a/assets/src/bundles/admin/deposit.js
+++ b/assets/src/bundles/admin/deposit.js
@@ -87,11 +87,15 @@ export function initDepositAdmin(username, isStaff) {
           },
           {
             data: 'status',
-            name: 'status',
+            name: 'status'
+          },
+          {
+            data: 'metadata',
+            name: 'metadata',
             render: (data, type, row) => {
               if (type === 'display') {
                 if (row.raw_metadata) {
-                  return `${data} <button class="btn btn-default metadata">metadata</button>`;
+                  return `<button class="btn btn-default metadata">Show</button>`;
                 }
               }
               return data;
@@ -158,7 +162,10 @@ export function initDepositAdmin(username, isStaff) {
       var row = depositsTable.row(this.parentNode.parentNode).data();
       var metadata = row.raw_metadata;
       var escapedMetadata = $('<div/>').text(metadata).html();
-      swh.webapp.showModalHtml(`Metadata of deposit ${row.id}`, `<pre>${escapedMetadata}</pre>`, '90%');
+      swh.webapp.showModalHtml(`Metadata of deposit ${row.id}`,
+                               `<pre style="max-height: 75vh;"><code class="xml">${escapedMetadata}</code></pre>`,
+                               '90%');
+      swh.webapp.highlightCode();
     });
 
     // Adding exclusion pattern update behavior, when typing, update search
diff --git a/swh/web/templates/admin/deposit.html b/swh/web/templates/admin/deposit.html
index ba4e8b23..e8f22fda 100644
--- a/swh/web/templates/admin/deposit.html
+++ b/swh/web/templates/admin/deposit.html
@@ -33,9 +33,10 @@ See top-level LICENSE file for more information
     <a class="toggle-col" href="#" data-column="1">origin</a> -
     <a class="toggle-col" href="#" data-column="2">reception date</a> -
     <a class="toggle-col" href="#" data-column="3">status</a> -
-    <a class="toggle-col col-hidden" href="#" data-column="4">status detail</a> -
-    <a class="toggle-col col-hidden" href="#" data-column="5">directory</a> -
-    <a class="toggle-col col-hidden" href="#" data-column="6">directory with context</a>
+    <a class="toggle-col" href="#" data-column="4">metadata</a> -
+    <a class="toggle-col col-hidden" href="#" data-column="5">status detail</a> -
+    <a class="toggle-col col-hidden" href="#" data-column="6">directory</a> -
+    <a class="toggle-col col-hidden" href="#" data-column="7">directory with context</a>
   </div>
   <br/>
   <table id="swh-admin-deposit-list" class="table swh-table swh-table-striped" width="100%">
@@ -45,6 +46,7 @@ See top-level LICENSE file for more information
         <th>origin</th>
         <th>reception date</th>
         <th>status</th>
+        <th>metadata</th>
         <th>status detail</th>
         <th>directory</th>
         <th>directory with context</th>

Also a cypress test should be added checking the popover is correctly displayed when clicking on the button.

This revision now requires changes to proceed.Wed, Nov 24, 2:55 PM