Icu 13897 create remove tag modal (#2377)

* feat: 🎸 add action dropdown in tags table

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* feat: 🎸 add removeApiTags method to worker

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* feat: 🎸 add tag removal workflow

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* test: 💍 add tests for removing a worker tag

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* test: 💍 add tests for removing a tag

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* refactor: 💡 fix spelling issue

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* refactor: 💡 use mirage worker serializer to remove empty tag

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* refactor: 💡 update modal id to class

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* refactor: 💡 add autofocus to remove tag modal

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* feat: 🎸 update tags controller attributes

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* refactor: 💡 create normalizer to add empty object for api_tags

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* refactor: 💡 update getter signature

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* refactor: 💡 remove unneeded parens

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* refactor: 💡 remove type from Hds::Button

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* refactor: 💡 update confirmation modal class name

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* feat: 🎸 add emptyObjectIfMissing model attribute

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* refactor: 💡 PR feedback

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

* refactor: 💡 remove default to null in toggleModal

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13897

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');
+  }
 }

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 },
+    });
+  }
 }

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.

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);

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) {

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;
+  },
 });

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');

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: [],
+    });
+  });
 });

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

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:

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;
+  }
 }

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;
+  }
+}

ui/admin/app/templates/scopes/scope/storage-buckets/index.hbs

@@ -101,7 +101,7 @@
                   <Hds::Dropdown as |dd|>
                     <dd.ToggleIcon
                       @icon='more-horizontal'
-                      @text='Overflow Options'
+                      @text={{t 'actions.overflow-options'}}
                       @hasChevron={{false}}
                       @size='small'
                     />

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 @@
                 <Hds::Badge @text={{B.data.type}} />
               {{/if}}
             </B.Td>
+            <B.Td>
+              {{#if (eq B.data.type this.tagTypeApi)}}
+                <Hds::Dropdown as |dd|>
+                  <dd.ToggleIcon
+                    @icon='more-horizontal'
+                    @text={{t 'actions.overflow-options'}}
+                    @hasChevron={{false}}
+                    @size='small'
+                  />
+                  {{#if (can 'removeTags worker' @model)}}
+                    <dd.Interactive
+                      @text={{t 'resources.worker.tags.actions.remove'}}
+                      {{on 'click' (fn this.toggleRemoveModal B.data)}}
+                    />
+                  {{/if}}
+                </Hds::Dropdown>
+              {{/if}}
+            </B.Td>
           </B.Tr>
         </:body>
       </Hds::Table>
     {{/if}}
 
+    {{#if this.removeModal}}
+      <Hds::Modal
+        class='confirmation-modal'
+        @color='critical'
+        @onClose={{this.toggleRemoveModal}}
+        as |M|
+      >
+        <M.Header @icon='alert-diamond'>
+          {{t 'resources.worker.tags.remove.title'}}
+        </M.Header>
+        <M.Body>
+          <Hds::Text::Body @color='primary'>
+            {{this.tagDisplayName}}
+            {{t 'resources.worker.tags.remove.warning'}}
+          </Hds::Text::Body>
+          {{! template-lint-disable no-autofocus-attribute }}
+          <Hds::Form::TextInput::Field
+            name='removal-confirmation'
+            autofocus={{true}}
+            @value={{this.removalConfirmation}}
+            {{on 'input' (set-from-event this 'removalConfirmation')}}
+            as |F|
+          >
+            <F.Label>{{t
+                'resources.worker.tags.remove.confirm.label'
+              }}</F.Label>
+            <F.HelperText>{{t
+                'resources.worker.tags.remove.confirm.help'
+              }}</F.HelperText>
+          </Hds::Form::TextInput::Field>
+        </M.Body>
+        <M.Footer as |F|>
+          <Hds::ButtonSet>
+            <Hds::Button
+              disabled={{not this.isRemovalConfirmed}}
+              @color='critical'
+              @text={{t 'actions.remove'}}
+              {{on 'click' this.removeApiTag}}
+            />
+            <Hds::Button
+              @color='secondary'
+              @text={{t 'actions.cancel'}}
+              {{on 'click' F.close}}
+            />
+          </Hds::ButtonSet>
+        </M.Footer>
+      </Hds::Modal>
+    {{/if}}
+
   </page.body>
 
 </Rose::Layout::Page>

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();
+  });
 });

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',
+      });
+    });
   },
 );