diff --git a/addons/api/addon/abilities/worker.js b/addons/api/addon/abilities/worker.js index ac5fe9e76..36d13d868 100644 --- a/addons/api/addon/abilities/worker.js +++ b/addons/api/addon/abilities/worker.js @@ -18,7 +18,17 @@ export default class WorkerAbility extends ModelAbility { return this.hasAuthorizedCollectionAction('create:worker-led'); } + /** + * @type {boolean} + */ get canSetWorkerTags() { return this.hasAuthorizedAction('set-worker-tags'); } + + /** + * @type {boolean} + */ + get canRemoveTags() { + return this.hasAuthorizedAction('remove-worker-tags'); + } } diff --git a/addons/api/addon/models/worker.js b/addons/api/addon/models/worker.js index b33603fa9..3a62cfc5c 100644 --- a/addons/api/addon/models/worker.js +++ b/addons/api/addon/models/worker.js @@ -86,6 +86,7 @@ export default class WorkerModel extends GeneratedWorkerModel { 'The deduplicated union of the tags reported by the worker ' + 'from its configuration and any tags added through other means.\nOutput only.', readOnly: true, + emptyObjectIfMissing: true, }) canonical_tags; @@ -93,11 +94,13 @@ export default class WorkerModel extends GeneratedWorkerModel { description: "The tags set in the worker's configuration file.\nOutput only.", readOnly: true, + emptyObjectIfMissing: true, }) config_tags; @attr({ description: 'The api tags set for the worker.\nOutput only.', + emptyObjectIfMissing: true, }) api_tags; @@ -132,4 +135,15 @@ export default class WorkerModel extends GeneratedWorkerModel { adapterOptions: { method: 'set-worker-tags', apiTags }, }); } + + /** + * Method to remove api tags on the worker. + * @param {object} apiTags + * @return {Promise} + */ + removeApiTags(apiTags) { + return this.save({ + adapterOptions: { method: 'remove-worker-tags', apiTags }, + }); + } } diff --git a/addons/api/addon/serializers/application.js b/addons/api/addon/serializers/application.js index 2d693c362..a6f46e402 100644 --- a/addons/api/addon/serializers/application.js +++ b/addons/api/addon/serializers/application.js @@ -179,6 +179,8 @@ export default class ApplicationSerializer extends RESTSerializer { payload = structuredClone(payload); // Check for and normalize missing arrays payload = this.normalizeMissingArrays(store, primaryModelClass, payload); + // Check for and normalize missing objects + payload = this.normalizeMissingObjects(store, primaryModelClass, payload); // Setup a new payload data structure. const transformedPayload = {}; // Find the Ember-data-expected root key name. @@ -199,7 +201,7 @@ export default class ApplicationSerializer extends RESTSerializer { /** * Resets missing array fields to an empty array if they are annotated by - * `normalizeToEmptyArray: true` in the associated model attribute + * `emptyArrayIfMissing: true` in the associated model attribute * declaration. Our API excludes arrays when they are empty from * singular responses. * @@ -222,6 +224,30 @@ export default class ApplicationSerializer extends RESTSerializer { return payload; } + /** + * Resets missing object fields to an empty object if they are annotated by + * `emptyObjectIfMissing: true` in the associated model attribute + * declaration. + * + * @param {Store} store + * @param {Model} primaryModelClass + * @param {object} payload + * @return {object} + */ + normalizeMissingObjects(store, primaryModelClass, payload) { + const attrDefs = store + .getSchemaDefinitionService() + .attributesDefinitionFor({ type: primaryModelClass.modelName }); + if (attrDefs) { + Object.keys(attrDefs).forEach((key) => { + if (!payload[key] && attrDefs[key]?.options?.emptyObjectIfMissing) { + payload[key] = {}; + } + }); + } + return payload; + } + /** * Hydrates the incoming scope JSON with additional boolean fields and * delegates nested attributes and secrets normalization. diff --git a/addons/api/mirage/config.js b/addons/api/mirage/config.js index e79d1358e..eda9e45d5 100644 --- a/addons/api/mirage/config.js +++ b/addons/api/mirage/config.js @@ -689,13 +689,23 @@ function routes() { const id = idMethod.split(':')[0]; const method = idMethod.split(':')[1]; const worker = workers.find(id); - let updatedAttrs = {}; + let updatedAttrs = { + version: attrs.version, + api_tags: worker.api_tags, + }; + + if (method === 'remove-worker-tags') { + const key = Object.keys(attrs.apiTags)[0]; + const value = attrs.apiTags[key][0]; + updatedAttrs.api_tags[key] = updatedAttrs.api_tags[key].filter( + (tag) => { + return tag !== value; + }, + ); + } if (method === 'set-worker-tags') { - updatedAttrs = { - version: attrs.version, - apiTags: attrs.apiTags, - }; + updatedAttrs.api_tags = attrs.apiTags; } return worker.update(updatedAttrs); diff --git a/addons/api/mirage/factories/worker.js b/addons/api/mirage/factories/worker.js index 0c711b20a..4dc9851d3 100644 --- a/addons/api/mirage/factories/worker.js +++ b/addons/api/mirage/factories/worker.js @@ -17,6 +17,7 @@ export default factory.extend({ 'update', 'delete', 'set-worker-tags', + 'remove-worker-tags', ], config_tags: (i) => { if (i % 3 === 0) { diff --git a/addons/api/mirage/serializers/worker.js b/addons/api/mirage/serializers/worker.js index e73d35706..4cd7c8795 100644 --- a/addons/api/mirage/serializers/worker.js +++ b/addons/api/mirage/serializers/worker.js @@ -7,4 +7,21 @@ import ApplicationSerializer from './application'; export default ApplicationSerializer.extend({ modelName: 'worker', + + _hashForModel(model) { + const json = ApplicationSerializer.prototype._hashForModel.apply( + this, + arguments, + ); + if (json.api_tags) { + const keys = Object.keys(model.api_tags); + keys.forEach((key) => { + if (json.api_tags[key]?.length === 0) { + delete json.api_tags[key]; + } + }); + } + + return json; + }, }); diff --git a/addons/api/tests/unit/models/worker-test.js b/addons/api/tests/unit/models/worker-test.js index 535d18df8..b6bc498a3 100644 --- a/addons/api/tests/unit/models/worker-test.js +++ b/addons/api/tests/unit/models/worker-test.js @@ -71,6 +71,38 @@ module('Unit | Model | worker', function (hooks) { const model = store.peekRecord('worker', workerId); await model.setApiTags(tags); }); + + test('it has a `removeApiTags` method that targets a specific POST API', async function (assert) { + assert.expect(1); + const workerId = 'w_123'; + const tags = { + tag1: ['value1'], + }; + this.server.post( + `/workers/${workerId}:remove-worker-tags`, + (schema, request) => { + const body = JSON.parse(request.requestBody); + assert.deepEqual(body, { api_tags: tags, version: 1 }); + return { id: workerId }; + }, + ); + + const store = this.owner.lookup('service:store'); + store.push({ + data: { + id: workerId, + type: 'worker', + attributes: { + name: 'fake worker', + version: 1, + api_tags: tags, + }, + }, + }); + const model = store.peekRecord('worker', workerId); + + await model.removeApiTags(tags); + }); test('it has a `configTagList` method that returns an array of key/value pair objects', function (assert) { const store = this.owner.lookup('service:store'); diff --git a/addons/api/tests/unit/serializers/worker-test.js b/addons/api/tests/unit/serializers/worker-test.js index 487fdd2d2..ab4f343f6 100644 --- a/addons/api/tests/unit/serializers/worker-test.js +++ b/addons/api/tests/unit/serializers/worker-test.js @@ -71,4 +71,45 @@ module('Unit | Serializer | worker', function (hooks) { api_tags: { key: ['value'] }, }); }); + + test('it normalizes missing `api_tags`, `config_tags`, and `canonical_tags`', function (assert) { + const store = this.owner.lookup('service:store'); + const serializer = store.serializerFor('worker'); + const worker = store.createRecord('worker').constructor; + const payload = { + id: 'w_123', + name: 'hardest worker', + scope: { id: 'global' }, + version: 1, + address: 'localhost', + }; + + const normalized = serializer.normalizeSingleResponse( + store, + worker, + payload, + ); + + assert.deepEqual(normalized, { + data: { + attributes: { + address: 'localhost', + api_tags: {}, + authorized_actions: [], + canonical_tags: {}, + config_tags: {}, + name: 'hardest worker', + scope: { + id: 'global', + scope_id: 'global', + }, + version: 1, + }, + id: 'w_123', + relationships: {}, + type: 'worker', + }, + included: [], + }); + }); }); diff --git a/addons/core/translations/actions/en-us.yaml b/addons/core/translations/actions/en-us.yaml index ec4783cc6..6ab0b3bbe 100644 --- a/addons/core/translations/actions/en-us.yaml +++ b/addons/core/translations/actions/en-us.yaml @@ -59,3 +59,4 @@ search: Search yes: Yes no: No narrow-results: Narrow results +overflow-options: Overflow Options diff --git a/addons/core/translations/resources/en-us.yaml b/addons/core/translations/resources/en-us.yaml index db18b2531..b69a8ff2c 100644 --- a/addons/core/translations/resources/en-us.yaml +++ b/addons/core/translations/resources/en-us.yaml @@ -999,6 +999,15 @@ 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. + remove: + title: Remove worker tag? + success: Tag successfully removed + warning: will be removed but removing these may affect certain targets + confirm: + label: Confirm removal + help: 'Enter “REMOVE” to confirm — this is case-sensitive.' + actions: + remove: Remove Tag flyout: title: Tags in {workerName} action: 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 af19dd0b7..a20aa07ba 100644 --- a/ui/admin/app/controllers/scopes/scope/workers/worker/tags.js +++ b/ui/admin/app/controllers/scopes/scope/workers/worker/tags.js @@ -4,10 +4,70 @@ */ import Controller, { inject as controller } from '@ember/controller'; -import { TAG_TYPE_CONFIG } from 'api/models/worker'; +import { inject as service } from '@ember/service'; +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'; export default class ScopesScopeWorkersWorkerTagsController extends Controller { @controller('scopes/scope/workers/index') workers; + // =services + @service intl; + + // =attributes + tagTypeConfig = TAG_TYPE_CONFIG; + tagTypeApi = TAG_TYPE_API; + + @tracked removeModal = false; + @tracked removalConfirmation = null; + @tracked modalTag = null; + + /** + * Build the display name for a worker tag + * @type {string} + */ + get tagDisplayName() { + return `"${this.modalTag?.key} = ${this.modalTag?.value}"`; + } + + /** + * Checks if user entered REMOVE to enable the remove button. + * @type {boolean} + */ + get isRemovalConfirmed() { + return ( + this.removalConfirmation === this.intl.t('actions.remove').toUpperCase() + ); + } + + /** + * Removes selected tag from the worker. + */ + @action + @loading + @notifyError(({ message }) => message, { catch: true }) + @notifySuccess('resources.worker.tags.remove.success') + async removeApiTag() { + await this.model.removeApiTags({ + [this.modalTag.key]: [this.modalTag.value], + }); + this.removeModal = false; + this.modalTag = null; + this.removalConfirmation = null; + } + + /** + * Opens the remove tag modal. + * @param {object} apiTag + */ + @action + toggleRemoveModal(apiTag) { + this.removeModal = !this.removeModal; + this.modalTag = apiTag; + this.removalConfirmation = null; + } } diff --git a/ui/admin/app/styles/app.scss b/ui/admin/app/styles/app.scss index 616fae50b..dac87638a 100644 --- a/ui/admin/app/styles/app.scss +++ b/ui/admin/app/styles/app.scss @@ -1057,3 +1057,14 @@ overflow: hidden; text-overflow: ellipsis; } + +.confirmation-modal { + .hds-button-set { + margin-bottom: 0; + } + + .hds-form-field--layout-vertical { + margin-top: 1.5rem; + margin-bottom: 0; + } +} diff --git a/ui/admin/app/templates/scopes/scope/storage-buckets/index.hbs b/ui/admin/app/templates/scopes/scope/storage-buckets/index.hbs index 12abbf978..058e2ea2d 100644 --- a/ui/admin/app/templates/scopes/scope/storage-buckets/index.hbs +++ b/ui/admin/app/templates/scopes/scope/storage-buckets/index.hbs @@ -101,7 +101,7 @@ 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 8d01950bf..f9aaca3fe 100644 --- a/ui/admin/app/templates/scopes/scope/workers/worker/tags.hbs +++ b/ui/admin/app/templates/scopes/scope/workers/worker/tags.hbs @@ -38,6 +38,7 @@ (hash label=(t 'resources.worker.table.key')) (hash label=(t 'resources.worker.table.value')) (hash label=(t 'resources.worker.table.type')) + (hash label=(t 'titles.actions') isVisuallyHidden=true width='60px') }} @valign='middle' > @@ -61,11 +62,78 @@ {{/if}} + + {{#if (eq B.data.type this.tagTypeApi)}} + + + {{#if (can 'removeTags worker' @model)}} + + {{/if}} + + {{/if}} + {{/if}} + {{#if this.removeModal}} + + + {{t 'resources.worker.tags.remove.title'}} + + + + {{this.tagDisplayName}} + {{t 'resources.worker.tags.remove.warning'}} + + {{! template-lint-disable no-autofocus-attribute }} + + {{t + 'resources.worker.tags.remove.confirm.label' + }} + {{t + 'resources.worker.tags.remove.confirm.help' + }} + + + + + + + + + + {{/if}} + \ No newline at end of file diff --git a/ui/admin/tests/acceptance/workers/tags-test.js b/ui/admin/tests/acceptance/workers/tags-test.js index fc9e2a12b..d08587b8b 100644 --- a/ui/admin/tests/acceptance/workers/tags-test.js +++ b/ui/admin/tests/acceptance/workers/tags-test.js @@ -4,7 +4,7 @@ */ import { module, test } from 'qunit'; -import { visit, currentURL, click, focus } from '@ember/test-helpers'; +import { visit, currentURL, click, focus, fillIn } 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'; @@ -18,6 +18,15 @@ 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_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 CONFIRMATION_MODAL_REMOVE_BUTTON_SELECTOR = + '.confirmation-modal button:first-child'; + const CONFIRMATION_MODAL_CANCEL_BUTTON_SELECTOR = + '.confirmation-modal button:last-child'; const instances = { scopes: { @@ -61,4 +70,50 @@ module('Acceptance | workers | worker | tags', function (hooks) { .dom(CONFIG_TAG_TOOLTIP_TEXT_SELECTOR) .hasText('To edit config tags, edit them in your worker config file.'); }); + + test('users can remove a specific tag', async function (assert) { + await visit(urls.tags); + + assert.dom('tbody tr').exists({ count: 11 }); + + await click(API_TAG_ACTION_SELECTOR); + await click(API_TAG_REMOVE_ACTION_SELECTOR); + + assert.dom(CONFIRMATION_MODAL_REMOVE_BUTTON_SELECTOR).isDisabled(); + + await fillIn(CONFIRMATION_MODAL_INPUT_SELECTOR, 'REMOVE'); + + assert.dom(CONFIRMATION_MODAL_REMOVE_BUTTON_SELECTOR).isEnabled(); + + await click(CONFIRMATION_MODAL_REMOVE_BUTTON_SELECTOR); + + assert.dom('tbody tr').exists({ count: 10 }); + }); + + test('user can cancel tag removal', async function (assert) { + await visit(urls.tags); + + assert.dom('tbody tr').exists({ count: 11 }); + + await click(API_TAG_ACTION_SELECTOR); + await click(API_TAG_REMOVE_ACTION_SELECTOR); + + await click(CONFIRMATION_MODAL_CANCEL_BUTTON_SELECTOR); + + assert.dom('tbody tr').exists({ count: 11 }); + }); + + test('user cannot remove a tag without proper permission', async function (assert) { + instances.worker.authorized_actions = + instances.worker.authorized_actions.filter( + (item) => item !== 'remove-worker-tags', + ); + await visit(urls.tags); + + assert.dom('tbody tr').exists({ count: 11 }); + + await click(API_TAG_ACTION_SELECTOR); + + assert.dom(API_TAG_REMOVE_ACTION_SELECTOR).doesNotExist(); + }); }); 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 16d60750d..059462255 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 @@ -4,18 +4,94 @@ */ import { module, test } from 'qunit'; -import { setupTest } from 'admin/tests/helpers'; +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'; module( 'Unit | Controller | scopes/scope/workers/worker/tags', function (hooks) { setupTest(hooks); + setupMirage(hooks); - test('it exists', function (assert) { - let controller = this.owner.lookup( + let store; + let controller; + + const instances = { + scopes: { + global: null, + }, + worker: null, + }; + + const urls = { + workers: null, + tags: null, + }; + + hooks.beforeEach(function () { + authenticateSession({}); + store = this.owner.lookup('service:store'); + controller = this.owner.lookup( 'controller:scopes/scope/workers/worker/tags', ); + + 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'] }, + }); + urls.workers = `/scopes/global/workers`; + urls.tags = `${urls.workers}/${instances.worker.id}/tags`; + }); + + test('it exists', function (assert) { assert.ok(controller); }); + + test('tagDisplayName returns the display name of the tag', function (assert) { + controller.modalTag = { key: 'test', value: 'test' }; + + assert.strictEqual(controller.tagDisplayName, '"test = test"'); + }); + + test('isRemovalConfirmed returns true if the user has entered REMOVE', function (assert) { + controller.removalConfirmation = 'REMOVE'; + + assert.true(controller.isRemovalConfirmed); + }); + + test('isRemovalConfirmed returns false if the user has entered anything but REMOVE', function (assert) { + controller.removalConfirmation = 'remove'; + + assert.false(controller.isRemovalConfirmed); + }); + + 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); + const apiTagCount = worker.apiTagList.length; + controller.modalTag = { key: 'test', value: 'test' }; + + await controller.removeApiTag(); + + assert.strictEqual(worker.apiTagList.length, apiTagCount - 1); + assert.false(controller.removeModal); + assert.strictEqual(controller.modalTag, null); + }); + + test('toggleRemoveModal toggles the modal and sets the modalTag', function (assert) { + assert.false(controller.removeModal); + assert.strictEqual(controller.modalTag, null); + + controller.toggleRemoveModal({ key: 'test', value: 'test' }); + + assert.true(controller.removeModal); + assert.deepEqual(controller.modalTag, { + key: 'test', + value: 'test', + }); + }); }, );