From 036ccaf72e09d3bd66268d45816de579c0583eee Mon Sep 17 00:00:00 2001 From: wenincode Date: Wed, 12 Oct 2022 18:16:39 -0600 Subject: [PATCH 01/11] Add banner for agentless node notice --- ui/packages/consul-ui/app/controllers/dc/nodes/index.js | 8 +++++++- .../consul-ui/app/styles/routes/dc/nodes/index.scss | 4 ++++ ui/packages/consul-ui/mock-api/v1/internal/ui/nodes | 3 ++- ui/packages/consul-ui/translations/routes/en-us.yaml | 5 +++++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/ui/packages/consul-ui/app/controllers/dc/nodes/index.js b/ui/packages/consul-ui/app/controllers/dc/nodes/index.js index 56c35902b..609bc5a58 100644 --- a/ui/packages/consul-ui/app/controllers/dc/nodes/index.js +++ b/ui/packages/consul-ui/app/controllers/dc/nodes/index.js @@ -1,3 +1,9 @@ import PeeredResourceController from 'consul-ui/controllers/_peered-resource'; +import { action } from '@ember/object'; -export default class DcNodesController extends PeeredResourceController {} +export default class DcNodesController extends PeeredResourceController { + @action + dismissAgentlessNotice() { + console.log('dismiss this here') + } +} diff --git a/ui/packages/consul-ui/app/styles/routes/dc/nodes/index.scss b/ui/packages/consul-ui/app/styles/routes/dc/nodes/index.scss index aa433dc90..eb2f30f54 100644 --- a/ui/packages/consul-ui/app/styles/routes/dc/nodes/index.scss +++ b/ui/packages/consul-ui/app/styles/routes/dc/nodes/index.scss @@ -4,3 +4,7 @@ html[data-route^='dc.nodes.show.metadata'] table tr { html[data-route^='dc.nodes.show.sessions'] .consul-lock-session-list { @extend %list-after-secondary-nav; } +html[data-route^='dc.nodes.index'] .agentless-node-notice header { + display: flex; + justify-content: space-between; +} diff --git a/ui/packages/consul-ui/mock-api/v1/internal/ui/nodes b/ui/packages/consul-ui/mock-api/v1/internal/ui/nodes index 8b63b7997..26a2ac501 100644 --- a/ui/packages/consul-ui/mock-api/v1/internal/ui/nodes +++ b/ui/packages/consul-ui/mock-api/v1/internal/ui/nodes @@ -24,7 +24,8 @@ "wan":"${fake.internet.ip()}" }, "Meta": { - "consul-network-segment":"" + "consul-network-segment":"", + "synthetic-node": "${fake.helpers.randomize(["true", "false", "false", "false"])}" }, "Services":[ ${ diff --git a/ui/packages/consul-ui/translations/routes/en-us.yaml b/ui/packages/consul-ui/translations/routes/en-us.yaml index 4e157d9df..43bd0fa67 100644 --- a/ui/packages/consul-ui/translations/routes/en-us.yaml +++ b/ui/packages/consul-ui/translations/routes/en-us.yaml @@ -55,6 +55,11 @@ dc: nodes: index: + agentless: + notice: + header: Consul 1.14 removes client nodes from Kubernetes Service Mesh deployments + body: Kubernetes clusters do not require client nodes to run as of Consul 1.14. Kubernetes nodes will not appear in this view. + footer: View documentation on Consul Dataplane empty: header: | {items, select, From 5fc979ee6e0cf558cabbe8fa149e2d8cb908cc58 Mon Sep 17 00:00:00 2001 From: wenincode Date: Thu, 13 Oct 2022 08:28:46 -0600 Subject: [PATCH 02/11] Move banner to component and make it dismissable --- .../consul/node/agentless-notice/index.hbs | 24 +++++++++++++++++++ .../consul/node/agentless-notice/index.js | 20 ++++++++++++++++ .../app/templates/dc/nodes/index.hbs | 1 + ui/packages/consul-ui/package.json | 1 + ui/yarn.lock | 16 +++++++++++++ 5 files changed, 62 insertions(+) create mode 100644 ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.hbs create mode 100644 ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js diff --git a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.hbs b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.hbs new file mode 100644 index 000000000..3f764e97a --- /dev/null +++ b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.hbs @@ -0,0 +1,24 @@ +{{#if isVisible}} + + +

+ {{t "routes.dc.nodes.index.agentless.notice.header"}} +

+ + + +

+ {{t "routes.dc.nodes.index.agentless.notice.body"}} +

+ + +

+ + {{t "routes.dc.nodes.index.agentless.notice.footer"}} + +

+ + +{{/if}} diff --git a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js new file mode 100644 index 000000000..fc41ca834 --- /dev/null +++ b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js @@ -0,0 +1,20 @@ +import Component from '@glimmer/component'; +import { action } from '@ember/object'; +import { trackedInLocalStorage } from 'ember-tracked-local-storage'; + +export default class AgentlessNotice extends Component { + @trackedInLocalStorage({ defaulValue: 'false' }) consulNodesAgentlessNoticeDismissed; + + get isVisible() { + const { items, filteredItems } = this.args; + + console.log(this.consulNodesAgentlessNoticeDismissed !== 'true' && items.length > filteredItems.length); + console.log('tracked prop: ', this.consulNodesAgentlessNoticeDismissed); + return this.consulNodesAgentlessNoticeDismissed !== 'true' && items.length > filteredItems.length; + } + + @action + dismissAgentlessNotice() { + this.consulNodesAgentlessNoticeDismissed = 'true'; + } +} diff --git a/ui/packages/consul-ui/app/templates/dc/nodes/index.hbs b/ui/packages/consul-ui/app/templates/dc/nodes/index.hbs index dcdc8c591..d43b35f90 100644 --- a/ui/packages/consul-ui/app/templates/dc/nodes/index.hbs +++ b/ui/packages/consul-ui/app/templates/dc/nodes/index.hbs @@ -60,6 +60,7 @@ {{/if}} + Date: Thu, 13 Oct 2022 10:21:56 -0600 Subject: [PATCH 03/11] Add tests for agentless-notice-banner --- .../consul/node/agentless-notice/index.hbs | 20 +++--- .../consul/node/agentless-notice/index.js | 6 +- .../consul-ui/mock-api/v1/internal/ui/nodes | 3 +- .../consul/node/agentless-notice-test.js | 65 +++++++++++++++++++ 4 files changed, 81 insertions(+), 13 deletions(-) create mode 100644 ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js diff --git a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.hbs b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.hbs index 3f764e97a..6ee6a3177 100644 --- a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.hbs +++ b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.hbs @@ -1,22 +1,26 @@ {{#if isVisible}} - +

- {{t "routes.dc.nodes.index.agentless.notice.header"}} + {{t 'routes.dc.nodes.index.agentless.notice.header'}}

-

- {{t "routes.dc.nodes.index.agentless.notice.body"}} + {{t 'routes.dc.nodes.index.agentless.notice.body'}}

-

- - {{t "routes.dc.nodes.index.agentless.notice.footer"}} +

+ + {{t 'routes.dc.nodes.index.agentless.notice.footer'}}

diff --git a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js index fc41ca834..3bcba47b5 100644 --- a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js +++ b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js @@ -8,9 +8,9 @@ export default class AgentlessNotice extends Component { get isVisible() { const { items, filteredItems } = this.args; - console.log(this.consulNodesAgentlessNoticeDismissed !== 'true' && items.length > filteredItems.length); - console.log('tracked prop: ', this.consulNodesAgentlessNoticeDismissed); - return this.consulNodesAgentlessNoticeDismissed !== 'true' && items.length > filteredItems.length; + return ( + this.consulNodesAgentlessNoticeDismissed !== 'true' && items.length > filteredItems.length + ); } @action diff --git a/ui/packages/consul-ui/mock-api/v1/internal/ui/nodes b/ui/packages/consul-ui/mock-api/v1/internal/ui/nodes index 26a2ac501..8b63b7997 100644 --- a/ui/packages/consul-ui/mock-api/v1/internal/ui/nodes +++ b/ui/packages/consul-ui/mock-api/v1/internal/ui/nodes @@ -24,8 +24,7 @@ "wan":"${fake.internet.ip()}" }, "Meta": { - "consul-network-segment":"", - "synthetic-node": "${fake.helpers.randomize(["true", "false", "false", "false"])}" + "consul-network-segment":"" }, "Services":[ ${ diff --git a/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js b/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js new file mode 100644 index 000000000..6f42c102c --- /dev/null +++ b/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js @@ -0,0 +1,65 @@ +import { module, test } from 'qunit'; +import { setupRenderingTest } from 'ember-qunit'; +import hbs from 'htmlbars-inline-precompile'; +import { click, render } from '@ember/test-helpers'; + +module('Integration | Component | consul node agentless-notice', function (hooks) { + setupRenderingTest(hooks); + + test('it does not display the notice if the filtered nodes are the same as the regular nodes', async function (assert) { + this.set('nodes', [ + { + Meta: { + 'synthetic-node': false, + }, + }, + ]); + + this.set('filteredNodes', [ + { + Meta: { + 'synthetic-node': false, + }, + }, + ]); + + await render( + hbs`` + ); + + assert + .dom('[data-test-node-agentless-notice]') + .doesNotExist( + 'The agentless notice should not display if the items are the same as the filtered items' + ); + }); + + test('it does display the notice when the filtered items are smaller then the regular items', async function (assert) { + this.set('nodes', [ + { + Meta: { + 'synthetic-node': false, + }, + }, + ]); + + this.set('filteredNodes', []); + + await render( + hbs`` + ); + + assert + .dom('[data-test-node-agentless-notice]') + .exists( + 'The agentless notice should display if their are less items then the filtered items' + ); + + await click('button'); + assert + .dom('[data-test-node-agentless-notice]') + .doesNotExist( + 'The agentless notice be dismissed' + ); + }); +}); From 31cbbc85e3f2478b9c69dc13604fb429f1fcfa68 Mon Sep 17 00:00:00 2001 From: wenincode Date: Thu, 13 Oct 2022 10:38:26 -0600 Subject: [PATCH 04/11] Move agentless-notice banner css to it's own file --- .../components/consul/node/agentless-notice/index.scss | 4 ++++ ui/packages/consul-ui/app/controllers/dc/nodes/index.js | 8 +------- ui/packages/consul-ui/app/styles/components.scss | 1 + .../consul-ui/app/styles/routes/dc/nodes/index.scss | 4 ---- .../components/consul/node/agentless-notice-test.js | 2 +- 5 files changed, 7 insertions(+), 12 deletions(-) create mode 100644 ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.scss diff --git a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.scss b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.scss new file mode 100644 index 000000000..fc4b21ac9 --- /dev/null +++ b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.scss @@ -0,0 +1,4 @@ +.agentless-node-notice header { + display: flex; + justify-content: space-between; +} diff --git a/ui/packages/consul-ui/app/controllers/dc/nodes/index.js b/ui/packages/consul-ui/app/controllers/dc/nodes/index.js index 609bc5a58..56c35902b 100644 --- a/ui/packages/consul-ui/app/controllers/dc/nodes/index.js +++ b/ui/packages/consul-ui/app/controllers/dc/nodes/index.js @@ -1,9 +1,3 @@ import PeeredResourceController from 'consul-ui/controllers/_peered-resource'; -import { action } from '@ember/object'; -export default class DcNodesController extends PeeredResourceController { - @action - dismissAgentlessNotice() { - console.log('dismiss this here') - } -} +export default class DcNodesController extends PeeredResourceController {} diff --git a/ui/packages/consul-ui/app/styles/components.scss b/ui/packages/consul-ui/app/styles/components.scss index 4a7e7b9e0..656b05cf1 100644 --- a/ui/packages/consul-ui/app/styles/components.scss +++ b/ui/packages/consul-ui/app/styles/components.scss @@ -110,3 +110,4 @@ @import 'consul-ui/components/consul/peer/info'; @import 'consul-ui/components/consul/peer/form'; @import 'consul-ui/components/consul/hcp/home'; +@import 'consul-ui/components/consul/node/agentless-notice'; diff --git a/ui/packages/consul-ui/app/styles/routes/dc/nodes/index.scss b/ui/packages/consul-ui/app/styles/routes/dc/nodes/index.scss index eb2f30f54..aa433dc90 100644 --- a/ui/packages/consul-ui/app/styles/routes/dc/nodes/index.scss +++ b/ui/packages/consul-ui/app/styles/routes/dc/nodes/index.scss @@ -4,7 +4,3 @@ html[data-route^='dc.nodes.show.metadata'] table tr { html[data-route^='dc.nodes.show.sessions'] .consul-lock-session-list { @extend %list-after-secondary-nav; } -html[data-route^='dc.nodes.index'] .agentless-node-notice header { - display: flex; - justify-content: space-between; -} diff --git a/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js b/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js index 6f42c102c..a03731712 100644 --- a/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js +++ b/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js @@ -38,7 +38,7 @@ module('Integration | Component | consul node agentless-notice', function (hooks this.set('nodes', [ { Meta: { - 'synthetic-node': false, + 'synthetic-node': true, }, }, ]); From 3006025bbe2f8c2346596039f65979b36ab89f23 Mon Sep 17 00:00:00 2001 From: wenincode Date: Thu, 13 Oct 2022 10:43:57 -0600 Subject: [PATCH 05/11] Add changelog --- .changelog/14971.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/14971.txt diff --git a/.changelog/14971.txt b/.changelog/14971.txt new file mode 100644 index 000000000..ce0b8b15c --- /dev/null +++ b/.changelog/14971.txt @@ -0,0 +1,3 @@ +```release-note:feature +ui: Display notice banner on nodes index page if synthetic nodes are being filtered. +``` From 9d56feb77e7ca997d90c5ee9652808e8ed6c47b8 Mon Sep 17 00:00:00 2001 From: wenincode Date: Thu, 13 Oct 2022 10:59:48 -0600 Subject: [PATCH 06/11] Fix linting error --- .../components/consul/node/agentless-notice-test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js b/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js index a03731712..851f29f54 100644 --- a/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js +++ b/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js @@ -58,8 +58,6 @@ module('Integration | Component | consul node agentless-notice', function (hooks await click('button'); assert .dom('[data-test-node-agentless-notice]') - .doesNotExist( - 'The agentless notice be dismissed' - ); + .doesNotExist('The agentless notice be dismissed'); }); }); From 9777ee00777ee4421a4abe0bba563c9ea9901ee4 Mon Sep 17 00:00:00 2001 From: wenincode Date: Fri, 14 Oct 2022 12:21:25 -0600 Subject: [PATCH 07/11] Save agentless node notice dismissal per dc --- .../consul/node/agentless-notice/index.js | 26 +++++++--- .../app/templates/dc/nodes/index.hbs | 2 +- ui/packages/consul-ui/package.json | 1 - .../consul/node/agentless-notice-test.js | 47 ++++++++++++++++++- ui/yarn.lock | 16 ------- 5 files changed, 67 insertions(+), 25 deletions(-) diff --git a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js index 3bcba47b5..cc7996ae8 100644 --- a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js +++ b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js @@ -1,20 +1,34 @@ import Component from '@glimmer/component'; import { action } from '@ember/object'; -import { trackedInLocalStorage } from 'ember-tracked-local-storage'; +import { tracked } from '@glimmer/tracking'; + +const DISMISSED_VALUE = 'true'; export default class AgentlessNotice extends Component { - @trackedInLocalStorage({ defaulValue: 'false' }) consulNodesAgentlessNoticeDismissed; + storageKey = 'consul-nodes-agentless-notice-dismissed'; + @tracked hasDismissedNotice = false; + + constructor(owner, args) { + super(owner, args); + + if (this.args.dc) { + this.storageKey = `consul-nodes-agentless-notice-dismissed-${this.args.dc}`; + } + + if (window.localStorage.getItem(this.storageKey) === DISMISSED_VALUE) { + this.hasDismissedNotice = true; + } + } get isVisible() { const { items, filteredItems } = this.args; - return ( - this.consulNodesAgentlessNoticeDismissed !== 'true' && items.length > filteredItems.length - ); + return !this.hasDismissedNotice && items.length > filteredItems.length; } @action dismissAgentlessNotice() { - this.consulNodesAgentlessNoticeDismissed = 'true'; + window.localStorage.setItem(this.storageKey, DISMISSED_VALUE); + this.hasDismissedNotice = true; } } diff --git a/ui/packages/consul-ui/app/templates/dc/nodes/index.hbs b/ui/packages/consul-ui/app/templates/dc/nodes/index.hbs index d43b35f90..f319ad0d5 100644 --- a/ui/packages/consul-ui/app/templates/dc/nodes/index.hbs +++ b/ui/packages/consul-ui/app/templates/dc/nodes/index.hbs @@ -60,7 +60,7 @@ {{/if}} - + { + const localStore = {}; + + sinon.stub(window.localStorage, 'getItem').callsFake((key) => localStore[key]); + sinon.stub(window.localStorage, 'setItem').callsFake((key, value) => (localStore[key] = value)); + }); test('it does not display the notice if the filtered nodes are the same as the regular nodes', async function (assert) { this.set('nodes', [ @@ -26,7 +33,7 @@ module('Integration | Component | consul node agentless-notice', function (hooks await render( hbs`` ); - + assert.true(window.localStorage.getItem.called); assert .dom('[data-test-node-agentless-notice]') .doesNotExist( @@ -59,5 +66,43 @@ module('Integration | Component | consul node agentless-notice', function (hooks assert .dom('[data-test-node-agentless-notice]') .doesNotExist('The agentless notice be dismissed'); + assert.true( + window.localStorage.setItem.calledOnceWith('consul-nodes-agentless-notice-dismissed', 'true'), + "Set the key in localstorage to 'true'" + ); + }); + + test('it does not display if the localstorage key is already set to true', async function (assert) { + this.set('nodes', [ + { + Meta: { + 'synthetic-node': false, + }, + }, + ]); + + this.set('filteredNodes', [ + { + Meta: { + 'synthetic-node': false, + }, + }, + ]); + + window.localStorage.setItem('consul-nodes-agentless-notice-dismissed-dc2', 'true'); + + await render( + hbs`` + ); + + assert.true( + window.localStorage.getItem.calledOnceWith('consul-nodes-agentless-notice-dismissed-dc2') + ); + + assert + .dom('[data-test-node-agentless-notice]') + .doesNotExist( + "The agentless notice should not display if the local storage key has already been set to 'true'" + ); }); }); diff --git a/ui/yarn.lock b/ui/yarn.lock index 24a3f492a..a1b3fb4c0 100644 --- a/ui/yarn.lock +++ b/ui/yarn.lock @@ -9247,17 +9247,6 @@ ember-text-measurer@^0.6.0: ember-cli-babel "^7.19.0" ember-cli-htmlbars "^4.3.1" -ember-tracked-local-storage@^1.1.1: - version "1.1.1" - resolved "https://registry.yarnpkg.com/ember-tracked-local-storage/-/ember-tracked-local-storage-1.1.1.tgz#16104ce5bddc6d055049af094c7e223ff2f61520" - integrity sha512-0n4EBdbFyIJWqtbhmEf+iAFRvUD6p+s3kL9WLD2GJOimfKjcbe2ybmNUT6Qdj3ge5vqQgh59mJejJg/PEP2R0w== - dependencies: - "@glimmer/tracking" "^1.0.1" - ember-auto-import "^1.6.0" - ember-cli-babel "^7.22.1" - ember-cli-htmlbars "^5.3.1" - macro-decorators "^0.1.2" - "ember-truth-helpers@^2.1.0 || ^3.0.0", ember-truth-helpers@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/ember-truth-helpers/-/ember-truth-helpers-3.0.0.tgz#86766bdca4ac9b86bce3d262dff2aabc4a0ea384" @@ -12626,11 +12615,6 @@ lru-cache@^6.0.0: dependencies: yallist "^4.0.0" -macro-decorators@^0.1.2: - version "0.1.2" - resolved "https://registry.yarnpkg.com/macro-decorators/-/macro-decorators-0.1.2.tgz#1d5cf1276d343371040af192901947f2a0af03c1" - integrity sha512-BV5XPmCm9kPSMtgfZiv0vTjOooe5pTIPIVkdoqbC49H1B7z22KB39H50R2ZNclZDQlmVyviLozRatKnOYZkwzg== - magic-string@^0.25.7: version "0.25.7" resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.25.7.tgz#3f497d6fd34c669c6798dcb821f2ef31f5445051" From f9f4ca8da4e1a5974876fa9b6c8118ce11da2a09 Mon Sep 17 00:00:00 2001 From: wenincode Date: Fri, 14 Oct 2022 14:08:40 -0600 Subject: [PATCH 08/11] Set postfix for agentless-notice storage key based on partition and dc --- .../app/components/consul/node/agentless-notice/index.js | 4 ++-- ui/packages/consul-ui/app/templates/dc/nodes/index.hbs | 2 +- .../components/consul/node/agentless-notice-test.js | 8 +++++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js index cc7996ae8..6cc18594c 100644 --- a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js +++ b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js @@ -11,8 +11,8 @@ export default class AgentlessNotice extends Component { constructor(owner, args) { super(owner, args); - if (this.args.dc) { - this.storageKey = `consul-nodes-agentless-notice-dismissed-${this.args.dc}`; + if (this.args.postfix) { + this.storageKey = `consul-nodes-agentless-notice-dismissed-${this.args.postfix}`; } if (window.localStorage.getItem(this.storageKey) === DISMISSED_VALUE) { diff --git a/ui/packages/consul-ui/app/templates/dc/nodes/index.hbs b/ui/packages/consul-ui/app/templates/dc/nodes/index.hbs index f319ad0d5..8459018cc 100644 --- a/ui/packages/consul-ui/app/templates/dc/nodes/index.hbs +++ b/ui/packages/consul-ui/app/templates/dc/nodes/index.hbs @@ -60,7 +60,7 @@ {{/if}} - + ` + hbs`` ); assert.true( - window.localStorage.getItem.calledOnceWith('consul-nodes-agentless-notice-dismissed-dc2') + window.localStorage.getItem.calledOnceWith( + 'consul-nodes-agentless-notice-dismissed-partition' + ) ); assert From 443b73435a6cbe1f7ba8cb51d5e7f9a7d90a89b1 Mon Sep 17 00:00:00 2001 From: wenincode Date: Tue, 18 Oct 2022 09:40:47 -0600 Subject: [PATCH 09/11] Use local-storage service to manage localStorage Use local-storage service, prototyped here https://github.com/LevelbossMike/local-storage-service, to manage local storage usage in an octane way. Does not write to local storage in tests by default and is easy to stub out. --- .../consul/node/agentless-notice/index.js | 19 +-- .../consul-ui/app/services/local-storage.js | 126 ++++++++++++++++++ ui/packages/consul-ui/app/storages/base.js | 14 ++ ui/packages/consul-ui/app/storages/notices.js | 24 ++++ ui/packages/consul-ui/package.json | 1 + .../consul/node/agentless-notice-test.js | 33 +---- ui/yarn.lock | 46 +++++-- 7 files changed, 214 insertions(+), 49 deletions(-) create mode 100644 ui/packages/consul-ui/app/services/local-storage.js create mode 100644 ui/packages/consul-ui/app/storages/base.js create mode 100644 ui/packages/consul-ui/app/storages/notices.js diff --git a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js index 6cc18594c..ed9d31127 100644 --- a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js +++ b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js @@ -1,34 +1,27 @@ import Component from '@glimmer/component'; import { action } from '@ember/object'; -import { tracked } from '@glimmer/tracking'; - -const DISMISSED_VALUE = 'true'; +import { storageFor } from '../../../../services/local-storage'; export default class AgentlessNotice extends Component { - storageKey = 'consul-nodes-agentless-notice-dismissed'; - @tracked hasDismissedNotice = false; + storageKey = 'nodes-agentless-dismissed'; + @storageFor('notices') notices; constructor(owner, args) { super(owner, args); if (this.args.postfix) { - this.storageKey = `consul-nodes-agentless-notice-dismissed-${this.args.postfix}`; - } - - if (window.localStorage.getItem(this.storageKey) === DISMISSED_VALUE) { - this.hasDismissedNotice = true; + this.storageKey = `nodes-agentless-dismissed-${this.args.postfix}`; } } get isVisible() { const { items, filteredItems } = this.args; - return !this.hasDismissedNotice && items.length > filteredItems.length; + return !this.notices.state.includes(this.storageKey) && items.length > filteredItems.length; } @action dismissAgentlessNotice() { - window.localStorage.setItem(this.storageKey, DISMISSED_VALUE); - this.hasDismissedNotice = true; + this.notices.add(this.storageKey); } } diff --git a/ui/packages/consul-ui/app/services/local-storage.js b/ui/packages/consul-ui/app/services/local-storage.js new file mode 100644 index 000000000..b88285570 --- /dev/null +++ b/ui/packages/consul-ui/app/services/local-storage.js @@ -0,0 +1,126 @@ +import Service from '@ember/service'; +import { getOwner } from '@ember/application'; +import ENV from 'consul-ui/config/environment'; + +export function storageFor(key) { + return function () { + return { + get() { + const owner = getOwner(this); + + const localStorageService = owner.lookup('service:localStorage'); + + return localStorageService.getBucket(key); + }, + }; + }; +} + +/** + * An in-memory stub of window.localStorage. Ideally this would + * implement the [Storage](https://developer.mozilla.org/en-US/docs/Web/API/Storage)-interface that localStorage implements + * as well. + * + * We use this implementation during testing to not pollute `window.localStorage` + */ +class MemoryStorage { + constructor() { + this.data = new Map(); + } + + getItem(key) { + return this.data.get(key); + } + + setItem(key, value) { + return this.data.set(key, value.toString()); + } + + /** + * A function to seed data into MemoryStorage. This expects an object to be + * passed. The passed values will be persisted as a string - i.e. the values + * passed will call their `toString()`-method before writing to storage. You need + * to take this into account when you want to persist complex values, like arrays + * or objects: + * + * Example: + * + * ```js + * const storage = new MemoryStorage(); + * storage.seed({ notices: ['notice-a', 'notice-b']}); + * + * storage.getItem('notices') // => 'notice-a,notice-b' + * + * // won't work + * storage.seed({ + * user: { name: 'Tomster' } + * }) + * + * storage.getItem('user') // => '[object Object]' + * + * // this works + * storage.seed({ + * . user: JSON.stringify({name: 'Tomster'}) + * }) + * + * storage.getItem('user') // => '{ "name": "Tomster" }' + * ``` + * @param {object} data - the data to seed + */ + seed(data) { + const newData = new Map(); + + const keys = Object.keys(data); + + keys.forEach((key) => { + newData.set(key, data[key].toString()); + }); + + this.data = newData; + } +} + +/** + * There might be better ways to do this but this is good enough for now. + * During testing we want to use MemoryStorage not window.localStorage. + */ +function initStorage() { + if (ENV.environment === 'test') { + return new MemoryStorage(); + } else { + return window.localStorage; + } +} + +/** + * A service that wraps access to local-storage. We wrap + * local-storage to not pollute local-storage during testing. + */ +export default class LocalStorageService extends Service { + constructor() { + super(...arguments); + + this.storage = initStorage(); + this.buckets = new Map(); + } + + getBucket(key) { + const bucket = this.buckets.get(key); + + if (bucket) { + return bucket; + } else { + return this._setupBucket(key); + } + } + + _setupBucket(key) { + const owner = getOwner(this); + const Klass = owner.factoryFor(`storage:${key}`).class; + const storage = new Klass(key, this.storage); + + this.buckets.set(key, storage); + + return storage; + } +} diff --git a/ui/packages/consul-ui/app/storages/base.js b/ui/packages/consul-ui/app/storages/base.js new file mode 100644 index 000000000..50a26fa66 --- /dev/null +++ b/ui/packages/consul-ui/app/storages/base.js @@ -0,0 +1,14 @@ +export default class Storage { + constructor(key, storage) { + this.key = key; + this.storage = storage; + + this.state = this.initState(this.key, this.storage); + } + + initState() { + const { key, storage } = this; + + return storage.getItem(key); + } +} diff --git a/ui/packages/consul-ui/app/storages/notices.js b/ui/packages/consul-ui/app/storages/notices.js new file mode 100644 index 000000000..549219764 --- /dev/null +++ b/ui/packages/consul-ui/app/storages/notices.js @@ -0,0 +1,24 @@ +import { TrackedArray } from 'tracked-built-ins'; +import Storage from './base'; + +export default class Notices extends Storage { + initState() { + const { key, storage } = this; + + const persisted = storage.getItem(key); + + if (persisted) { + return new TrackedArray(persisted.split(',')); + } else { + return new TrackedArray(); + } + } + + add(value) { + const { key, storage, state } = this; + + state.push(value); + + storage.setItem(key, [...state]); + } +} diff --git a/ui/packages/consul-ui/package.json b/ui/packages/consul-ui/package.json index 146ff10df..54bf1e27a 100644 --- a/ui/packages/consul-ui/package.json +++ b/ui/packages/consul-ui/package.json @@ -191,6 +191,7 @@ "text-encoding": "^0.7.0", "tippy.js": "^6.2.7", "torii": "^0.10.1", + "tracked-built-ins": "^3.1.0", "unist-util-visit": "^2.0.3", "wayfarer": "^7.0.1", "webpack": "^5.74.0" diff --git a/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js b/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js index 6260447ef..db05c8162 100644 --- a/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js +++ b/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js @@ -2,16 +2,9 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; import hbs from 'htmlbars-inline-precompile'; import { click, render } from '@ember/test-helpers'; -import sinon from 'sinon'; module('Integration | Component | consul node agentless-notice', function (hooks) { setupRenderingTest(hooks); - hooks.beforeEach(() => { - const localStore = {}; - - sinon.stub(window.localStorage, 'getItem').callsFake((key) => localStore[key]); - sinon.stub(window.localStorage, 'setItem').callsFake((key, value) => (localStore[key] = value)); - }); test('it does not display the notice if the filtered nodes are the same as the regular nodes', async function (assert) { this.set('nodes', [ @@ -33,7 +26,6 @@ module('Integration | Component | consul node agentless-notice', function (hooks await render( hbs`` ); - assert.true(window.localStorage.getItem.called); assert .dom('[data-test-node-agentless-notice]') .doesNotExist( @@ -66,10 +58,6 @@ module('Integration | Component | consul node agentless-notice', function (hooks assert .dom('[data-test-node-agentless-notice]') .doesNotExist('The agentless notice be dismissed'); - assert.true( - window.localStorage.setItem.calledOnceWith('consul-nodes-agentless-notice-dismissed', 'true'), - "Set the key in localstorage to 'true'" - ); }); test('it does not display if the localstorage key is already set to true', async function (assert) { @@ -81,30 +69,21 @@ module('Integration | Component | consul node agentless-notice', function (hooks }, ]); - this.set('filteredNodes', [ - { - Meta: { - 'synthetic-node': false, - }, - }, - ]); + this.set('filteredNodes', []); - window.localStorage.setItem('consul-nodes-agentless-notice-dismissed-partition', 'true'); + const localStorage = this.owner.lookup('service:local-storage'); + localStorage.storage.seed({ + notices: ['nodes-agentless-dismissed-partition'], + }); await render( hbs`` ); - assert.true( - window.localStorage.getItem.calledOnceWith( - 'consul-nodes-agentless-notice-dismissed-partition' - ) - ); - assert .dom('[data-test-node-agentless-notice]') .doesNotExist( - "The agentless notice should not display if the local storage key has already been set to 'true'" + 'The agentless notice should not display if the dismissal has already been stored in local storage' ); }); }); diff --git a/ui/yarn.lock b/ui/yarn.lock index a1b3fb4c0..88940cfa4 100644 --- a/ui/yarn.lock +++ b/ui/yarn.lock @@ -7862,7 +7862,7 @@ ember-auto-import@^2.2.3, ember-auto-import@^2.4.1, ember-auto-import@^2.4.2: typescript-memoize "^1.0.0-alpha.3" walk-sync "^3.0.0" -ember-basic-dropdown@3.0.21, ember-basic-dropdown@^3.0.16: +ember-basic-dropdown@^3.0.16: version "3.0.21" resolved "https://registry.yarnpkg.com/ember-basic-dropdown/-/ember-basic-dropdown-3.0.21.tgz#5711d071966919c9578d2d5ac2c6dcadbb5ea0e0" integrity sha512-Wu9hJWyqorKo+ZT2PMSIO1BxAeAdaiIC2IjSic0+HcKjmMU47botvG0xbxlprimOWaS9vM+nHat6Pt3xPvcB0A== @@ -7907,7 +7907,7 @@ ember-changeset-validations@~3.9.0: ember-get-config "^0.2.4" ember-validators "^2.0.0" -ember-changeset@3.10.1, ember-changeset@^3.9.1: +ember-changeset@^3.9.1: version "3.10.1" resolved "https://registry.yarnpkg.com/ember-changeset/-/ember-changeset-3.10.1.tgz#d6f06bc55f867a2c1ac7c5fd780776bd1e5a9b60" integrity sha512-4FoGKRcKxixSr+NBQ+ZoiwwbJE0/fuZRULUp9M1RIHejYhst+U8/ni47SsphrMhoRAcZCeyl+JqlBMlwR7v50g== @@ -7982,7 +7982,7 @@ ember-cli-babel@^6.0.0, ember-cli-babel@^6.0.0-beta.4, ember-cli-babel@^6.11.0, ember-cli-version-checker "^2.1.2" semver "^5.5.0" -ember-cli-babel@^7.12.0, ember-cli-babel@^7.13.2, ember-cli-babel@^7.26.1, ember-cli-babel@^7.26.11, ember-cli-babel@^7.26.3, ember-cli-babel@^7.26.5, ember-cli-babel@^7.4.0: +ember-cli-babel@^7.12.0, ember-cli-babel@^7.13.2, ember-cli-babel@^7.26.1, ember-cli-babel@^7.26.10, ember-cli-babel@^7.26.11, ember-cli-babel@^7.26.3, ember-cli-babel@^7.26.5, ember-cli-babel@^7.4.0: version "7.26.11" resolved "https://registry.yarnpkg.com/ember-cli-babel/-/ember-cli-babel-7.26.11.tgz#50da0fe4dcd99aada499843940fec75076249a9f" integrity sha512-JJYeYjiz/JTn34q7F5DSOjkkZqy8qwFOOxXfE6pe9yEJqWGu4qErKxlz8I22JoVEQ/aBUO+OcKTpmctvykM9YA== @@ -8448,6 +8448,22 @@ ember-cli-typescript@^5.0.0: stagehand "^1.0.0" walk-sync "^2.2.0" +ember-cli-typescript@^5.1.0: + version "5.1.1" + resolved "https://registry.yarnpkg.com/ember-cli-typescript/-/ember-cli-typescript-5.1.1.tgz#cf561026f3e7bd05312c1c212acffa1c30d5fa0c" + integrity sha512-DbzATYWY8nbXwSxXqtK8YlqGJTcyFyL+eg6IGCc2ur0AMnq/H+o6Z9np9eGoq1sI+HwX7vBkOVoD3k0WurAwXg== + dependencies: + ansi-to-html "^0.6.15" + broccoli-stew "^3.0.0" + debug "^4.0.0" + execa "^4.0.0" + fs-extra "^9.0.1" + resolve "^1.5.0" + rsvp "^4.8.1" + semver "^7.3.2" + stagehand "^1.0.0" + walk-sync "^2.2.0" + ember-cli-version-checker@^2.1.0, ember-cli-version-checker@^2.1.2: version "2.2.0" resolved "https://registry.yarnpkg.com/ember-cli-version-checker/-/ember-cli-version-checker-2.2.0.tgz#47771b731fe0962705e27c8199a9e3825709f3b3" @@ -9247,6 +9263,14 @@ ember-text-measurer@^0.6.0: ember-cli-babel "^7.19.0" ember-cli-htmlbars "^4.3.1" +ember-tracked-storage-polyfill@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/ember-tracked-storage-polyfill/-/ember-tracked-storage-polyfill-1.0.0.tgz#84d307a1e4badc5f84dca681db2cfea9bdee8a77" + integrity sha512-eL7lZat68E6P/D7b9UoTB5bB5Oh/0aju0Z7PCMi3aTwhaydRaxloE7TGrTRYU+NdJuyNVZXeGyxFxn2frvd3TA== + dependencies: + ember-cli-babel "^7.26.3" + ember-cli-htmlbars "^5.7.1" + "ember-truth-helpers@^2.1.0 || ^3.0.0", ember-truth-helpers@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/ember-truth-helpers/-/ember-truth-helpers-3.0.0.tgz#86766bdca4ac9b86bce3d262dff2aabc4a0ea384" @@ -16641,6 +16665,15 @@ tr46@~0.0.3: resolved "https://registry.yarnpkg.com/tr46/-/tr46-0.0.3.tgz#8184fd347dac9cdc185992f3a6622e14b9d9ab6a" integrity sha512-N3WMsuqV66lT30CrXNbEjx4GEwlow3v6rr4mCcv6prnfwhS01rkgyFdjPNBYd9br7LpXV1+Emh01fHnq2Gdgrw== +tracked-built-ins@^3.1.0: + version "3.1.0" + resolved "https://registry.yarnpkg.com/tracked-built-ins/-/tracked-built-ins-3.1.0.tgz#827703e8e8857e45ac449dfc41e8706e0d6da309" + integrity sha512-yPEZV1aYaw7xFWdoEluvdwNxIJIA834HaBQaMATjNAYPwd1fRqIJ46YnuRo6+9mRRWu6nM6sJqrVVa5H6UhFuw== + dependencies: + ember-cli-babel "^7.26.10" + ember-cli-typescript "^5.1.0" + ember-tracked-storage-polyfill "^1.0.0" + tracked-maps-and-sets@^2.1.0: version "2.2.1" resolved "https://registry.yarnpkg.com/tracked-maps-and-sets/-/tracked-maps-and-sets-2.2.1.tgz#323dd40540c561e8b0ffdec8bf129c68ec5025f9" @@ -17177,7 +17210,7 @@ validate-peer-dependencies@^1.2.0: resolve-package-path "^3.1.0" semver "^7.3.2" -validated-changeset@0.10.0, validated-changeset@~0.10.0: +validated-changeset@~0.10.0: version "0.10.0" resolved "https://registry.yarnpkg.com/validated-changeset/-/validated-changeset-0.10.0.tgz#2e8188c089ab282c1b51fba3c289073f6bd14c8b" integrity sha512-n8NB3ol6Tbi0O7bnq1wz81m5Wd1gfHw0HUcH4MatOfqO3DyXzWZV+bUaNq6wThXn20rMFB82C8pTNFSWbgXJLA== @@ -17675,11 +17708,6 @@ xmlchars@^2.2.0: resolved "https://registry.yarnpkg.com/xmlchars/-/xmlchars-2.2.0.tgz#060fe1bcb7f9c76fe2a17db86a9bc3ab894210cb" integrity sha512-JZnDKK8B0RCDw84FNdDAIpZK+JuJw+s7Lz8nksI7SIuU3UXJJslUthsi+uWBUYOwPFwW7W7PRLRfUKpxjtjFCw== -xmlhttprequest-ssl@^1.6.3: - version "1.6.3" - resolved "https://registry.yarnpkg.com/xmlhttprequest-ssl/-/xmlhttprequest-ssl-1.6.3.tgz#03b713873b01659dfa2c1c5d056065b27ddc2de6" - integrity sha512-3XfeQE/wNkvrIktn2Kf0869fC0BN6UpydVasGIeSm2B1Llihf7/0UfZM+eCkOw3P7bP4+qPgqhm7ZoxuJtFU0Q== - xtend@^4.0.0, xtend@^4.0.1, xtend@^4.0.2, xtend@~4.0.1: version "4.0.2" resolved "https://registry.yarnpkg.com/xtend/-/xtend-4.0.2.tgz#bb72779f5fa465186b1f438f674fa347fdb5db54" From 33186ab5437dbc18cba07d9219d07cef64273fbd Mon Sep 17 00:00:00 2001 From: wenincode Date: Tue, 18 Oct 2022 10:17:03 -0600 Subject: [PATCH 10/11] Update yarn lockfile --- ui/yarn.lock | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ui/yarn.lock b/ui/yarn.lock index 88940cfa4..371678631 100644 --- a/ui/yarn.lock +++ b/ui/yarn.lock @@ -7862,7 +7862,7 @@ ember-auto-import@^2.2.3, ember-auto-import@^2.4.1, ember-auto-import@^2.4.2: typescript-memoize "^1.0.0-alpha.3" walk-sync "^3.0.0" -ember-basic-dropdown@^3.0.16: +ember-basic-dropdown@3.0.21, ember-basic-dropdown@^3.0.16: version "3.0.21" resolved "https://registry.yarnpkg.com/ember-basic-dropdown/-/ember-basic-dropdown-3.0.21.tgz#5711d071966919c9578d2d5ac2c6dcadbb5ea0e0" integrity sha512-Wu9hJWyqorKo+ZT2PMSIO1BxAeAdaiIC2IjSic0+HcKjmMU47botvG0xbxlprimOWaS9vM+nHat6Pt3xPvcB0A== @@ -7907,7 +7907,7 @@ ember-changeset-validations@~3.9.0: ember-get-config "^0.2.4" ember-validators "^2.0.0" -ember-changeset@^3.9.1: +ember-changeset@3.10.1, ember-changeset@^3.9.1: version "3.10.1" resolved "https://registry.yarnpkg.com/ember-changeset/-/ember-changeset-3.10.1.tgz#d6f06bc55f867a2c1ac7c5fd780776bd1e5a9b60" integrity sha512-4FoGKRcKxixSr+NBQ+ZoiwwbJE0/fuZRULUp9M1RIHejYhst+U8/ni47SsphrMhoRAcZCeyl+JqlBMlwR7v50g== @@ -17210,7 +17210,7 @@ validate-peer-dependencies@^1.2.0: resolve-package-path "^3.1.0" semver "^7.3.2" -validated-changeset@~0.10.0: +validated-changeset@0.10.0, validated-changeset@~0.10.0: version "0.10.0" resolved "https://registry.yarnpkg.com/validated-changeset/-/validated-changeset-0.10.0.tgz#2e8188c089ab282c1b51fba3c289073f6bd14c8b" integrity sha512-n8NB3ol6Tbi0O7bnq1wz81m5Wd1gfHw0HUcH4MatOfqO3DyXzWZV+bUaNq6wThXn20rMFB82C8pTNFSWbgXJLA== @@ -17708,6 +17708,11 @@ xmlchars@^2.2.0: resolved "https://registry.yarnpkg.com/xmlchars/-/xmlchars-2.2.0.tgz#060fe1bcb7f9c76fe2a17db86a9bc3ab894210cb" integrity sha512-JZnDKK8B0RCDw84FNdDAIpZK+JuJw+s7Lz8nksI7SIuU3UXJJslUthsi+uWBUYOwPFwW7W7PRLRfUKpxjtjFCw== +xmlhttprequest-ssl@^1.6.3: + version "1.6.3" + resolved "https://registry.yarnpkg.com/xmlhttprequest-ssl/-/xmlhttprequest-ssl-1.6.3.tgz#03b713873b01659dfa2c1c5d056065b27ddc2de6" + integrity sha512-3XfeQE/wNkvrIktn2Kf0869fC0BN6UpydVasGIeSm2B1Llihf7/0UfZM+eCkOw3P7bP4+qPgqhm7ZoxuJtFU0Q== + xtend@^4.0.0, xtend@^4.0.1, xtend@^4.0.2, xtend@~4.0.1: version "4.0.2" resolved "https://registry.yarnpkg.com/xtend/-/xtend-4.0.2.tgz#bb72779f5fa465186b1f438f674fa347fdb5db54" From 7c5e109e179eeada306815c923ad85c62350d7a8 Mon Sep 17 00:00:00 2001 From: wenincode Date: Tue, 18 Oct 2022 11:53:24 -0600 Subject: [PATCH 11/11] Call super with arguments to safeguard against future changes --- .../app/components/consul/node/agentless-notice/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js index ed9d31127..7d0850812 100644 --- a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js +++ b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js @@ -6,8 +6,8 @@ export default class AgentlessNotice extends Component { storageKey = 'nodes-agentless-dismissed'; @storageFor('notices') notices; - constructor(owner, args) { - super(owner, args); + constructor() { + super(...arguments); if (this.args.postfix) { this.storageKey = `nodes-agentless-dismissed-${this.args.postfix}`;