Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disabling License Banners #19116

Merged
merged 20 commits into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/19116.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: Allows license-banners to be dismissed. Saves preferences in localStorage.
```
46 changes: 42 additions & 4 deletions ui/app/components/license-banners.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
import Component from '@glimmer/component';
import { action } from '@ember/object';
import { tracked } from '@glimmer/tracking';
import { inject as service } from '@ember/service';
import isAfter from 'date-fns/isAfter';
import differenceInDays from 'date-fns/differenceInDays';
import localStorage from 'vault/lib/local-storage';

/**
* @module LicenseBanners
* LicenseBanners components are used to display Vault-specific license expiry messages
Expand All @@ -9,11 +17,23 @@
* @param {string} expiry - RFC3339 date timestamp
*/

import Component from '@glimmer/component';
Copy link
Contributor Author

@Monkeychip Monkeychip Feb 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved the imports above documentation to match the component template

import isAfter from 'date-fns/isAfter';
import differenceInDays from 'date-fns/differenceInDays';

export default class LicenseBanners extends Component {
@service version;

@tracked warningDismissed;
@tracked expiredDismissed;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thought about this approach, is that localStorage will continue to store past version dismissals. Thinking through this, there really isn't a way for me to get the previous version so I'd have to do some string finding to remove a past version's (dismiss-license-banner-${this.currentVersion}). Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true. We could have a helper in the localStorage lib which goes through each of the localStorage values and matches on the first part of the string, then removes the matching ones

const relevantKeys = Object.keys(localStorage).filter(str => str.startsWith("dismiss-license-banner")
relevantKeys.forEach(key => {
localStorage.removeItem(key)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea. I'll add that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a method to lib/local-storage. Let me know what you think. (see row 51 here and the new method on local-storage.js).

constructor() {
super(...arguments);
// do not dismiss any banners if the user has updated their version
const dismissedBanner = localStorage.getItem(`dismiss-license-banner-${this.currentVersion}`); // returns either warning or expired
this.updateDismissType(dismissedBanner);
}

get currentVersion() {
return this.version.version;
}

get licenseExpired() {
if (!this.args.expiry) return false;
return isAfter(new Date(), new Date(this.args.expiry));
Expand All @@ -24,4 +44,22 @@ export default class LicenseBanners extends Component {
if (!this.args.expiry) return 99;
return differenceInDays(new Date(this.args.expiry), new Date());
}

@action
dismissBanner(dismissAction) {
// if a client's version changed their old localStorage key will still exists.
localStorage.cleanUpStorage('dismiss-license-banner', `dismiss-license-banner-${this.currentVersion}`);
// updates localStorage and then updates the template by calling updateDismissType
localStorage.setItem(`dismiss-license-banner-${this.currentVersion}`, dismissAction);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm understanding this correctly, if I've previously dismissed a warning and I come back and dismiss an expired, we will replace the localStorage value so that the warning will be able to show up again?

Copy link
Contributor Author

@Monkeychip Monkeychip Feb 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. The key here is that both will never show up at the same time. So let's say you dismissed a warning, but then you let your license expire, the expired license will show. If we dismiss that, then next time a warning should display it will.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great context, thanks!

this.updateDismissType(dismissAction);
}

updateDismissType(dismissType) {
// updates tracked properties to update template
if (dismissType === 'warning') {
this.warningDismissed = true;
} else if (dismissType === 'expired') {
this.expiredDismissed = true;
}
}
}
9 changes: 9 additions & 0 deletions ui/app/lib/local-storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,13 @@ export default {
keys() {
return Object.keys(window.localStorage);
},

cleanUpStorage(string, keyToKeep) {
Monkeychip marked this conversation as resolved.
Show resolved Hide resolved
const relevantKeys = this.keys().filter((str) => str.startsWith(string));
relevantKeys.forEach((key) => {
if (key !== keyToKeep) {
localStorage.removeItem(key);
}
});
},
};
10 changes: 8 additions & 2 deletions ui/app/templates/components/license-banners.hbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{#if this.licenseExpired}}
{{#if (and this.licenseExpired (not this.expiredDismissed))}}
<div class="license-banner-wrapper" data-test-license-banner data-test-license-banner-expired>
<AlertBanner
@type="danger"
Expand All @@ -13,9 +13,12 @@
<Icon @name="learn-link" />
Read documentation
</DocLink>
<button type="button" class="close-button" {{on "click" (fn this.dismissBanner "expired")}} data-test-dismiss-expired>
<Icon @name="x" aria-label="dismiss license expired warning" />
</button>
</AlertBanner>
</div>
{{else if (lte this.licenseExpiringInDays 30)}}
{{else if (and (lte this.licenseExpiringInDays 30) (not this.warningDismissed))}}
<div class="license-banner-wrapper" data-test-license-banner data-test-license-banner-warning>
<AlertBanner
@type="warning"
Expand All @@ -34,6 +37,9 @@
<Icon @name="learn-link" />
Read documentation
</DocLink>
<button type="button" class="close-button" {{on "click" (fn this.dismissBanner "warning")}} data-test-dismiss-warning>
<Icon @name="x" aria-label="dismiss license expire soon warning" />
</button>
</AlertBanner>
</div>
{{/if}}
78 changes: 73 additions & 5 deletions ui/tests/integration/components/license-banners-test.js
Original file line number Diff line number Diff line change
@@ -1,39 +1,107 @@
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { render } from '@ember/test-helpers';
import { render, click } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';
import subDays from 'date-fns/subDays';
import addDays from 'date-fns/addDays';
import formatRFC3339 from 'date-fns/formatRFC3339';

const YESTERDAY = subDays(new Date(), 1);
const NEXT_MONTH = addDays(new Date(), 30);

module('Integration | Component | license-banners', function (hooks) {
setupRenderingTest(hooks);

hooks.beforeEach(function () {
this.version = this.owner.lookup('service:version');
this.version.version = '1.13.1+ent';
});

test('it does not render if no expiry', async function (assert) {
assert.expect(1);
await render(hbs`<LicenseBanners />`);
assert.dom('[data-test-license-banner]').doesNotExist('License banner does not render');
});

test('it renders an error if expiry is before now', async function (assert) {
const yesterday = subDays(new Date(), 1);
this.set('expiry', formatRFC3339(yesterday));
assert.expect(2);
this.set('expiry', formatRFC3339(YESTERDAY));
await render(hbs`<LicenseBanners @expiry={{this.expiry}} />`);
assert.dom('[data-test-license-banner-expired]').exists('Expired license banner renders');
assert.dom('.message-title').hasText('License expired', 'Shows correct title on alert');
});

test('it renders a warning if expiry is within 30 days', async function (assert) {
const nextMonth = addDays(new Date(), 30);
this.set('expiry', formatRFC3339(nextMonth));
assert.expect(2);
this.set('expiry', formatRFC3339(NEXT_MONTH));
await render(hbs`<LicenseBanners @expiry={{this.expiry}} />`);
assert.dom('[data-test-license-banner-warning]').exists('Warning license banner renders');
assert.dom('.message-title').hasText('Vault license expiring', 'Shows correct title on alert');
});

test('it does not render a banner if expiry is outside 30 days', async function (assert) {
assert.expect(1);
const outside30 = addDays(new Date(), 32);
this.set('expiry', formatRFC3339(outside30));
await render(hbs`<LicenseBanners @expiry={{this.expiry}} />`);
assert.dom('[data-test-license-banner]').doesNotExist('License banner does not render');
});

test('it does not render the expired banner if it has been dismissed', async function (assert) {
assert.expect(3);
this.set('expiry', formatRFC3339(YESTERDAY));
await render(hbs`<LicenseBanners @expiry={{this.expiry}} />`);
await click('[data-test-dismiss-expired]');
assert.dom('[data-test-license-banner-expired]').doesNotExist('Expired license banner does not render');

await render(hbs`<LicenseBanners @expiry={{this.expiry}} />`);
const localStorageResult = JSON.parse(localStorage.getItem(`dismiss-license-banner-1.13.1+ent`));
assert.strictEqual(localStorageResult, 'expired');
assert
.dom('[data-test-license-banner-expired]')
.doesNotExist('The expired banner still does not render after a re-render.');
localStorage.removeItem(`dismiss-license-banner-1.13.1+ent`);
});

test('it does not render the warning banner if it has been dismissed', async function (assert) {
assert.expect(3);
this.set('expiry', formatRFC3339(NEXT_MONTH));
await render(hbs`<LicenseBanners @expiry={{this.expiry}} />`);
await click('[data-test-dismiss-warning]');
assert.dom('[data-test-license-banner-warning]').doesNotExist('Warning license banner does not render');

await render(hbs`<LicenseBanners @expiry={{this.expiry}} />`);
const localStorageResult = JSON.parse(localStorage.getItem(`dismiss-license-banner-1.13.1+ent`));
assert.strictEqual(localStorageResult, 'warning');
assert
.dom('[data-test-license-banner-warning]')
.doesNotExist('The warning banner still does not render after a re-render.');
localStorage.removeItem(`dismiss-license-banner-1.13.1+ent`);
});

test('it renders a banner if the vault license has changed', async function (assert) {
assert.expect(3);
this.version.version = '1.12.1+ent';
this.set('expiry', formatRFC3339(NEXT_MONTH));
await render(hbs`<LicenseBanners @expiry={{this.expiry}} />`);
await click('[data-test-dismiss-warning]');
this.version.version = '1.13.1+ent';
await render(hbs`<LicenseBanners @expiry={{this.expiry}} />`);
assert
.dom('[data-test-license-banner-warning]')
.exists('The warning banner shows even though we have dismissed it earlier.');

await click('[data-test-dismiss-warning]');
const localStorageResultNewVersion = JSON.parse(
localStorage.getItem(`dismiss-license-banner-1.13.1+ent`)
);
const localStorageResultOldVersion = JSON.parse(
localStorage.getItem(`dismiss-license-banner-1.12.1+ent`)
);
// Check that localStorage was cleaned and no longer contains the old version storage key.
assert.strictEqual(localStorageResultOldVersion, null);
assert.strictEqual(localStorageResultNewVersion, 'warning');
// If debugging this test remember to clear localStorage if the test was not run to completion.
localStorage.removeItem(`dismiss-license-banner-1.13.1+ent`);
});
});