Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GS] add search syntax support #83422

Merged
merged 30 commits into from
Nov 24, 2020
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
ac7a5b8
add search syntax parsing logic
pgayvallet Nov 16, 2020
16ab001
Merge remote-tracking branch 'upstream/master' into kbn-74290-gs-sear…
pgayvallet Nov 17, 2020
46f6543
fix ts types
pgayvallet Nov 17, 2020
a12bbdb
use type filter in providers
pgayvallet Nov 17, 2020
d4582d3
move search syntax logic to the searchbar
pgayvallet Nov 17, 2020
b1b1af4
fix test plugin types
pgayvallet Nov 17, 2020
7b59a53
Merge remote-tracking branch 'upstream/master' into kbn-74290-gs-sear…
pgayvallet Nov 17, 2020
334c0f2
fix test plugin types again
pgayvallet Nov 17, 2020
3e77eb3
Merge remote-tracking branch 'upstream/master' into kbn-74290-gs-sear…
pgayvallet Nov 18, 2020
952c76b
use `onSearch` prop to disable internal component search
pgayvallet Nov 18, 2020
ef76562
add tag filter support
pgayvallet Nov 18, 2020
db0d913
add FTR tests
pgayvallet Nov 18, 2020
b2e661e
move away from CI group 7
pgayvallet Nov 18, 2020
063a96d
fix unit tests
pgayvallet Nov 18, 2020
37d1dd3
add unit tests
pgayvallet Nov 18, 2020
47cd1e1
remove the API test suite
pgayvallet Nov 18, 2020
bee03aa
Add icons to the SO results
pgayvallet Nov 18, 2020
b28c441
add test for unknown type / tag
pgayvallet Nov 18, 2020
515743e
nits
pgayvallet Nov 18, 2020
4b4ed79
Merge remote-tracking branch 'upstream/master' into kbn-74290-gs-sear…
pgayvallet Nov 19, 2020
baa2013
ignore case for the `type` filter
pgayvallet Nov 19, 2020
76aadcf
Add syntax help text
ryankeairns Nov 19, 2020
e55fd0a
Merge pull request #3 from ryankeairns/kbn-74290-gs-search-syntax
pgayvallet Nov 20, 2020
45424b4
Merge remote-tracking branch 'upstream/master' into kbn-74290-gs-sear…
pgayvallet Nov 20, 2020
c38c529
remove unused import
pgayvallet Nov 20, 2020
194f13b
Merge remote-tracking branch 'upstream/master' into kbn-74290-gs-sear…
pgayvallet Nov 22, 2020
9a21e53
hide icon for non-application results
pgayvallet Nov 22, 2020
35e3fbd
add tsdoc on query utils
pgayvallet Nov 22, 2020
829c8f0
coerce known filter values to string
pgayvallet Nov 22, 2020
6322554
Merge remote-tracking branch 'upstream/master' into kbn-74290-gs-sear…
pgayvallet Nov 24, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/plugins/saved_objects_tagging_oss/public/api.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const createApiUiMock = (): SavedObjectsTaggingApiUiMock => {
convertNameToReference: jest.fn(),
parseSearchQuery: jest.fn(),
getTagIdsFromReferences: jest.fn(),
getTagIdFromName: jest.fn(),
updateTagsReferences: jest.fn(),
};

Expand Down
8 changes: 7 additions & 1 deletion src/plugins/saved_objects_tagging_oss/public/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export interface SavedObjectsTaggingApiUi {
/**
* Convert given tag name to a {@link SavedObjectsFindOptionsReference | reference }
* to be used to search using the savedObjects `_find` API. Will return `undefined`
* is the given name does not match any existing tag.
* if the given name does not match any existing tag.
*/
convertNameToReference(tagName: string): SavedObjectsFindOptionsReference | undefined;

Expand Down Expand Up @@ -124,6 +124,12 @@ export interface SavedObjectsTaggingApiUi {
references: Array<SavedObjectReference | SavedObjectsFindOptionsReference>
): string[];

/**
* Returns the id for given tag name. Will return `undefined`
* if the given name does not match any existing tag.
*/
getTagIdFromName(tagName: string): string | undefined;

/**
* Returns a new references array that replace the old tag references with references to the
* new given tag ids, while preserving all non-tag references.
Expand Down
25 changes: 25 additions & 0 deletions x-pack/plugins/global_search/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,28 @@ export interface GlobalSearchBatchedResults {
*/
results: GlobalSearchResult[];
}

/**
* Search parameters for the {@link GlobalSearchPluginStart.find | `find` API}
*
* @public
*/
export interface GlobalSearchFindParams {
/**
* The term to search for. Can be undefined if searching by filters.
*/
term?: string;
/**
* The types of results to search for.
*/
types?: string[];
/**
* The tag ids to filter search by.
*/
tags?: string[];
}

/**
* @public
*/
export type GlobalSearchProviderFindParams = GlobalSearchFindParams;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this type? Do we expect this type to diverge in the future?

Copy link
Contributor Author

@pgayvallet pgayvallet Nov 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may, depending on how we decide to internally process the parameters from the service before forwarding them to providers. Also atm the service and provider types are all separated, so it was mostly for consistency.

2 changes: 2 additions & 0 deletions x-pack/plugins/global_search/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export {
GlobalSearchProviderResult,
GlobalSearchProviderResultUrl,
GlobalSearchResult,
GlobalSearchFindParams,
GlobalSearchProviderFindParams,
} from '../common/types';
export {
GlobalSearchPluginSetup,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,18 @@ describe('fetchServerResults', () => {
it('perform a POST request to the endpoint with valid options', () => {
http.post.mockResolvedValue({ results: [] });

fetchServerResults(http, 'some term', { preference: 'pref' });
fetchServerResults(
http,
{ term: 'some term', types: ['dashboard', 'map'] },
{ preference: 'pref' }
);

expect(http.post).toHaveBeenCalledTimes(1);
expect(http.post).toHaveBeenCalledWith('/internal/global_search/find', {
body: JSON.stringify({ term: 'some term', options: { preference: 'pref' } }),
body: JSON.stringify({
params: { term: 'some term', types: ['dashboard', 'map'] },
options: { preference: 'pref' },
}),
});
});

Expand All @@ -47,7 +54,11 @@ describe('fetchServerResults', () => {

http.post.mockResolvedValue({ results: [resultA, resultB] });

const results = await fetchServerResults(http, 'some term', { preference: 'pref' }).toPromise();
const results = await fetchServerResults(
http,
{ term: 'some term' },
{ preference: 'pref' }
).toPromise();

expect(http.post).toHaveBeenCalledTimes(1);
expect(results).toHaveLength(2);
Expand All @@ -65,7 +76,7 @@ describe('fetchServerResults', () => {
getTestScheduler().run(({ expectObservable, hot }) => {
http.post.mockReturnValue(hot('---(a|)', { a: { results: [] } }) as any);

const results = fetchServerResults(http, 'term', {});
const results = fetchServerResults(http, { term: 'term' }, {});

expectObservable(results).toBe('---(a|)', {
a: [],
Expand All @@ -77,7 +88,7 @@ describe('fetchServerResults', () => {
getTestScheduler().run(({ expectObservable, hot }) => {
http.post.mockReturnValue(hot('---(a|)', { a: { results: [] } }) as any);
const aborted$ = hot('-(a|)', { a: undefined });
const results = fetchServerResults(http, 'term', { aborted$ });
const results = fetchServerResults(http, { term: 'term' }, { aborted$ });

expectObservable(results).toBe('-|', {
a: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { Observable, from, EMPTY } from 'rxjs';
import { map, takeUntil } from 'rxjs/operators';
import { HttpStart } from 'src/core/public';
import { GlobalSearchResult } from '../../common/types';
import { GlobalSearchResult, GlobalSearchProviderFindParams } from '../../common/types';
import { GlobalSearchFindOptions } from './types';

interface ServerFetchResponse {
Expand All @@ -24,7 +24,7 @@ interface ServerFetchResponse {
*/
export const fetchServerResults = (
http: HttpStart,
term: string,
params: GlobalSearchProviderFindParams,
{ preference, aborted$ }: GlobalSearchFindOptions
): Observable<GlobalSearchResult[]> => {
let controller: AbortController | undefined;
Expand All @@ -36,7 +36,7 @@ export const fetchServerResults = (
}
return from(
http.post<ServerFetchResponse>('/internal/global_search/find', {
body: JSON.stringify({ term, options: { preference } }),
body: JSON.stringify({ params, options: { preference } }),
signal: controller?.signal,
})
).pipe(
Expand Down
40 changes: 23 additions & 17 deletions x-pack/plugins/global_search/public/services/search_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,14 @@ describe('SearchService', () => {
registerResultProvider(provider);

const { find } = service.start(startDeps());
find('foobar', { preference: 'pref' });
find(
{ term: 'foobar', types: ['dashboard', 'map'], tags: ['tag-id'] },
{ preference: 'pref' }
);

expect(provider.find).toHaveBeenCalledTimes(1);
expect(provider.find).toHaveBeenCalledWith(
'foobar',
{ term: 'foobar', types: ['dashboard', 'map'], tags: ['tag-id'] },
expect.objectContaining({ preference: 'pref' })
);
});
Expand All @@ -129,12 +132,15 @@ describe('SearchService', () => {
service.setup({ config: createConfig() });

const { find } = service.start(startDeps());
find('foobar', { preference: 'pref' });
find(
{ term: 'foobar', types: ['dashboard', 'map'], tags: ['tag-id'] },
{ preference: 'pref' }
);

expect(fetchServerResultsMock).toHaveBeenCalledTimes(1);
expect(fetchServerResultsMock).toHaveBeenCalledWith(
httpStart,
'foobar',
{ term: 'foobar', types: ['dashboard', 'map'], tags: ['tag-id'] },
expect.objectContaining({ preference: 'pref', aborted$: expect.any(Object) })
);
});
Expand All @@ -148,25 +154,25 @@ describe('SearchService', () => {
registerResultProvider(provider);

const { find } = service.start(startDeps());
find('foobar', { preference: 'pref' });
find({ term: 'foobar' }, { preference: 'pref' });

expect(getDefaultPreferenceMock).not.toHaveBeenCalled();

expect(provider.find).toHaveBeenNthCalledWith(
1,
'foobar',
{ term: 'foobar' },
expect.objectContaining({
preference: 'pref',
})
);

find('foobar', {});
find({ term: 'foobar' }, {});

expect(getDefaultPreferenceMock).toHaveBeenCalledTimes(1);

expect(provider.find).toHaveBeenNthCalledWith(
2,
'foobar',
{ term: 'foobar' },
expect.objectContaining({
preference: 'default_pref',
})
Expand All @@ -186,7 +192,7 @@ describe('SearchService', () => {
registerResultProvider(createProvider('A', providerResults));

const { find } = service.start(startDeps());
const results = find('foo', {});
const results = find({ term: 'foobar' }, {});

expectObservable(results).toBe('a-b-|', {
a: expectedBatch('1'),
Expand All @@ -207,7 +213,7 @@ describe('SearchService', () => {
fetchServerResultsMock.mockReturnValue(serverResults);

const { find } = service.start(startDeps());
const results = find('foo', {});
const results = find({ term: 'foobar' }, {});

expectObservable(results).toBe('a-b-|', {
a: expectedBatch('1'),
Expand Down Expand Up @@ -242,7 +248,7 @@ describe('SearchService', () => {
);

const { find } = service.start(startDeps());
const results = find('foo', {});
const results = find({ term: 'foobar' }, {});

expectObservable(results).toBe('ab-cd-|', {
a: expectedBatch('A1', 'A2'),
Expand Down Expand Up @@ -276,7 +282,7 @@ describe('SearchService', () => {
);

const { find } = service.start(startDeps());
const results = find('foo', {});
const results = find({ term: 'foobar' }, {});

expectObservable(results).toBe('a-b--(c|)', {
a: expectedBatch('P1'),
Expand All @@ -301,7 +307,7 @@ describe('SearchService', () => {
const aborted$ = hot('----a--|', { a: undefined });

const { find } = service.start(startDeps());
const results = find('foo', { aborted$ });
const results = find({ term: 'foobar' }, { aborted$ });

expectObservable(results).toBe('--a-|', {
a: expectedBatch('1'),
Expand All @@ -323,7 +329,7 @@ describe('SearchService', () => {
registerResultProvider(createProvider('A', providerResults));

const { find } = service.start(startDeps());
const results = find('foo', {});
const results = find({ term: 'foobar' }, {});

expectObservable(results).toBe('a 24ms b 74ms |', {
a: expectedBatch('1'),
Expand Down Expand Up @@ -359,7 +365,7 @@ describe('SearchService', () => {
);

const { find } = service.start(startDeps());
const results = find('foo', {});
const results = find({ term: 'foobar' }, {});

expectObservable(results).toBe('ab-(c|)', {
a: expectedBatch('A1', 'A2'),
Expand Down Expand Up @@ -392,7 +398,7 @@ describe('SearchService', () => {
registerResultProvider(provider);

const { find } = service.start(startDeps());
const batch = await find('foo', {}).pipe(take(1)).toPromise();
const batch = await find({ term: 'foobar' }, {}).pipe(take(1)).toPromise();

expect(batch.results).toHaveLength(2);
expect(batch.results[0]).toEqual({
Expand Down Expand Up @@ -420,7 +426,7 @@ describe('SearchService', () => {
registerResultProvider(createProvider('A', providerResults));

const { find } = service.start(startDeps());
const results = find('foo', {});
const results = find({ term: 'foobar' }, {});

expectObservable(results).toBe(
'#',
Expand Down
21 changes: 14 additions & 7 deletions x-pack/plugins/global_search/public/services/search_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import { map, takeUntil } from 'rxjs/operators';
import { duration } from 'moment';
import { i18n } from '@kbn/i18n';
import { HttpStart } from 'src/core/public';
import { GlobalSearchProviderResult, GlobalSearchBatchedResults } from '../../common/types';
import {
GlobalSearchFindParams,
GlobalSearchProviderResult,
GlobalSearchBatchedResults,
} from '../../common/types';
import { GlobalSearchFindError } from '../../common/errors';
import { takeInArray } from '../../common/operators';
import { defaultMaxProviderResults } from '../../common/constants';
Expand Down Expand Up @@ -52,7 +56,7 @@ export interface SearchServiceStart {
*
* @example
* ```ts
* startDeps.globalSearch.find('some term').subscribe({
* startDeps.globalSearch.find({term: 'some term'}).subscribe({
* next: ({ results }) => {
* addNewResultsToList(results);
* },
Expand All @@ -67,7 +71,10 @@ export interface SearchServiceStart {
* Emissions from the resulting observable will only contains **new** results. It is the consumer's
* responsibility to aggregate the emission and sort the results if required.
*/
find(term: string, options: GlobalSearchFindOptions): Observable<GlobalSearchBatchedResults>;
find(
params: GlobalSearchFindParams,
options: GlobalSearchFindOptions
): Observable<GlobalSearchBatchedResults>;
}

interface SetupDeps {
Expand Down Expand Up @@ -110,11 +117,11 @@ export class SearchService {
this.licenseChecker = licenseChecker;

return {
find: (term, options) => this.performFind(term, options),
find: (params, options) => this.performFind(params, options),
};
}

private performFind(term: string, options: GlobalSearchFindOptions) {
private performFind(params: GlobalSearchFindParams, options: GlobalSearchFindOptions) {
const licenseState = this.licenseChecker!.getState();
if (!licenseState.valid) {
return throwError(
Expand Down Expand Up @@ -142,13 +149,13 @@ export class SearchService {
const processResult = (result: GlobalSearchProviderResult) =>
processProviderResult(result, this.http!.basePath);

const serverResults$ = fetchServerResults(this.http!, term, {
const serverResults$ = fetchServerResults(this.http!, params, {
preference,
aborted$,
});

const providersResults$ = [...this.providers.values()].map((provider) =>
provider.find(term, providerOptions).pipe(
provider.find(params, providerOptions).pipe(
takeInArray(this.maxProviderResults),
takeUntil(aborted$),
map((results) => results.map((r) => processResult(r)))
Expand Down
10 changes: 7 additions & 3 deletions x-pack/plugins/global_search/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
*/

import { Observable } from 'rxjs';
import { GlobalSearchProviderFindOptions, GlobalSearchProviderResult } from '../common/types';
import {
GlobalSearchProviderFindOptions,
GlobalSearchProviderResult,
GlobalSearchProviderFindParams,
} from '../common/types';
import { SearchServiceSetup, SearchServiceStart } from './services';

export type GlobalSearchPluginSetup = Pick<SearchServiceSetup, 'registerResultProvider'>;
Expand All @@ -29,15 +33,15 @@ export interface GlobalSearchResultProvider {
* // returning all results in a single batch
* setupDeps.globalSearch.registerResultProvider({
* id: 'my_provider',
* find: (term, { aborted$, preference, maxResults }, context) => {
* find: ({ term, filters }, { aborted$, preference, maxResults }, context) => {
* const resultPromise = myService.search(term, { preference, maxResults }, context.core.savedObjects.client);
* return from(resultPromise).pipe(takeUntil(aborted$));
* },
* });
* ```
*/
find(
term: string,
search: GlobalSearchProviderFindParams,
options: GlobalSearchProviderFindOptions
): Observable<GlobalSearchProviderResult[]>;
}
Loading