From 6397cd80a60560d99403bd8bc80ebd7c17cad2f4 Mon Sep 17 00:00:00 2001 From: Dharini Jeeva Date: Mon, 3 Jun 2024 13:38:42 -0700 Subject: [PATCH] Refactor key value comp (#2180) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: 💡 keyvalue comp --- .../generated/models/credential-library.js | 2 +- addons/api/addon/models/credential-library.js | 15 +- .../addon/serializers/credential-library.js | 44 ++-- .../transforms/account-value-map-array.js | 6 +- .../unit/serializers/application-test.js | 2 +- .../unit/serializers/auth-method-test.js | 16 +- .../serializers/credential-library-test.js | 16 +- .../account-value-map-array-test.js | 8 +- .../form/auth-method/ldap/index.hbs | 157 ++++--------- .../form/auth-method/oidc/index.hbs | 163 +++----------- .../vault-generic/index.hbs | 19 +- .../credential-library/vault-generic/index.js | 37 +++- .../vault-ssh-certificate/index.hbs | 18 +- .../form/field/list-wrapper/index.hbs | 3 - .../field/list-wrapper/key-value/index.hbs | 206 ++++++++++++++---- .../field/list-wrapper/key-value/index.js | 15 ++ .../field/list-wrapper/key-value/select.hbs | 19 ++ .../field/list-wrapper/key-value/text.hbs | 6 + .../form/worker/create-worker-led/index.hbs | 9 +- .../acceptance/auth-methods/create-test.js | 12 +- .../acceptance/auth-methods/update-test.js | 21 +- .../form/field/list-wrapper-test.js | 142 +++++++++--- 22 files changed, 531 insertions(+), 405 deletions(-) create mode 100644 ui/admin/app/components/form/field/list-wrapper/key-value/select.hbs create mode 100644 ui/admin/app/components/form/field/list-wrapper/key-value/text.hbs diff --git a/addons/api/addon/generated/models/credential-library.js b/addons/api/addon/generated/models/credential-library.js index 0fcb12adb..3d8ba617e 100644 --- a/addons/api/addon/generated/models/credential-library.js +++ b/addons/api/addon/generated/models/credential-library.js @@ -81,7 +81,7 @@ export default class GeneratedCredentialLibraryModel extends BaseModel { }) credential_type; - @attr('object', { + @attr('object-as-array', { for: 'vault-generic', description: 'It indicates the credential mapping overrides.', }) diff --git a/addons/api/addon/models/credential-library.js b/addons/api/addon/models/credential-library.js index fd96b52e7..1ca9a7728 100644 --- a/addons/api/addon/models/credential-library.js +++ b/addons/api/addon/models/credential-library.js @@ -13,15 +13,12 @@ export const options = { key_type: ['ed25519', 'ecdsa', 'rsa'], credential_types: ['ssh_private_key', 'username_password'], mapping_overrides: { - username_password: { - username_attribute: 'Username', - password_attribute: 'Password', - }, - ssh_private_key: { - private_key_attribute: 'Private Key', - private_key_passphrase_attribute: 'Private Key Passphrase', - username_attribute: 'Username', - }, + username_password: ['username_attribute', 'password_attribute'], + ssh_private_key: [ + 'private_key_attribute', + 'private_key_passphrase_attribute', + 'username_attribute', + ], }, }; diff --git a/addons/api/addon/serializers/credential-library.js b/addons/api/addon/serializers/credential-library.js index 52adb0d50..8a466ae97 100644 --- a/addons/api/addon/serializers/credential-library.js +++ b/addons/api/addon/serializers/credential-library.js @@ -32,32 +32,9 @@ export default class CredentialLibrarySerializer extends ApplicationSerializer { return super.serialize(...arguments); } } - serializeAttribute(snapshot, json) { - const value = super.serializeAttribute(...arguments); - const { credential_type, type, credential_mapping_overrides, isNew } = - snapshot.record; - if ( - type === TYPE_CREDENTIAL_LIBRARY_VAULT_GENERIC && - credential_mapping_overrides && - credential_type && - !isNew - ) { - // API expects to send null to fields if it is undefined or deleted - json.credential_mapping_overrides = Object.keys( - options.mapping_overrides[credential_type], - ).reduce((obj, key) => { - if (credential_mapping_overrides[key]) { - obj[key] = credential_mapping_overrides[key]; - } else { - obj[key] = null; - } - return obj; - }, {}); - return value; - } - } - serializeVaultGeneric() { + serializeVaultGeneric(snapshot) { + const { isNew } = snapshot; const serialized = super.serialize(...arguments); if (serialized.attributes) { @@ -66,7 +43,22 @@ export default class CredentialLibrarySerializer extends ApplicationSerializer { if (!serialized.attributes?.http_method?.match(/post/i)) { serialized.attributes.http_request_body = null; } + const { credential_type, credential_mapping_overrides } = serialized; + + // API expects to send null to fields if it is undefined or deleted + if (credential_mapping_overrides && !isNew) { + serialized.credential_mapping_overrides = options.mapping_overrides[ + credential_type + ].reduce((obj, key) => { + if (credential_mapping_overrides[key]) { + obj[key] = credential_mapping_overrides[key]; + } else { + obj[key] = null; + } + return obj; + }, {}); + } + return serialized; } - return serialized; } } diff --git a/addons/api/addon/transforms/account-value-map-array.js b/addons/api/addon/transforms/account-value-map-array.js index 41598d6de..0c9f820c1 100644 --- a/addons/api/addon/transforms/account-value-map-array.js +++ b/addons/api/addon/transforms/account-value-map-array.js @@ -9,13 +9,13 @@ export default class AccountValueMapArrayTransform extends Transform { deserialize(serialized) { const accountValues = serialized || []; return accountValues.map((accountValueString) => ({ - from: accountValueString.split('=')[0], - to: accountValueString.split('=')[1], + key: accountValueString.split('=')[0], + value: accountValueString.split('=')[1], })); } serialize(deserialized) { const accountValues = deserialized || []; - return accountValues.map(({ from, to }) => `${from}=${to}`); + return accountValues.map(({ key, value }) => `${key}=${value}`); } } diff --git a/addons/api/tests/unit/serializers/application-test.js b/addons/api/tests/unit/serializers/application-test.js index 65b121687..c2eb20e21 100644 --- a/addons/api/tests/unit/serializers/application-test.js +++ b/addons/api/tests/unit/serializers/application-test.js @@ -139,7 +139,7 @@ module('Unit | Serializer | application', function (hooks) { type: TYPE_AUTH_METHOD_OIDC, //has both string and array `for` option name: 'OIDC Auth Method', state: 'foo', - account_claim_maps: [{ from: 'foo', to: 'bar' }], + account_claim_maps: [{ key: 'foo', value: 'bar' }], claims_scopes: [{ value: 'profile' }, { value: 'email' }], signing_algorithms: [{ value: 'RS256' }, { value: 'RS384' }], allowed_audiences: [ diff --git a/addons/api/tests/unit/serializers/auth-method-test.js b/addons/api/tests/unit/serializers/auth-method-test.js index 840d44e29..99578e7a3 100644 --- a/addons/api/tests/unit/serializers/auth-method-test.js +++ b/addons/api/tests/unit/serializers/auth-method-test.js @@ -45,7 +45,7 @@ module('Unit | Serializer | auth method', function (hooks) { type: TYPE_AUTH_METHOD_OIDC, name: 'OIDC Auth Method', state: 'foo', - account_claim_maps: [{ from: 'foo', to: 'bar' }], + account_claim_maps: [{ key: 'foo', value: 'bar' }], claims_scopes: [{ value: 'profile' }, { value: 'email' }], signing_algorithms: [{ value: 'RS256' }, { value: 'RS384' }], allowed_audiences: [ @@ -96,7 +96,7 @@ module('Unit | Serializer | auth method', function (hooks) { type: TYPE_AUTH_METHOD_LDAP, name: 'LDAP Auth Method', state: 'baz', - account_attribute_maps: [{ from: 'foo', to: 'bar' }], + account_attribute_maps: [{ key: 'foo', value: 'bar' }], urls: [{ value: 'ldap://ldap.example.com' }], use_token_groups: false, start_tls: true, @@ -160,7 +160,7 @@ module('Unit | Serializer | auth method', function (hooks) { type: TYPE_AUTH_METHOD_OIDC, name: 'OIDC Auth Method', state: 'foo', - account_claim_maps: [{ from: 'foo', to: 'bar' }], + account_claim_maps: [{ key: 'foo', value: 'bar' }], claims_scopes: [{ value: 'profile' }, { value: 'email' }], signing_algorithms: [{ value: 'RS256' }, { value: 'RS384' }], allowed_audiences: [ @@ -210,7 +210,7 @@ module('Unit | Serializer | auth method', function (hooks) { type: TYPE_AUTH_METHOD_LDAP, name: 'LDAP Auth Method', state: 'baz', - account_attribute_maps: [{ from: 'foo', to: 'bar' }], + account_attribute_maps: [{ key: 'foo', value: 'bar' }], urls: [{ value: 'ldap://ldap.example.com' }], use_token_groups: false, start_tls: true, @@ -410,8 +410,8 @@ module('Unit | Serializer | auth method', function (hooks) { prompts, } = record; assert.deepEqual(account_claim_maps, [ - { from: 'from', to: 'to' }, - { from: 'foo', to: 'bar' }, + { key: 'from', value: 'to' }, + { key: 'foo', value: 'bar' }, ]); assert.deepEqual(claims_scopes, [{ value: 'profile' }, { value: 'email' }]); assert.deepEqual(allowed_audiences, [ @@ -481,8 +481,8 @@ module('Unit | Serializer | auth method', function (hooks) { certificates: recordCertificates, } = record; assert.deepEqual(account_attribute_maps, [ - { from: 'from', to: 'to' }, - { from: 'foo', to: 'bar' }, + { key: 'from', value: 'to' }, + { key: 'foo', value: 'bar' }, ]); assert.deepEqual(urls, [{ value: ldapUrl }]); assert.deepEqual(recordCertificates, [ diff --git a/addons/api/tests/unit/serializers/credential-library-test.js b/addons/api/tests/unit/serializers/credential-library-test.js index 49a943a1d..61e936535 100644 --- a/addons/api/tests/unit/serializers/credential-library-test.js +++ b/addons/api/tests/unit/serializers/credential-library-test.js @@ -24,7 +24,9 @@ module('Unit | Serializer | credential library', function (hooks) { http_method: 'GET', version: 1, credential_type: 'ssh_private_key', - credential_mapping_overrides: { username_attribute: 'user' }, + credential_mapping_overrides: [ + { key: 'username_attribute', value: 'user' }, + ], }); const snapshot = record._createSnapshot(); const serializedRecord = serializer.serialize(snapshot); @@ -61,7 +63,9 @@ module('Unit | Serializer | credential library', function (hooks) { http_method: 'GET', version: 1, credential_type: 'ssh_private_key', - credential_mapping_overrides: {}, + credential_mapping_overrides: [ + { key: 'private_key_attribute', value: 'test' }, + ], }, }, }); @@ -75,7 +79,7 @@ module('Unit | Serializer | credential library', function (hooks) { description: 'Description', credential_type: 'ssh_private_key', credential_mapping_overrides: { - private_key_attribute: null, + private_key_attribute: 'test', private_key_passphrase_attribute: null, username_attribute: null, }, @@ -116,7 +120,7 @@ module('Unit | Serializer | credential library', function (hooks) { name: null, type: TYPE_CREDENTIAL_LIBRARY_VAULT_GENERIC, credential_type: null, - credential_mapping_overrides: {}, + credential_mapping_overrides: null, }); assert.deepEqual(serializedVaultSSHCertificateRecord, { attributes: { @@ -157,7 +161,7 @@ module('Unit | Serializer | credential library', function (hooks) { name: null, type: TYPE_CREDENTIAL_LIBRARY_VAULT_GENERIC, credential_type: null, - credential_mapping_overrides: {}, + credential_mapping_overrides: null, }, 'http_request_body attribute is not expected', ); @@ -177,7 +181,7 @@ module('Unit | Serializer | credential library', function (hooks) { name: null, type: TYPE_CREDENTIAL_LIBRARY_VAULT_GENERIC, credential_type: null, - credential_mapping_overrides: {}, + credential_mapping_overrides: null, }, 'http_request_body attribute is expected', ); diff --git a/addons/api/tests/unit/transforms/account-value-map-array-test.js b/addons/api/tests/unit/transforms/account-value-map-array-test.js index 655f4d8e4..b776b3a99 100644 --- a/addons/api/tests/unit/transforms/account-value-map-array-test.js +++ b/addons/api/tests/unit/transforms/account-value-map-array-test.js @@ -13,16 +13,16 @@ module('Unit | Transform | account value map array', function (hooks) { const transform = this.owner.lookup('transform:account-value-map-array'); const deserialized = transform.deserialize(['oid=sub', 'full_name=name']); assert.deepEqual(deserialized, [ - { from: 'oid', to: 'sub' }, - { from: 'full_name', to: 'name' }, + { key: 'oid', value: 'sub' }, + { key: 'full_name', value: 'name' }, ]); }); test('it serializes an array of account claims', function (assert) { const transform = this.owner.lookup('transform:account-value-map-array'); const serialized = transform.serialize([ - { from: 'oid', to: 'sub' }, - { from: 'full_name', to: 'name' }, + { key: 'oid', value: 'sub' }, + { key: 'full_name', value: 'name' }, ]); assert.deepEqual(serialized, ['oid=sub', 'full_name=name']); }); diff --git a/ui/admin/app/components/form/auth-method/ldap/index.hbs b/ui/admin/app/components/form/auth-method/ldap/index.hbs index b58b53b0a..cc3d5096d 100644 --- a/ui/admin/app/components/form/auth-method/ldap/index.hbs +++ b/ui/admin/app/components/form/auth-method/ldap/index.hbs @@ -382,124 +382,45 @@ }} - - {{t - 'resources.auth-method.form.account_attribute_maps.label' - }} - {{t - 'resources.auth-method.form.account_attribute_maps.help' - }} - {{#let (unique-id) (unique-id) as |fromHeaderID toHeaderID|}} - - - <:head as |H|> - - {{t - 'resources.auth-method.titles.from-attr' - }} - {{t - 'resources.auth-method.titles.to-attr' - }} - {{t 'titles.actions'}} - - - <:body as |B|> - {{#each @model.account_attribute_maps as |attrMap i|}} - - - - - - - - {{#each this.toAttributes as |toAttrOption|}} - - {{/each}} - - - - - - - - {{/each}} - - - - - - - - {{#each this.toAttributes as |toAttrOption|}} - - {{/each}} - - - - - - - - - - - {{/let}} - {{#if @model.errors.account_attribute_maps}} - - {{#each @model.errors.account_attribute_maps as |error|}} - {{error.message}} - {{/each}} - - {{/if}} - + + <:fieldset as |F|> + + {{t 'resources.auth-method.form.account_attribute_maps.label'}} + + + {{t 'resources.auth-method.form.account_attribute_maps.help'}} + + + {{#if @model.errors.account_attribute_maps}} + + {{#each @model.errors.account_attribute_maps as |error|}} + {{error.message}} + {{/each}} + + {{/if}} + + <:field as |F|> + + <:key as |K|> + + + <:value as |V|> + + + + +

{{t diff --git a/ui/admin/app/components/form/auth-method/oidc/index.hbs b/ui/admin/app/components/form/auth-method/oidc/index.hbs index d59c79b3b..8e65760d0 100644 --- a/ui/admin/app/components/form/auth-method/oidc/index.hbs +++ b/ui/admin/app/components/form/auth-method/oidc/index.hbs @@ -247,138 +247,45 @@ {{! Account Claim Maps }} - - <:title>{{t 'form.account_claim_maps.label'}} - <:description>{{t 'form.account_claim_maps.help'}} - <:body> + + <:fieldset as |F|> + + {{t 'form.account_claim_maps.label'}} + + + {{t 'form.account_claim_maps.help'}} + + {{#if @model.errors.account_claim_maps}} - + {{#each @model.errors.account_claim_maps as |error|}} - {{error.message}} + {{error.message}} {{/each}} - + {{/if}} - - - - {{t 'form.from_claim.label'}} - {{t 'form.to_claim.label'}} - {{t 'titles.actions'}} - - - - {{#each @model.account_claim_maps as |claimMap i|}} - - - - {{t 'form.from_claim.label'}} - - - - - - - {{t 'form.to_claim.label'}} - - {{#each this.toClaims as |toClaimOption|}} - - {{/each}} - - - - - - - {{t 'actions.remove'}} - - - - {{/each}} - - - - - - - - - {{t - 'resources.auth-method.titles.new-to-claim' - }} - - {{#each this.toClaims as |toClaimOption|}} - - {{/each}} - - - - - - - {{t 'actions.add'}} - - - - - - - + + <:field as |F|> + + <:key as |K|> + + + <:value as |V|> + + + + + {{! Certificates }} <:field as |F|> - + @width='100%' + @showNewRow={{this.showNewRow}} + > + <:newKey as |N|> + + + <:key as |K|> + + + <:value as |V|> + + + {{/if}} diff --git a/ui/admin/app/components/form/credential-library/vault-generic/index.js b/ui/admin/app/components/form/credential-library/vault-generic/index.js index 0fddbfd8f..caf5b682a 100644 --- a/ui/admin/app/components/form/credential-library/vault-generic/index.js +++ b/ui/admin/app/components/form/credential-library/vault-generic/index.js @@ -17,6 +17,7 @@ export default class FormCredentialLibraryVaultGenericComponent extends Componen httpMethodOptions = options.http_method; credentialTypes = options.credential_types; + /** * * @type {Array.} @@ -35,12 +36,46 @@ export default class FormCredentialLibraryVaultGenericComponent extends Componen return options.mapping_overrides[this.args.model.credential_type]; } + get allowedEntries() { + return this.mappingOverrides.length; + } + /** * Clear the previously selected key value pair when toggling between credential types on a new form */ @action selectCredentialType({ target: { value } }) { - this.args.model.credential_mapping_overrides = {}; + this.args.model.credential_mapping_overrides = []; this.args.model.credential_type = value; } + + /** + * Prevents users from selecting duplicate keys from the select list if the arg is set to true + * @type {array} + */ + + get selectOptions() { + const previouslySelectedKeys = + this.args.model.credential_mapping_overrides || []; + if (previouslySelectedKeys.length) { + return this.mappingOverrides.filter((key) => + previouslySelectedKeys.every((obj) => obj.key !== key), + ); + } else { + return this.mappingOverrides; + } + } + + /** + * Determines if we need to show an empty row to the users to enter more key/value pairs based on removeDuplicates arg, + * by default it is true + * @type {object} + */ + @action + showNewRow() { + return ( + this.args.model.credential_mapping_overrides?.length !== + this.allowedEntries + ); + } } diff --git a/ui/admin/app/components/form/credential-library/vault-ssh-certificate/index.hbs b/ui/admin/app/components/form/credential-library/vault-ssh-certificate/index.hbs index be7e62919..28b558390 100644 --- a/ui/admin/app/components/form/credential-library/vault-ssh-certificate/index.hbs +++ b/ui/admin/app/components/form/credential-library/vault-ssh-certificate/index.hbs @@ -266,7 +266,14 @@ @options={{@model.critical_options}} @model={{@model}} @disabled={{form.disabled}} - /> + > + <:key as |K|> + + + <:value as |K|> + + + @@ -297,7 +304,14 @@ @name='extensions' @options={{@model.extensions}} @model={{@model}} - /> + > + <:key as |K|> + + + <:value as |V|> + + + diff --git a/ui/admin/app/components/form/field/list-wrapper/index.hbs b/ui/admin/app/components/form/field/list-wrapper/index.hbs index 5942736ea..e96417ab6 100644 --- a/ui/admin/app/components/form/field/list-wrapper/index.hbs +++ b/ui/admin/app/components/form/field/list-wrapper/index.hbs @@ -25,9 +25,6 @@ 'form/field/list-wrapper/textarea' disabled=@disabled ) Select=(component 'form/field/list-wrapper/select' disabled=@disabled) - SelectText=(component - 'form/field/list-wrapper/select-text' disabled=@disabled - ) ) to='field' }} diff --git a/ui/admin/app/components/form/field/list-wrapper/key-value/index.hbs b/ui/admin/app/components/form/field/list-wrapper/key-value/index.hbs index e454fce69..2a53da3f8 100644 --- a/ui/admin/app/components/form/field/list-wrapper/key-value/index.hbs +++ b/ui/admin/app/components/form/field/list-wrapper/key-value/index.hbs @@ -2,16 +2,26 @@ Copyright (c) HashiCorp, Inc. SPDX-License-Identifier: BUSL-1.1 }} -{{#let (unique-id) (unique-id) as |keyHeaderID valueHeaderID|}} +{{#let (unique-id) (unique-id) as |keyHeaderID valueHeaderID|}} <:head as |H|> - {{t 'form.key.label'}} - {{t 'form.value.label'}} + + {{#if @keyLabel}} + {{@keyLabel}} + {{else}} + {{t 'form.key.label'}} + {{/if}} + + {{#if @valueLabel}} + {{@valueLabel}} + {{else}} + {{t 'form.value.label'}} + {{/if}} + {{t 'titles.actions'}} - @@ -19,23 +29,49 @@ {{#each this.options as |option index|}} - + {{yield + (hash + text=(component + 'form/field/list-wrapper/key-value/text' + disabled=@disabled + value=option.key + ariaLabel=keyHeaderID + setContext=(set-from-event option 'key') + ) + select=(component + 'form/field/list-wrapper/key-value/select' + disabled=@disabled + value=option.key + ariaLabel=keyHeaderID + setContext=(set-from-event option 'key') + width=@width + ) + ) + to='key' + }} - + {{yield + (hash + text=(component + 'form/field/list-wrapper/key-value/text' + disabled=@disabled + value=option.value + ariaLabel=valueHeaderID + setContext=(set-from-event option 'value') + ) + select=(component + 'form/field/list-wrapper/key-value/select' + disabled=@disabled + value=option.value + ariaLabel=valueHeaderID + width=@width + setContext=(set-from-event option 'value') + ) + ) + to='value' + }} @@ -53,40 +89,114 @@ {{/each}} + {{#if this.showNewRow}} + + + {{#if (has-block 'newKey')}} + {{yield + (hash + text=(component + 'form/field/list-wrapper/key-value/text' + disabled=@disabled + value=this.newOptionKey + ariaLabel=keyHeaderID + setContext=(set-from-event this 'newOptionKey') + ) + select=(component + 'form/field/list-wrapper/key-value/select' + disabled=@disabled + value=this.newOptionKey + ariaLabel=keyHeaderID + width=@width + setContext=(set-from-event this 'newOptionKey') + ) + ) + to='newKey' + }} - - - - + {{else}} + {{yield + (hash + text=(component + 'form/field/list-wrapper/key-value/text' + disabled=@disabled + value=this.newOptionKey + ariaLabel=keyHeaderID + setContext=(set-from-event this 'newOptionKey') + ) + select=(component + 'form/field/list-wrapper/key-value/select' + disabled=@disabled + value=this.newOptionKey + ariaLabel=keyHeaderID + width=@width + setContext=(set-from-event this 'newOptionKey') + ) + ) + to='key' + }} + {{/if}} + - - - + + {{#if (has-block 'newValue')}} + {{yield + (hash + text=(component + 'form/field/list-wrapper/key-value/text' + disabled=@disabled + value=this.newOptionValue + ariaLabel=valueHeaderID + setContext=(set-from-event this 'newOptionValue') + ) + select=(component + 'form/field/list-wrapper/key-value/select' + disabled=@disabled + value=this.newOptionValue + ariaLabel=valueHeaderID + width=@width + setContext=(set-from-event this 'newOptionValue') + ) + ) + to='newValue' + }} - - - - + {{else}} + {{yield + (hash + text=(component + 'form/field/list-wrapper/key-value/text' + disabled=@disabled + value=this.newOptionValue + ariaLabel=valueHeaderID + setContext=(set-from-event this 'newOptionValue') + ) + select=(component + 'form/field/list-wrapper/key-value/select' + disabled=@disabled + value=this.newOptionValue + ariaLabel=valueHeaderID + width=@width + setContext=(set-from-event this 'newOptionValue') + ) + ) + to='value' + }} + {{/if}} + + + + + + + {{/if}} - {{/let}} \ No newline at end of file diff --git a/ui/admin/app/components/form/field/list-wrapper/key-value/index.js b/ui/admin/app/components/form/field/list-wrapper/key-value/index.js index 74502b18c..a0451a91d 100644 --- a/ui/admin/app/components/form/field/list-wrapper/key-value/index.js +++ b/ui/admin/app/components/form/field/list-wrapper/key-value/index.js @@ -29,6 +29,20 @@ export default class MappingListComponent extends Component { get options() { return this.args?.options || this.args?.model?.[this.args.name]; } + + /** + * Determines if we need to show an empty row to the users to enter more key/value pairs based on removeDuplicates arg, + * by default it is true + * @type {object} + */ + get showNewRow() { + if (this.args.showNewRow) { + return this.args.showNewRow(); + } else { + return true; + } + } + // =actions /** @@ -49,6 +63,7 @@ export default class MappingListComponent extends Component { } else { const field = this.args.name; const existingArray = this.args.model[field] ?? []; + const newArray = [ ...existingArray, { key: this.newOptionKey, value: this.newOptionValue }, diff --git a/ui/admin/app/components/form/field/list-wrapper/key-value/select.hbs b/ui/admin/app/components/form/field/list-wrapper/key-value/select.hbs new file mode 100644 index 000000000..57327a882 --- /dev/null +++ b/ui/admin/app/components/form/field/list-wrapper/key-value/select.hbs @@ -0,0 +1,19 @@ + + + + {{#each @selectOptions as |selectOption|}} + + {{/each}} + + \ No newline at end of file diff --git a/ui/admin/app/components/form/field/list-wrapper/key-value/text.hbs b/ui/admin/app/components/form/field/list-wrapper/key-value/text.hbs new file mode 100644 index 000000000..ee0f59323 --- /dev/null +++ b/ui/admin/app/components/form/field/list-wrapper/key-value/text.hbs @@ -0,0 +1,6 @@ + \ No newline at end of file diff --git a/ui/admin/app/components/form/worker/create-worker-led/index.hbs b/ui/admin/app/components/form/worker/create-worker-led/index.hbs index 3b5151ece..171b64b37 100644 --- a/ui/admin/app/components/form/worker/create-worker-led/index.hbs +++ b/ui/admin/app/components/form/worker/create-worker-led/index.hbs @@ -95,7 +95,14 @@ @disabled={{form.disabled}} @addOption={{this.addWorkerTag}} @removeOptionByIndex={{this.removeWorkerTagByIndex}} - /> + > + <:key as |K|> + + + <:value as |V|> + + + diff --git a/ui/admin/tests/acceptance/auth-methods/create-test.js b/ui/admin/tests/acceptance/auth-methods/create-test.js index 251385b29..9a4ed2cc1 100644 --- a/ui/admin/tests/acceptance/auth-methods/create-test.js +++ b/ui/admin/tests/acceptance/auth-methods/create-test.js @@ -128,10 +128,16 @@ module('Acceptance | auth-methods | create', function (hooks) { await fillIn(CLAIMS_SCOPES_INPUT_SELECTOR, 'claims_scopes'); await click(CLAIMS_SCOPES_BTN_SELECTOR, 'claims_scopes'); - await fillIn('[name="from_claim"]', 'from_claim'); - await select('form fieldset:nth-of-type(4) select', 'email'); + await fillIn( + '[name="account_claim_maps"] tbody td:nth-of-type(1) input', + 'from_claim', + ); + await select( + '[name="account_claim_maps"] tbody td:nth-of-type(2) select', + 'email', + ); - await click('form fieldset:nth-of-type(4) [title="Add"]'); + await click('[name="account_claim_maps"] button'); await fillIn(IDP_CERTS_INPUT_SELECTOR, 'certificates'); await click(IDP_CERTS_BTN_SELECTOR); diff --git a/ui/admin/tests/acceptance/auth-methods/update-test.js b/ui/admin/tests/acceptance/auth-methods/update-test.js index f90d5066b..dfc505741 100644 --- a/ui/admin/tests/acceptance/auth-methods/update-test.js +++ b/ui/admin/tests/acceptance/auth-methods/update-test.js @@ -164,14 +164,23 @@ module('Acceptance | auth-methods | update', function (hooks) { // Remove all claim maps const claimMaps = await Promise.all( - findAll('form fieldset:nth-of-type(4) [title="Remove"]'), + findAll('[name="account_claim_maps"] [data-test-remove-button]'), ); for (const element of claimMaps) { await click(element); } - await fillIn('[name="from_claim"]', 'from_claim'); - await select('form fieldset:nth-of-type(4) select', 'email'); - await click('form fieldset:nth-of-type(4) [title="Add"]'); + + await fillIn( + '[name="account_claim_maps"] tbody td:nth-of-type(1) input', + 'from_claim', + ); + + await select( + '[name="account_claim_maps"] tbody td:nth-of-type(2) select', + 'email', + ); + + await click('[name="account_claim_maps"] button'); // Remove all certificates const certificatesList = findAll(IDP_CERTS_REMOVE_BTN_SELECTOR); @@ -179,15 +188,15 @@ module('Acceptance | auth-methods | update', function (hooks) { for (const element of certificatesList) { await click(element); } - await fillIn(IDP_CERTS_INPUT_SELECTOR, 'certificates'); await click(IDP_CERTS_BTN_SELECTOR); await fillIn('[name="max_age"]', '5'); await fillIn('[name="api_url_prefix"]', 'api_url_prefix'); await click(TOGGLE_SELECTOR); - await click('form [type="submit"]:not(:disabled)'); + await click('.rose-form-actions [type="submit"]'); const authMethod = this.server.schema.authMethods.findBy({ name }); + assert.strictEqual(authMethod.name, name); assert.strictEqual(authMethod.description, 'description'); assert.strictEqual(authMethod.attributes.issuer, 'issuer'); diff --git a/ui/admin/tests/integration/components/form/field/list-wrapper-test.js b/ui/admin/tests/integration/components/form/field/list-wrapper-test.js index 4131d9a35..787eb963e 100644 --- a/ui/admin/tests/integration/components/form/field/list-wrapper-test.js +++ b/ui/admin/tests/integration/components/form/field/list-wrapper-test.js @@ -5,7 +5,7 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { click, render, select } from '@ember/test-helpers'; +import { click, findAll, render, select } from '@ember/test-helpers'; import { hbs } from 'ember-cli-htmlbars'; import { setupIntl } from 'ember-intl/test-support'; @@ -55,7 +55,14 @@ module('Integration | Component | list-wrapper', function (hooks) { await render(hbs` <:field as |F|> - + + <:key as |K|> + + + <:value as |V|> + + + `); @@ -165,42 +172,56 @@ module('Integration | Component | list-wrapper', function (hooks) { }); test('it renders multiple options with select and text input as key value pair', async function (assert) { - this.options = { - username_attribute: 'user', - password_attribute: 'pass', - }; + this.options = [ + { + key: 'username_attribute', + value: 'user', + }, + { + key: 'password_attribute', + value: 'pass', + }, + ]; + + this.selectOptions = ['username_attribute', 'password_attribute']; - this.selectOptions = { - username_attribute: 'User Key', - password_attribute: 'Pass Key', - }; await render(hbs` <:field as |F|> - + + <:key as |K|> + + + <:value as |V|> + + + `); - assert.dom('tbody tr').exists({ count: 2 }); - assert - .dom('.list-wrapper-field tbody tr:last-child input') - .hasValue('pass'); - assert - .dom('.list-wrapper-field tbody tr:last-child select') - .hasValue('password_attribute'); + assert.dom('tbody tr').exists({ count: 1 }); }); - test('it does not render new rows when the select option limit is reached by passing in @removeDuplicates', async function (assert) { - this.options = { username_attribute: 'user', password_attribute: 'pass' }; - this.selectOptions = { - username_attribute: 'User Key', - password_attribute: 'Pass Key', - }; + test('it does not render new rows when the select option limit is reached when showNewRow is passed', async function (assert) { + this.options = [ + { key: 'username_attribute', value: 'user' }, + { key: 'password_attribute', value: 'pass' }, + ]; + + this.selectOptions = ['username_attribute', 'password_attribute']; + this.showNewRow = () => false; await render(hbs` <:field as |F|> - + + <:key as |K|> + + + <:value as |V|> + + + `); @@ -208,20 +229,77 @@ module('Integration | Component | list-wrapper', function (hooks) { assert.dom('tbody tr').exists({ count: 2 }); }); - test('it does render unlimited new rows when @removeDuplicates is not passed', async function (assert) { - this.options = { username_attribute: 'user', password_attribute: 'pass' }; - this.selectOptions = { - username_attribute: 'User Key', - password_attribute: 'Pass Key', - }; + test('it does render unlimited new rows when @showNewRow is not passed', async function (assert) { + this.options = [ + { + key: 'username_attribute', + value: 'user', + }, + { + key: 'password_attribute', + value: 'pass', + }, + ]; + this.selectOptions = ['username_attribute', 'password_attribute']; await render(hbs` <:field as |F|> - + + <:key as |K|> + + + <:value as |V|> + + + `); assert.dom('tbody tr').exists({ count: 3 }); }); + + test('it renders `newKey` and `newValue` named blocks when passed instead of key and value blocks', async function (assert) { + this.options = ['option1', 'option2', 'option3']; + this.selectOptionsNewValue = [ + 'username_attribute_new_val', + 'password_attribute_new_val', + ]; + + this.selectOptionsNewKey = [ + 'username_attribute_new_key', + 'password_attribute_new_key', + ]; + await render(hbs` + + <:field as |F|> + + <:key as |K|> + + + <:newKey as |N|> + + + <:newValue as |S|> + + + <:value as |V|> + + + + + + `); + + assert.strictEqual( + findAll('.list-wrapper-field tbody td:nth-of-type(1) select option') + .length, + 3, + ); + assert.strictEqual( + findAll('.list-wrapper-field tbody td:nth-of-type(2) select option') + .length, + 3, + ); + }); });