-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Disabling License Banners #19116
Changes from 9 commits
f88973a
f64bd76
a2f37bb
378aec4
37f9855
956b744
095c065
31ec229
c34012c
5fa1db3
2dd9d40
ab9d86e
699a72a
a6ff940
2a43b95
3a4e34f
a5a731c
ced6740
06882d6
c9c54ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. | ||
``` |
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 | ||
|
@@ -9,11 +17,24 @@ | |
* @param {string} expiry - RFC3339 date timestamp | ||
*/ | ||
|
||
import Component from '@glimmer/component'; | ||
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; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great idea. I'll add that. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, show the license banners. | ||
const localStorageLicenseBannerObject = localStorage.getItem('licenseBanner'); | ||
if (!localStorageLicenseBannerObject || localStorageLicenseBannerObject.version !== this.currentVersion) { | ||
localStorage.setItem('licenseBanner', { dismissType: '', version: this.currentVersion }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
} | ||
this.setDismissType(localStorageLicenseBannerObject.dismissType); | ||
} | ||
|
||
get licenseExpired() { | ||
if (!this.args.expiry) return false; | ||
return isAfter(new Date(), new Date(this.args.expiry)); | ||
|
@@ -24,4 +45,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; | ||
} | ||
} | ||
} |
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); | ||
}); | ||
}); |
There was a problem hiding this comment.
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