fix: 🐛 policy UI bug fix (#2098)

* fix: 🐛 policy UI bug fix

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:

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

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 @@
     </Hds::Form::Select::Field>
     {{#if @model.scope.isGlobal}}
       <Hds::Form::Toggle::Field
-        checked={{this.isOverridable}}
-        disabled={{@disabled}}
         name={{@customInputName}}
+        checked={{this.isOverridable}}
+        disabled={{this.toggleDisabled}}
+        data-toggle={{@customInputName}}
         {{on 'change' this.handleOverridableToggle}}
         as |F|
       >
@@ -53,12 +55,12 @@
     <F.Control>
       <Hds::SegmentedGroup as |S|>
         <S.TextInput
+          name={{@customInputName}}
           @value={{@inputValue}}
           disabled={{@disabled}}
           aria-label={{t 'resources.policy.title'}}
+          data-input={{@customInputName}}
           {{on 'input' this.handleInputChange}}
-          name={{@customInputName}}
-          data-input-type={{@customInputName}}
         />
         <S.Generic>
           <Hds::Badge

ui/admin/app/components/form/policy/policy-selection/index.js

@@ -13,52 +13,35 @@ export default class FormPolicySelectionComponent extends Component {
    * @type {boolean}
    */
   get showCustomInput() {
-    if (this.args.name === 'retention_policy') {
-      return this.isCustomRetentionSelected;
-    } else if (
-      this.args.name === 'deletion_policy' &&
-      this.args.model.retain_for?.days >= 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,
+      };
+    }
   }
 
   /**

ui/admin/app/components/form/scope/add-storage-policy/create.hbs

@@ -15,9 +15,7 @@
   </page.breadcrumbs>
 
   <page.header>
-    <Hds::Text::Display @tag='h2'>
-      {{t 'resources.policy.titles.new'}}
-    </Hds::Text::Display>
+    <h2>{{t 'resources.policy.titles.new'}}</h2>
   </page.header>
 
   <page.body>

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

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

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;

ui/admin/app/templates/scopes/scope/add-storage-policy/create.hbs

@@ -15,9 +15,7 @@
   </page.breadcrumbs>
 
   <page.header>
-    <Hds::Text::Display @tag='h2'>
-      {{t 'resources.policy.titles.new'}}
-    </Hds::Text::Display>
+    <h2> {{t 'resources.policy.titles.new'}}</h2>
   </page.header>
 
   <page.body>

ui/admin/app/templates/scopes/scope/edit.hbs

@@ -122,7 +122,7 @@
                 <P.Item
                   @icon='hourglass'
                   @color='boundary'
-                  @text={{this.storage_policy.name}}
+                  @text={{this.storage_policy.displayName}}
                 >
                   {{#if this.storage_policy.retain_for.days}}
                     <Hds::Text::Body

ui/admin/app/templates/scopes/scope/policies/index.hbs

@@ -9,9 +9,10 @@
 
   <page.header>
     <h2>
-      {{t 'resources.policy.title_plural'}}
-      {{! TODO: Add doc link here }}
+      {{t 'resources.policy.title'}}
+      {{! TODO: ADD DOC LINK }}
     </h2>
+    <p>{{t 'resources.policy.description'}}</p>
   </page.header>
   <page.actions>
     {{#if (can 'create model' this.scope collection='policies')}}

ui/admin/app/templates/scopes/scope/policies/policy/index.hbs

@@ -9,10 +9,11 @@
   </page.breadcrumbs>
 
   <page.header>
-    <Hds::Text::Display @tag='h2'>
+    <h2>
       {{t 'resources.policy.title'}}
-    </Hds::Text::Display>
-
+      {{! TODO: ADD DOC LINK }}
+    </h2>
+    <p>{{t 'resources.policy.description'}}</p>
     <Hds::Copy::Snippet @color='secondary' @textToCopy={{@model.id}} />
   </page.header>
 

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

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

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