UI/fix kmip role form (hashicorp#13585)

* Fix info-table-row not rendering if alwaysRender=false and only block content present * use defaultFields for form and nonOperationFields for adapter * WIP: Move info table row template to addon component dir * Refactor InfoTableRow to glimmer component * Add changelog * passthrough attributes, change @data-test-x to data-test-x on InfoTableRow invocations
qk4l · Feb 4, 2022 · 7ffb0d3 · 7ffb0d3
1 parent a820792
commit 7ffb0d3
Show file tree

Hide file tree

Showing 14 changed files with 252 additions and 155 deletions.
diff --git a/changelog/13585.txt b/changelog/13585.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Fixes issue saving KMIP role correctly
+```
diff --git a/ui/app/adapters/kmip/role.js b/ui/app/adapters/kmip/role.js
@@ -38,7 +38,7 @@ export default BaseAdapter.extend({
 
   serialize(snapshot) {
     // the endpoint here won't allow sending `operation_all` and `operation_none` at the same time or with
-    // other values, so we manually check for them and send an abbreviated object
+    // other operation_ values, so we manually check for them and send an abbreviated object
     let json = snapshot.serialize();
     let keys = snapshot.record.nonOperationFields.map(decamelize);
     let nonOperationFields = getProperties(json, keys);

diff --git a/ui/app/models/kmip/role.js b/ui/app/models/kmip/role.js
@@ -17,10 +17,16 @@ export const COMPUTEDS = {
     return ['tlsClientKeyBits', 'tlsClientKeyType', 'tlsClientTtl'];
   }),
 
-  nonOperationFields: computed('newFields', 'operationFields', 'tlsFields', function () {
+  // For rendering on the create/edit pages
+  defaultFields: computed('newFields', 'operationFields', 'tlsFields', function () {
     let excludeFields = ['role'].concat(this.operationFields, this.tlsFields);
     return this.newFields.slice().removeObjects(excludeFields);
   }),
+
+  // For adapter/serializer
+  nonOperationFields: computed('newFields', 'operationFields', function () {
+    return this.newFields.slice().removeObjects(this.operationFields);
+  }),
 };
 
 const ModelExport = Model.extend(COMPUTEDS, {
@@ -31,10 +37,10 @@ const ModelExport = Model.extend(COMPUTEDS, {
   getHelpUrl(path) {
     return `/v1/${path}/scope/example/role/example?help=1`;
   },
-  fieldGroups: computed('fields', 'nonOperationFields.length', 'tlsFields', function () {
+  fieldGroups: computed('fields', 'defaultFields.length', 'tlsFields', function () {
     const groups = [{ TLS: this.tlsFields }];
-    if (this.nonOperationFields.length) {
-      groups.unshift({ default: this.nonOperationFields });
+    if (this.defaultFields.length) {
+      groups.unshift({ default: this.defaultFields });
     }
     let ret = fieldToAttrs(this, groups);
     return ret;
@@ -61,16 +67,16 @@ const ModelExport = Model.extend(COMPUTEDS, {
     ];
     if (others.length) {
       groups.push({
-        '': others,
+        Other: others,
       });
     }
     return fieldToAttrs(this, groups);
   }),
   tlsFormFields: computed('tlsFields', function () {
     return expandAttributeMeta(this, this.tlsFields);
   }),
-  fields: computed('nonOperationFields', function () {
-    return expandAttributeMeta(this, this.nonOperationFields);
+  fields: computed('defaultFields', function () {
+    return expandAttributeMeta(this, this.defaultFields);
   }),
 });
 

diff --git a/ui/app/templates/components/generate-credentials.hbs b/ui/app/templates/components/generate-credentials.hbs
@@ -56,12 +56,19 @@
             attr.options.masked
           )
         }}
-          <InfoTableRow
-            @label={{capitalize (or attr.options.label (humanize (dasherize attr.name)))}}
-            @value={{get this.model attr.name}}
-          >
-            <MaskedInput @value={{get this.model attr.name}} @name={{attr.name}} @displayOnly={{true}} @allowCopy={{true}} />
-          </InfoTableRow>
+          {{#if (get this.model attr.name)}}
+            <InfoTableRow
+              @label={{capitalize (or attr.options.label (humanize (dasherize attr.name)))}}
+              @value={{get this.model attr.name}}
+            >
+              <MaskedInput
+                @value={{get this.model attr.name}}
+                @name={{attr.name}}
+                @displayOnly={{true}}
+                @allowCopy={{true}}
+              />
+            </InfoTableRow>
+          {{/if}}
         {{else if (and (get this.model attr.name) (or (eq attr.name "issueDate") (eq attr.name "expiryDate")))}}
           <InfoTableRow
             data-test-table-row

diff --git a/ui/app/templates/components/identity/item-alias/alias-details.hbs b/ui/app/templates/components/identity/item-alias/alias-details.hbs
@@ -1,4 +1,4 @@
-<InfoTableRow @label="Name" @value={{@model.name}} @data-test-alias-name={{true}} />
+<InfoTableRow @label="Name" @value={{@model.name}} data-test-alias-name={{true}} />
 <InfoTableRow @label="ID" @value={{@model.id}} />
 <InfoTableRow @label={{if (eq @model.identityType "entity-alias") "Entity ID" "Group ID"}} @value={{this.model.canonicalId}}>
   <LinkTo

diff --git a/ui/app/templates/components/identity/item-details.hbs b/ui/app/templates/components/identity/item-details.hbs
@@ -13,7 +13,7 @@
       {{/if}}
     </AlertBanner>
   {{/if}}
-  <InfoTableRow @label="Name" @value={{@model.name}} @data-test-identity-item-name={{true}} />
+  <InfoTableRow @label="Name" @value={{@model.name}} data-test-identity-item-name={{true}} />
   <InfoTableRow @label="Type" @value={{@model.type}} />
   <InfoTableRow @label="ID" @value={{@model.id}} />
   <InfoTableRow @label="Merged Ids" @value={{@model.mergedEntityIds}}>

diff --git a/ui/app/templates/components/license-info.hbs b/ui/app/templates/components/license-info.hbs
@@ -7,13 +7,13 @@
 <section class="box is-sideless is-marginless is-shadowless is-fullwidth">
   <span class="title is-5">Details</span>
   <div class="field box is-fullwidth is-shadowless is-paddingless is-marginless">
-    <InfoTableRow @label="License ID" @value={{@licenseId}} @data-test-detail-row={{true}} />
-    <InfoTableRow @label="Valid from" @value={{@startTime}} @data-test-detail-row={{true}}>
+    <InfoTableRow @label="License ID" @value={{@licenseId}} data-test-detail-row />
+    <InfoTableRow @label="Valid from" @value={{@startTime}} data-test-detail-row>
       {{date-format @startTime "MMM dd, yyyy hh:mm:ss a"}}
       to
       {{date-format @expirationTime "MMM dd, yyyy hh:mm:ss a"}}
     </InfoTableRow>
-    <InfoTableRow @label="License state" @value={{if @autoloaded "Autoloaded" "Stored"}} @data-test-detail-row={{true}}>
+    <InfoTableRow @label="License state" @value={{if @autoloaded "Autoloaded" "Stored"}} data-test-detail-row>
       {{#if @autoloaded}}
         Autoloaded
       {{else}}
@@ -32,11 +32,7 @@
   <span class="title is-5">Features</span>
   <div class="field box is-fullwidth is-shadowless is-paddingless is-marginless">
     {{#each this.featuresInfo as |info|}}
-      <InfoTableRow
-        @label={{info.name}}
-        @value={{if info.active "Active" "Not Active"}}
-        @data-test-feature-row="data-test-feature-row"
-      >
+      <InfoTableRow @label={{info.name}} @value={{if info.active "Active" "Not Active"}} data-test-feature-row>
         {{#if info.active}}
           <Icon @name="check-circle" class="icon-true" />
           <span data-test-feature-status>

diff --git a/ui/app/templates/components/tool-lookup.hbs b/ui/app/templates/components/tool-lookup.hbs
@@ -8,12 +8,12 @@
 
 {{#if (or @creation_time @creation_ttl)}}
   <div class="box is-fullwidth is-sideless is-paddingless is-marginless">
-    <InfoTableRow @label="Creation path" @value={{@creation_path}} @data-test-tools="token-lookup-row" />
-    <InfoTableRow @label="Creation time" @value={{@creation_time}} @data-test-tools="token-lookup-row" />
-    <InfoTableRow @label="Creation TTL" @value={{@creation_ttl}} @data-test-tools="token-lookup-row" />
+    <InfoTableRow @label="Creation path" @value={{@creation_path}} data-test-tools="token-lookup-row" />
+    <InfoTableRow @label="Creation time" @value={{@creation_time}} data-test-tools="token-lookup-row" />
+    <InfoTableRow @label="Creation TTL" @value={{@creation_ttl}} data-test-tools="token-lookup-row" />
     {{#if @expirationDate}}
-      <InfoTableRow @label="Expiration date" @value={{@expirationDate}} @data-test-tools="token-lookup-row" />
-      <InfoTableRow @label="Expires in" @value={{date-from-now @expirationDate}} @data-test-tools="token-lookup-row" />
+      <InfoTableRow @label="Expiration date" @value={{@expirationDate}} data-test-tools="token-lookup-row" />
+      <InfoTableRow @label="Expires in" @value={{date-from-now @expirationDate}} data-test-tools="token-lookup-row" />
     {{/if}}
   </div>
   <div class="field is-grouped box is-fullwidth is-bottomless">

diff --git a/ui/lib/core/addon/components/info-table-row.hbs b/ui/lib/core/addon/components/info-table-row.hbs
@@ -0,0 +1,90 @@
+{{#if (or (has-block) this.isVisible)}}
+  <div class="info-table-row" data-test-component="info-table-row" ...attributes>
+    <div
+      class="column is-one-quarter {{if this.hasLabelOverflow "label-overflow"}}"
+      data-test-label-div
+      {{did-insert this.calculateLabelOverflow}}
+    >
+      {{#if @label}}
+        {{#if this.hasLabelOverflow}}
+          <ToolTip @verticalPosition="below" @horizontalPosition="left" as |T|>
+            <T.Trigger @tabindex={{false}}>
+              <span class="is-label has-text-grey-dark" data-test-row-label={{@label}}>{{@label}}</span>
+            </T.Trigger>
+            <T.Content class="tool-tip">
+              <div class="box fit-content" data-test-label-tooltip>
+                {{@label}}
+              </div>
+            </T.Content>
+          </ToolTip>
+        {{else}}
+          <span class="is-label has-text-grey-dark" data-test-row-label={{@label}}>{{@label}}</span>
+        {{/if}}
+        {{#if @helperText}}
+          <div>
+            <span class="is-label helper-text has-text-grey">{{@helperText}}</span>
+          </div>
+        {{/if}}
+      {{else}}
+        <Icon @name="minus" />
+      {{/if}}
+    </div>
+    <div class="column is-flex foobar" data-test-value-div={{@label}}>
+      {{#if (has-block)}}
+        {{yield}}
+      {{else if this.valueIsBoolean}}
+        {{#if @value}}
+          <Icon class="icon-true" @name="check-circle" data-test-boolean-true />
+          Yes
+        {{else}}
+          <Icon @name="x-square" class="icon-false" data-test-boolean-false />
+          No
+        {{/if}}
+        {{! alwaysRender is still true }}
+      {{else if this.valueIsEmpty}}
+        {{#if @defaultShown}}
+          <span data-test-row-value={{@label}}>{{@defaultShown}}</span>
+        {{else}}
+          <Icon @name="minus" />
+        {{/if}}
+      {{else}}
+        {{#if (eq @type "array")}}
+          <InfoTableItemArray
+            @backend={{@backend}}
+            @displayArray={{@value}}
+            @isLink={{@isLink}}
+            @label={{@label}}
+            @modelType={{@modelType}}
+            @queryParam={{@queryParam}}
+            @viewAll={{@viewAll}}
+            @wildcardLabel={{@wildcardLabel}}
+          />
+        {{else}}
+          {{#if @tooltipText}}
+            <ToolTip @verticalPosition="above" @horizontalPosition="left" as |T|>
+              <T.Trigger @tabindex={{false}}>
+                <span class="is-word-break has-text-black" data-test-row-value={{this.label}}>{{this.value}}</span>
+              </T.Trigger>
+              <T.Content class="tool-tip">
+                <CopyButton
+                  @clipboardText={{@tooltipText}}
+                  @success={{action (set-flash-message "Data copied!")}}
+                  @tagName="div"
+                  @disabled={{not @isTooltipCopyable}}
+                  class={{if @isTooltipCopyable "has-pointer"}}
+                  data-test-tooltip-copy
+                >
+                  <div class="box">
+                    {{@tooltipText}}
+                  </div>
+                </CopyButton>
+              </T.Content>
+            </ToolTip>
+          {{else}}
+            <span class="is-word-break has-text-black" data-test-row-value={{@label}}>{{@value}}</span>
+          {{/if}}
+        {{/if}}
+      {{/if}}
+    </div>
+  </div>
+{{/if}}
diff --git a/ui/lib/core/addon/components/info-table-row.js b/ui/lib/core/addon/components/info-table-row.js
@@ -1,8 +1,8 @@
 import { typeOf } from '@ember/utils';
-import { computed } from '@ember/object';
-import { or } from '@ember/object/computed';
-import Component from '@ember/component';
-import layout from '../templates/components/info-table-row';
+import Component from '@glimmer/component';
+import { tracked } from '@glimmer/tracking';
+import { action } from '@ember/object';
+// import layout from '../templates/components/info-table-row';
 
 /**
  * @module InfoTableRow
@@ -14,42 +14,35 @@ import layout from '../templates/components/info-table-row';
  * <InfoTableRow @value={{5}} @label="TTL" @helperText="Some description"/>
  * ```
  *
- * @param value=null {any} - The the data to be displayed - by default the content of the component will only show if there is a value. Also note that special handling is given to boolean values - they will render `Yes` for true and `No` for false.
  * @param label=null {string} - The display name for the value.
  * @param helperText=null {string} - Text to describe the value displayed beneath the label.
- * @param alwaysRender=false {Boolean} - Indicates if the component content should be always be rendered.  When false, the value of `value` will be used to determine if the component should render.
+ * @param value=null {any} - The the data to be displayed - by default the content of the component will only show if there is a value. Also note that special handling is given to boolean values - they will render `Yes` for true and `No` for false. Overridden by block if exists
+ * @param [alwaysRender=false] {Boolean} - Indicates if the component content should be always be rendered.  When false, the value of `value` will be used to determine if the component should render.
+ * @param [defaultShown] {String} - Text that renders as value if alwaysRender=true. Eg. "Vault default"
+ * @param [tooltipText] {String} - Text if a tooltip should display over the value.
+ * @param [isTooltipCopyable] {Boolean} - Allows tooltip click to copy
  * @param [type=array] {string} - The type of value being passed in.  This is used for when you want to trim an array.  For example, if you have an array value that can equal length 15+ this will trim to show 5 and count how many more are there
  * @param [isLink=true] {Boolean} - Passed through to InfoTableItemArray. Indicates if the item should contain a link-to component.  Only setup for arrays, but this could be changed if needed.
  * @param [modelType=null] {string} - Passed through to InfoTableItemArray. Tells what model you want data for the allOptions to be returned from.  Used in conjunction with the the isLink.
- * @param [queryParam] {String} - Passed through to InfoTableItemArray. If you want to specific a tab for the View All XX to display to.  Ex: role
- * @param [backend] {String} - Passed through to InfoTableItemArray. To specify secrets backend to point link to  Ex: transformation
+ * @param [queryParam] {String} - Passed through to InfoTableItemArray. If you want to specific a tab for the View All XX to display to.  Ex= role
+ * @param [backend] {String} - Passed through to InfoTableItemArray. To specify secrets backend to point link to  Ex= transformation
  * @param [viewAll] {String} - Passed through to InfoTableItemArray. Specify the word at the end of the link View all.
- * @param [tooltipText] {String} - Text if a tooltip should display over the value.
- * @param [isTooltipCopyable] {Boolean} - Allows tooltip click to copy
- * @param [defaultShown] {String} - Text that renders as value if alwaysRender=true. Eg. "Vault default"
  */
 
-export default Component.extend({
-  layout,
-  'data-test-component': 'info-table-row',
-  classNames: ['info-table-row'],
-  isVisible: or('alwaysRender', 'value'),
+export default class InfoTableRowComponent extends Component {
+  @tracked
+  hasLabelOverflow = false; // is calculated and set in didInsertElement
 
-  alwaysRender: false,
-  label: null,
-  helperText: null,
-  value: null,
-  tooltipText: '',
-  isTooltipCopyable: false,
-  defaultShown: '',
-  hasLabelOverflow: false, // is calculated and set in didInsertElement
+  get isVisible() {
+    return this.args.alwaysRender || !this.valueIsEmpty;
+  }
 
-  valueIsBoolean: computed('value', function () {
-    return typeOf(this.value) === 'boolean';
-  }),
+  get valueIsBoolean() {
+    return typeOf(this.args.value) === 'boolean';
+  }
 
-  valueIsEmpty: computed('value', function () {
-    let { value } = this;
+  get valueIsEmpty() {
+    let { value } = this.args;
     if (typeOf(value) === 'array' && value.length === 0) {
       return true;
     }
@@ -63,17 +56,16 @@ export default Component.extend({
       default:
         return false;
     }
-  }),
+  }
 
-  didInsertElement() {
-    this._super(...arguments);
-    const labelDiv = this.element.querySelector('div');
-    const labelText = this.element.querySelector('.is-label');
+  @action
+  calculateLabelOverflow(el) {
+    const labelDiv = el;
+    const labelText = el.querySelector('.is-label');
     if (labelDiv && labelText) {
       if (labelText.offsetWidth > labelDiv.offsetWidth) {
-        labelDiv.classList.add('label-overflow');
-        this.set('hasLabelOverflow', true);
+        this.hasLabelOverflow = true;
       }
     }
-  },
-});
+  }
+}