From 540c9e21026b3aa426b8c7fd26dd1c1c6524997e Mon Sep 17 00:00:00 2001 From: Cameron Perera Date: Thu, 9 Feb 2023 16:00:19 -0600 Subject: [PATCH] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20Update=20how=20Worke?= =?UTF-8?q?rDiagram=20is=20used=20in=20Target=20form=20(#1626)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: 💡 Update how WorkerDiagram is used in Target form * refactor: 💡 remove extra div wrappers Remove div wrappers for worker diagrams that were no longer needed * refactor: 💡 rename worker diagram attributes to be more clear * refactor: 💡 remove style changes out of scope for updates * refactor: 💡 update logic for how HCP diagram is shown * chore: 🤖 Add multi-hop flags to OSS block for verbosity --- .../components/form/target/details/index.hbs | 19 +- .../components/form/target/details/index.js | 16 -- .../worker-diagram/dual-filter/hcp/index.hbs | 171 ++++++++---------- .../worker-diagram/dual-filter/index.hbs | 156 ++++++++-------- .../app/components/worker-diagram/index.hbs | 23 ++- .../worker-diagram/single-filter/index.hbs | 6 +- ui/admin/app/styles/app.scss | 2 +- ui/admin/config/environment.js | 2 + .../tests/acceptance/targets/update-test.js | 68 ++++++- .../components/form/target/details-test.js | 57 ------ .../dual-filter/hcp/index-test.js | 16 +- .../worker-diagram/dual-filter/index-test.js | 16 +- .../components/worker-diagram/index-test.js | 18 +- .../single-filter/index-test.js | 10 +- 14 files changed, 288 insertions(+), 292 deletions(-) delete mode 100644 ui/admin/tests/integration/components/form/target/details-test.js diff --git a/ui/admin/app/components/form/target/details/index.hbs b/ui/admin/app/components/form/target/details/index.hbs index 70c04a20a..c3741d826 100644 --- a/ui/admin/app/components/form/target/details/index.hbs +++ b/ui/admin/app/components/form/target/details/index.hbs @@ -224,21 +224,10 @@ {{/if}} {{#if this.showTargetFilters}} - {{#if this.workerDiagramHCPDualFilter}} - - {{else if this.workerDiagramDualFilter}} - - {{else}} - - {{/if}} + {{#if (feature-flag 'target-worker-filters-v2-ingress')}} - {{#if (and @egressFilter @ingressFilter)}} - - - {{t 'resources.target.workers.diagram.client'}} - - - {{t 'resources.target.workers.diagram.ingress-worker'}} - - - - {{t 'resources.target.workers.diagram.egress-worker'}} - - - {{t 'resources.target.workers.diagram.host'}} - - - -
- - {{t - 'resources.target.workers.filter-explainer.dual-egress-on-ingress-on' - }} - -
- {{else if @ingressFilter}} - + + {{t 'resources.target.workers.diagram.client'}} + + + {{t 'resources.target.workers.diagram.ingress-worker'}} + + - - {{t 'resources.target.workers.diagram.client'}} - - {{t 'resources.target.workers.diagram.ingress-worker'}} + {{t 'resources.target.workers.diagram.egress-worker'}} {{t 'resources.target.workers.diagram.host'}} - -
- - {{t 'resources.target.workers.filter-explainer.ingress-worker'}} - -
- {{else if @egressFilter}} - - - {{t 'resources.target.workers.diagram.client'}} - - - {{#if (feature-flag 'target-worker-filters-v2-hcp')}} - {{t 'resources.target.workers.diagram.hcp-worker'}} - {{else}} - {{t 'resources.target.workers.diagram.frontline-worker'}} - {{/if}} - - - - {{t 'resources.target.workers.diagram.egress-worker'}} - - - {{t 'resources.target.workers.diagram.host'}} - - - -
- - {{t 'resources.target.workers.filter-explainer.hcp-dual-egress-on'}} - -
- {{else}} - - - {{t 'resources.target.workers.diagram.client'}} - - + + +
+ + {{t + 'resources.target.workers.filter-explainer.dual-egress-on-ingress-on' + }} + +
+{{else if @ingressWorkerFilterEnabled}} + + + {{t 'resources.target.workers.diagram.client'}} + + + {{t 'resources.target.workers.diagram.ingress-worker'}} + + + {{t 'resources.target.workers.diagram.host'}} + + +
+ + {{t 'resources.target.workers.filter-explainer.ingress-worker'}} + +
+{{else if @egressWorkerFilterEnabled}} + + + {{t 'resources.target.workers.diagram.client'}} + + + {{#if (feature-flag 'target-worker-filters-v2-hcp')}} {{t 'resources.target.workers.diagram.hcp-worker'}} + {{else}} + {{t 'resources.target.workers.diagram.frontline-worker'}} + {{/if}} + + + + {{t 'resources.target.workers.diagram.egress-worker'}} {{t 'resources.target.workers.diagram.host'}} - -
- - {{t 'resources.target.workers.filter-explainer.hcp-worker'}} - -
- {{/if}} - \ No newline at end of file + + +
+ + {{t 'resources.target.workers.filter-explainer.hcp-dual-egress-on'}} + +
+{{else}} + + + {{t 'resources.target.workers.diagram.client'}} + + + {{t 'resources.target.workers.diagram.hcp-worker'}} + + + {{t 'resources.target.workers.diagram.host'}} + + +
+ + {{t 'resources.target.workers.filter-explainer.hcp-worker'}} + +
+{{/if}} \ No newline at end of file diff --git a/ui/admin/app/components/worker-diagram/dual-filter/index.hbs b/ui/admin/app/components/worker-diagram/dual-filter/index.hbs index c3e8ada91..27c6e5000 100644 --- a/ui/admin/app/components/worker-diagram/dual-filter/index.hbs +++ b/ui/admin/app/components/worker-diagram/dual-filter/index.hbs @@ -1,89 +1,87 @@ -
- {{#if (and @egressFilter @ingressFilter)}} - - - {{t 'resources.target.workers.diagram.client'}} - +{{#if (and @egressWorkerFilterEnabled @ingressWorkerFilterEnabled)}} + + + {{t 'resources.target.workers.diagram.client'}} + + + {{t 'resources.target.workers.diagram.ingress-worker'}} + + - {{t 'resources.target.workers.diagram.ingress-worker'}} - - - - {{t 'resources.target.workers.diagram.egress-worker'}} - - - {{t 'resources.target.workers.diagram.host'}} - - - -
- - {{t - 'resources.target.workers.filter-explainer.dual-egress-on-ingress-on' - }} - -
- {{else if @ingressFilter}} - - - {{t 'resources.target.workers.diagram.client'}} - - - {{t 'resources.target.workers.diagram.ingress-worker'}} + {{t 'resources.target.workers.diagram.egress-worker'}} {{t 'resources.target.workers.diagram.host'}} - -
- - {{t 'resources.target.workers.filter-explainer.ingress-worker'}} - -
- {{else if @egressFilter}} - - - {{t 'resources.target.workers.diagram.client'}} - + + +
+ + {{t + 'resources.target.workers.filter-explainer.dual-egress-on-ingress-on' + }} + +
+{{else if @ingressWorkerFilterEnabled}} + + + {{t 'resources.target.workers.diagram.client'}} + + + {{t 'resources.target.workers.diagram.ingress-worker'}} + + + {{t 'resources.target.workers.diagram.host'}} + + +
+ + {{t 'resources.target.workers.filter-explainer.ingress-worker'}} + +
+{{else if @egressWorkerFilterEnabled}} + + + {{t 'resources.target.workers.diagram.client'}} + + + {{t 'resources.target.workers.diagram.frontline-worker'}} + + - {{t 'resources.target.workers.diagram.frontline-worker'}} - - - - {{t 'resources.target.workers.diagram.egress-worker'}} - - - {{t 'resources.target.workers.diagram.host'}} - - - -
- - {{t 'resources.target.workers.filter-explainer.dual-egress-on'}} - -
- {{else}} - - - {{t 'resources.target.workers.diagram.client'}} - - - {{t 'resources.target.workers.diagram.any-worker'}} + {{t 'resources.target.workers.diagram.egress-worker'}} {{t 'resources.target.workers.diagram.host'}} - -
- - {{t 'resources.target.workers.filter-explainer.any-worker'}} - -
- {{/if}} -
\ No newline at end of file + + +
+ + {{t 'resources.target.workers.filter-explainer.dual-egress-on'}} + +
+{{else}} + + + {{t 'resources.target.workers.diagram.client'}} + + + {{t 'resources.target.workers.diagram.any-worker'}} + + + {{t 'resources.target.workers.diagram.host'}} + + +
+ + {{t 'resources.target.workers.filter-explainer.any-worker'}} + +
+{{/if}} \ No newline at end of file diff --git a/ui/admin/app/components/worker-diagram/index.hbs b/ui/admin/app/components/worker-diagram/index.hbs index 166a39661..a3b8e2838 100644 --- a/ui/admin/app/components/worker-diagram/index.hbs +++ b/ui/admin/app/components/worker-diagram/index.hbs @@ -1,6 +1,17 @@ -{{yield - (hash - SingleFilter=(component 'worker-diagram/single-filter') - DualFilter=(component 'worker-diagram/dual-filter') - ) -}} \ No newline at end of file +{{#if (feature-flag 'target-worker-filters-v2-ingress')}} + {{#if (feature-flag 'target-worker-filters-v2-hcp')}} + + {{else}} + + {{/if}} +{{else}} + +{{/if}} \ No newline at end of file diff --git a/ui/admin/app/components/worker-diagram/single-filter/index.hbs b/ui/admin/app/components/worker-diagram/single-filter/index.hbs index 991f605b2..848b27d5d 100644 --- a/ui/admin/app/components/worker-diagram/single-filter/index.hbs +++ b/ui/admin/app/components/worker-diagram/single-filter/index.hbs @@ -1,4 +1,4 @@ -{{#if @egressFilter}} +{{#if @egressWorkerFilterEnabled}} {{t 'resources.target.workers.diagram.client'}} @@ -15,7 +15,7 @@ -
+
{{t 'resources.target.workers.filter-explainer.egress-worker'}} @@ -32,7 +32,7 @@ {{t 'resources.target.workers.diagram.host'}} -
+
{{t 'resources.target.workers.filter-explainer.any-worker'}} diff --git a/ui/admin/app/styles/app.scss b/ui/admin/app/styles/app.scss index 3f451bf83..d208c10ca 100644 --- a/ui/admin/app/styles/app.scss +++ b/ui/admin/app/styles/app.scss @@ -626,7 +626,7 @@ } } - .target-worker-filter-explainer { + .worker-diagram-explainer { min-height: 4rem; // 1 } diff --git a/ui/admin/config/environment.js b/ui/admin/config/environment.js index cf9742693..208f6bbbc 100644 --- a/ui/admin/config/environment.js +++ b/ui/admin/config/environment.js @@ -14,6 +14,8 @@ const featureEditions = { 'byow-pki-upstream': true, 'vault-worker-filter': false, 'target-worker-filters-v2': true, + 'target-worker-filters-v2-ingress': false, + 'target-worker-filters-v2-hcp': false, 'target-network-address': true, 'credential-library-vault-ssh-certificate': false, }, diff --git a/ui/admin/tests/acceptance/targets/update-test.js b/ui/admin/tests/acceptance/targets/update-test.js index 2275adad7..b1a89b182 100644 --- a/ui/admin/tests/acceptance/targets/update-test.js +++ b/ui/admin/tests/acceptance/targets/update-test.js @@ -106,7 +106,7 @@ module('Acceptance | targets | update', function (hooks) { assert.dom('[name=worker_filter]').isDisabled(); }); - test('show update filter button when the old worker filter field is not empty', async function (assert) { + test('show update filter button when the old worker filter field is has value', async function (assert) { featuresService.enable('target-worker-filters-v2'); assert.expect(2); await visit(urls.target); @@ -129,6 +129,25 @@ module('Acceptance | targets | update', function (hooks) { ); }); + test('onClick update filter should select the egress and ingress toggle and the worker_filter value is copied into egress_worker_filter and ingress_worker_filter', async function (assert) { + featuresService.enable('target-worker-filters-v2'); + featuresService.enable('target-worker-filters-v2-ingress'); + assert.expect(3); + await visit(urls.target); + assert.dom('[name=worker_filter]').isVisible(); + const worker_filter_value = this.server.schema.targets.first().workerFilter; + await click('.rose-form-actions [type="button"]', 'Activate edit mode'); + await click('.hds-button[type="button"]', 'Update Filter'); + assert.strictEqual( + this.server.schema.targets.first().egressWorkerFilter, + worker_filter_value + ); + assert.strictEqual( + this.server.schema.targets.first().ingressWorkerFilter, + worker_filter_value + ); + }); + test('show filter input field when the `egress_worker_filter` toggle is on', async function (assert) { featuresService.enable('target-worker-filters-v2'); assert.expect(1); @@ -173,6 +192,53 @@ module('Acceptance | targets | update', function (hooks) { ); }); + test('show filter input field when the `ingress_worker_filter` toggle is on', async function (assert) { + featuresService.enable('target-worker-filters-v2'); + featuresService.enable('target-worker-filters-v2-ingress'); + assert.expect(1); + await visit(urls.target); + await click('.rose-form-actions [type="button"]', 'Activate edit mode'); + await click('.hds-button[type="button"]', 'Update Filter'); + assert.dom('[name=ingress_worker_filter]').isVisible(); + }); + + test('hide filter input field when the `ingress_worker_filter` toggled is off', async function (assert) { + featuresService.enable('target-worker-filters-v2'); + featuresService.enable('target-worker-filters-v2-ingress'); + assert.expect(1); + await visit(urls.target); + await click('.rose-form-actions [type="button"]', 'Activate edit mode'); + await click('.hds-button[type="button"]', 'Update Filter'); + await click('.hds-form-toggle__control', 'Ingress worker filter'); + assert.dom('[name=ingress_worker_filter]').isNotVisible(); + }); + + test('clear `ingress_worker_field` value from a target when the toggle is off and form is saved', async function (assert) { + featuresService.enable('target-worker-filters-v2'); + featuresService.enable('target-worker-filters-v2-ingress'); + assert.expect(5); + await visit(urls.target); + await click('.rose-form-actions [type="button"]', 'Activate edit mode'); + await click('.hds-button[type="button"]', 'Update Filter'); + await fillIn('[name=ingress_worker_filter]', 'random filter string'); + await click('.rose-form-actions [type="submit"]'); + assert.strictEqual(currentURL(), urls.target); + assert.strictEqual( + this.server.schema.targets.first().ingressWorkerFilter, + 'random filter string' + ); + await click('.rose-form-actions [type="button"]', 'Activate edit mode'); + await click('.hds-form-toggle__control', 'Ingress worker filter'); + await click('.rose-form-actions [type="submit"]'); + //clear egress_worker_filter when the toggle is off + assert.strictEqual(currentURL(), urls.target); + assert.dom('[name=ingress_worker_filter]').doesNotExist(); + assert.strictEqual( + this.server.schema.targets.first().ingressWorkerFilter, + null + ); + }); + test('can cancel changes to existing target', async function (assert) { assert.expect(2); await visit(urls.targets); diff --git a/ui/admin/tests/integration/components/form/target/details-test.js b/ui/admin/tests/integration/components/form/target/details-test.js deleted file mode 100644 index 599aeaab1..000000000 --- a/ui/admin/tests/integration/components/form/target/details-test.js +++ /dev/null @@ -1,57 +0,0 @@ -import { module, test } from 'qunit'; -import { setupRenderingTest } from 'ember-qunit'; -import { render, getContext } from '@ember/test-helpers'; -import { hbs } from 'ember-cli-htmlbars'; -import { setupIntl } from 'ember-intl/test-support'; - -module('Integration | Component | form/target/details', function (hooks) { - setupRenderingTest(hooks); - setupIntl(hooks); - - let featuresService; - - hooks.beforeEach(function () { - const { owner } = getContext(); - featuresService = owner.lookup('service:features'); - this.model = { type: 'ssh' }; - this.submit = () => {}; - this.cancel = () => {}; - this.changeType = () => {}; - }); - - test('shows the correct worker diagram when `target-worker-filters-v2` is enabled', async function (assert) { - featuresService.enable('target-worker-filters-v2'); - featuresService.disable('target-worker-filters-v2-ingress'); - featuresService.disable('target-worker-filters-v2-hcp'); - assert.expect(1); - await render( - hbs`` - ); - - assert.dom('[data-test-single-filter-egress-off]').isVisible(); - }); - - test('shows the correct worker diagram when `target-worker-filters-v2` and `target-worker-filters-v2-ingress` is enabled', async function (assert) { - featuresService.enable('target-worker-filters-v2'); - featuresService.enable('target-worker-filters-v2-ingress'); - featuresService.disable('target-worker-filters-v2-hcp'); - assert.expect(1); - await render( - hbs`` - ); - - assert.dom('[data-test-dual-filters]').isVisible(); - }); - - test('shows the correct worker diagram when `target-worker-filters-v2` and `target-worker-filters-v2-ingress` and `target-worker-filters-v2-hcp` is enabled', async function (assert) { - featuresService.enable('target-worker-filters-v2'); - featuresService.enable('target-worker-filters-v2-ingress'); - featuresService.enable('target-worker-filters-v2-hcp'); - assert.expect(1); - await render( - hbs`` - ); - - assert.dom('[data-test-dual-filters-hcp]').isVisible(); - }); -}); diff --git a/ui/admin/tests/integration/components/worker-diagram/dual-filter/hcp/index-test.js b/ui/admin/tests/integration/components/worker-diagram/dual-filter/hcp/index-test.js index 965e1f32b..4bb190094 100644 --- a/ui/admin/tests/integration/components/worker-diagram/dual-filter/hcp/index-test.js +++ b/ui/admin/tests/integration/components/worker-diagram/dual-filter/hcp/index-test.js @@ -20,33 +20,33 @@ module( featuresService.disable(targetWorkerFilterIngress); }); - test('it renders the correct diagram when egressFilter is false and ingress is false', async function (assert) { + test('it renders the correct diagram when egress and ingress filter is false', async function (assert) { await render( - hbs`` + hbs`` ); assert.dom('[data-test-dual-filter-hcp-egress-off-ingress-off]').exists(); }); - test('it renders the correct diagram when egressFilter is true and ingress is false', async function (assert) { + test('it renders the correct diagram when egress filter is true and ingress filter is false', async function (assert) { await render( - hbs`` + hbs`` ); assert.dom('[data-test-dual-filter-hcp-egress-on-ingress-off]').exists(); }); - test('it renders the correct diagram when egressFilter is false and ingress is true', async function (assert) { + test('it renders the correct diagram when egress filter is false and ingress filter is true', async function (assert) { await render( - hbs`` + hbs`` ); assert.dom('[data-test-dual-filter-hcp-egress-off-ingress-on]').exists(); }); - test('it renders the correct diagram when egressFilter is true and ingress is true', async function (assert) { + test('it renders the correct diagram when egress and ingress filter is true', async function (assert) { await render( - hbs`` + hbs`` ); assert.dom('[data-test-dual-filter-hcp-egress-on-ingress-on]').exists(); diff --git a/ui/admin/tests/integration/components/worker-diagram/dual-filter/index-test.js b/ui/admin/tests/integration/components/worker-diagram/dual-filter/index-test.js index ceaf11016..db14d38e7 100644 --- a/ui/admin/tests/integration/components/worker-diagram/dual-filter/index-test.js +++ b/ui/admin/tests/integration/components/worker-diagram/dual-filter/index-test.js @@ -20,33 +20,33 @@ module( featuresService.disable(targetWorkerFilterHCP); }); - test('it renders the correct diagram when egressFilter is false and ingress is false', async function (assert) { + test('it renders the correct diagram when egress and ingress filter is false', async function (assert) { await render( - hbs`` + hbs`` ); assert.dom('[data-test-dual-filter-egress-off-ingress-off]').exists(); }); - test('it renders the correct diagram when egressFilter is true and ingress is false', async function (assert) { + test('it renders the correct diagram when egress filter is true and ingress filter is false', async function (assert) { await render( - hbs`` + hbs`` ); assert.dom('[data-test-dual-filter-egress-on-ingress-off]').exists(); }); - test('it renders the correct diagram when egressFilter is false and ingress is true', async function (assert) { + test('it renders the correct diagram when egress filter is false and ingress filter is true', async function (assert) { await render( - hbs`` + hbs`` ); assert.dom('[data-test-dual-filter-egress-off-ingress-on]').exists(); }); - test('it renders the correct diagram when egressFilter is true and ingress is true', async function (assert) { + test('it renders the correct diagram when egress and ingress filter is true', async function (assert) { await render( - hbs`` + hbs`` ); assert.dom('[data-test-dual-filter-egress-on-ingress-on]').exists(); diff --git a/ui/admin/tests/integration/components/worker-diagram/index-test.js b/ui/admin/tests/integration/components/worker-diagram/index-test.js index 4942bb8b6..a117a3371 100644 --- a/ui/admin/tests/integration/components/worker-diagram/index-test.js +++ b/ui/admin/tests/integration/components/worker-diagram/index-test.js @@ -17,18 +17,30 @@ module('Integration | Component | worker-diagram/index', function (hooks) { featuresService.disable(targetWorkerFilterIngress); featuresService.disable(targetWorkerFilterHCP); assert.expect(1); - await render(hbs``); + await render(hbs``); assert.dom('[data-test-single-filter-egress-off]').isVisible(); }); - test('it renders a dual filter diagram', async function (assert) { + test('it renders a dual filter diagram when `target-worker-filters-v2-ingress` is enabled', async function (assert) { featuresService = this.owner.lookup('service:features'); featuresService.enable(targetWorkerFilterIngress); featuresService.disable(targetWorkerFilterHCP); assert.expect(1); - await render(hbs``); + await render(hbs``); assert.dom('[data-test-dual-filter-egress-off-ingress-off]').isVisible(); }); + + test('it renders a HCP dual filter diagram when `target-worker-filters-v2-hcp` is enabled', async function (assert) { + featuresService = this.owner.lookup('service:features'); + featuresService.enable(targetWorkerFilterIngress); + featuresService.enable(targetWorkerFilterHCP); + assert.expect(1); + await render(hbs``); + + assert + .dom('[data-test-dual-filter-hcp-egress-off-ingress-off]') + .isVisible(); + }); }); diff --git a/ui/admin/tests/integration/components/worker-diagram/single-filter/index-test.js b/ui/admin/tests/integration/components/worker-diagram/single-filter/index-test.js index f8d6071f8..6a9e24351 100644 --- a/ui/admin/tests/integration/components/worker-diagram/single-filter/index-test.js +++ b/ui/admin/tests/integration/components/worker-diagram/single-filter/index-test.js @@ -10,19 +10,21 @@ module( setupRenderingTest(hooks); setupIntl(hooks); - test('it renders the correct diagram when egressFilter is false', async function (assert) { + test('it renders the correct diagram when egress filter is false', async function (assert) { assert.expect(2); await render( - hbs`` + hbs`` ); assert.dom('[data-test-single-filter-egress-off]').isVisible(); assert.dom('[data-test-single-filter-egress-on]').doesNotExist(); }); - test('it renders the correct diagram when egressFilter is true', async function (assert) { + test('it renders the correct diagram when egress filter is true', async function (assert) { assert.expect(2); - await render(hbs``); + await render( + hbs`` + ); assert.dom('[data-test-single-filter-egress-on]').isVisible(); assert.dom('[data-test-single-filter-egress-off]').doesNotExist();