From 8da06f9b546d438b59f62b3eddebbcd4fba60ad3 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com> Date: Thu, 31 Aug 2023 16:30:00 -0500 Subject: [PATCH] UI: Update kv list filter to not search on type (#22648) --- ui/lib/kv/addon/components/kv-list-filter.hbs | 56 ++---- ui/lib/kv/addon/components/kv-list-filter.js | 155 ++++---------- ui/lib/kv/addon/components/page/list.hbs | 7 +- ui/tests/helpers/kv/kv-selectors.js | 2 +- .../components/kv/kv-list-filter-test.js | 190 +++++++----------- 5 files changed, 140 insertions(+), 270 deletions(-) diff --git a/ui/lib/kv/addon/components/kv-list-filter.hbs b/ui/lib/kv/addon/components/kv-list-filter.hbs index 521be2e3f4..dae80dd4e4 100644 --- a/ui/lib/kv/addon/components/kv-list-filter.hbs +++ b/ui/lib/kv/addon/components/kv-list-filter.hbs @@ -1,35 +1,21 @@ -
-
-

- - -

-
-
-{{#if (and this.filterIsFocused this.isFilterMatch)}} -

- ENTER - to go to see details. - ESC - to clear input. -

-{{else}} -

- TAB - to autocomplete. - ESC - to clear input. -

-{{/if}} \ No newline at end of file +
+ + + + +
\ No newline at end of file diff --git a/ui/lib/kv/addon/components/kv-list-filter.js b/ui/lib/kv/addon/components/kv-list-filter.js index 03bc4effbc..d2980f8b35 100644 --- a/ui/lib/kv/addon/components/kv-list-filter.js +++ b/ui/lib/kv/addon/components/kv-list-filter.js @@ -3,33 +3,41 @@ * SPDX-License-Identifier: MPL-2.0 */ +import Ember from 'ember'; import Component from '@glimmer/component'; import { inject as service } from '@ember/service'; import { action } from '@ember/object'; import keys from 'core/utils/key-codes'; import { keyIsFolder, parentKeyForKey, keyWithoutParentKey } from 'core/utils/key-utils'; -import escapeStringRegexp from 'escape-string-regexp'; import { tracked } from '@glimmer/tracking'; +import { task, timeout } from 'ember-concurrency'; /** * @module KvListFilter - * `KvListFilter` filters through the KV metadata LIST response. It allows users to search through the current list, navigate into directories, and use keyboard functions to: autocomplete, view a secret, create a new secret, or clear the input field. + * `KvListFilter` is used for filtering on the KV metadata LIST response. + * It allows users to search for any text, and will transition to the list + * page with the appropriate parameters depending on the query. This component + * expects that the component will be re-constructed after search, since the + * route will reload the model and completely refresh the page. * * * * @param {array} secrets - An array of secret models. * @param {string} mountPoint - Where in the router files we're located. For this component it will always be vault.cluster.secrets.backend.kv - * @param {string} filterValue - A concatenation between the list-directory's dynamic path "path-to-secret" and the queryParam "pageFilter". For example, if we're inside the beep/ directory searching for any secret that starts with "my-" this value will equal "beep/my-". - * @param {string} pageFilter - The queryParam value, does not include pathToSecret ex: my-. + * @param {string} filterValue - Full initial search value. A concatenation between the list-directory's dynamic path "path-to-secret" and the queryParam "pageFilter". For example, if we're inside the beep/ directory searching for any secret that starts with "my-" this value will equal "beep/my-". */ export default class KvListFilterComponent extends Component { @service router; - @tracked filterIsFocused = false; + @tracked query; + + constructor() { + super(...arguments); + this.query = this.args.filterValue; + } navigate(pathToSecret, pageFilter) { const route = pathToSecret ? `${this.args.mountPoint}.list-directory` : `${this.args.mountPoint}.list`; @@ -45,122 +53,37 @@ export default class KvListFilterComponent extends Component { this.router.transitionTo(...args); } - /* - - partialMatch returns the secret that most closely matches the pageFilter queryParam. - - Searches pageFilter and not filterValue because if we're inside a directory we only care about the secrets listed there and not the directory. - - If pageFilter is empty this returns the first secret model in the list. -**/ - get partialMatch() { - // If pageFilter is empty we replace it with an empty string because you cannot pass 'undefined' to RegEx. - const value = !this.args.pageFilter ? '' : this.args.pageFilter; - const reg = new RegExp('^' + escapeStringRegexp(value)); - const match = this.args.secrets.filter((path) => reg.test(path.fullSecretPath))[0]; - if (this.isFilterMatch || !match) return null; - - return match.fullSecretPath; - } - /* - - isFilterMatch returns true if the filterValue matches a fullSecretPath. -**/ - get isFilterMatch() { - return !!this.args.secrets?.findBy('fullSecretPath', this.args.filterValue); - } - /* - -handleInput is triggered after the value of the input has changed. It is not triggered when input looses focus. -**/ @action - handleInput(event) { - const input = event.target.value; - const isDirectory = keyIsFolder(input); - const parentDirectory = parentKeyForKey(input); - const secretWithinDirectory = keyWithoutParentKey(input); + handleKeyDown(event) { + if (event.keyCode === keys.ESC) { + // On escape, transition to the nearest parentDirectory. + // If no parentDirectory, then to the list route. + const input = event.target.value; + const parentDirectory = parentKeyForKey(input); + !parentDirectory ? this.navigate() : this.navigate(parentDirectory); + } + // ignore all other key events + } + @action handleInput(evt) { + this.query = evt.target.value; + } + + @task + *handleSearch(evt) { + evt.preventDefault(); + // shows loader to indicate that the search was executed + yield timeout(Ember.testing ? 0 : 250); + const searchTerm = this.query; + const isDirectory = keyIsFolder(searchTerm); + const parentDirectory = parentKeyForKey(searchTerm); + const secretWithinDirectory = keyWithoutParentKey(searchTerm); if (isDirectory) { - this.navigate(input); + this.navigate(searchTerm); } else if (parentDirectory) { this.navigate(parentDirectory, secretWithinDirectory); } else { - this.navigate(null, input); - } - } - /* - -handleKeyDown handles: tab, enter, backspace and escape. Ignores everything else. -**/ - @action - handleKeyDown(event) { - const input = event.target.value; - const parentDirectory = parentKeyForKey(input); - - if (event.keyCode === keys.BACKSPACE) { - this.handleBackspace(input, parentDirectory); - } - - if (event.keyCode === keys.TAB) { - event.preventDefault(); - this.handleTab(); - } - - if (event.keyCode === keys.ENTER) { - event.preventDefault(); - this.handleEnter(input); - } - - if (event.keyCode === keys.ESC) { - this.handleEscape(parentDirectory); - } - // ignore all other key events - return; - } - // key-code specific methods - handleBackspace(input, parentDirectory) { - const isInputDirectory = keyIsFolder(input); - const inputWithoutParentKey = keyWithoutParentKey(input); - const pageFilter = isInputDirectory ? '' : inputWithoutParentKey.slice(0, -1); - this.navigate(parentDirectory, pageFilter); - } - handleTab() { - const isMatchDirectory = keyIsFolder(this.partialMatch); - const matchParentDirectory = parentKeyForKey(this.partialMatch); - const matchWithinDirectory = keyWithoutParentKey(this.partialMatch); - - if (isMatchDirectory) { - // ex: beep/boop/ - this.navigate(this.partialMatch); - } else if (!isMatchDirectory && matchParentDirectory) { - // ex: beep/boop/my- - this.navigate(matchParentDirectory, matchWithinDirectory); - } else { - // ex: my- - this.navigate(null, this.partialMatch); - } - } - handleEnter(input) { - if (this.isFilterMatch) { - // if secret exists send to details - this.router.transitionTo(`${this.args.mountPoint}.secret.details`, input); - } else { - // if secret does not exists send to create with the path prefilled with input value. - this.router.transitionTo(`${this.args.mountPoint}.create`, { - queryParams: { initialKey: input }, - }); - } - } - handleEscape(parentDirectory) { - // transition to the nearest parentDirectory. If no parentDirectory, then to the list route. - !parentDirectory ? this.navigate() : this.navigate(parentDirectory); - } - - @action - setFilterIsFocused() { - // tracked property used to show or hide the help-text next to the input. Not involved in focus event itself. - this.filterIsFocused = true; - } - - @action - focusInput() { - // set focus to the input when there is either a pageFilter queryParam value and/or list-directory's dynamic path-to-secret has a value. - if (this.args.filterValue) { - document.getElementById('secret-filter')?.focus(); + this.navigate(null, searchTerm); } } } diff --git a/ui/lib/kv/addon/components/page/list.hbs b/ui/lib/kv/addon/components/page/list.hbs index b6b632276a..e03e5744c0 100644 --- a/ui/lib/kv/addon/components/page/list.hbs +++ b/ui/lib/kv/addon/components/page/list.hbs @@ -7,12 +7,7 @@ <:toolbarFilters> {{#unless @noMetadataListPermissions}} {{#if (or @secrets @filterValue)}} - + {{/if}} {{/unless}} diff --git a/ui/tests/helpers/kv/kv-selectors.js b/ui/tests/helpers/kv/kv-selectors.js index 2d935aaca2..3c9e5efb69 100644 --- a/ui/tests/helpers/kv/kv-selectors.js +++ b/ui/tests/helpers/kv/kv-selectors.js @@ -60,7 +60,7 @@ export const PAGE = { list: { createSecret: '[data-test-toolbar-create-secret]', item: (secret) => (!secret ? '[data-test-list-item]' : `[data-test-list-item="${secret}"]`), - filter: `[data-test-component="kv-list-filter"]`, + filter: `[data-test-kv-list-filter]`, overviewCard: '[data-test-overview-card-container="View secret"]', overviewInput: '[data-test-view-secret] input', overviewButton: '[data-test-get-secret-detail]', diff --git a/ui/tests/integration/components/kv/kv-list-filter-test.js b/ui/tests/integration/components/kv/kv-list-filter-test.js index edeee644a8..c3d2616b39 100644 --- a/ui/tests/integration/components/kv/kv-list-filter-test.js +++ b/ui/tests/integration/components/kv/kv-list-filter-test.js @@ -4,10 +4,11 @@ */ import { module, test } from 'qunit'; +import sinon from 'sinon'; import { setupRenderingTest } from 'vault/tests/helpers'; import { setupEngine } from 'ember-engines/test-support'; import { setupMirage } from 'ember-cli-mirage/test-support'; -import { render, focus, triggerKeyEvent, fillIn } from '@ember/test-helpers'; +import { render, triggerKeyEvent, typeIn, click } from '@ember/test-helpers'; import { hbs } from 'ember-cli-htmlbars'; import { kvMetadataPath } from 'vault/utils/kv-path'; @@ -47,14 +48,16 @@ module('Integration | Component | kv | kv-list-filter', function (hooks) { this.mountPoint = MOUNT_POINT; }); - test('it renders and TAB defaults to first secret in list', async function (assert) { - assert.expect(4); - // mirage hook for TAB - this.owner.lookup('service:router').reopen({ - transitionTo(route, { queryParams: { pageFilter } }) { - assert.strictEqual(route, `${MOUNT_POINT}.list`, 'List route sent when TAB on empty input.'); - assert.deepEqual(pageFilter, 'my-secret', 'Filters to the first secret in the list.'); - }, + test('it transitions correctly for query without slash', async function (assert) { + assert.expect(3); + const routerSvc = this.owner.lookup('service:router'); + sinon.stub(routerSvc, 'transitionTo').callsFake((route, params) => { + assert.strictEqual(route, `${MOUNT_POINT}.list`, 'List route sent when no pathToSecret.'); + assert.deepEqual( + params, + { queryParams: { pageFilter: 'my-secret' } }, + 'Sends correct transition params.' + ); }); await render(hbs``, { @@ -62,136 +65,99 @@ module('Integration | Component | kv | kv-list-filter', function (hooks) { }); assert - .dom('[data-test-component="kv-list-filter"]') - .hasAttribute('placeholder', 'Filter secrets', 'Placeholder applied to input.'); + .dom('[data-test-kv-list-filter]') + .hasAttribute('placeholder', 'Search secret path', 'Placeholder applied to input.'); - await focus('[data-test-component="kv-list-filter"]'); - assert.dom('[data-test-help-tab]').exists('on focus, with no filterValue, displays help text'); - // trigger tab - await triggerKeyEvent('[data-test-component="kv-list-filter"]', 'keydown', 9); + await typeIn('[data-test-kv-list-filter]', 'my-secret'); + await click('[data-test-kv-list-filter-submit]'); }); - test('it filters partial matches', async function (assert) { - assert.expect(2); - // mirage hook for TAB - this.owner.lookup('service:router').reopen({ - transitionTo(route, { queryParams: { pageFilter } }) { - assert.strictEqual(route, `${MOUNT_POINT}.list`, 'List route sent when no pathToSecret.'); - assert.deepEqual(pageFilter, 'my-secret', 'Sets page filter to my-secret.'); - }, + test('it transitions correctly for query ending in /', async function (assert) { + assert.expect(3); + const routerSvc = this.owner.lookup('service:router'); + sinon.stub(routerSvc, 'transitionTo').callsFake((route, params) => { + assert.strictEqual(route, `${MOUNT_POINT}.list-directory`, 'List route sent when params'); + assert.deepEqual(params, 'beep/', 'Sends directory as param'); }); + await render(hbs``, { + owner: this.engine, + }); + + assert + .dom('[data-test-kv-list-filter]') + .hasAttribute('placeholder', 'Search secret path', 'Placeholder applied to input.'); + + await typeIn('[data-test-kv-list-filter]', 'beep/'); + await click('[data-test-kv-list-filter-submit]'); + }); + + test('it transitions correctly for nested query', async function (assert) { + assert.expect(4); + const routerSvc = this.owner.lookup('service:router'); + sinon.stub(routerSvc, 'transitionTo').callsFake((route, params, { queryParams }) => { + assert.strictEqual(route, `${MOUNT_POINT}.list-directory`, 'List route sent when params'); + assert.deepEqual(params, 'beep/', 'Sends directory as url param'); + assert.deepEqual(queryParams, { pageFilter: 'boo' }, 'Sends directory as query param'); + }); + + await render(hbs``, { + owner: this.engine, + }); + + assert + .dom('[data-test-kv-list-filter]') + .hasAttribute('placeholder', 'Search secret path', 'Placeholder applied to input.'); + + await typeIn('[data-test-kv-list-filter]', 'beep/boo'); + await click('[data-test-kv-list-filter-submit]'); + }); + + test('it prefills filterbar from pageFilter', async function (assert) { await render( - hbs``, + hbs``, { owner: this.engine, } ); - // focus on input and trigger TAB - await focus('[data-test-component="kv-list-filter"]'); - await triggerKeyEvent('[data-test-component="kv-list-filter"]', 'keydown', 9); + assert.dom('[data-test-kv-list-filter]').hasValue('beep/boop/bop'); }); - test('it clears last item on backspace and clears to directory on esc', async function (assert) { - assert.expect(8); - // mirage hook for filling in the input - this.owner.lookup('service:router').reopen({ - transitionTo(route, pathToSecret, { queryParams: { pageFilter } }) { - assert.deepEqual(pageFilter, 'boop-', 'Sends the correct pageFilter on fillIn.'); - }, + test('it clears to directory on esc', async function (assert) { + assert.expect(3); + const routerSvc = this.owner.lookup('service:router'); + sinon.stub(routerSvc, 'transitionTo').callsFake((route, params, { queryParams }) => { + assert.strictEqual(route, `${MOUNT_POINT}.list-directory`, 'List route sent when params'); + assert.deepEqual(params, 'beep/boop/', 'Sends base directory as url param'); + assert.deepEqual(queryParams, { pageFilter: null }, 'clears pageFilter param'); }); await render( - hbs``, + hbs``, { owner: this.engine, } ); - // focus on input and trigger backspace - await focus('[data-test-component="kv-list-filter"]'); - await fillIn('[data-test-component="kv-list-filter"]', 'beep/boop-'); - - this.owner.lookup('service:router').reopen({ - transitionTo(route, pathToSecret, { queryParams: { pageFilter } }) { - assert.strictEqual(route, `${MOUNT_POINT}.list-directory`, 'Correct route sent.'); - assert.strictEqual(pathToSecret, 'beep/', 'PathToSecret is the parent directory.'); - assert.deepEqual(pageFilter, 'boop', 'Clears last item in pageFilter on backspace.'); - }, - }); - await triggerKeyEvent('[data-test-component="kv-list-filter"]', 'keydown', 8); - assert.strictEqual( - document.activeElement.id, - 'secret-filter', - 'the input still remains focused after delete.' - ); - - this.owner.lookup('service:router').reopen({ - transitionTo(route, pathToSecret, { queryParams: { pageFilter } }) { - assert.strictEqual(route, `${MOUNT_POINT}.list-directory`, 'Still on a directory route.'); - assert.strictEqual(pathToSecret, 'beep/', 'Parent directory still shown.'); - assert.deepEqual(pageFilter, null, 'Clears pageFilter on escape.'); - }, - }); - // trigger escape - await triggerKeyEvent('[data-test-component="kv-list-filter"]', 'keydown', 27); + // trigger esc + await triggerKeyEvent('[data-test-kv-list-filter]', 'keydown', 27); }); - test('it transitions create page on enter when secret path is new', async function (assert) { - assert.expect(5); - // mirage hook for fillIn - this.owner.lookup('service:router').reopen({ - transitionTo(route, pathToSecret, { queryParams: { pageFilter } }) { - assert.strictEqual(route, `${MOUNT_POINT}.list-directory`, 'Still on a directory route.'); - assert.strictEqual(pathToSecret, 'beep/boop/', 'Parent directory still shown.'); - assert.deepEqual(pageFilter, 'new-secret', 'Sends correct pageFilter.'); - }, + test('it clears to previous directory on esc', async function (assert) { + assert.expect(3); + const routerSvc = this.owner.lookup('service:router'); + sinon.stub(routerSvc, 'transitionTo').callsFake((route, params, { queryParams }) => { + assert.strictEqual(route, `${MOUNT_POINT}.list-directory`, 'List route sent when params'); + assert.deepEqual(params, 'beep/', 'Sends base directory as url param'); + assert.deepEqual(queryParams, { pageFilter: null }, 'clears pageFilter param'); }); await render( - hbs``, + hbs``, { owner: this.engine, } ); - // focus on input, fillIn and then trigger enter - await focus('[data-test-component="kv-list-filter"]'); - await fillIn('[data-test-component="kv-list-filter"]', 'beep/boop/new-secret'); - - // mirage hook for entering to create - this.owner.lookup('service:router').reopen({ - transitionTo(route, { queryParams: { initialKey } }) { - assert.strictEqual( - route, - `${MOUNT_POINT}.create`, - 'Sends to create route when secret does not exists.' - ); - assert.deepEqual(initialKey, 'beep/boop/new-secret', 'It sends full secret path.'); - }, - }); - await triggerKeyEvent('[data-test-component="kv-list-filter"]', 'keydown', 13); - }); - - test('it transitions details page on enter when secret path exists', async function (assert) { - assert.expect(1); - // mirage hook for entering to details - this.owner.lookup('service:router').reopen({ - transitionTo(route) { - assert.strictEqual( - route, - `${MOUNT_POINT}.secret.details`, - 'Sends to details route when secret does exists.' - ); - }, - }); - - await render( - hbs``, - { - owner: this.engine, - } - ); - // focus on input, fillIn and then trigger enter - await focus('[data-test-component="kv-list-filter"]'); - await triggerKeyEvent('[data-test-component="kv-list-filter"]', 'keydown', 13); + // trigger esc + await triggerKeyEvent('[data-test-kv-list-filter]', 'keydown', 27); }); });