From d34943435d63593bc80b71a86d95dc59665bf3ba Mon Sep 17 00:00:00 2001
From: Phil Renaud <phil.renaud@hashicorp.com>
Date: Wed, 12 Jun 2024 14:28:17 -0400
Subject: [PATCH] [ui] Rework of node/job attributes/meta using PathTree
 (#23290)

* Rework of attributes using pathTree

* Pack meta reintroduced and made local

* attributes table test updated for new pathTree syntax

* removed flat import and extended the PathTree type signature to include prefix

* Slightly darken the is-faded text in tables
---
 .changelog/23290.txt                          |  3 +
 ui/app/components/job-page/parts/stats-box.js | 19 +++++
 ui/app/models/node-driver.js                  |  8 +-
 ui/app/models/structured-attributes.js        | 11 +--
 ui/app/styles/core/table.scss                 |  2 +-
 ui/app/templates/clients/client/index.hbs     |  8 +-
 .../components/attributes-section.hbs         | 32 ++++----
 .../components/job-page/parts/meta.hbs        |  4 +-
 .../components/job-page/parts/stats-box.hbs   | 16 ++--
 .../components/job-page/parts/title.hbs       |  2 +-
 ui/app/utils/path-tree.js                     | 13 +++-
 ui/package.json                               |  1 -
 .../components/attributes-table-test.js       | 78 ++++++++-----------
 ui/yarn.lock                                  |  9 +--
 14 files changed, 112 insertions(+), 94 deletions(-)
 create mode 100644 .changelog/23290.txt

diff --git a/.changelog/23290.txt b/.changelog/23290.txt
new file mode 100644
index 00000000000..142f57d5692
--- /dev/null
+++ b/.changelog/23290.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+ui: shorten client/node metadata/attributes display and make parent-terminal attributes show up
+```
diff --git a/ui/app/components/job-page/parts/stats-box.js b/ui/app/components/job-page/parts/stats-box.js
index ee1b7dbec67..a6dd61dfa25 100644
--- a/ui/app/components/job-page/parts/stats-box.js
+++ b/ui/app/components/job-page/parts/stats-box.js
@@ -8,4 +8,23 @@ import { inject as service } from '@ember/service';
 
 export default class StatsBox extends Component {
   @service system;
+
+  get packDetails() {
+    let packMeta = this.args.job?.meta?.structured.root.children.pack;
+    if (!packMeta) {
+      return null;
+    } else {
+      return packMeta.files
+        .map((file) => {
+          return {
+            key: file.name,
+            value: file.variable.value,
+          };
+        })
+        .reduce((acc, file) => {
+          acc[file.key] = file.value;
+          return acc;
+        }, {});
+    }
+  }
 }
diff --git a/ui/app/models/node-driver.js b/ui/app/models/node-driver.js
index a41a4e19250..30ee81a8404 100644
--- a/ui/app/models/node-driver.js
+++ b/ui/app/models/node-driver.js
@@ -5,7 +5,7 @@
 
 import classic from 'ember-classic-decorator';
 import Fragment from 'ember-data-model-fragments/fragment';
-import { get, computed } from '@ember/object';
+import { computed } from '@ember/object';
 import { attr } from '@ember-data/model';
 import { fragmentOwner } from 'ember-data-model-fragments/attributes';
 import { fragment } from 'ember-data-model-fragments/attributes';
@@ -18,8 +18,10 @@ export default class NodeDriver extends Fragment {
 
   @computed('name', 'attributes.structured')
   get attributesShort() {
-    const attributes = this.get('attributes.structured');
-    return get(attributes, `driver.${this.name}`);
+    const attributes = this.get(
+      `attributes.structured.root.children.driver.children.${this.name}`
+    );
+    return attributes;
   }
 
   @attr('string') name;
diff --git a/ui/app/models/structured-attributes.js b/ui/app/models/structured-attributes.js
index 6dc5b4df936..9708918ad47 100644
--- a/ui/app/models/structured-attributes.js
+++ b/ui/app/models/structured-attributes.js
@@ -7,9 +7,7 @@ import { set } from '@ember/object';
 import { get, computed } from '@ember/object';
 import { attr } from '@ember-data/model';
 import Fragment from 'ember-data-model-fragments/fragment';
-import flat from 'flat';
-
-const { unflatten } = flat;
+import PathTree from 'nomad-ui/utils/path-tree';
 
 export default class StructuredAttributes extends Fragment {
   @attr() raw;
@@ -26,14 +24,17 @@ export default class StructuredAttributes extends Fragment {
       return undefined;
     }
 
-    // `unflatten` doesn't sort keys before unflattening, so manual preprocessing is necessary.
     const attrs = Object.keys(original)
       .sort()
       .reduce((obj, key) => {
         obj[key] = original[key];
         return obj;
       }, {});
-    return unflatten(attrs, { overwrite: true });
+    let pathValueArray = Object.entries(attrs).map(([path, value]) => {
+      return { path, value };
+    });
+    let pathTree = new PathTree(pathValueArray, { delimiter: '.' });
+    return pathTree;
   }
 
   unknownProperty(key) {
diff --git a/ui/app/styles/core/table.scss b/ui/app/styles/core/table.scss
index b98f82cc3cf..549fdd11637 100644
--- a/ui/app/styles/core/table.scss
+++ b/ui/app/styles/core/table.scss
@@ -290,7 +290,7 @@
   }
 
   .is-faded {
-    color: $grey-light;
+    color: darken($grey-light, 10%);
   }
 }
 
diff --git a/ui/app/templates/clients/client/index.hbs b/ui/app/templates/clients/client/index.hbs
index 8958cca1627..bea905f4e52 100644
--- a/ui/app/templates/clients/client/index.hbs
+++ b/ui/app/templates/clients/client/index.hbs
@@ -793,7 +793,7 @@
               {{capitalize a.item.name}}
               Attributes
             </div>
-            {{#if a.item.attributes.structured}}
+            {{#if a.item.attributesShort}}
               <div class="boxed-section-body is-full-bleed">
                 <AttributesTable
                   @attributePairs={{a.item.attributesShort}}
@@ -821,7 +821,7 @@
     <div class="boxed-section-body is-full-bleed">
       <AttributesTable
         data-test-attributes
-        @attributePairs={{this.model.attributes.structured}}
+        @attributePairs={{this.model.attributes.structured.root}}
         @class="attributes-table"
         @copyable={{true}}
       />
@@ -835,7 +835,7 @@
       <div class="boxed-section-body is-full-bleed">
         <AttributesTable
           data-test-meta
-          @attributePairs={{this.model.meta.structured}}
+          @attributePairs={{this.model.meta.structured.root}}
           @editable={{can "write client"}}
           @onKVSave={{this.addDynamicMetaData}}
           @onKVEdit={{this.validateMetadata}}
@@ -907,4 +907,4 @@
       {{/if}}
     {{/if}}
   </div>
-</section>
\ No newline at end of file
+</section>
diff --git a/ui/app/templates/components/attributes-section.hbs b/ui/app/templates/components/attributes-section.hbs
index 7defaef8d11..b825aaa8a8a 100644
--- a/ui/app/templates/components/attributes-section.hbs
+++ b/ui/app/templates/components/attributes-section.hbs
@@ -3,17 +3,23 @@
   SPDX-License-Identifier: BUSL-1.1
 ~}}
 
-{{#each-in this.attributes as |key value|}}
-  {{#if (is-object value)}}
-    <tr data-test-attributes-section>
-      <td data-test-heading class="is-subheading" colspan="2">
-        {{#if this.prefix}}<span class="is-faded" data-test-prefix>{{this.prefix}}.</span>{{/if}}{{key}}
-      </td>
-    </tr>
-    <AttributesSection @prefix={{if this.prefix (concat this.prefix "." key) key}} @attributes={{value}}
-    @key={{key}} @value={{value}} @editable={{@editable}} @onKVSave={{@onKVSave}} @copyable={{@copyable}} />
-  {{else}}
-    <MetadataKv @prefix={{this.prefix}}
-    @key={{key}} @value={{value}} @editable={{@editable}} @onKVSave={{@onKVSave}} @copyable={{@copyable}} />
-  {{/if}}
+{{#each this.attributes.files as |file|}}
+  <MetadataKv
+    @prefix={{file.prefix}}
+    @key={{file.name}}
+    @value={{file.variable.value}}
+    @editable={{@editable}}
+    @onKVSave={{@onKVSave}}
+    @copyable={{@copyable}}
+  />
+{{/each}}
+{{#each-in this.attributes.children as |key value|}}
+  <AttributesSection
+    @attributes={{value}}
+    @key={{key}}
+    @value={{value}}
+    @editable={{@editable}}
+    @onKVSave={{@onKVSave}}
+    @copyable={{@copyable}}
+  />
 {{/each-in}}
diff --git a/ui/app/templates/components/job-page/parts/meta.hbs b/ui/app/templates/components/job-page/parts/meta.hbs
index 9e7157ad5ff..27b67ca5850 100644
--- a/ui/app/templates/components/job-page/parts/meta.hbs
+++ b/ui/app/templates/components/job-page/parts/meta.hbs
@@ -11,8 +11,8 @@
     <div class="boxed-section-body is-full-bleed">
       <AttributesTable
         data-test-meta
-        @attributePairs={{@job.meta.structured}}
+        @attributePairs={{@job.meta.structured.root}}
         @class="attributes-table" />
     </div>
   </div>
-{{/if}}
\ No newline at end of file
+{{/if}}
diff --git a/ui/app/templates/components/job-page/parts/stats-box.hbs b/ui/app/templates/components/job-page/parts/stats-box.hbs
index 567edf14842..296f61648be 100644
--- a/ui/app/templates/components/job-page/parts/stats-box.hbs
+++ b/ui/app/templates/components/job-page/parts/stats-box.hbs
@@ -38,29 +38,29 @@
     {{yield to="after-namespace"}}
   </div>
 
-  {{#if @job.meta.structured.pack.name}}
+  {{#if this.packDetails.name}}
     <div class="boxed-section-body inline-definitions">
       <span class="label" style="width: 6.125rem;">Pack Details</span>
       <span class="pair" data-test-pack-stat="name">
         <span class="term">Name</span>
-        {{@job.meta.structured.pack.name}}
+        {{this.packDetails.name}}
       </span>
-      {{#if @job.meta.structured.pack.registry}}
+      {{#if this.packDetails.registry}}
         <span class="pair" data-test-pack-stat="registry">
           <span class="term">Registry</span>
-          {{@job.meta.structured.pack.registry}}
+          {{this.packDetails.registry}}
         </span>
       {{/if}}
-      {{#if @job.meta.structured.pack.version}}
+      {{#if this.packDetails.version}}
         <span class="pair" data-test-pack-stat="version">
           <span class="term">Version</span>
-          {{@job.meta.structured.pack.version}}
+          {{this.packDetails.version}}
         </span>
       {{/if}}
-      {{#if @job.meta.structured.pack.revision}}
+      {{#if this.packDetails.revision}}
         <span class="pair" data-test-pack-stat="revision">
           <span class="term">Revision</span>
-          {{@job.meta.structured.pack.revision}}
+          {{this.packDetails.revision}}
         </span>
       {{/if}}
       {{yield to="pack"}}
diff --git a/ui/app/templates/components/job-page/parts/title.hbs b/ui/app/templates/components/job-page/parts/title.hbs
index d2fc9cae51b..cd8d835923c 100644
--- a/ui/app/templates/components/job-page/parts/title.hbs
+++ b/ui/app/templates/components/job-page/parts/title.hbs
@@ -6,7 +6,7 @@
 <Hds::PageHeader class="job-page-header" as |PH|>
   <PH.Title data-test-job-name>
     {{or this.title this.job.name}}
-    {{#if @job.meta.structured.pack}}
+    {{#if @job.meta.structured.root.children.pack}}
       <span data-test-pack-tag class="tag is-hollow">
         {{x-icon "box" class= "test"}}
         <span>Pack</span>
diff --git a/ui/app/utils/path-tree.js b/ui/app/utils/path-tree.js
index 4db8a6a0ed2..4c105645549 100644
--- a/ui/app/utils/path-tree.js
+++ b/ui/app/utils/path-tree.js
@@ -17,6 +17,7 @@ import { trimPath } from '../helpers/trim-path';
  * @property {string} path - the folder path containing our "file", relative to parent
  * @property {string} name - the variable "file" name
  * @property {string} absoluteFilePath - the folder path containing our "file", absolute
+ * @property {string} prefix - the path prefix
  * @property {VariableModel} variable - the variable itself
  */
 
@@ -39,11 +40,14 @@ export default class PathTree {
   /**
    * @param {MutableArray<VariableModel>} variables
    */
-  constructor(variables) {
+  constructor(variables, { delimiter = '/' } = {}) {
+    this.delimiter = delimiter;
     this.variables = variables;
     this.paths = this.generatePaths(variables);
   }
 
+  delimiter = '/';
+
   /**
    * @type {VariableFolder}
    */
@@ -56,14 +60,15 @@ export default class PathTree {
    */
   generatePaths = (variables) => {
     variables.forEach((variable) => {
-      const path = trimPath([variable.path]).split('/');
+      const path = trimPath([variable.path]).split(this.delimiter);
       path.reduce((acc, segment, index, arr) => {
         if (index === arr.length - 1) {
           // If it's a file (end of the segment array)
           acc.files.push({
             name: segment,
-            absoluteFilePath: path.join('/'),
-            path: arr.slice(0, index + 1).join('/'),
+            absoluteFilePath: path.join(this.delimiter),
+            path: arr.slice(0, index + 1).join(this.delimiter),
+            prefix: arr.slice(0, index).join(this.delimiter),
             variable,
           });
         } else {
diff --git a/ui/package.json b/ui/package.json
index c9d2182f645..7bb6f6cafae 100644
--- a/ui/package.json
+++ b/ui/package.json
@@ -123,7 +123,6 @@
     "eslint-plugin-prettier": "^3.4.1",
     "eslint-plugin-qunit": "^6.2.0",
     "faker": "^4.1.0",
-    "flat": "^5.0.2",
     "fuse.js": "^3.4.4",
     "glob": "^7.2.0",
     "http-proxy": "^1.1.6",
diff --git a/ui/tests/integration/components/attributes-table-test.js b/ui/tests/integration/components/attributes-table-test.js
index 9ae6adda229..cc730ee5085 100644
--- a/ui/tests/integration/components/attributes-table-test.js
+++ b/ui/tests/integration/components/attributes-table-test.js
@@ -8,37 +8,49 @@ import { module, test } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
 import hbs from 'htmlbars-inline-precompile';
 import { componentA11yAudit } from 'nomad-ui/tests/helpers/a11y-audit';
-import flat from 'flat';
-
-const { flatten } = flat;
+import PathTree from 'nomad-ui/utils/path-tree';
 
 module('Integration | Component | attributes table', function (hooks) {
   setupRenderingTest(hooks);
 
-  const commonAttributes = {
-    key: 'value',
-    nested: {
-      props: 'are',
-      supported: 'just',
-      fine: null,
+  const commonAttributes = [
+    {
+      path: 'key',
+      value: 'value',
+    },
+    {
+      path: 'nested.props',
+      value: 'are',
+    },
+    {
+      path: 'nested.supported',
+      value: 'just',
+    },
+    {
+      path: 'nested.fine',
+      value: null,
     },
-    so: {
-      are: {
-        deeply: {
-          nested: 'properties',
-          like: 'these ones',
-        },
-      },
+    {
+      path: 'so.are.deeply.nested',
+      value: 'properties',
     },
-  };
+    {
+      path: 'so.are.deeply.like',
+      value: 'these ones',
+    },
+  ];
+
+  const commonAttributesTree = new PathTree(commonAttributes, {
+    delimiter: '.',
+  });
 
   test('should render a row for each key/value pair in a deep object', async function (assert) {
     assert.expect(2);
 
-    this.set('attributes', commonAttributes);
+    this.set('attributes', commonAttributesTree.root);
     await render(hbs`<AttributesTable @attributePairs={{attributes}} />`);
 
-    const rowsCount = Object.keys(flatten(commonAttributes)).length;
+    const rowsCount = commonAttributes.length;
     assert.equal(
       this.element.querySelectorAll(
         '[data-test-attributes-section] [data-test-value]'
@@ -51,7 +63,7 @@ module('Integration | Component | attributes table', function (hooks) {
   });
 
   test('should render the full path of key/value pair from the root of the object', async function (assert) {
-    this.set('attributes', commonAttributes);
+    this.set('attributes', commonAttributesTree.root);
     await render(hbs`<AttributesTable @attributePairs={{attributes}} />`);
 
     assert.equal(
@@ -64,8 +76,7 @@ module('Integration | Component | attributes table', function (hooks) {
       'value',
       'Row renders the value'
     );
-
-    const deepRow = findAll('[data-test-attributes-section]')[8];
+    const deepRow = findAll('[data-test-attributes-section]')[4];
     assert.equal(
       deepRow.querySelector('[data-test-key]').textContent.trim(),
       'so.are.deeply.nested',
@@ -81,27 +92,4 @@ module('Integration | Component | attributes table', function (hooks) {
       'properties'
     );
   });
-
-  test('should render a row for key/value pairs even when the value is another object', async function (assert) {
-    this.set('attributes', commonAttributes);
-    await render(hbs`<AttributesTable @attributePairs={{attributes}} />`);
-
-    const countOfParentRows = countOfParentKeys(commonAttributes);
-    assert.equal(
-      findAll('[data-test-heading]').length,
-      countOfParentRows,
-      'Each key for a nested object gets a row with no value'
-    );
-  });
-
-  function countOfParentKeys(obj) {
-    return Object.keys(obj).reduce((count, key) => {
-      const value = obj[key];
-      return isObject(value) ? count + 1 + countOfParentKeys(value) : count;
-    }, 0);
-  }
-
-  function isObject(value) {
-    return !Array.isArray(value) && value != null && typeof value === 'object';
-  }
 });
diff --git a/ui/yarn.lock b/ui/yarn.lock
index 030c14f0da2..e0ce1a57336 100644
--- a/ui/yarn.lock
+++ b/ui/yarn.lock
@@ -7008,9 +7008,9 @@ ember-truth-helpers@^3.0.0:
   dependencies:
     ember-cli-babel "^7.22.1"
 
-"ember-usable@git+https://github.com/pzuraq/ember-usable.git#0d03a50":
+"ember-usable@https://github.com/pzuraq/ember-usable#0d03a50":
   version "0.0.0"
-  resolved "git+https://github.com/pzuraq/ember-usable.git#0d03a500a2f49041a4ddff0bb05b077c3907ed7d"
+  resolved "https://github.com/pzuraq/ember-usable#0d03a500a2f49041a4ddff0bb05b077c3907ed7d"
   dependencies:
     ember-cli-babel "^7.13.0"
     ember-cli-htmlbars "^4.2.0"
@@ -7988,11 +7988,6 @@ flat-cache@^3.0.4:
     keyv "^4.5.3"
     rimraf "^3.0.2"
 
-flat@^5.0.2:
-  version "5.0.2"
-  resolved "https://registry.yarnpkg.com/flat/-/flat-5.0.2.tgz#8ca6fe332069ffa9d324c327198c598259ceb241"
-  integrity sha512-b6suED+5/3rTpUBdG1gupIl8MPFCAMA0QXwmljLhvCUKcUvdE4gWky9zpuGCcXHOsz4J9wPGNWq6OKpmIzz3hQ==
-
 flatted@^3.2.9:
   version "3.3.1"
   resolved "https://registry.yarnpkg.com/flatted/-/flatted-3.3.1.tgz#21db470729a6734d4997002f439cb308987f567a"