From 0e580e3c9f29f6039b33f319595667caac201b25 Mon Sep 17 00:00:00 2001 From: Dharini Date: Mon, 1 Jun 2026 10:59:51 -0700 Subject: [PATCH] chore: migrate listwrapper Target and worker fields to keyvalue component (#3284) --- .../form/auth-method/oidc/index.hbs | 1 - .../components/form/field/key-value/index.hbs | 4 +- .../components/form/field/key-value/index.js | 22 ++- .../form/storage-bucket/aws/index.hbs | 8 +- .../components/form/target/details/index.hbs | 47 ++--- .../form/worker/create-tags/index.hbs | 50 +++-- .../form/worker/create-tags/index.js | 24 ++- .../form/worker/create-worker-led/index.hbs | 60 +++--- .../form/worker/create-worker-led/index.js | 25 ++- ui/admin/app/components/form/worker/tag.js | 2 +- .../controllers/scopes/scope/targets/index.js | 5 + .../scope/workers/worker/create-tags.js | 7 +- .../app/routes/scopes/scope/targets/new.js | 1 + .../scope/workers/worker/create-tags.js | 7 +- .../tests/acceptance/targets/create-test.js | 5 +- .../tests/acceptance/targets/selectors.js | 5 +- .../acceptance/workers/create-tags-test.js | 5 +- .../tests/acceptance/workers/selectors.js | 7 +- .../tests/acceptance/workers/tags-test.js | 21 +- .../components/form/field/key-value-test.js | 181 +++++++++++++----- .../form/worker/create-tags/index-test.js | 5 +- .../form/worker/create-worker-led-test.js | 37 ++-- .../scope/workers/worker/create-tags-test.js | 4 +- 23 files changed, 349 insertions(+), 184 deletions(-) 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 8bb777a49..ee5f542c5 100644 --- a/ui/admin/app/components/form/auth-method/oidc/index.hbs +++ b/ui/admin/app/components/form/auth-method/oidc/index.hbs @@ -98,7 +98,6 @@ name='client_id' @value={{@model.client_id}} @isRequired={{true}} - @isInvalid={{@model.errors.client_id}} disabled={{form.disabled}} {{on 'input' (set-from-event @model 'client_id')}} as |F| diff --git a/ui/admin/app/components/form/field/key-value/index.hbs b/ui/admin/app/components/form/field/key-value/index.hbs index 8a926a4ba..53cc0a6c6 100644 --- a/ui/admin/app/components/form/field/key-value/index.hbs +++ b/ui/admin/app/components/form/field/key-value/index.hbs @@ -35,9 +35,9 @@ @onClick={{this.handleAdd}} /> - {{#if (get @model.errors @name)}} + {{#if this.errors}} - {{#each (get @model.errors @name) as |error|}} + {{#each this.errors as |error|}} {{error.message}} {{/each}} diff --git a/ui/admin/app/components/form/field/key-value/index.js b/ui/admin/app/components/form/field/key-value/index.js index cdacf67fd..a3685b6c6 100644 --- a/ui/admin/app/components/form/field/key-value/index.js +++ b/ui/admin/app/components/form/field/key-value/index.js @@ -21,13 +21,33 @@ export default class FormFieldKeyValueComponent extends Component { // For a single row, only show if it has any non-empty values if (rows.length === 1) { - return Object.values(rows[0]).some((value) => value); + const row = rows[0]; + for (const key in row) { + if (row[key]) { + return true; + } + } } // Return false if there are no rows return false; } + get errors() { + // If errors are passed directly as an argument, use those + if (this.args.errors) { + return this.args.errors; + } + + const { model, name } = this.args; + + if (model && name && model.errors) { + return model.errors[name]; + } + + return null; + } + /** * Default handler for adding a row. * Uses @model, @name, and @properties if provided, otherwise calls @onAdd. diff --git a/ui/admin/app/components/form/storage-bucket/aws/index.hbs b/ui/admin/app/components/form/storage-bucket/aws/index.hbs index 71ec0d282..f4f4b67d3 100644 --- a/ui/admin/app/components/form/storage-bucket/aws/index.hbs +++ b/ui/admin/app/components/form/storage-bucket/aws/index.hbs @@ -434,7 +434,6 @@ data-test-key-input @value={{R.rowData.key}} {{on 'input' (fn R.updateRow 'key')}} - disabled={{form.disabled}} /> @@ -442,15 +441,10 @@ data-test-value-input @value={{R.rowData.value}} {{on 'input' (fn R.updateRow 'value')}} - disabled={{form.disabled}} /> {{#if R.canDelete}} - + {{/if}} diff --git a/ui/admin/app/components/form/target/details/index.hbs b/ui/admin/app/components/form/target/details/index.hbs index 95583d044..049ad55e7 100644 --- a/ui/admin/app/components/form/target/details/index.hbs +++ b/ui/admin/app/components/form/target/details/index.hbs @@ -244,37 +244,30 @@ {{#if (and @model.isNew (can 'create model' @globalScope collection='aliases')) }} - - <:fieldset as |F|> - - {{t 'resources.alias.form.alias.label'}} - - - {{t 'resources.alias.form.alias.help'}} - + <:row as |R|> + + + + {{#if R.canDelete}} + - {{#if @model.errors.with_aliases}} - - {{#each @model.errors.with_aliases as |error|}} - {{error.message}} - {{/each}} - {{/if}} - - - <:field as |F|> - - - - + + {{/if}} {{#if (can 'save model' @model)}} - - <:field as |F|> - - <:key as |K|> - - - <:value as |V|> - - - - - + + <:row as |R|> + + {{t 'form.key.label'}} + + + + + {{t 'form.value.label'}} + + + {{#if R.canDelete}} + + {{/if}} + + tag.key?.trim()); + if (tagsToProcess.length === 0) { this.router.transitionTo('scopes.scope.workers.worker.tags'); return; } const apiTags = structuredClone(this.args.model.api_tags ?? {}); - this.args.apiTags.forEach((tag) => { + tagsToProcess.forEach((tag) => { let key = tag.key; let values = tag.value.split(',').map((value) => value.trim()); 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 1494b555e..4c466d192 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 @@ -78,35 +78,39 @@ {{/if}} - - <:fieldset as |F|> - {{t 'resources.worker.form.worker_tags.label'}} - - {{t 'resources.worker.form.worker_tags.help'}} - - - <:field as |F|> - - <:key as |K|> - - - <:value as |V|> - - - - - + + <:row as |R|> + + + + + + + + {{#if R.canDelete}} + + {{/if}} + + {{#if (feature-flag 'ssh-session-recording')}} diff --git a/ui/admin/app/components/form/worker/create-worker-led/index.js b/ui/admin/app/components/form/worker/create-worker-led/index.js index 333b600da..2b3146c1a 100644 --- a/ui/admin/app/components/form/worker/create-worker-led/index.js +++ b/ui/admin/app/components/form/worker/create-worker-led/index.js @@ -21,7 +21,8 @@ export default class FormWorkerCreateWorkerLedComponent extends Component { @tracked ipAddress; @tracked configFilePath; @tracked initialUpstreams; - @tracked workerTags = new TrackedArray([]); + @tracked workerTags = new TrackedArray([new Tag()]); + @tracked enableRecordingStoragePath = false; @tracked recording_storage_path = ''; @@ -66,7 +67,7 @@ touch ${this.configFilePath || ''}/pki-worker.hcl`; const tagsText = `tags { ${ - this.workerTags.length + this.workerTags.filter((t) => t.key?.trim() || t.value?.trim()).length ? this.getTagConfigString() : 'key = ["", ""]' } @@ -141,6 +142,7 @@ unzip *.zip ;\\ getTagConfigString() { return this.workerTags + .filter((t) => t.key?.trim() || t.value?.trim()) .map( (tag) => `${tag.key} = [${this.convertCommaSeparatedValuesToArray( @@ -151,13 +153,24 @@ unzip *.zip ;\\ } @action - addWorkerTag(e) { - this.workerTags.push(new Tag(e.key, e.value)); + addWorkerTag() { + this.workerTags.push(new Tag()); } @action - removeWorkerTagByIndex(index) { - this.workerTags.splice(index, 1); + removeWorkerTagByIndex(rowData) { + const index = this.workerTags.indexOf(rowData); + if (index !== -1) { + this.workerTags.splice(index, 1); + } + if (this.workerTags.length === 0) { + this.workerTags.push(new Tag()); + } + } + + @action + updateWorkerTagValue(rowData, property, event) { + rowData[property] = event.target.value; } @action diff --git a/ui/admin/app/components/form/worker/tag.js b/ui/admin/app/components/form/worker/tag.js index 837cdccc8..528e82c76 100644 --- a/ui/admin/app/components/form/worker/tag.js +++ b/ui/admin/app/components/form/worker/tag.js @@ -9,7 +9,7 @@ export default class Tag { @tracked key; @tracked value; - constructor(key, value) { + constructor(key = '', value = '') { this.key = key; this.value = value; } diff --git a/ui/admin/app/controllers/scopes/scope/targets/index.js b/ui/admin/app/controllers/scopes/scope/targets/index.js index a1bfdc38d..f89725e2a 100644 --- a/ui/admin/app/controllers/scopes/scope/targets/index.js +++ b/ui/admin/app/controllers/scopes/scope/targets/index.js @@ -186,6 +186,11 @@ export default class ScopesScopeTargetsIndexController extends Controller { isNew ? 'notifications.create-success' : 'notifications.save-success', ) async save(target) { + if (target.with_aliases) { + target.with_aliases = target.with_aliases.filter((item) => + item.value?.trim(), + ); + } await target.save(); if (this.abilities.can('read model', target)) { await this.router.transitionTo('scopes.scope.targets.target', target); diff --git a/ui/admin/app/controllers/scopes/scope/workers/worker/create-tags.js b/ui/admin/app/controllers/scopes/scope/workers/worker/create-tags.js index f068cea0e..d504e9a56 100644 --- a/ui/admin/app/controllers/scopes/scope/workers/worker/create-tags.js +++ b/ui/admin/app/controllers/scopes/scope/workers/worker/create-tags.js @@ -9,6 +9,7 @@ import { action } from '@ember/object'; import { loading } from 'ember-loading'; import { notifySuccess, notifyError } from 'core/decorators/notify'; import { TrackedArray } from 'tracked-built-ins'; +import Tag from 'admin/components/form/worker/tag'; export default class ScopesScopeWorkersWorkerCreateTagsController extends Controller { // =services @@ -16,7 +17,7 @@ export default class ScopesScopeWorkersWorkerCreateTagsController extends Contro @service router; // =attributes - apiTags = new TrackedArray([]); + apiTags = new TrackedArray([new Tag()]); // =actions @@ -32,7 +33,7 @@ export default class ScopesScopeWorkersWorkerCreateTagsController extends Contro const worker = this.model; await worker.setApiTags(apiTags); // Clear the apiTags array after saving - this.apiTags = new TrackedArray([]); + this.apiTags = new TrackedArray([new Tag()]); this.router.transitionTo('scopes.scope.workers.worker.tags', worker); } @@ -41,7 +42,7 @@ export default class ScopesScopeWorkersWorkerCreateTagsController extends Contro */ @action cancel() { - this.apiTags = new TrackedArray([]); + this.apiTags = new TrackedArray([new Tag()]); this.router.replaceWith('scopes.scope.workers.worker.tags'); } } diff --git a/ui/admin/app/routes/scopes/scope/targets/new.js b/ui/admin/app/routes/scopes/scope/targets/new.js index 3c20e7aed..ad44fb4f2 100644 --- a/ui/admin/app/routes/scopes/scope/targets/new.js +++ b/ui/admin/app/routes/scopes/scope/targets/new.js @@ -60,6 +60,7 @@ export default class ScopesScopeTargetsNewRoute extends Route { description, }); record.scopeModel = scopeModel; + record.with_aliases = [{ value: '' }]; return record; } diff --git a/ui/admin/app/routes/scopes/scope/workers/worker/create-tags.js b/ui/admin/app/routes/scopes/scope/workers/worker/create-tags.js index 5a7a66cf7..2b1c1ba1e 100644 --- a/ui/admin/app/routes/scopes/scope/workers/worker/create-tags.js +++ b/ui/admin/app/routes/scopes/scope/workers/worker/create-tags.js @@ -7,6 +7,7 @@ import Route from '@ember/routing/route'; import { action } from '@ember/object'; import { service } from '@ember/service'; import { TrackedArray } from 'tracked-built-ins'; +import Tag from 'admin/components/form/worker/tag'; export default class ScopesScopeWorkersWorkerCreateTagsRoute extends Route { // =services @@ -18,14 +19,16 @@ export default class ScopesScopeWorkersWorkerCreateTagsRoute extends Route { async willTransition(transition) { // eslint-disable-next-line ember/no-controller-access-in-routes const controller = this.controllerFor(this.routeName); - if (controller.apiTags.length) { + if ( + controller.apiTags.some((tag) => tag.key?.trim() || tag.value?.trim()) + ) { transition.abort(); try { await this.confirm.confirm(this.intl.t('questions.abandon-confirm'), { title: 'titles.abandon-confirm', confirm: 'actions.discard', }); - controller.set('apiTags', new TrackedArray([])); + controller.set('apiTags', new TrackedArray([new Tag()])); transition.retry(); } catch { // if user denies, do nothing diff --git a/ui/admin/tests/acceptance/targets/create-test.js b/ui/admin/tests/acceptance/targets/create-test.js index 492704b9e..73b2eaae7 100644 --- a/ui/admin/tests/acceptance/targets/create-test.js +++ b/ui/admin/tests/acceptance/targets/create-test.js @@ -318,17 +318,14 @@ module('Acceptance | targets | create', function (hooks) { ), ); + await click(selectors.FIELD_ALIASES_ADD_BTN); await fillIn(selectors.FIELD_ALIASES, 'alias 1'); - await click(selectors.FIELD_ALIASES_ADD_BTN); - await fillIn(selectors.FIELD_ALIASES, 'alias 2'); - await click(selectors.FIELD_ALIASES_ADD_BTN); await click(commonSelectors.SAVE_BTN); assert.strictEqual(getTargetCount(), targetCount + 1); const target = this.server.schema.targets.findBy({ name }); assert.deepEqual(target.withAliases, [ { value: 'alias 1', scope_id: 'global' }, - { value: 'alias 2', scope_id: 'global' }, ]); assert.strictEqual(getTCPTargetCount(), tcpTargetCount + 1); }); diff --git a/ui/admin/tests/acceptance/targets/selectors.js b/ui/admin/tests/acceptance/targets/selectors.js index 48afe0879..5743ff86c 100644 --- a/ui/admin/tests/acceptance/targets/selectors.js +++ b/ui/admin/tests/acceptance/targets/selectors.js @@ -14,10 +14,9 @@ export const FIELD_INFO = '.info-field'; export const FIELD_INFO_LABEL = '.info-field .hds-form-helper-text'; export const FIELD_ALIAS = '[name="value"]'; export const FIELD_DESTINATION_ID = '[name="destination_id"]'; -export const FIELD_ALIASES = - '[name="with_aliases"] tbody tr:last-of-type input'; +export const FIELD_ALIASES = '[name=with_aliases] [data-test-value-input]'; export const FIELD_ALIASES_ADD_BTN = - '[name="with_aliases"] tbody tr:last-of-type button'; + '[name=with_aliases] [data-test-add-button]'; export const FIELD_ADDRESS = '[name="address"]'; export const FIELD_ADDRESS_VALUE = '0.0.0.0'; export const FIELD_NAME_ERROR = '[data-test-error-message-name]'; diff --git a/ui/admin/tests/acceptance/workers/create-tags-test.js b/ui/admin/tests/acceptance/workers/create-tags-test.js index 5d749bdb2..5129ab66a 100644 --- a/ui/admin/tests/acceptance/workers/create-tags-test.js +++ b/ui/admin/tests/acceptance/workers/create-tags-test.js @@ -96,6 +96,7 @@ module('Acceptance | workers | worker | create-tags', function (hooks) { await click(selectors.MANAGE_DROPDOWN_WORKER); await click(selectors.MANAGE_DROPDOWN_WORKER_CREATE_TAGS); + await click(selectors.ADD_WORKER_TAG_ACTION); await fillIn(selectors.FIELD_KEY, selectors.FIELD_KEY_VALUE); await fillIn(selectors.FIELD_VALUE, selectors.FIELD_VALUE_VALUE); await click(selectors.ADD_WORKER_TAG_ACTION); @@ -137,9 +138,11 @@ module('Acceptance | workers | worker | create-tags', function (hooks) { confirmService.enabled = true; await visit(urls.createTags); + await click(selectors.ADD_WORKER_TAG_ACTION); + await fillIn(selectors.FIELD_KEY, selectors.FIELD_KEY_VALUE); await fillIn(selectors.FIELD_VALUE, selectors.FIELD_VALUE_VALUE); - await click(selectors.ADD_WORKER_TAG_ACTION); + await click(commonSelectors.HREF(urls.workers)); assert.dom(commonSelectors.MODAL_WARNING).isVisible(); diff --git a/ui/admin/tests/acceptance/workers/selectors.js b/ui/admin/tests/acceptance/workers/selectors.js index 854998496..d9de085ea 100644 --- a/ui/admin/tests/acceptance/workers/selectors.js +++ b/ui/admin/tests/acceptance/workers/selectors.js @@ -10,8 +10,7 @@ export const FIELD_WORKER_AUTH_REGISTRATION_VALUE = 'token'; export const WORKER_CREATE_SECTION_FORM_LABEL = '.worker-create-section label.hds-form-label'; export const WORKER_CREATE_SECTION = '.worker-create-section'; -export const ADD_WORKER_TAG_ACTION = - '[name="api_tags"] tr td:last-child button'; +export const ADD_WORKER_TAG_ACTION = '[name="api_tags"] [data-test-add-button]'; export const MANAGE_DROPDOWN_WORKER = '[data-test-manage-worker-dropdown] button:first-child'; @@ -20,10 +19,10 @@ export const MANAGE_DROPDOWN_WORKER_REMOVE = export const MANAGE_DROPDOWN_WORKER_CREATE_TAGS = '[data-test-manage-worker-dropdown] ul a:first-child'; -export const FIELD_KEY = '[name="api_tags"] tr td:first-child input'; +export const FIELD_KEY = '[name="api_tags"] [data-test-key-input]'; export const FIELD_KEY_VALUE = 'key'; -export const FIELD_VALUE = '[name="api_tags"] tr td:nth-child(2) input'; +export const FIELD_VALUE = '[name="api_tags"] [data-test-value-input]'; export const FIELD_VALUE_VALUE = 'value'; export const WORKER_TAGS_FLYOUT = '[data-test-worker-tags-flyout]'; diff --git a/ui/admin/tests/acceptance/workers/tags-test.js b/ui/admin/tests/acceptance/workers/tags-test.js index 50a0102b3..18dc50081 100644 --- a/ui/admin/tests/acceptance/workers/tags-test.js +++ b/ui/admin/tests/acceptance/workers/tags-test.js @@ -183,7 +183,7 @@ module('Acceptance | workers | worker | tags', function (hooks) { .doesNotExist(); }); - test('users can edit a specific tag', async function (assert) { + test('users can edit a specific tag and create a new one afterwards', async function (assert) { setRunOptions({ rules: { 'color-contrast': { @@ -210,6 +210,25 @@ module('Acceptance | workers | worker | tags', function (hooks) { assert.dom(selectors.TABLE_ROW_API_TAG_KEY(11)).hasText('red'); assert.dom(selectors.TABLE_ROW_API_TAG_VALUE(11)).hasText('blue'); + + // After editing, go to create tags route and create a new tag + + await click(selectors.MANAGE_DROPDOWN_WORKER); + await click(selectors.MANAGE_DROPDOWN_WORKER_CREATE_TAGS); + await click(selectors.ADD_WORKER_TAG_ACTION); + + await fillIn(selectors.FIELD_KEY, selectors.FIELD_KEY_VALUE); + await fillIn(selectors.FIELD_VALUE, selectors.FIELD_VALUE_VALUE); + + await click(selectors.ADD_WORKER_TAG_ACTION); + await click(commonSelectors.SAVE_BTN); + + assert + .dom(selectors.TABLE_ROW_API_TAG_KEY(12)) + .hasText(selectors.FIELD_KEY_VALUE); + assert + .dom(selectors.TABLE_ROW_API_TAG_VALUE(12)) + .hasText(selectors.FIELD_VALUE_VALUE); }); test('users can cancel editing a tag', async function (assert) { diff --git a/ui/admin/tests/integration/components/form/field/key-value-test.js b/ui/admin/tests/integration/components/form/field/key-value-test.js index 9e9a0916e..f05b4e35a 100644 --- a/ui/admin/tests/integration/components/form/field/key-value-test.js +++ b/ui/admin/tests/integration/components/form/field/key-value-test.js @@ -13,10 +13,17 @@ module('Integration | Component | form/field/key-value', function (hooks) { setupRenderingTest(hooks); setupIntl(hooks, 'en-us'); + // Test selectors + const KEY_INPUT = '[data-test-key-input]'; + const VALUE_INPUT = '[data-test-value-input]'; + const DELETE_BUTTON = '[data-test-delete-button]'; + const ADD_BUTTON = '[data-test-add-button]'; + const ERROR_MESSAGE = '[data-test-error-message]'; + // Helper functions - const getKeyInputs = () => findAll('[data-test-key-input]'); - const getValueInputs = () => findAll('[data-test-value-input]'); - const getDeleteButtons = () => findAll('[data-test-delete-button]'); + const getKeyInputs = () => findAll(KEY_INPUT); + const getValueInputs = () => findAll(VALUE_INPUT); + const getDeleteButtons = () => findAll(DELETE_BUTTON); test('it renders with default and provided data', async function (assert) { // Default empty row @@ -39,10 +46,10 @@ module('Integration | Component | form/field/key-value', function (hooks) { `); - assert.dom('[data-test-key-input]').exists(); - assert.dom('[data-test-value-input]').exists(); - assert.dom('[data-test-add-button]').exists(); - assert.dom('[data-test-delete-button]').doesNotExist(); + assert.dom(KEY_INPUT).exists(); + assert.dom(VALUE_INPUT).exists(); + assert.dom(ADD_BUTTON).exists(); + assert.dom(DELETE_BUTTON).doesNotExist(); // Provided data this.set('data', [ @@ -112,7 +119,9 @@ module('Integration | Component | form/field/key-value', function (hooks) { `); assert.strictEqual(getKeyInputs().length, 1, 'Initially has one row'); - await click('[data-test-add-button]'); + + await click(ADD_BUTTON); + assert.strictEqual( getKeyInputs().length, 2, @@ -156,8 +165,8 @@ module('Integration | Component | form/field/key-value', function (hooks) { `); - assert.dom('[data-test-key-input]').hasValue('k'); - assert.dom('[data-test-value-input]').hasValue('v'); + assert.dom(KEY_INPUT).hasValue('k'); + assert.dom(VALUE_INPUT).hasValue('v'); assert.dom('[data-test-id-input]').hasValue('1'); // Test Textarea field @@ -177,7 +186,7 @@ module('Integration | Component | form/field/key-value', function (hooks) { `); - assert.dom('[data-test-key-input]').hasValue('k'); + assert.dom(KEY_INPUT).hasValue('k'); // Test single field this.set('data', [{ key: '' }]); @@ -197,14 +206,14 @@ module('Integration | Component | form/field/key-value', function (hooks) { `); - assert.dom('[data-test-key-input]').exists(); - assert.dom('[data-test-value-input]').doesNotExist(); - assert.dom('[data-test-add-button]').exists(); - assert.dom('[data-test-delete-button]').doesNotExist(); + assert.dom(KEY_INPUT).exists(); + assert.dom(VALUE_INPUT).doesNotExist(); + assert.dom(ADD_BUTTON).exists(); + assert.dom(DELETE_BUTTON).doesNotExist(); }); - test('it displays error messages in footer (model errors)', async function (assert) { - const model = { + test('it displays error messages in footer from model errors', async function (assert) { + this.set('model', { tags: [{ key: '', value: '' }], errors: { tags: [ @@ -212,17 +221,10 @@ module('Integration | Component | form/field/key-value', function (hooks) { { message: 'Value must be unique' }, ], }, - }; - this.set('model', model); - this.set('properties', ['key', 'value']); + }); - // With errors await render(hbs` - + <:row as |R|> @@ -239,19 +241,18 @@ module('Integration | Component | form/field/key-value', function (hooks) { `); - assert.dom('[data-test-error-message]').exists().hasText('Key is required'); + assert.dom(ERROR_MESSAGE).exists().hasText('Key is required'); assert .dom('[data-test-error-message]:nth-child(2)') .hasText('Value must be unique'); - // Without errors - this.set('model', { ...model, errors: {} }); + this.set('model', { + tags: [{ key: '', value: '' }], + errors: {}, + }); + await render(hbs` - + <:row as |R|> @@ -268,7 +269,33 @@ module('Integration | Component | form/field/key-value', function (hooks) { `); - assert.dom('[data-test-error-message]').doesNotExist(); + assert.dom(ERROR_MESSAGE).doesNotExist(); + }); + + test('it displays error messages in footer from @errors arg', async function (assert) { + this.set('errors', [ + { message: 'Direct error one' }, + { message: 'Direct error two' }, + ]); + this.set('data', []); + + await render(hbs` + + <:row as |R|> + + + + + <:footer as |F|> + + + + `); + + assert.dom(ERROR_MESSAGE).exists().hasText('Direct error one'); + assert + .dom('[data-test-error-message]:nth-child(2)') + .hasText('Direct error two'); }); test('it updates both key and value fields using custom handlers', async function (assert) { @@ -296,8 +323,8 @@ module('Integration | Component | form/field/key-value', function (hooks) { `); - await fillIn('[data-test-key-input]', 'test-key'); - await fillIn('[data-test-value-input]', 'test-value'); + await fillIn(KEY_INPUT, 'test-key'); + await fillIn(VALUE_INPUT, 'test-value'); assert.strictEqual(this.data.length, 1); assert.strictEqual(this.data[0].key, 'test-key'); @@ -336,12 +363,6 @@ module('Integration | Component | form/field/key-value', function (hooks) { test('custom handlers take precedence over default handlers', async function (assert) { const model = { tags: [{ key: '', value: '' }], - errors: { - tags: [ - { message: 'Key is required' }, - { message: 'Value must be unique' }, - ], - }, }; this.set('model', model); @@ -379,7 +400,7 @@ module('Integration | Component | form/field/key-value', function (hooks) { `); // Test custom add handler takes precedence - await click('[data-test-add-button]'); + await click(ADD_BUTTON); assert.strictEqual(model.tags.length, 2); assert.strictEqual(model.tags[1].key, 'custom'); @@ -425,7 +446,7 @@ module('Integration | Component | form/field/key-value', function (hooks) { assert.strictEqual(model.tags.length, 1); - await click('[data-test-add-button]'); + await click(ADD_BUTTON); assert.strictEqual(model.tags.length, 2); @@ -502,10 +523,76 @@ module('Integration | Component | form/field/key-value', function (hooks) { `); - await fillIn('[data-test-key-input]', 'updated-key'); - await fillIn('[data-test-value-input]', 'updated-value'); + await fillIn(KEY_INPUT, 'updated-key'); + await fillIn(VALUE_INPUT, 'updated-value'); assert.strictEqual(model.tags[0].key, 'updated-key'); assert.strictEqual(model.tags[0].value, 'updated-value'); }); + + test('canDelete uses properties argument correctly (trash icon visible)', async function (assert) { + // Multiple rows: trash icon should be visible + this.set('data', [ + { key: 'foo', value: 'bar' }, + { key: '', value: '' }, + ]); + this.set('properties', ['key', 'value']); + await render(hbs` + + <:row as |R|> + + + + + + + {{#if R.canDelete}} + + {{/if}} + + + `); + + assert.dom(DELETE_BUTTON).isVisible(); + + // Single row, property is empty: trash icon should not be visible + this.set('data', [{ key: '', value: '' }]); + await render(hbs` + + <:row as |R|> + + + + + + + {{#if R.canDelete}} + + {{/if}} + + + `); + + assert.dom(DELETE_BUTTON).isNotVisible(); + + // Single row, property is set: trash icon should be visible + this.set('data', [{ key: 'foo', value: '' }]); + await render(hbs` + + <:row as |R|> + + + + + + + {{#if R.canDelete}} + + {{/if}} + + + `); + + assert.dom(DELETE_BUTTON).isVisible(); + }); }); diff --git a/ui/admin/tests/integration/components/form/worker/create-tags/index-test.js b/ui/admin/tests/integration/components/form/worker/create-tags/index-test.js index cd01e5623..a8a78423a 100644 --- a/ui/admin/tests/integration/components/form/worker/create-tags/index-test.js +++ b/ui/admin/tests/integration/components/form/worker/create-tags/index-test.js @@ -28,9 +28,8 @@ module( hbs``, ); - assert - .dom(this.element) - .includesText('Key Value Actions Add Save Cancel'); + assert.dom(this.element).includesText('Add Save Cancel'); + assert.dom('[data-test-add-button]').exists(); }); test('it calls the passed in save action', async function (assert) { diff --git a/ui/admin/tests/integration/components/form/worker/create-worker-led-test.js b/ui/admin/tests/integration/components/form/worker/create-worker-led-test.js index 2e6ae9f04..dacd84d84 100644 --- a/ui/admin/tests/integration/components/form/worker/create-worker-led-test.js +++ b/ui/admin/tests/integration/components/form/worker/create-worker-led-test.js @@ -5,7 +5,7 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { click, fillIn, render } from '@ember/test-helpers'; +import { click, fillIn, render, findAll } from '@ember/test-helpers'; import { hbs } from 'ember-cli-htmlbars'; import { v4 as uuidv4 } from 'uuid'; import { setupIntl } from 'ember-intl/test-support'; @@ -18,6 +18,10 @@ module( setupBrowserFakes(hooks, { window: true }); setupIntl(hooks, 'en-us'); + const ADD_BTN = '[name="worker_tags"] [data-test-add-button]'; + const KEY_VALUE_ROW = + '[name="worker_tags"] .hds-form-key-value-inputs__row'; + hooks.beforeEach(function () { const featuresService = this.owner.lookup('service:features'); featuresService.enable('byow-pki-hcp-cluster-id'); @@ -91,11 +95,12 @@ module( this.submit = () => {}; this.cancel = () => {}; - const KEY_VALUE_INPUT_SELECTOR = (row, column) => - `[name="worker_tags"] tbody :nth-child(${row}) td:nth-child(${column}) input`; - const KEY_VALUE_BUTTON_SELECTOR = (row) => - `[name="worker_tags"] tbody :nth-child(${row}) button`; - const KEY_VALUE_ROW = '[name="worker_tags"] tbody tr'; + const getKeyInputs = () => + findAll('[name="worker_tags"] [data-test-key-input]'); + const getValueInputs = () => + findAll('[name="worker_tags"] [data-test-value-input]'); + const getDeleteBtns = () => + findAll('[name="worker_tags"] [data-test-delete-button]'); await render( hbs``, @@ -103,21 +108,17 @@ module( assert.dom(KEY_VALUE_ROW).exists({ count: 1 }); - // Test adding a worker tag - await fillIn(KEY_VALUE_INPUT_SELECTOR(1, 1), 'environment'); - await fillIn(KEY_VALUE_INPUT_SELECTOR(1, 2), 'dev'); - await click(KEY_VALUE_BUTTON_SELECTOR(1)); + // Add second worker tag + await click(ADD_BTN); + assert.dom(KEY_VALUE_ROW).exists({ count: 2 }); + await fillIn(getKeyInputs()[1], 'environment'); + await fillIn(getValueInputs()[1], 'test'); - await fillIn(KEY_VALUE_INPUT_SELECTOR(2, 1), 'environment'); - await fillIn(KEY_VALUE_INPUT_SELECTOR(2, 2), 'test'); - await click(KEY_VALUE_BUTTON_SELECTOR(2)); - - assert.dom(KEY_VALUE_ROW).exists({ count: 3 }); + // Remove the first worker tag + await click(getDeleteBtns()[0]); // Remove the worker tag - await click(KEY_VALUE_BUTTON_SELECTOR(1)); - - assert.dom(KEY_VALUE_ROW).exists({ count: 2 }); + assert.dom(KEY_VALUE_ROW).exists({ count: 1 }); }); }, ); diff --git a/ui/admin/tests/unit/controllers/scopes/scope/workers/worker/create-tags-test.js b/ui/admin/tests/unit/controllers/scopes/scope/workers/worker/create-tags-test.js index a89eb043d..87799a6b9 100644 --- a/ui/admin/tests/unit/controllers/scopes/scope/workers/worker/create-tags-test.js +++ b/ui/admin/tests/unit/controllers/scopes/scope/workers/worker/create-tags-test.js @@ -75,7 +75,9 @@ module( await controller.cancel(); - assert.deepEqual(controller.apiTags, []); + assert.strictEqual(controller.apiTags.length, 1); + assert.strictEqual(controller.apiTags[0].key, ''); + assert.strictEqual(controller.apiTags[0].value, ''); assert.strictEqual(currentURL(), urls.tags); }); },