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 11 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.
```
51 changes: 47 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,25 @@
* @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 currentVersion = this.version.version;
Monkeychip marked this conversation as resolved.
Show resolved Hide resolved
@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);
// If nothing is saved in localStorage or the user has updated their Vault version, do not dismiss any of the banners.
const localStorageLicenseBannerObject = localStorage.getItem('licenseBanner');
if (!localStorageLicenseBannerObject || localStorageLicenseBannerObject.version !== this.currentVersion) {
localStorage.setItem('licenseBanner', { dismissType: '', version: this.currentVersion });
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the logic here could be simplified by storing the dismissal at a variable key, rather than storing an object that has to be parsed and reasoned about. So for example, if I'm on version 1.12.2, this component would check for a value (any value, really) at licenseBanner-warning-1.12.2
The get and set calls would be able to use a key variable that is calculated based on the version:

const warningBannerKey = `licenseBanner-warning-${this.currentVersion}`
const dismissedWarning = localStorage.getItem(warningBannerKey)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the idea. Let me know what you think of the changes. Not exactly your suggestion, but a similar concept.

return;
}
// if dismissType has previously been saved in localStorage, update tracked properties.
this.setDismissType(localStorageLicenseBannerObject.dismissType);
}

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

@action
dismissBanner(dismissAction) {
// dismissAction is either 'dismiss-warning' or 'dismiss-expired'
const updatedLocalStorageObject = { dismissType: dismissAction, version: this.currentVersion };
localStorage.setItem('licenseBanner', updatedLocalStorageObject);
this.setDismissType(dismissAction);
}

setDismissType(dismissType) {
// reset tracked properties to false
this.warningDismissed = this.expiredDismissed = false;
if (dismissType === 'dismiss-warning') {
this.warningDismissed = true;
} else if (dismissType === 'dismiss-expired') {
this.expiredDismissed = true;
} else {
// if dismissType is empty do nothing.
return;
}
}
}
20 changes: 18 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,17 @@
<Icon @name="learn-link" />
Read documentation
</DocLink>
<button
type="button"
class="close-button"
{{on "click" (fn this.dismissBanner "dismiss-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 +42,14 @@
<Icon @name="learn-link" />
Read documentation
</DocLink>
<button
type="button"
class="close-button"
{{on "click" (fn this.dismissBanner "dismiss-warning")}}
data-test-dismiss-warning
>
<Icon @name="x" aria-label="dismiss license expire soon warning" />
</button>
</AlertBanner>
</div>
{{/if}}
70 changes: 65 additions & 5 deletions ui/tests/integration/components/license-banners-test.js
Original file line number Diff line number Diff line change
@@ -1,39 +1,99 @@
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 () {
localStorage.removeItem('licenseBanner');
});

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 localStorageObject = JSON.parse(localStorage.getItem('licenseBanner'));
assert.strictEqual(localStorageObject.dismissType, 'dismiss-expired');
assert
.dom('[data-test-license-banner-expired]')
.doesNotExist('The expired banner still does not render after a re-render.');
});

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 localStorageObject = JSON.parse(localStorage.getItem('licenseBanner'));
assert.strictEqual(localStorageObject.dismissType, 'dismiss-warning');
assert
.dom('[data-test-license-banner-warning]')
.doesNotExist('The warning banner still does not render after a re-render.');
});

test('it renders a banner if the vault license has changed', async function (assert) {
assert.expect(2);
this.version = this.owner.lookup('service:version');
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]');

const localStorageObjectEarlierVersion = JSON.parse(localStorage.getItem('licenseBanner'));
this.version.version = '1.13.1+ent';
await render(hbs`<LicenseBanners @expiry={{this.expiry}} />`);

const localStorageObjectLaterVersion = JSON.parse(localStorage.getItem('licenseBanner'));
assert
.dom('[data-test-license-banner-warning]')
.exists('The warning banner shows even though we have dismissed it earlier.');

assert.notEqual(localStorageObjectEarlierVersion.version, localStorageObjectLaterVersion.version);
});
});