From 5a74220f382e5bfbf6bc8551851be3b95d7d01d2 Mon Sep 17 00:00:00 2001 From: Dharini Jeeva Date: Fri, 19 Jan 2024 14:06:51 -0800 Subject: [PATCH] =?UTF-8?q?fix:=20=F0=9F=90=9B=20policy=20UI=20bug=20fix?= =?UTF-8?q?=20(#2098)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: 🐛 policy UI bug fix --- addons/core/translations/resources/en-us.yaml | 3 +- ui/admin/app/components/form/policy/index.js | 4 + .../form/policy/policy-selection/index.hbs | 10 +- .../form/policy/policy-selection/index.js | 53 +++---- .../form/scope/add-storage-policy/create.hbs | 4 +- ui/admin/app/routes/scopes.js | 2 +- .../scopes/scope/add-storage-policy/create.js | 3 +- ui/admin/app/styles/app.scss | 4 +- .../scope/add-storage-policy/create.hbs | 4 +- ui/admin/app/templates/scopes/scope/edit.hbs | 2 +- .../templates/scopes/scope/policies/index.hbs | 5 +- .../scopes/scope/policies/policy/index.hbs | 7 +- .../tests/acceptance/policy/create-test.js | 59 ++++++- .../tests/acceptance/policy/delete-test.js | 89 +++++++++++ .../tests/acceptance/policy/update-test.js | 150 ++++++++++++++++++ 15 files changed, 341 insertions(+), 58 deletions(-) create mode 100644 ui/admin/tests/acceptance/policy/delete-test.js create mode 100644 ui/admin/tests/acceptance/policy/update-test.js diff --git a/addons/core/translations/resources/en-us.yaml b/addons/core/translations/resources/en-us.yaml index 28ad32546..4c3f84961 100644 --- a/addons/core/translations/resources/en-us.yaml +++ b/addons/core/translations/resources/en-us.yaml @@ -927,6 +927,7 @@ worker: policy: title: Storage Policy title_plural: Storage Policies + description: A Storage Policy defines how long Session Recordings must be kept and when they should be deleted. titles: new: New Storage Policy policy_name: Policy name @@ -951,7 +952,7 @@ policy: reapply: Successfully reapplied none: title: No storage policies yet - description: Storage policies define how long to store recordings before they are automatically deleted. We recommend checking your data retention policies for compliance requirements. + description: A Storage Policy defines how long Session Recordings must be kept and when they should be deleted. We recommend checking your data retention policies for compliance requirements. link: Create a new storage policy form: retention_policy: diff --git a/ui/admin/app/components/form/policy/index.js b/ui/admin/app/components/form/policy/index.js index deb87acfe..2e2ab8fc5 100644 --- a/ui/admin/app/components/form/policy/index.js +++ b/ui/admin/app/components/form/policy/index.js @@ -43,9 +43,13 @@ export default class FormPolicyComponent extends Component { /** * Returns policy type + * API doesn't return days for do_not_protect, + * so we set option to do_not_protect when there's no retain days * @type {string} */ get selectRetentionPolicyType() { + if (!this.args.model.retain_for?.days) return 'do_not_protect'; + const val = Object.keys(RETENTION_POLICY).find( (i) => RETENTION_POLICY[i] === this.args.model.retain_for?.days, ); diff --git a/ui/admin/app/components/form/policy/policy-selection/index.hbs b/ui/admin/app/components/form/policy/policy-selection/index.hbs index 60da1b7a2..e99eceec9 100644 --- a/ui/admin/app/components/form/policy/policy-selection/index.hbs +++ b/ui/admin/app/components/form/policy/policy-selection/index.hbs @@ -15,6 +15,7 @@ name={{@name}} disabled={{or @disabled this.isDeleteDisable}} aria-label={{t (concat 'resources.policy.form.' @name '.label')}} + data-select={{@name}} @width='70%' {{on 'change' this.handlePolicyTypeSelection}} as |F| @@ -35,9 +36,10 @@ {{#if @model.scope.isGlobal}} @@ -53,12 +55,12 @@ = 0 - ) { - return this.isCustomDeletionSelected; - } else { - return null; - } + return this.args.selectedOption === 'custom'; } /** - * Returns true if rentention days are greater than 0 + * Disable toggle when the form is disabled and when the retain/delete after days are 0 * @type {boolean} */ - get isCustomRetentionSelected() { - //no need to show custom input for the below days - const arr = [-1, 0, 2555, 2190]; - return arr.includes(this.args.model.retain_for?.days) ? false : true; + get toggleDisabled() { + return ( + this.args.disabled || !this.args.model[this.args.customInputName]?.days + ); } - /** - * Returns true if deletion days are greater than 0 - * @type {boolean} - */ - get isCustomDeletionSelected() { - return this.args.model.delete_after?.days > 0; - } /** * Returns true if the toggle is on * @type {boolean} */ get isOverridable() { - return this.args.model[this.args.customInputName]?.overridable; + return ( + this.args.model[this.args.customInputName]?.days && + this.args.model[this.args.customInputName]?.overridable + ); } get isDeleteDisable() { - if ( + return ( this.args.name === 'deletion_policy' && this.args.model.retain_for?.days === -1 - ) { - return true; - } else { - return false; - } + ); } //actions /** @@ -67,11 +50,13 @@ export default class FormPolicySelectionComponent extends Component { @action handleInputChange({ target: { value, name: field } }) { // Select options return type is string and we want the `days` in integer format - const val = Number(value); - this.args.model[field] = { - ...this.args.model[field], - days: val, - }; + if (value) { + const val = Number(value); + this.args.model[field] = { + ...this.args.model[field], + days: val, + }; + } } /** diff --git a/ui/admin/app/components/form/scope/add-storage-policy/create.hbs b/ui/admin/app/components/form/scope/add-storage-policy/create.hbs index e5709b1c2..71e5c3ae3 100644 --- a/ui/admin/app/components/form/scope/add-storage-policy/create.hbs +++ b/ui/admin/app/components/form/scope/add-storage-policy/create.hbs @@ -15,9 +15,7 @@ - - {{t 'resources.policy.titles.new'}} - +

{{t 'resources.policy.titles.new'}}

 diff --git a/ui/admin/app/routes/scopes.js b/ui/admin/app/routes/scopes.js index 589106584..6454ee690 100644 --- a/ui/admin/app/routes/scopes.js +++ b/ui/admin/app/routes/scopes.js @@ -49,7 +49,7 @@ export default class ScopesRoute extends Route { @notifySuccess('resources.policy.messages.detach') async detachStoragePolicy(scope) { const { storage_policy_id } = scope; - await scope.detachStoragePolicy(storage_policy_id); scope.storage_policy_id = ''; + await scope.detachStoragePolicy(storage_policy_id); } } diff --git a/ui/admin/app/routes/scopes/scope/add-storage-policy/create.js b/ui/admin/app/routes/scopes/scope/add-storage-policy/create.js index dbdbe80be..c460cdc8c 100644 --- a/ui/admin/app/routes/scopes/scope/add-storage-policy/create.js +++ b/ui/admin/app/routes/scopes/scope/add-storage-policy/create.js @@ -22,7 +22,8 @@ export default class ScopesScopeAddStoragePolicyCreateRoute extends Route { * @return {Policy} */ model() { - const scopeModel = this.store.peekRecord('scope', 'global'); + const { id: scope_id } = this.modelFor('scopes.scope'); + const scopeModel = this.store.peekRecord('scope', scope_id); const record = this.store.createRecord('policy', { type: TYPE_POLICY, }); diff --git a/ui/admin/app/styles/app.scss b/ui/admin/app/styles/app.scss index 7db2a85c6..4809d8e26 100644 --- a/ui/admin/app/styles/app.scss +++ b/ui/admin/app/styles/app.scss @@ -1005,8 +1005,8 @@ .policy-sidebar { background-color: var(--token-form-control-disabled-surface-color); - padding: 1rem; - width: 380px; + padding: 1.5rem 1rem 0.7rem; + width: 300px; .hds-card__container > ul { display: flex; diff --git a/ui/admin/app/templates/scopes/scope/add-storage-policy/create.hbs b/ui/admin/app/templates/scopes/scope/add-storage-policy/create.hbs index e5709b1c2..b0b3e0024 100644 --- a/ui/admin/app/templates/scopes/scope/add-storage-policy/create.hbs +++ b/ui/admin/app/templates/scopes/scope/add-storage-policy/create.hbs @@ -15,9 +15,7 @@ - - {{t 'resources.policy.titles.new'}} - +

{{t 'resources.policy.titles.new'}}

 diff --git a/ui/admin/app/templates/scopes/scope/edit.hbs b/ui/admin/app/templates/scopes/scope/edit.hbs index 67ee9a020..8b8a9e12d 100644 --- a/ui/admin/app/templates/scopes/scope/edit.hbs +++ b/ui/admin/app/templates/scopes/scope/edit.hbs @@ -122,7 +122,7 @@ {{#if this.storage_policy.retain_for.days}}

- {{t 'resources.policy.title_plural'}} - {{! TODO: Add doc link here }} + {{t 'resources.policy.title'}} + {{! TODO: ADD DOC LINK }}

+

{{t 'resources.policy.description'}}

{{#if (can 'create model' this.scope collection='policies')}} diff --git a/ui/admin/app/templates/scopes/scope/policies/policy/index.hbs b/ui/admin/app/templates/scopes/scope/policies/policy/index.hbs index a0885e036..d5ee257a6 100644 --- a/ui/admin/app/templates/scopes/scope/policies/policy/index.hbs +++ b/ui/admin/app/templates/scopes/scope/policies/policy/index.hbs @@ -9,10 +9,11 @@ - +

{{t 'resources.policy.title'}} - - + {{! TODO: ADD DOC LINK }} +

+

{{t 'resources.policy.description'}}

 diff --git a/ui/admin/tests/acceptance/policy/create-test.js b/ui/admin/tests/acceptance/policy/create-test.js index dee3803c7..a131d9dff 100644 --- a/ui/admin/tests/acceptance/policy/create-test.js +++ b/ui/admin/tests/acceptance/policy/create-test.js @@ -4,7 +4,7 @@ */ import { module, test } from 'qunit'; -import { visit, currentURL, click, fillIn } from '@ember/test-helpers'; +import { visit, currentURL, click, fillIn, select } from '@ember/test-helpers'; import { setupApplicationTest } from 'admin/tests/helpers'; import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; import a11yAudit from 'ember-a11y-testing/test-support/audit'; @@ -21,10 +21,15 @@ module('Acceptance | policies | create', function (hooks) { const SAVE_BTN_SELECTOR = '[type="submit"]'; const CANCEL_BTN_SELECTOR = '.rose-form-actions [type="button"]'; const NAME_FIELD_SELECTOR = '[name="name"]'; - const RETAIN_FOR_INPUT = 'input[data-input-type="retain_for"]'; + const RETAIN_FOR_TEXT_INPUT = '[data-input="retain_for"]'; const ALERT_TEXT_SELECTOR = '[role="alert"] div'; const FIELD_ERROR_TEXT_SELECTOR = '.hds-form-error__message'; const NAME_FIELD_TEXT = 'random string'; + const DELETE_AFTER_TEXT_INPUT = '[data-input="delete_after"]'; + const RETAIN_OVERRIDE = '[data-toggle="retain_for"]'; + const DELETE_OVERRIDE = '[data-toggle="delete_after"]'; + const RETAIN_SELECT_LIST = '[data-select="retention_policy"]'; + const DELETE_SELECT_LIST = '[data-select="deletion_policy"]'; const instances = { scopes: { @@ -63,7 +68,7 @@ module('Acceptance | policies | create', function (hooks) { await click('.hds-form-select'); await fillIn('.hds-form-select', 1); - await fillIn(RETAIN_FOR_INPUT, 90); + await fillIn(RETAIN_FOR_TEXT_INPUT, 90); await click(SAVE_BTN_SELECTOR); const policy = this.server.schema.policies.findBy({ name: NAME_FIELD_TEXT, @@ -75,6 +80,54 @@ module('Acceptance | policies | create', function (hooks) { assert.strictEqual(getPolicyCount(), policyCount + 1); }); + test('delete policy is automatically disabled when forever retention policy is chosen', async function (assert) { + await visit(urls.policies); + await click(`[href="${urls.newPolicy}"]`); + await select(RETAIN_SELECT_LIST, '-1'); + assert.dom(DELETE_SELECT_LIST).isDisabled(); + }); + + test('user can enter custom value when custom retain option is selected', async function (assert) { + await visit(urls.policies); + await click(`[href="${urls.newPolicy}"]`); + await select(RETAIN_SELECT_LIST, '1'); + assert.dom(RETAIN_FOR_TEXT_INPUT).isVisible(); + }); + + test('user can enter custom value when custom delete option is selected', async function (assert) { + await visit(urls.policies); + await click(`[href="${urls.newPolicy}"]`); + await select(DELETE_SELECT_LIST, '1'); + assert.dom(DELETE_AFTER_TEXT_INPUT).isVisible(); + }); + test('user can select SOC option and will not see custom input', async function (assert) { + await visit(urls.policies); + await click(`[href="${urls.newPolicy}"]`); + await select(RETAIN_SELECT_LIST, '2555'); + assert.dom(RETAIN_FOR_TEXT_INPUT).isNotVisible(); + }); + + test('user can select do_not_delete option and will not see custom input', async function (assert) { + await visit(urls.policies); + await click(`[href="${urls.newPolicy}"]`); + await select(DELETE_SELECT_LIST, '0'); + assert.dom(DELETE_AFTER_TEXT_INPUT).isNotVisible(); + }); + + test('org override is disabled when do_not_protect is selected', async function (assert) { + await visit(urls.policies); + await click(`[href="${urls.newPolicy}"]`); + await select(RETAIN_SELECT_LIST, '0'); + assert.dom(RETAIN_OVERRIDE).isDisabled(); + }); + + test('org override is disabled when do_not_delete is selected', async function (assert) { + await visit(urls.policies); + await click(`[href="${urls.newPolicy}"]`); + await select(DELETE_SELECT_LIST, '0'); + assert.dom(DELETE_OVERRIDE).isDisabled(); + }); + test('user can cancel new policy creation', async function (assert) { const policyCount = getPolicyCount(); await visit(urls.policies); diff --git a/ui/admin/tests/acceptance/policy/delete-test.js b/ui/admin/tests/acceptance/policy/delete-test.js new file mode 100644 index 000000000..7e1f14b23 --- /dev/null +++ b/ui/admin/tests/acceptance/policy/delete-test.js @@ -0,0 +1,89 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: MPL-2.0 + */ + +import { module, test } from 'qunit'; +import { visit, click } from '@ember/test-helpers'; +import { setupApplicationTest } from 'admin/tests/helpers'; +import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; +import { authenticateSession } from 'ember-simple-auth/test-support'; + +module('Acceptance | policies | delete', function (hooks) { + setupApplicationTest(hooks); + setupMirage(hooks); + + let featuresService; + let policyCount; + + const DROPDOWN_BUTTON_SELECTOR = '.hds-dropdown-toggle-icon'; + const DROPDOWN_ITEM_SELECTOR = '.hds-dropdown-list-item button'; + + const instances = { + scopes: { + global: null, + org: null, + }, + policy: null, + }; + + const urls = { + globalScope: null, + policies: null, + policy: null, + }; + + hooks.beforeEach(function () { + authenticateSession({}); + instances.scopes.global = this.server.create('scope', { id: 'global' }); + instances.scopes.org = this.server.create('scope', { + type: 'org', + scope: { id: 'global', type: 'global' }, + }); + + instances.policy = this.server.create('policy', { + scope: instances.scopes.global, + }); + + urls.globalScope = `/scopes/global`; + urls.policies = `${urls.globalScope}/policies`; + + featuresService = this.owner.lookup('service:features'); + policyCount = () => this.server.schema.policies.all().models.length; + }); + + test('users can delete a storage policy', async function (assert) { + const count = policyCount(); + featuresService.enable('ssh-session-recording'); + + assert.true(instances.policy.authorized_actions.includes('delete')); + await visit(urls.globalScope); + + urls.policy = `${urls.policies}/${instances.policy.id}`; + + await click(`[href="${urls.policies}"]`); + await click(DROPDOWN_BUTTON_SELECTOR); + + assert.dom(DROPDOWN_ITEM_SELECTOR).exists(); + assert.dom(DROPDOWN_ITEM_SELECTOR).hasText('Delete Storage Policy'); + + await click(DROPDOWN_ITEM_SELECTOR); + assert.strictEqual(policyCount(), count - 1); + }); + + test('users cannot delete a storage policy without proper authorization', async function (assert) { + featuresService.enable('ssh-session-recording'); + + instances.policy.authorized_actions = + instances.policy.authorized_actions.filter((item) => item !== 'delete'); + + await visit(urls.globalScope); + + urls.policy = `${urls.policies}/${instances.policy.id}`; + + await click(`[href="${urls.policies}"]`); + await click(DROPDOWN_BUTTON_SELECTOR); + + assert.dom(DROPDOWN_ITEM_SELECTOR).isNotVisible(); + }); +}); diff --git a/ui/admin/tests/acceptance/policy/update-test.js b/ui/admin/tests/acceptance/policy/update-test.js new file mode 100644 index 000000000..fcb3c2f60 --- /dev/null +++ b/ui/admin/tests/acceptance/policy/update-test.js @@ -0,0 +1,150 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: MPL-2.0 + */ + +import { module, test } from 'qunit'; +import { visit, click, fillIn, select } from '@ember/test-helpers'; +import { setupApplicationTest } from 'admin/tests/helpers'; +import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; + +import { authenticateSession } from 'ember-simple-auth/test-support'; + +module('Acceptance | policies | update', function (hooks) { + setupApplicationTest(hooks); + setupMirage(hooks); + + let features; + + const SAVE_BTN_SELECTOR = '[type="submit"]'; + + const NAME_FIELD_SELECTOR = '[name="name"]'; + const RETAIN_FOR_TEXT_INPUT = '[data-input="retain_for"]'; + + const NAME_FIELD_TEXT = 'random string'; + const DELETE_AFTER_TEXT_INPUT = '[data-input="delete_after"]'; + const DELETE_OVERRIDE = '[data-toggle="delete_after"]'; + const RETAIN_SELECT_LIST = '[data-select="retention_policy"]'; + const DELETE_SELECT_LIST = '[data-select="deletion_policy"]'; + + const BUTTON_SELECTOR = '.rose-form-actions [type="button"]'; + + const instances = { + scopes: { + global: null, + }, + storageBucket: null, + }; + + const urls = { + globalScope: null, + policies: null, + newPolicy: null, + policy: null, + }; + + hooks.beforeEach(function () { + instances.scopes.global = this.server.create('scope', { id: 'global' }); + + instances.policy = this.server.create('policy', { + scope: instances.scopes.global, + }); + urls.globalScope = `/scopes/global`; + urls.policies = `${urls.globalScope}/policies`; + urls.newPolicy = `${urls.policies}/new`; + urls.policy = `${urls.policies}/${instances.policy.id}`; + + features = this.owner.lookup('service:features'); + features.enable('ssh-session-recording'); + authenticateSession({}); + }); + + test('users can update forever select option to a custom input', async function (assert) { + await visit(urls.policies); + await click(`[href="${urls.policy}"]`); + await click(BUTTON_SELECTOR, 'Click edit mode'); + await select(RETAIN_SELECT_LIST, '-1'); + await click(SAVE_BTN_SELECTOR); + assert.strictEqual( + this.server.schema.policies.first().attributes.retain_for.days, + -1, + ); + await click(BUTTON_SELECTOR, 'Click edit mode'); + await select(RETAIN_SELECT_LIST, '1'); + await fillIn(RETAIN_FOR_TEXT_INPUT, 100); + + await click(SAVE_BTN_SELECTOR); + assert.strictEqual( + this.server.schema.policies.first().attributes.retain_for.days, + 100, + ); + assert.dom(RETAIN_FOR_TEXT_INPUT).isVisible(); + }); + + test('users can update a custom input', async function (assert) { + await visit(urls.policies); + await click(`[href="${urls.policy}"]`); + await click(BUTTON_SELECTOR, 'Click edit mode'); + await select(RETAIN_SELECT_LIST, '1'); + await fillIn(RETAIN_FOR_TEXT_INPUT, 100); + + await click(SAVE_BTN_SELECTOR); + assert.strictEqual( + this.server.schema.policies.first().attributes.retain_for.days, + 100, + ); + await click(BUTTON_SELECTOR, 'Click edit mode'); + await select(RETAIN_SELECT_LIST, '1'); + await fillIn(RETAIN_FOR_TEXT_INPUT, 10); + + await click(SAVE_BTN_SELECTOR); + assert.strictEqual( + this.server.schema.policies.first().attributes.retain_for.days, + 10, + ); + assert.dom(RETAIN_FOR_TEXT_INPUT).isVisible(); + }); + + test('users can update from do_not_delete to a custom input', async function (assert) { + await visit(urls.policies); + await click(`[href="${urls.policy}"]`); + await click(BUTTON_SELECTOR, 'Click edit mode'); + await select(DELETE_SELECT_LIST, '0'); + await click(SAVE_BTN_SELECTOR); + assert.strictEqual( + this.server.schema.policies.first().attributes.delete_after.days, + 0, + ); + await click(BUTTON_SELECTOR, 'Click edit mode'); + await select(DELETE_SELECT_LIST, '1'); + await fillIn(DELETE_AFTER_TEXT_INPUT, 100); + + await click(SAVE_BTN_SELECTOR); + assert.strictEqual( + this.server.schema.policies.first().attributes.delete_after.days, + 100, + ); + assert.dom(DELETE_AFTER_TEXT_INPUT).isVisible(); + }); + + test('users cannot update delete override when do_not_protect is selected', async function (assert) { + await visit(urls.policies); + await click(`[href="${urls.policy}"]`); + await click(BUTTON_SELECTOR, 'Click edit mode'); + await select(DELETE_SELECT_LIST, '0'); + assert.dom(DELETE_OVERRIDE).isDisabled(); + }); + + test('can cancel changes to an existing storage policy', async function (assert) { + const name = instances.policy.name; + await visit(urls.policies); + + await click(`[href="${urls.policy}"]`); + await click(BUTTON_SELECTOR, 'Click edit mode'); + await fillIn(NAME_FIELD_SELECTOR, NAME_FIELD_TEXT); + await click(BUTTON_SELECTOR, 'Click cancel'); + + assert.dom(NAME_FIELD_SELECTOR).hasValue(`${name}`); + assert.strictEqual(instances.policy.name, name); + }); +});