From 8cb16dc46ce2f1af902e1e1f844e74a9f2314388 Mon Sep 17 00:00:00 2001 From: Cameron Perera Date: Tue, 16 Jul 2024 12:54:44 -0500 Subject: [PATCH] Icu 13894 create edit tags modal (#2384) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: 🎸 add "no tags" state for worker tags ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13894 * feat: 🎸 add edit workflow for worker tags ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13894 * refactor: 💡 update tag action permissions logic ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13894 * refactor: 💡 small change to updateApiTags ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13894 * feat: 🎸 show different tooltip for hcp managed worker tags ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13894 * refactor: 💡 address PR feedback ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13894 * refactor: 💡 address more PR feedback ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13894 --- addons/api/addon/models/worker.js | 2 + addons/core/translations/resources/en-us.yaml | 16 +- addons/rose/addon/styles/hds/overrides.scss | 10 +- .../scopes/scope/workers/worker/tags.js | 71 ++++++++- ui/admin/app/styles/app.scss | 13 +- .../scopes/scope/workers/worker/tags.hbs | 119 ++++++++++++--- .../tests/acceptance/workers/tags-test.js | 137 ++++++++++++++++-- .../scopes/scope/workers/worker/tags-test.js | 92 +++++++++++- 8 files changed, 412 insertions(+), 48 deletions(-) diff --git a/addons/api/addon/models/worker.js b/addons/api/addon/models/worker.js index 3a62cfc5c..60814989d 100644 --- a/addons/api/addon/models/worker.js +++ b/addons/api/addon/models/worker.js @@ -11,6 +11,8 @@ export const TYPE_WORKER_PKI = 'pki'; export const TAG_TYPE_CONFIG = 'config'; export const TAG_TYPE_API = 'api'; +export const HCP_MANAGED_KEY = 'boundary.cloud.hashicorp.com:managed'; + export default class WorkerModel extends GeneratedWorkerModel { // =attributes diff --git a/addons/core/translations/resources/en-us.yaml b/addons/core/translations/resources/en-us.yaml index b69a8ff2c..b830cad51 100644 --- a/addons/core/translations/resources/en-us.yaml +++ b/addons/core/translations/resources/en-us.yaml @@ -981,7 +981,9 @@ worker: messages: welcome: title: Welcome to Workers - tooltip: To edit config tags, edit them in your worker config file. + tooltip: + hcp: This is a HCP managed worker, you cannot edit or delete this worker’s tags. + self: To edit config tags, edit them in your worker config file. table: session_count: Session Count ip_address: IP Address @@ -999,6 +1001,10 @@ worker: link: Config tags are added in the worker config file. success: Tag(s) successfully created description: Tags are used to define the type of worker and to make it distinguishable amongst other workers. + messages: + no-tags: + title: No tags added + body: You haven’t added any tags to this worker yet remove: title: Remove worker tag? success: Tag successfully removed @@ -1006,8 +1012,14 @@ worker: confirm: label: Confirm removal help: 'Enter “REMOVE” to confirm — this is case-sensitive.' + edit: + title: Edit tag + description: Edit the existing worker tag + success: Tag successfully edited + helper-text: Separate multiple values by comma actions: - remove: Remove Tag + remove: Remove tag + add: Add tags to this worker flyout: title: Tags in {workerName} action: diff --git a/addons/rose/addon/styles/hds/overrides.scss b/addons/rose/addon/styles/hds/overrides.scss index 7aaf141bf..4d645d135 100644 --- a/addons/rose/addon/styles/hds/overrides.scss +++ b/addons/rose/addon/styles/hds/overrides.scss @@ -4,10 +4,12 @@ */ // field and button bottom margin (HDS leaves this to implementers) -[class*='hds-form-field--layout'], -[class*='hds-form-group--layout'], -.hds-button-set { - margin-bottom: 1.5rem; +form { + [class*='hds-form-field--layout'], + [class*='hds-form-group--layout'], + .rose-form-actions.hds-button-set { + margin-bottom: 1.5rem; + } } /* stylelint-disable selector-class-pattern */ diff --git a/ui/admin/app/controllers/scopes/scope/workers/worker/tags.js b/ui/admin/app/controllers/scopes/scope/workers/worker/tags.js index a20aa07ba..88a556f5a 100644 --- a/ui/admin/app/controllers/scopes/scope/workers/worker/tags.js +++ b/ui/admin/app/controllers/scopes/scope/workers/worker/tags.js @@ -9,7 +9,11 @@ import { tracked } from '@glimmer/tracking'; import { action } from '@ember/object'; import { loading } from 'ember-loading'; import { notifySuccess, notifyError } from 'core/decorators/notify'; -import { TAG_TYPE_CONFIG, TAG_TYPE_API } from 'api/models/worker'; +import { + TAG_TYPE_CONFIG, + TAG_TYPE_API, + HCP_MANAGED_KEY, +} from 'api/models/worker'; export default class ScopesScopeWorkersWorkerTagsController extends Controller { @controller('scopes/scope/workers/index') workers; @@ -24,7 +28,10 @@ export default class ScopesScopeWorkersWorkerTagsController extends Controller { @tracked removeModal = false; @tracked removalConfirmation = null; + @tracked editModal = false; @tracked modalTag = null; + @tracked editKey = null; + @tracked editValue = null; /** * Build the display name for a worker tag @@ -44,6 +51,40 @@ export default class ScopesScopeWorkersWorkerTagsController extends Controller { ); } + /** + * Determines if a config tag is managed by HCP + * @param {object} tag + * @returns {boolean} + */ + isHcpManaged(tag) { + return tag.key === HCP_MANAGED_KEY && tag.value === 'true'; + } + + updateApiTags() { + const apiTags = structuredClone(this.model.api_tags); + const { key, value } = this.modalTag; + const editedValues = this.editValue.split(',').map((value) => value.trim()); + + // remove the old value + apiTags[key] = apiTags[key].filter((val) => val !== value); + + // remove key if value is empty array + if (apiTags[key].length === 0) { + delete apiTags[key]; + } + + // add new key/value to apiTags + if (this.editKey in apiTags) { + apiTags[this.editKey] = [ + ...new Set([...apiTags[this.editKey], ...editedValues]), + ]; + } else { + apiTags[this.editKey] = editedValues; + } + + return apiTags; + } + /** * Removes selected tag from the worker. */ @@ -56,12 +97,24 @@ export default class ScopesScopeWorkersWorkerTagsController extends Controller { [this.modalTag.key]: [this.modalTag.value], }); this.removeModal = false; - this.modalTag = null; this.removalConfirmation = null; } /** - * Opens the remove tag modal. + * Edits selected tag on the worker + */ + @action + @loading + @notifyError(({ message }) => message, { catch: true }) + @notifySuccess('resources.worker.tags.edit.success') + async editApiTag() { + const apiTags = this.updateApiTags(); + await this.model.setApiTags(apiTags); + this.editModal = false; + } + + /** + * Toggles the remove tag modal. * @param {object} apiTag */ @action @@ -70,4 +123,16 @@ export default class ScopesScopeWorkersWorkerTagsController extends Controller { this.modalTag = apiTag; this.removalConfirmation = null; } + + /** + * Toggles the edit tag modal. + * @param {object} apiTag + */ + @action + toggleEditModal(apiTag) { + this.editModal = !this.editModal; + this.modalTag = apiTag; + this.editKey = apiTag?.key; + this.editValue = apiTag?.value; + } } diff --git a/ui/admin/app/styles/app.scss b/ui/admin/app/styles/app.scss index dac87638a..b79823621 100644 --- a/ui/admin/app/styles/app.scss +++ b/ui/admin/app/styles/app.scss @@ -1058,13 +1058,10 @@ text-overflow: ellipsis; } -.confirmation-modal { - .hds-button-set { - margin-bottom: 0; - } - - .hds-form-field--layout-vertical { - margin-top: 1.5rem; - margin-bottom: 0; +.worker-tags { + .modal-body { + display: flex; + flex-direction: column; + gap: 1.5rem; } } diff --git a/ui/admin/app/templates/scopes/scope/workers/worker/tags.hbs b/ui/admin/app/templates/scopes/scope/workers/worker/tags.hbs index f9aaca3fe..efdfab87a 100644 --- a/ui/admin/app/templates/scopes/scope/workers/worker/tags.hbs +++ b/ui/admin/app/templates/scopes/scope/workers/worker/tags.hbs @@ -30,7 +30,7 @@ /> - + {{#if @model.allTags}} {{#if (eq B.data.type this.tagTypeConfig)}} @@ -64,30 +70,107 @@ {{#if (eq B.data.type this.tagTypeApi)}} - - - {{#if (can 'removeTags worker' @model)}} - + {{#let + (hash + canEdit=(can 'setWorkerTags worker' @model) + canRemove=(can 'removeTags worker' @model) + ) + as |perms| + }} + {{#if (or perms.canEdit perms.canRemove)}} + + + {{#if perms.canEdit}} + + {{/if}} + {{#if perms.canRemove}} + + {{/if}} + {{/if}} - + {{/let}} {{/if}} + {{else}} + + + + + + + + {{/if}} + + {{#if this.editModal}} + + + {{t 'resources.worker.tags.edit.title'}} + + + + {{t 'resources.worker.tags.edit.description'}} + + {{! template-lint-disable no-autofocus-attribute }} + + {{t 'resources.worker.table.key'}} + + + {{t 'resources.worker.table.value'}} + {{t + 'resources.worker.tags.edit.helper-text' + }} + + + + + + + + + {{/if}} {{#if this.removeModal}} {{t 'resources.worker.tags.remove.title'}} - - + + {{this.tagDisplayName}} {{t 'resources.worker.tags.remove.warning'}} diff --git a/ui/admin/tests/acceptance/workers/tags-test.js b/ui/admin/tests/acceptance/workers/tags-test.js index d08587b8b..f0d49edfa 100644 --- a/ui/admin/tests/acceptance/workers/tags-test.js +++ b/ui/admin/tests/acceptance/workers/tags-test.js @@ -4,11 +4,19 @@ */ import { module, test } from 'qunit'; -import { visit, currentURL, click, focus, fillIn } from '@ember/test-helpers'; +import { + visit, + currentURL, + click, + focus, + fillIn, + find, +} from '@ember/test-helpers'; import { setupApplicationTest } from 'ember-qunit'; import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; import a11yAudit from 'ember-a11y-testing/test-support/audit'; import { authenticateSession } from 'ember-simple-auth/test-support'; +import { HCP_MANAGED_KEY } from 'api/models/worker'; module('Acceptance | workers | worker | tags', function (hooks) { setupApplicationTest(hooks); @@ -18,15 +26,30 @@ module('Acceptance | workers | worker | tags', function (hooks) { 'tbody tr:first-child td:nth-child(3) button'; const CONFIG_TAG_TOOLTIP_TEXT_SELECTOR = 'tbody tr:first-child td:nth-child(3) > div'; + const API_TAG_KEY_SELECTOR = (row) => + `tbody tr:nth-child(${row}) td:first-child pre`; + const API_TAG_VALUE_SELECTOR = (row) => + `tbody tr:nth-child(${row}) td:nth-child(2) pre`; const API_TAG_ACTION_SELECTOR = 'tbody tr:nth-child(4) td:nth-child(4) button'; - const API_TAG_REMOVE_ACTION_SELECTOR = - 'tbody tr:nth-child(4) td:nth-child(4) ul li button'; - const CONFIRMATION_MODAL_INPUT_SELECTOR = '.confirmation-modal input'; + const API_TAG_FIRST_BUTTON_SELECTOR = + 'tbody tr:nth-child(4) td:nth-child(4) ul li:first-child button'; + const API_TAG_LAST_BUTTON_SELECTOR = + 'tbody tr:nth-child(4) td:nth-child(4) ul li:nth-child(2) button'; + const EDIT_TAG_KEY_INPUT_SELECTOR = + '[data-test-edit-modal] [name="edit-tag-key"]'; + const EDIT_TAG_VALUE_INPUT_SELECTOR = + '[data-test-edit-modal] [name="edit-tag-value"]'; + const EDIT_TAG_CONFIRM_BUTTON = '[data-test-edit-modal] button:first-child'; + const EDIT_TAG_CANCEL_BUTTON = '[data-test-edit-modal] button:last-child'; + const CONFIRMATION_MODAL_INPUT_SELECTOR = + '[data-test-confirmation-modal] input'; const CONFIRMATION_MODAL_REMOVE_BUTTON_SELECTOR = - '.confirmation-modal button:first-child'; + '[data-test-confirmation-modal] button:first-child'; const CONFIRMATION_MODAL_CANCEL_BUTTON_SELECTOR = - '.confirmation-modal button:last-child'; + '[data-test-confirmation-modal] button:last-child'; + const NO_TAGS_STATE_TITLE = '[data-test-no-tags] div:first-child'; + const NO_TAGS_STATE_ACTION = '[data-test-no-tags] div:nth-child(3) a'; const instances = { scopes: { @@ -38,6 +61,7 @@ module('Acceptance | workers | worker | tags', function (hooks) { workers: null, worker: null, tags: null, + createTags: null, }; hooks.beforeEach(function () { @@ -48,6 +72,7 @@ module('Acceptance | workers | worker | tags', function (hooks) { urls.workers = `/scopes/global/workers`; urls.worker = `${urls.workers}/${instances.worker.id}`; urls.tags = `${urls.worker}/tags`; + urls.createTags = `${urls.worker}/create-tags`; authenticateSession({}); }); @@ -60,9 +85,21 @@ module('Acceptance | workers | worker | tags', function (hooks) { assert.dom('tbody tr').exists({ count: 11 }); }); - test('config tags display a tooltip', async function (assert) { + test('config tags display a tooltip for HCP managed tag', async function (assert) { + instances.worker.config_tags = { [HCP_MANAGED_KEY]: ['true'] }; + await visit(urls.tags); + + await focus(CONFIG_TAG_TOOLTIP_SELECTOR); + + assert + .dom(CONFIG_TAG_TOOLTIP_TEXT_SELECTOR) + .hasText( + 'This is a HCP managed worker, you cannot edit or delete this worker’s tags.', + ); + }); + + test('config tags display a tooltip self managed tag', async function (assert) { await visit(urls.tags); - await a11yAudit(); await focus(CONFIG_TAG_TOOLTIP_SELECTOR); @@ -77,7 +114,7 @@ module('Acceptance | workers | worker | tags', function (hooks) { assert.dom('tbody tr').exists({ count: 11 }); await click(API_TAG_ACTION_SELECTOR); - await click(API_TAG_REMOVE_ACTION_SELECTOR); + await click(API_TAG_LAST_BUTTON_SELECTOR); assert.dom(CONFIRMATION_MODAL_REMOVE_BUTTON_SELECTOR).isDisabled(); @@ -96,7 +133,7 @@ module('Acceptance | workers | worker | tags', function (hooks) { assert.dom('tbody tr').exists({ count: 11 }); await click(API_TAG_ACTION_SELECTOR); - await click(API_TAG_REMOVE_ACTION_SELECTOR); + await click(API_TAG_LAST_BUTTON_SELECTOR); await click(CONFIRMATION_MODAL_CANCEL_BUTTON_SELECTOR); @@ -114,6 +151,84 @@ module('Acceptance | workers | worker | tags', function (hooks) { await click(API_TAG_ACTION_SELECTOR); - assert.dom(API_TAG_REMOVE_ACTION_SELECTOR).doesNotExist(); + // Asserts first button is for editing and there is no second button + assert.dom(API_TAG_FIRST_BUTTON_SELECTOR).hasText('Edit tag'); + assert.dom(API_TAG_LAST_BUTTON_SELECTOR).doesNotExist(); + }); + + test('users can edit a specific tag', async function (assert) { + await visit(urls.tags); + const key = find(API_TAG_KEY_SELECTOR(4)).textContent.trim(); + const value = find(API_TAG_VALUE_SELECTOR(4)).textContent.trim(); + + assert.dom(API_TAG_KEY_SELECTOR(4)).hasText(key); + assert.dom(API_TAG_VALUE_SELECTOR(4)).hasText(value); + + await click(API_TAG_ACTION_SELECTOR); + await click(API_TAG_FIRST_BUTTON_SELECTOR); + + await fillIn(EDIT_TAG_KEY_INPUT_SELECTOR, 'red'); + await fillIn(EDIT_TAG_VALUE_INPUT_SELECTOR, 'blue'); + + await click(EDIT_TAG_CONFIRM_BUTTON); + + assert.dom(API_TAG_KEY_SELECTOR(11)).hasText('red'); + assert.dom(API_TAG_VALUE_SELECTOR(11)).hasText('blue'); + }); + + test('users can cancel editing a tag', async function (assert) { + await visit(urls.tags); + const key = find(API_TAG_KEY_SELECTOR(4)).textContent.trim(); + const value = find(API_TAG_VALUE_SELECTOR(4)).textContent.trim(); + + assert.dom(API_TAG_KEY_SELECTOR(4)).hasText(key); + assert.dom(API_TAG_VALUE_SELECTOR(4)).hasText(value); + + await click(API_TAG_ACTION_SELECTOR); + await click(API_TAG_FIRST_BUTTON_SELECTOR); + + await fillIn(EDIT_TAG_KEY_INPUT_SELECTOR, 'red'); + await fillIn(EDIT_TAG_VALUE_INPUT_SELECTOR, 'blue'); + + await click(EDIT_TAG_CANCEL_BUTTON); + + assert.dom(API_TAG_KEY_SELECTOR(4)).hasText(key); + assert.dom(API_TAG_VALUE_SELECTOR(4)).hasText(value); + }); + + test('user cannot edit a tag without proper permission', async function (assert) { + instances.worker.authorized_actions = + instances.worker.authorized_actions.filter( + (item) => item !== 'set-worker-tags', + ); + await visit(urls.tags); + + await click(API_TAG_ACTION_SELECTOR); + + // Asserts first button is for removing and there is no second button + assert.dom(API_TAG_FIRST_BUTTON_SELECTOR).hasText('Remove tag'); + assert.dom(API_TAG_LAST_BUTTON_SELECTOR).doesNotExist(); + }); + + test('user does not see tag action dropdown if they cannot edit or remove tags', async function (assert) { + instances.worker.authorized_actions = + instances.worker.authorized_actions.filter( + (item) => item !== 'set-worker-tags' && item !== 'remove-worker-tags', + ); + + await visit(urls.tags); + + assert.dom(API_TAG_ACTION_SELECTOR).doesNotExist(); + }); + + test('shows "No tags added" message when there are no worker tags', async function (assert) { + instances.worker.api_tags = {}; + instances.worker.config_tags = {}; + instances.worker.canonical_tags = {}; + + await visit(urls.tags); + + assert.dom(NO_TAGS_STATE_TITLE).hasText('No tags added'); + assert.dom(NO_TAGS_STATE_ACTION).hasAttribute('href', urls.createTags); }); }); diff --git a/ui/admin/tests/unit/controllers/scopes/scope/workers/worker/tags-test.js b/ui/admin/tests/unit/controllers/scopes/scope/workers/worker/tags-test.js index 059462255..c859c41f6 100644 --- a/ui/admin/tests/unit/controllers/scopes/scope/workers/worker/tags-test.js +++ b/ui/admin/tests/unit/controllers/scopes/scope/workers/worker/tags-test.js @@ -8,6 +8,7 @@ import { visit } from '@ember/test-helpers'; import { setupTest } from 'ember-qunit'; import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; import { authenticateSession } from 'ember-simple-auth/test-support'; +import { HCP_MANAGED_KEY } from 'api/models/worker'; module( 'Unit | Controller | scopes/scope/workers/worker/tags', @@ -17,6 +18,7 @@ module( let store; let controller; + const apiTags = { test: ['test'], key: ['value'] }; const instances = { scopes: { @@ -40,7 +42,7 @@ module( instances.scopes.global = this.server.create('scope', { id: 'global' }); instances.worker = this.server.create('worker', { scope: instances.scopes.global, - api_tags: { test: ['test'], key: ['value'] }, + api_tags: apiTags, }); urls.workers = `/scopes/global/workers`; urls.tags = `${urls.workers}/${instances.worker.id}/tags`; @@ -68,6 +70,21 @@ module( assert.false(controller.isRemovalConfirmed); }); + test('isHcpManged returns true if tag key matches `HCP_MANAGED_KEY` and value is true', async function (assert) { + assert.true( + controller.isHcpManaged({ key: HCP_MANAGED_KEY, value: 'true' }), + ); + }); + + test('isHcpManged returns false if tag key does not match `HCP_MANAGED_KEY` or value is not true', async function (assert) { + assert.false( + controller.isHcpManaged({ key: 'self.managed', value: 'true' }), + ); + assert.false( + controller.isHcpManaged({ key: HCP_MANAGED_KEY, value: 'false' }), + ); + }); + test('removeApiTag calls removeApiTags on the model and closes modal', async function (assert) { await visit(urls.tags); const worker = await store.findRecord('worker', instances.worker.id); @@ -78,7 +95,65 @@ module( assert.strictEqual(worker.apiTagList.length, apiTagCount - 1); assert.false(controller.removeModal); - assert.strictEqual(controller.modalTag, null); + }); + + test('editApiTags calls setApiTags on the model and closes modal', async function (assert) { + await visit(urls.tags); + const worker = await store.findRecord('worker', instances.worker.id); + + assert.deepEqual(worker.api_tags, apiTags); + + // set controller values needed for editing + controller.modalTag = { key: 'test', value: 'test' }; + controller.editKey = 'unit'; + controller.editValue = 'test'; + + await controller.editApiTag(); + + assert.deepEqual(worker.api_tags, { unit: ['test'], key: ['value'] }); + assert.false(controller.editModal); + }); + + test('editApiTags can handle comma separate string value', async function (assert) { + await visit(urls.tags); + const worker = await store.findRecord('worker', instances.worker.id); + + assert.deepEqual(worker.api_tags, apiTags); + + // set controller values needed for editing + controller.modalTag = { key: 'test', value: 'test' }; + controller.editKey = 'unit'; + controller.editValue = 'test, are, fun'; + + await controller.editApiTag(); + + assert.deepEqual(worker.api_tags, { + unit: ['test', 'are', 'fun'], + key: ['value'], + }); + assert.false(controller.editModal); + }); + + test('updateApiTags returns an update apiTags object with edited values added', async function (assert) { + await visit(urls.tags); + const worker = await store.findRecord('worker', instances.worker.id); + + assert.deepEqual(worker.api_tags, { + key: ['value'], + test: ['test'], + }); + + // set controller values needed for editing + controller.modalTag = { key: 'test', value: 'test' }; + controller.editKey = 'unit'; + controller.editValue = 'test, are, fun'; + + const result = controller.updateApiTags(); + + assert.deepEqual(result, { + key: ['value'], + unit: ['test', 'are', 'fun'], + }); }); test('toggleRemoveModal toggles the modal and sets the modalTag', function (assert) { @@ -93,5 +168,18 @@ module( value: 'test', }); }); + + test('toggleEditModal toggles the modal and sets the modalTag', function (assert) { + assert.false(controller.editModal); + assert.strictEqual(controller.modalTag, null); + + controller.toggleEditModal({ key: 'test', value: 'test' }); + + assert.true(controller.editModal); + assert.deepEqual(controller.modalTag, { + key: 'test', + value: 'test', + }); + }); }, );