From 911b6136931c2c19a6dee31a5863a8485661cf12 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Wed, 28 Apr 2021 13:31:05 -0500 Subject: [PATCH] ui: Change global search to use fuzzy search API (#10412) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This updates the UI to use the new fuzzy search API. It’s a drop-in replacement so the / shortcut to jump to search is preserved, and results can be cycled through and chosen via arrow keys and the enter key. It doesn’t use everything returned by the API: * deployments and evaluations: these match by id, doesn’t seem like people would know those or benefit from quick navigation to them * namespaces: doesn’t seem useful as they currently function * scaling policies * tasks: the response doesn’t include an allocation id, which means they can’t be navigated to in the UI without an additional query * CSI volumes: aren’t actually returned by the API Since there’s no API to check the server configuration and know whether the feature has been disabled, this adds another query in route:application#beforeModel that acts as feature detection: if the attempt to query fails (500), the global search field is hidden. Upon having added another query on load, I realised that beforeModel was being triggered any time service:router#transitionTo was being called, which happens upon navigating to a search result, for instance, because of refreshModel being present on the region query parameter. This PR adds a check for transition.queryParamsOnly and skips rerunning the onload queries (token permissions check, license check, fuzzy search feature detection). Implementation notes: * there are changes to unrelated tests to ignore the on-load feature detection query * some lifecycle-related guards against undefined were required to address failures when navigating to an allocation * the minimum search length of 2 characters is hard-coded as there’s currently no way to determine min_term_length in the UI --- CHANGELOG.md | 1 + ui/app/components/global-header.js | 1 + ui/app/components/global-search/control.js | 210 +++++++++--------- ui/app/components/global-search/match.js | 54 ----- ui/app/components/lifecycle-chart-row.js | 4 + ui/app/components/lifecycle-chart.js | 5 +- .../allocations/allocation/index.js | 2 +- ui/app/routes/application.js | 62 ++++-- ui/app/services/data-caches.js | 33 --- ui/app/services/system.js | 18 ++ .../components/global-search-dropdown.scss | 4 - ui/app/templates/components/global-header.hbs | 8 +- .../components/global-search/control.hbs | 3 +- .../components/global-search/match.hbs | 5 - ui/mirage/config.js | 68 ++++++ ui/tests/acceptance/allocation-detail-test.js | 2 +- ui/tests/acceptance/client-detail-test.js | 30 +-- ui/tests/acceptance/regions-test.js | 2 + ui/tests/acceptance/search-test.js | 166 ++++++-------- ui/tests/pages/layout.js | 29 +-- 20 files changed, 336 insertions(+), 371 deletions(-) delete mode 100644 ui/app/components/global-search/match.js delete mode 100644 ui/app/services/data-caches.js delete mode 100644 ui/app/templates/components/global-search/match.hbs diff --git a/CHANGELOG.md b/CHANGELOG.md index d32a654c750f..707e57a7540f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ IMPROVEMENTS: * networking: Added support for interpolating host network names with node attributes. [[GH-10196](https://github.com/hashicorp/nomad/issues/10196)] * nomad/structs: Removed deprecated Node.Drain field, added API extensions to restore it [[GH-10202](https://github.com/hashicorp/nomad/issues/10202)] * ui: Added a job reversion button [[GH-10336](https://github.com/hashicorp/nomad/pull/10336)] + * ui: Updated global search to use fuzzy search API [[GH-10412](https://github.com/hashicorp/nomad/pull/10412)] BUG FIXES: * core (Enterprise): Update licensing library to v0.0.11 to include race condition fix. [[GH-10253](https://github.com/hashicorp/nomad/issues/10253)] diff --git a/ui/app/components/global-header.js b/ui/app/components/global-header.js index a1b13deb1a25..2e89fcb55181 100644 --- a/ui/app/components/global-header.js +++ b/ui/app/components/global-header.js @@ -5,6 +5,7 @@ import { inject as service } from '@ember/service'; @classic export default class GlobalHeader extends Component { @service config; + @service system; 'data-test-global-header' = true; onHamburgerClick() {} diff --git a/ui/app/components/global-search/control.js b/ui/app/components/global-search/control.js index a72f2ef1c31e..84bc9a2bfff3 100644 --- a/ui/app/components/global-search/control.js +++ b/ui/app/components/global-search/control.js @@ -1,39 +1,23 @@ import Component from '@ember/component'; import { classNames } from '@ember-decorators/component'; import { task } from 'ember-concurrency'; -import EmberObject, { action, computed, set } from '@ember/object'; -import { alias } from '@ember/object/computed'; +import { action, set } from '@ember/object'; import { inject as service } from '@ember/service'; import { debounce, run } from '@ember/runloop'; -import Searchable from 'nomad-ui/mixins/searchable'; -import classic from 'ember-classic-decorator'; const SLASH_KEY = 191; const MAXIMUM_RESULTS = 10; @classNames('global-search-container') export default class GlobalSearchControl extends Component { - @service dataCaches; @service router; - @service store; + @service token; searchString = null; constructor() { super(...arguments); this['data-test-search-parent'] = true; - - this.jobSearch = JobSearch.create({ - dataSource: this, - }); - - this.nodeNameSearch = NodeNameSearch.create({ - dataSource: this, - }); - - this.nodeIdSearch = NodeIdSearch.create({ - dataSource: this, - }); } keyDownHandler(e) { @@ -57,34 +41,85 @@ export default class GlobalSearchControl extends Component { } @task(function*(string) { - try { - set(this, 'searchString', string); - - const jobs = yield this.dataCaches.fetch('job'); - const nodes = yield this.dataCaches.fetch('node'); - - set(this, 'jobs', jobs.toArray()); - set(this, 'nodes', nodes.toArray()); - - const jobResults = this.jobSearch.listSearched.slice(0, MAXIMUM_RESULTS); - - const mergedNodeListSearched = this.nodeIdSearch.listSearched.concat(this.nodeNameSearch.listSearched).uniq(); - const nodeResults = mergedNodeListSearched.slice(0, MAXIMUM_RESULTS); - - return [ - { - groupName: resultsGroupLabel('Jobs', jobResults, this.jobSearch.listSearched), - options: jobResults, - }, - { - groupName: resultsGroupLabel('Clients', nodeResults, mergedNodeListSearched), - options: nodeResults, - }, - ]; - } catch (e) { - // eslint-disable-next-line - console.log('exception searching', e); - } + const searchResponse = yield this.token.authorizedRequest('/v1/search/fuzzy', { + method: 'POST', + body: JSON.stringify({ + Text: string, + Context: 'all', + }), + }); + + const results = yield searchResponse.json(); + + const allJobResults = results.Matches.jobs || []; + const allNodeResults = results.Matches.nodes || []; + const allAllocationResults = results.Matches.allocs || []; + const allTaskGroupResults = results.Matches.groups || []; + const allCSIPluginResults = results.Matches.plugins || []; + + const jobResults = allJobResults.slice(0, MAXIMUM_RESULTS).map(({ ID: name, Scope: [ namespace, id ]}) => ({ + type: 'job', + id, + namespace, + label: name, + })); + + const nodeResults = allNodeResults.slice(0, MAXIMUM_RESULTS).map(({ ID: name, Scope: [ id ]}) => ({ + type: 'node', + id, + label: name, + })); + + const allocationResults = allAllocationResults.slice(0, MAXIMUM_RESULTS).map(({ ID: name, Scope: [ , id ]}) => ({ + type: 'allocation', + id, + label: name, + })); + + const taskGroupResults = allTaskGroupResults.slice(0, MAXIMUM_RESULTS).map(({ ID: id, Scope: [ namespace, jobId ]}) => ({ + type: 'task-group', + id, + namespace, + jobId, + label: id, + })); + + const csiPluginResults = allCSIPluginResults.slice(0, MAXIMUM_RESULTS).map(({ ID: id }) => ({ + type: 'plugin', + id, + label: id, + })); + + const { + jobs: jobsTruncated, + nodes: nodesTruncated, + allocs: allocationsTruncated, + groups: taskGroupsTruncated, + plugins: csiPluginsTruncated, + } = results.Truncations; + + return [ + { + groupName: resultsGroupLabel('Jobs', jobResults, allJobResults, jobsTruncated), + options: jobResults, + }, + { + groupName: resultsGroupLabel('Clients', nodeResults, allNodeResults, nodesTruncated), + options: nodeResults, + }, + { + groupName: resultsGroupLabel('Allocations', allocationResults, allAllocationResults, allocationsTruncated), + options: allocationResults, + }, + { + groupName: resultsGroupLabel('Task Groups', taskGroupResults, allTaskGroupResults, taskGroupsTruncated), + options: taskGroupResults, + }, + { + groupName: resultsGroupLabel('CSI Plugins', csiPluginResults, allCSIPluginResults, csiPluginsTruncated), + options: csiPluginResults, + } + ]; }) search; @@ -96,15 +131,26 @@ export default class GlobalSearchControl extends Component { } @action - selectOption(model) { - const itemModelName = model.constructor.modelName; + ensureMinimumLength(string) { + return string.length > 1; + } - if (itemModelName === 'job') { - this.router.transitionTo('jobs.job', model.plainId, { - queryParams: { namespace: model.get('namespace.name') }, + @action + selectOption(model) { + if (model.type === 'job') { + this.router.transitionTo('jobs.job', model.id, { + queryParams: { namespace: model.namespace }, }); - } else if (itemModelName === 'node') { + } else if (model.type === 'node') { this.router.transitionTo('clients.client', model.id); + } else if (model.type === 'task-group') { + this.router.transitionTo('jobs.job.task-group', model.jobId, model.id, { + queryParams: { namespace: model.namespace }, + }); + } else if (model.type === 'plugin') { + this.router.transitionTo('csi.plugins.plugin', model.id); + } else if (model.type === 'allocation') { + this.router.transitionTo('allocations.allocation', model.id); } } @@ -150,61 +196,7 @@ export default class GlobalSearchControl extends Component { } } -@classic -class JobSearch extends EmberObject.extend(Searchable) { - @computed - get searchProps() { - return ['id', 'name']; - } - - @computed - get fuzzySearchProps() { - return ['name']; - } - - @alias('dataSource.jobs') listToSearch; - @alias('dataSource.searchString') searchTerm; - - fuzzySearchEnabled = true; - includeFuzzySearchMatches = true; -} -@classic -class NodeNameSearch extends EmberObject.extend(Searchable) { - @computed - get searchProps() { - return ['name']; - } - - @computed - get fuzzySearchProps() { - return ['name']; - } - - @alias('dataSource.nodes') listToSearch; - @alias('dataSource.searchString') searchTerm; - - fuzzySearchEnabled = true; - includeFuzzySearchMatches = true; -} - -@classic -class NodeIdSearch extends EmberObject.extend(Searchable) { - @computed - get regexSearchProps() { - return ['id']; - } - - @alias('dataSource.nodes') listToSearch; - @computed('dataSource.searchString') - get searchTerm() { - return `^${this.get('dataSource.searchString')}`; - } - - exactMatchEnabled = false; - regexEnabled = true; -} - -function resultsGroupLabel(type, renderedResults, allResults) { +function resultsGroupLabel(type, renderedResults, allResults, truncated) { let countString; if (renderedResults.length < allResults.length) { @@ -213,5 +205,7 @@ function resultsGroupLabel(type, renderedResults, allResults) { countString = renderedResults.length; } - return `${type} (${countString})`; + const truncationIndicator = truncated ? '+' : ''; + + return `${type} (${countString}${truncationIndicator})`; } diff --git a/ui/app/components/global-search/match.js b/ui/app/components/global-search/match.js deleted file mode 100644 index 4360adeeda39..000000000000 --- a/ui/app/components/global-search/match.js +++ /dev/null @@ -1,54 +0,0 @@ -import Component from '@ember/component'; -import { tagName } from '@ember-decorators/component'; -import { computed, get } from '@ember/object'; -import { alias } from '@ember/object/computed'; - -@tagName('') -export default class GlobalSearchMatch extends Component { - @alias('match.fuzzySearchMatches.firstObject') firstMatch; - - @computed('match.name') - get label() { - return get(this, 'match.name') || ''; - } - - @computed('firstMatch.indices.[]', 'label.length') - get substrings() { - const indices = get(this, 'firstMatch.indices'); - const labelLength = this.label.length; - - if (indices) { - return indices.reduce((substrings, [startIndex, endIndex], indicesIndex) => { - if (indicesIndex === 0 && startIndex > 0) { - substrings.push({ - isHighlighted: false, - string: this.label.substring(0, startIndex) - }); - } - - substrings.push({ - isHighlighted: true, - string: this.label.substring(startIndex, endIndex + 1) - }); - - let endIndexOfNextUnhighlightedSubstring; - - if (indicesIndex === indices.length - 1) { - endIndexOfNextUnhighlightedSubstring = labelLength; - } else { - const nextIndices = indices[indicesIndex + 1]; - endIndexOfNextUnhighlightedSubstring = nextIndices[0]; - } - - substrings.push({ - isHighlighted: false, - string: this.label.substring(endIndex + 1, endIndexOfNextUnhighlightedSubstring) - }); - - return substrings; - }, []); - } else { - return null; - } - } -} diff --git a/ui/app/components/lifecycle-chart-row.js b/ui/app/components/lifecycle-chart-row.js index 5a2422cb9d29..25203683b329 100644 --- a/ui/app/components/lifecycle-chart-row.js +++ b/ui/app/components/lifecycle-chart-row.js @@ -26,6 +26,10 @@ export default class LifecycleChartRow extends Component { @computed('task.lifecycleName') get lifecycleLabel() { + if (!this.task) { + return ''; + } + const name = this.task.lifecycleName; if (name.includes('sidecar')) { diff --git a/ui/app/components/lifecycle-chart.js b/ui/app/components/lifecycle-chart.js index a58e6b83999b..07a938109d90 100644 --- a/ui/app/components/lifecycle-chart.js +++ b/ui/app/components/lifecycle-chart.js @@ -24,7 +24,10 @@ export default class LifecycleChart extends Component { tasksOrStates.forEach(taskOrState => { const task = taskOrState.task || taskOrState; - lifecycles[`${task.lifecycleName}s`].push(taskOrState); + + if (task.lifecycleName) { + lifecycles[`${task.lifecycleName}s`].push(taskOrState); + } }); const phases = []; diff --git a/ui/app/controllers/allocations/allocation/index.js b/ui/app/controllers/allocations/allocation/index.js index 04771dbb1977..4068de300d5a 100644 --- a/ui/app/controllers/allocations/allocation/index.js +++ b/ui/app/controllers/allocations/allocation/index.js @@ -47,7 +47,7 @@ export default class IndexController extends Controller.extend(Sortable) { @computed('model.taskGroup.services.@each.name') get services() { - return this.get('model.taskGroup.services').sortBy('name'); + return (this.get('model.taskGroup.services') || []).sortBy('name'); } onDismiss() { diff --git a/ui/app/routes/application.js b/ui/app/routes/application.js index a52f185c7fcb..e0162369eff2 100644 --- a/ui/app/routes/application.js +++ b/ui/app/routes/application.js @@ -26,35 +26,49 @@ export default class ApplicationRoute extends Route { } async beforeModel(transition) { - let exchangeOneTimeToken; + let promises; - if (transition.to.queryParams.ott) { - exchangeOneTimeToken = this.get('token').exchangeOneTimeToken(transition.to.queryParams.ott); + // service:router#transitionTo can cause this to rerun because of refreshModel on + // the region query parameter, this skips rerunning the detection/loading queries. + if (transition.queryParamsOnly) { + promises = Promise.resolve(true); } else { - exchangeOneTimeToken = Promise.resolve(true); - } - try { - await exchangeOneTimeToken; - } catch (e) { - this.controllerFor('application').set('error', e); + let exchangeOneTimeToken; + + if (transition.to.queryParams.ott) { + exchangeOneTimeToken = this.get('token').exchangeOneTimeToken(transition.to.queryParams.ott); + } else { + exchangeOneTimeToken = Promise.resolve(true); + } + + try { + await exchangeOneTimeToken; + } catch (e) { + this.controllerFor('application').set('error', e); + } + + const fetchSelfTokenAndPolicies = this.get('token.fetchSelfTokenAndPolicies') + .perform() + .catch(); + + const fetchLicense = this.get('system.fetchLicense') + .perform() + .catch(); + + const checkFuzzySearchPresence = this.get('system.checkFuzzySearchPresence') + .perform() + .catch(); + + promises = await RSVP.all([ + this.get('system.regions'), + this.get('system.defaultRegion'), + fetchLicense, + fetchSelfTokenAndPolicies, + checkFuzzySearchPresence, + ]); } - const fetchSelfTokenAndPolicies = this.get('token.fetchSelfTokenAndPolicies') - .perform() - .catch(); - - const fetchLicense = this.get('system.fetchLicense') - .perform() - .catch(); - - const promises = await RSVP.all([ - this.get('system.regions'), - this.get('system.defaultRegion'), - fetchLicense, - fetchSelfTokenAndPolicies, - ]); - if (!this.get('system.shouldShowRegions')) return promises; const queryParam = transition.to.queryParams.region; diff --git a/ui/app/services/data-caches.js b/ui/app/services/data-caches.js deleted file mode 100644 index 7617c61c5830..000000000000 --- a/ui/app/services/data-caches.js +++ /dev/null @@ -1,33 +0,0 @@ -import Service, { inject as service } from '@ember/service'; - -export const COLLECTION_CACHE_DURATION = 60000; // one minute - -export default class DataCachesService extends Service { - @service router; - @service store; - @service system; - - collectionLastFetched = {}; - - async fetch(modelName) { - const modelNameToRoute = { - job: 'jobs', - node: 'clients', - }; - - const route = modelNameToRoute[modelName]; - const lastFetched = this.collectionLastFetched[modelName]; - const now = Date.now(); - - if (this.router.isActive(route)) { - // TODO Incorrect because it’s constantly being fetched by watchers, shouldn’t be marked as last fetched only on search - this.collectionLastFetched[modelName] = now; - return this.store.peekAll(modelName); - } else if (lastFetched && now - lastFetched < COLLECTION_CACHE_DURATION) { - return this.store.peekAll(modelName); - } else { - this.collectionLastFetched[modelName] = now; - return this.store.findAll(modelName); - } - } -} diff --git a/ui/app/services/system.js b/ui/app/services/system.js index 0f526b571754..106987c6a396 100644 --- a/ui/app/services/system.js +++ b/ui/app/services/system.js @@ -157,7 +157,25 @@ export default class SystemService extends Service { }) fetchLicense; + @task(function*() { + try { + const request = yield this.token.authorizedRequest('/v1/search/fuzzy', { + method: 'POST', + body: JSON.stringify({ + Text: 'feature-detection-query', + Context: 'namespaces', + }), + }); + + return request.ok; + } catch (e) { + return false; + } + }) + checkFuzzySearchPresence; + @alias('fetchLicense.lastSuccessful.value') license; + @alias('checkFuzzySearchPresence.last.value') fuzzySearchEnabled; @computed('license.License.Features.[]') get features() { diff --git a/ui/app/styles/components/global-search-dropdown.scss b/ui/app/styles/components/global-search-dropdown.scss index 27e5030020d1..0a4d52972e2e 100644 --- a/ui/app/styles/components/global-search-dropdown.scss +++ b/ui/app/styles/components/global-search-dropdown.scss @@ -42,10 +42,6 @@ background: transparentize($blue, 0.8); color: $blue; } - - .highlighted { - font-weight: $weight-semibold; - } } } diff --git a/ui/app/templates/components/global-header.hbs b/ui/app/templates/components/global-header.hbs index 4beb56b39aa8..2fd688ee6c14 100644 --- a/ui/app/templates/components/global-header.hbs +++ b/ui/app/templates/components/global-header.hbs @@ -7,9 +7,11 @@ - {{#unless (media "isMobile")}} - - {{/unless}} + {{#if this.system.fuzzySearchEnabled}} + {{#unless (media "isMobile")}} + + {{/unless}} + {{/if}}
{{#if this.config.APP.showStorybookLink}} Storybook diff --git a/ui/app/templates/components/global-search/control.hbs b/ui/app/templates/components/global-search/control.hbs index 4c1226b47298..fd044f76f65f 100644 --- a/ui/app/templates/components/global-search/control.hbs +++ b/ui/app/templates/components/global-search/control.hbs @@ -3,6 +3,7 @@ data-test-search @searchEnabled={{true}} @search={{perform this.search}} + @onInput={{action 'ensureMinimumLength'}} @onChange={{action 'selectOption'}} @onFocus={{action 'openOnClickOrTab'}} @onClose={{action 'onCloseEvent'}} @@ -12,5 +13,5 @@ @triggerComponent="global-search/trigger" @registerAPI={{action 'storeSelect'}} as |option|> - + {{option.label}} diff --git a/ui/app/templates/components/global-search/match.hbs b/ui/app/templates/components/global-search/match.hbs deleted file mode 100644 index 97671e3033c3..000000000000 --- a/ui/app/templates/components/global-search/match.hbs +++ /dev/null @@ -1,5 +0,0 @@ -{{#if this.substrings}} - {{#each this.substrings as |substring|}}{{substring.string}}{{/each}} -{{else}} - {{this.label}} -{{/if}} diff --git a/ui/mirage/config.js b/ui/mirage/config.js index 1641fca8c6c4..2cee56d13375 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -577,6 +577,74 @@ export default function() { }); }); + this.post('/search/fuzzy', function( { allocations, jobs, nodes, taskGroups, csiPlugins }, { requestBody }) { + const { Text } = JSON.parse(requestBody); + + const matchedAllocs = allocations.where(allocation => allocation.name.includes(Text)); + const matchedGroups = taskGroups.where(taskGroup => taskGroup.name.includes(Text)); + const matchedJobs = jobs.where(job => job.name.includes(Text)); + const matchedNodes = nodes.where(node => node.name.includes(Text)); + const matchedPlugins = csiPlugins.where(plugin => plugin.id.includes(Text)); + + const transformedAllocs = matchedAllocs.models.map(alloc => ({ + ID: alloc.name, + Scope: [ + alloc.namespace.id, + alloc.id, + ], + })); + + const transformedGroups = matchedGroups.models.map(group => ({ + ID: group.name, + Scope: [ + group.job.namespace, + group.job.id, + ], + })); + + const transformedJobs = matchedJobs.models.map(job => ({ + ID: job.name, + Scope: [ + job.namespace, + job.id, + ] + })); + + const transformedNodes = matchedNodes.models.map(node => ({ + ID: node.name, + Scope: [ + node.id, + ], + })); + + const transformedPlugins = matchedPlugins.models.map(plugin => ({ + ID: plugin.id, + })); + + const truncatedAllocs = transformedAllocs.slice(0, 20); + const truncatedGroups = transformedGroups.slice(0, 20); + const truncatedJobs = transformedJobs.slice(0, 20); + const truncatedNodes = transformedNodes.slice(0, 20); + const truncatedPlugins = transformedPlugins.slice(0, 20); + + return { + Matches: { + allocs: truncatedAllocs, + groups: truncatedGroups, + jobs: truncatedJobs, + nodes: truncatedNodes, + plugins: truncatedPlugins, + }, + Truncations: { + allocs: truncatedAllocs.length < truncatedAllocs.length, + groups: truncatedGroups.length < transformedGroups.length, + jobs: truncatedJobs.length < transformedJobs.length, + nodes: truncatedNodes.length < transformedNodes.length, + plugins: truncatedPlugins.length < transformedPlugins.length, + }, + } + }); + this.get('/recommendations', function( { jobs, namespaces, recommendations }, { queryParams: { job: id, namespace } } diff --git a/ui/tests/acceptance/allocation-detail-test.js b/ui/tests/acceptance/allocation-detail-test.js index af0d97292e19..0fa549395e1b 100644 --- a/ui/tests/acceptance/allocation-detail-test.js +++ b/ui/tests/acceptance/allocation-detail-test.js @@ -274,7 +274,7 @@ module('Acceptance | allocation detail', function(hooks) { await Allocation.stop.confirm(); assert.equal( - server.pretender.handledRequests.findBy('method', 'POST').url, + server.pretender.handledRequests.reject(request => request.url.includes('fuzzy')).findBy('method', 'POST').url, `/v1/allocation/${allocation.id}/stop`, 'Stop request is made for the allocation' ); diff --git a/ui/tests/acceptance/client-detail-test.js b/ui/tests/acceptance/client-detail-test.js index 890a03982370..c5f9638655a8 100644 --- a/ui/tests/acceptance/client-detail-test.js +++ b/ui/tests/acceptance/client-detail-test.js @@ -17,6 +17,10 @@ let clientToken; const wasPreemptedFilter = allocation => !!allocation.preemptedByAllocation; +function nonSearchPOSTS() { + return server.pretender.handledRequests.reject(request => request.url.includes('fuzzy')).filterBy('method', 'POST'); +} + module('Acceptance | client detail', function(hooks) { setupApplicationTest(hooks); setupMirage(hooks); @@ -576,7 +580,7 @@ module('Acceptance | client detail', function(hooks) { assert.ok(ClientDetail.eligibilityToggle.isActive); ClientDetail.eligibilityToggle.toggle(); - await waitUntil(() => server.pretender.handledRequests.findBy('method', 'POST')); + await waitUntil(() => nonSearchPOSTS()); assert.ok(ClientDetail.eligibilityToggle.isDisabled); server.pretender.resolve(server.pretender.requestReferences[0].request); @@ -586,7 +590,7 @@ module('Acceptance | client detail', function(hooks) { assert.notOk(ClientDetail.eligibilityToggle.isActive); assert.notOk(ClientDetail.eligibilityToggle.isDisabled); - const request = server.pretender.handledRequests.findBy('method', 'POST'); + const request = nonSearchPOSTS()[0]; assert.equal(request.url, `/v1/node/${node.id}/eligibility`); assert.deepEqual(JSON.parse(request.requestBody), { NodeID: node.id, @@ -594,11 +598,11 @@ module('Acceptance | client detail', function(hooks) { }); ClientDetail.eligibilityToggle.toggle(); - await waitUntil(() => server.pretender.handledRequests.filterBy('method', 'POST').length === 2); + await waitUntil(() => nonSearchPOSTS().length === 2); server.pretender.resolve(server.pretender.requestReferences[0].request); assert.ok(ClientDetail.eligibilityToggle.isActive); - const request2 = server.pretender.handledRequests.filterBy('method', 'POST')[1]; + const request2 = nonSearchPOSTS()[1]; assert.equal(request2.url, `/v1/node/${node.id}/eligibility`); assert.deepEqual(JSON.parse(request2.requestBody), { @@ -619,7 +623,7 @@ module('Acceptance | client detail', function(hooks) { await ClientDetail.drainPopover.toggle(); await ClientDetail.drainPopover.submit(); - request = server.pretender.handledRequests.filterBy('method', 'POST').pop(); + request = nonSearchPOSTS().pop(); assert.equal(request.url, `/v1/node/${node.id}/drain`); assert.deepEqual( @@ -638,7 +642,7 @@ module('Acceptance | client detail', function(hooks) { await ClientDetail.drainPopover.deadlineToggle.toggle(); await ClientDetail.drainPopover.submit(); - request = server.pretender.handledRequests.filterBy('method', 'POST').pop(); + request = nonSearchPOSTS().pop(); assert.deepEqual( JSON.parse(request.requestBody), @@ -657,7 +661,7 @@ module('Acceptance | client detail', function(hooks) { await ClientDetail.drainPopover.deadlineOptions.options[1].choose(); await ClientDetail.drainPopover.submit(); - request = server.pretender.handledRequests.filterBy('method', 'POST').pop(); + request = nonSearchPOSTS().pop(); assert.deepEqual( JSON.parse(request.requestBody), @@ -678,7 +682,7 @@ module('Acceptance | client detail', function(hooks) { await ClientDetail.drainPopover.setCustomDeadline('1h40m20s'); await ClientDetail.drainPopover.submit(); - request = server.pretender.handledRequests.filterBy('method', 'POST').pop(); + request = nonSearchPOSTS().pop(); assert.deepEqual( JSON.parse(request.requestBody), @@ -697,7 +701,7 @@ module('Acceptance | client detail', function(hooks) { await ClientDetail.drainPopover.forceDrainToggle.toggle(); await ClientDetail.drainPopover.submit(); - request = server.pretender.handledRequests.filterBy('method', 'POST').pop(); + request = nonSearchPOSTS().pop(); assert.deepEqual( JSON.parse(request.requestBody), @@ -715,7 +719,7 @@ module('Acceptance | client detail', function(hooks) { await ClientDetail.drainPopover.systemJobsToggle.toggle(); await ClientDetail.drainPopover.submit(); - request = server.pretender.handledRequests.filterBy('method', 'POST').pop(); + request = nonSearchPOSTS().pop(); assert.deepEqual( JSON.parse(request.requestBody), @@ -744,7 +748,7 @@ module('Acceptance | client detail', function(hooks) { await ClientDetail.drainPopover.cancel(); assert.notOk(ClientDetail.drainPopover.isOpen); - assert.equal(server.pretender.handledRequests.filterBy('method', 'POST'), 0); + assert.equal(nonSearchPOSTS(), 0); }); test('toggling eligibility is disabled while a drain is active', async function(assert) { @@ -769,7 +773,7 @@ module('Acceptance | client detail', function(hooks) { await ClientDetail.stopDrain.idle(); await ClientDetail.stopDrain.confirm(); - const request = server.pretender.handledRequests.findBy('method', 'POST'); + const request = nonSearchPOSTS()[0]; assert.equal(request.url, `/v1/node/${node.id}/drain`); assert.deepEqual(JSON.parse(request.requestBody), { NodeID: node.id, @@ -801,7 +805,7 @@ module('Acceptance | client detail', function(hooks) { await ClientDetail.drainDetails.force.idle(); await ClientDetail.drainDetails.force.confirm(); - const request = server.pretender.handledRequests.findBy('method', 'POST'); + const request = nonSearchPOSTS()[0]; assert.equal(request.url, `/v1/node/${node.id}/drain`); assert.deepEqual(JSON.parse(request.requestBody), { NodeID: node.id, diff --git a/ui/tests/acceptance/regions-test.js b/ui/tests/acceptance/regions-test.js index d667bf30dd40..0e48903ff210 100644 --- a/ui/tests/acceptance/regions-test.js +++ b/ui/tests/acceptance/regions-test.js @@ -162,10 +162,12 @@ module('Acceptance | regions (many)', function(hooks) { await Layout.gutter.visitClients(); await Layout.gutter.visitServers(); const [ + , , , // License request // Token/policies request + // Search feature detection regionsRequest, defaultRegionRequest, ...appRequests diff --git a/ui/tests/acceptance/search-test.js b/ui/tests/acceptance/search-test.js index 10d9f7aa6fb1..8045071ea03e 100644 --- a/ui/tests/acceptance/search-test.js +++ b/ui/tests/acceptance/search-test.js @@ -6,46 +6,37 @@ import { setupMirage } from 'ember-cli-mirage/test-support'; import Layout from 'nomad-ui/tests/pages/layout'; import JobsList from 'nomad-ui/tests/pages/jobs/list'; import { selectSearch } from 'ember-power-select/test-support'; -import sinon from 'sinon'; - -import { COLLECTION_CACHE_DURATION } from 'nomad-ui/services/data-caches'; - -function getRequestCount(server, url) { - return server.pretender.handledRequests.filterBy('url', url).length; -} +import Response from 'ember-cli-mirage/response'; module('Acceptance | search', function(hooks) { setupApplicationTest(hooks); setupMirage(hooks); - test('search searches jobs and nodes with route- and time-based caching and navigates to chosen items', async function(assert) { + test('search exposes and navigates to results from the fuzzy search endpoint', async function(assert) { server.create('node', { name: 'xyz' }); const otherNode = server.create('node', { name: 'ghi' }); - server.create('job', { id: 'vwxyz', namespaceId: 'default' }); - server.create('job', { id: 'xyz', name: 'xyz job', namespace: 'default' }); - server.create('job', { id: 'abc', namespace: 'default' }); + server.create('job', { id: 'vwxyz', namespaceId: 'default', groupsCount: 1, groupTaskCount: 1 }); + server.create('job', { id: 'xyz', name: 'xyz job', namespaceId: 'default', groupsCount: 1, groupTaskCount: 1 }); + server.create('job', { id: 'abc', namespaceId: 'default', groupsCount: 1, groupTaskCount: 1 }); - await visit('/'); + const firstAllocation = server.schema.allocations.all().models[0]; + const firstTaskGroup = server.schema.taskGroups.all().models[0]; - const clock = sinon.useFakeTimers({ - now: new Date(), - shouldAdvanceTime: true, - }); + server.create('csi-plugin', { id: 'xyz-plugin', createVolumes: false }); - let presearchJobsRequestCount = getRequestCount(server, '/v1/jobs'); - let presearchNodesRequestCount = getRequestCount(server, '/v1/nodes'); + await visit('/'); await selectSearch(Layout.navbar.search.scope, 'xy'); Layout.navbar.search.as(search => { - assert.equal(search.groups.length, 2); + assert.equal(search.groups.length, 5); search.groups[0].as(jobs => { assert.equal(jobs.name, 'Jobs (2)'); assert.equal(jobs.options.length, 2); - assert.equal(jobs.options[0].text, 'xyz job'); - assert.equal(jobs.options[1].text, 'vwxyz'); + assert.equal(jobs.options[0].text, 'vwxyz'); + assert.equal(jobs.options[1].text, 'xyz job'); }); search.groups[1].as(clients => { @@ -53,99 +44,75 @@ module('Acceptance | search', function(hooks) { assert.equal(clients.options.length, 1); assert.equal(clients.options[0].text, 'xyz'); }); + + search.groups[2].as(allocs => { + assert.equal(allocs.name, 'Allocations (0)'); + assert.equal(allocs.options.length, 0); + }); + + search.groups[3].as(groups => { + assert.equal(groups.name, 'Task Groups (0)'); + assert.equal(groups.options.length, 0); + }); + + search.groups[4].as(plugins => { + assert.equal(plugins.name, 'CSI Plugins (1)'); + assert.equal(plugins.options.length, 1); + assert.equal(plugins.options[0].text, 'xyz-plugin'); + }); }); - assert.equal( - getRequestCount(server, '/v1/jobs'), - presearchJobsRequestCount, - 'no new jobs request should be sent when in the jobs hierarchy' - ); - assert.equal( - getRequestCount(server, '/v1/nodes'), - presearchNodesRequestCount + 1, - 'a nodes request should happen when not in the clients hierarchy' - ); - - await Layout.navbar.search.groups[0].options[0].click(); + await Layout.navbar.search.groups[0].options[1].click(); assert.equal(currentURL(), '/jobs/xyz'); - await selectSearch(Layout.navbar.search.scope, otherNode.id.substr(0, 3)); - + await selectSearch(Layout.navbar.search.scope, otherNode.name); await Layout.navbar.search.groups[1].options[0].click(); assert.equal(currentURL(), `/clients/${otherNode.id}`); - presearchJobsRequestCount = getRequestCount(server, '/v1/jobs'); - presearchNodesRequestCount = getRequestCount(server, '/v1/nodes'); - - await selectSearch(Layout.navbar.search.scope, 'zzzzzzzzzzz'); - - assert.equal( - getRequestCount(server, '/v1/jobs'), - presearchJobsRequestCount, - 'a jobs request should not happen because the cache hasn’t expired' - ); - assert.equal( - presearchNodesRequestCount, - getRequestCount(server, '/v1/nodes'), - 'no new nodes request should happen when in the clients hierarchy' - ); + await selectSearch(Layout.navbar.search.scope, firstAllocation.name); + assert.equal(Layout.navbar.search.groups[2].options[0].text, firstAllocation.name); + await Layout.navbar.search.groups[2].options[0].click(); + assert.equal(currentURL(), `/allocations/${firstAllocation.id}`); - clock.tick(COLLECTION_CACHE_DURATION * 2); + await selectSearch(Layout.navbar.search.scope, firstTaskGroup.name); + assert.equal(Layout.navbar.search.groups[3].options[0].text, firstTaskGroup.name); + await Layout.navbar.search.groups[3].options[0].click(); + assert.equal(currentURL(), `/jobs/vwxyz/${firstTaskGroup.name}`); - await selectSearch(Layout.navbar.search.scope, otherNode.id.substr(0, 3)); + await selectSearch(Layout.navbar.search.scope, 'xy'); + await Layout.navbar.search.groups[4].options[0].click(); + assert.equal(currentURL(), '/csi/plugins/xyz-plugin'); - assert.equal( - getRequestCount(server, '/v1/jobs'), - presearchJobsRequestCount + 1, - 'a jobs request should happen because the cache has expired' - ); + const featureDetectionQueries = server.pretender.handledRequests + .filterBy('url', '/v1/search/fuzzy') + .filter(request => request.requestBody.includes('feature-detection-query')); - clock.restore(); + assert.equal(featureDetectionQueries.length, 1, 'expect the feature detection query to only run once'); }); - test('search highlights matching substrings', async function(assert) { - server.create('node', { name: 'xyz' }); - - server.create('job', { id: 'traefik', namespaceId: 'default' }); - server.create('job', { id: 'tracking', namespace: 'default' }); - server.create('job', { id: 'smtp-sensor', namespaceId: 'default' }); - + test('search does not perform a request when only one character has been entered', async function(assert) { await visit('/'); - await selectSearch(Layout.navbar.search.scope, 'trae'); - - Layout.navbar.search.as(search => { - search.groups[0].as(jobs => { - assert.equal(jobs.options[0].text, 'traefik'); - assert.equal(jobs.options[0].formattedText, '*trae*fik'); + await selectSearch(Layout.navbar.search.scope, 'q'); - assert.equal(jobs.options[1].text, 'tracking'); - assert.equal(jobs.options[1].formattedText, '*tra*cking'); - }); - }); - - await selectSearch(Layout.navbar.search.scope, 'ra'); + assert.ok(Layout.navbar.search.noOptionsShown); + assert.equal(server.pretender.handledRequests.filterBy('url', '/v1/search/fuzzy').length, 1, 'expect the feature detection query'); + }); - Layout.navbar.search.as(search => { - search.groups[0].as(jobs => { - assert.equal(jobs.options[0].formattedText, 't*ra*efik'); - assert.equal(jobs.options[1].formattedText, 't*ra*cking'); - }); + test('when fuzzy search is disabled on the server, the search control is hidden', async function(assert) { + server.post('/search/fuzzy', function() { + return new Response(500, {}, ''); }); - await selectSearch(Layout.navbar.search.scope, 'sensor'); + await visit('/'); - Layout.navbar.search.as(search => { - search.groups[0].as(jobs => { - assert.equal(jobs.options[0].formattedText, '*s*mtp-*sensor*'); - }); - }); + assert.ok(Layout.navbar.search.isHidden); }); test('results are truncated at 10 per group', async function(assert) { server.create('node', { name: 'xyz' }); - for (let i = 0; i < 15; i++) { + for (let i = 0; i < 11; i++) { server.create('job', { id: `job-${i}`, namespaceId: 'default' }); } @@ -155,29 +122,26 @@ module('Acceptance | search', function(hooks) { Layout.navbar.search.as(search => { search.groups[0].as(jobs => { - assert.equal(jobs.name, 'Jobs (showing 10 of 15)'); + assert.equal(jobs.name, 'Jobs (showing 10 of 11)'); assert.equal(jobs.options.length, 10); }); }); }); - test('node id prefix matches take priority over node name matches', async function(assert) { - const nodeToMatchById = server.create('node', { name: 'xyz' }); - - const idPrefix = nodeToMatchById.id.substr(0, 5); + test('server-side truncation is indicated in the group label', async function(assert) { + server.create('node', { name: 'xyz' }); - const nodeToMatchByName = server.create('node', { - name: `node-name-with-id-piece-${idPrefix}`, - }); + for (let i = 0; i < 21; i++) { + server.create('job', { id: `job-${i}`, namespaceId: 'default' }); + } await visit('/'); - await selectSearch(Layout.navbar.search.scope, idPrefix); + await selectSearch(Layout.navbar.search.scope, 'job'); Layout.navbar.search.as(search => { - search.groups[1].as(clients => { - assert.equal(clients.options[0].text, nodeToMatchById.name); - assert.equal(clients.options[1].text, nodeToMatchByName.name); + search.groups[0].as(jobs => { + assert.equal(jobs.name, 'Jobs (showing 10 of 20+)'); }); }); }); diff --git a/ui/tests/pages/layout.js b/ui/tests/pages/layout.js index c2a34c343f0e..e9d3de2c2cda 100644 --- a/ui/tests/pages/layout.js +++ b/ui/tests/pages/layout.js @@ -4,6 +4,7 @@ import { clickable, collection, hasClass, + isHidden, isPresent, text, } from 'ember-cli-page-object'; @@ -31,28 +32,12 @@ export default create({ resetScope: true, name: text('.ember-power-select-group-name'), - options: collection( - '.ember-power-select-option', - create({ - label: text(), - - substrings: collection('[data-test-match-substring]', { - isHighlighted: hasClass('highlighted'), - }), - - get formattedText() { - return this.substrings - .map(string => { - if (string.isHighlighted) { - return `*${string.text}*`; - } else { - return string.text; - } - }) - .join(''); - }, - }) - ), + options: collection('.ember-power-select-option'), + }), + + noOptionsShown: isHidden('.ember-power-select-options', { + testContainer: '.ember-basic-dropdown-content', + resetScope: true, }), field: {