-
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 all 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,23 @@ | |
* @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 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); | ||
// 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)); | ||
|
@@ -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); | ||
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. 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? 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. 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. 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 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; | ||
} | ||
} | ||
} |
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`); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
import { module, test } from 'qunit'; | ||
import { setupTest } from 'ember-qunit'; | ||
import LocalStorage from 'vault/lib/local-storage'; | ||
|
||
module('Unit | lib | local-storage', function (hooks) { | ||
setupTest(hooks); | ||
|
||
hooks.beforeEach(function () { | ||
window.localStorage.clear(); | ||
}); | ||
|
||
test('it does not error if nothing is in local storage', async function (assert) { | ||
assert.expect(1); | ||
assert.strictEqual( | ||
LocalStorage.cleanUpStorage('something', 'something-key'), | ||
undefined, | ||
'returns undefined and does not throw an error when method is called and nothing exist in localStorage.' | ||
); | ||
}); | ||
|
||
test('it does not remove anything in localStorage that does not start with the string or we have specified to keep.', async function (assert) { | ||
assert.expect(3); | ||
LocalStorage.setItem('string-key-remove', 'string-key-remove-value'); | ||
LocalStorage.setItem('beep-boop-bop-key', 'beep-boop-bop-value'); | ||
LocalStorage.setItem('string-key', 'string-key-value'); | ||
const storageLengthBefore = window.localStorage.length; | ||
LocalStorage.cleanUpStorage('string', 'string-key'); | ||
const storageLengthAfter = window.localStorage.length; | ||
assert.strictEqual( | ||
storageLengthBefore - storageLengthAfter, | ||
1, | ||
'the method should only remove one key from localStorage.' | ||
); | ||
assert.strictEqual( | ||
LocalStorage.getItem('string-key'), | ||
'string-key-value', | ||
'the key we asked to keep still exists in localStorage.' | ||
); | ||
assert.strictEqual( | ||
LocalStorage.getItem('string-key-remove'), | ||
null, | ||
'the key we did not specify to keep was removed from localStorage.' | ||
); | ||
// clear storage | ||
window.localStorage.clear(); | ||
}); | ||
}); |
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