Skip to content

Commit

Permalink
[GS] Show all applications when opening the search bar (#78741) (#79422)
Browse files Browse the repository at this point in the history
* increase default number of results to show all apps

* fix circuit breaker

* fix ut

* add unit test
  • Loading branch information
pgayvallet committed Oct 5, 2020
1 parent a91638c commit 2ace108
Show file tree
Hide file tree
Showing 7 changed files with 171 additions and 72 deletions.
7 changes: 7 additions & 0 deletions x-pack/plugins/global_search/common/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

export const defaultMaxProviderResults = 40;
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { HttpStart } from 'src/core/public';
import { GlobalSearchProviderResult, GlobalSearchBatchedResults } from '../../common/types';
import { GlobalSearchFindError } from '../../common/errors';
import { takeInArray } from '../../common/operators';
import { defaultMaxProviderResults } from '../../common/constants';
import { processProviderResult } from '../../common/process_result';
import { ILicenseChecker } from '../../common/license_checker';
import { GlobalSearchResultProvider } from '../types';
Expand Down Expand Up @@ -79,7 +80,6 @@ interface StartDeps {
licenseChecker: ILicenseChecker;
}

const defaultMaxProviderResults = 20;
const mapToUndefined = () => undefined;

/** @internal */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { KibanaRequest, CoreStart, IBasePath } from 'src/core/server';
import { GlobalSearchProviderResult, GlobalSearchBatchedResults } from '../../common/types';
import { GlobalSearchFindError } from '../../common/errors';
import { takeInArray } from '../../common/operators';
import { defaultMaxProviderResults } from '../../common/constants';
import { ILicenseChecker } from '../../common/license_checker';

import { processProviderResult } from '../../common/process_result';
Expand Down Expand Up @@ -80,7 +81,6 @@ interface StartDeps {
licenseChecker: ILicenseChecker;
}

const defaultMaxProviderResults = 20;
const mapToUndefined = () => undefined;

/** @internal */
Expand Down
132 changes: 96 additions & 36 deletions x-pack/plugins/global_search_bar/public/components/search_bar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,19 @@
*/

import React from 'react';
import { wait } from '@testing-library/react';
import { of } from 'rxjs';
import { waitFor, act } from '@testing-library/react';
import { ReactWrapper } from 'enzyme';
import { of, BehaviorSubject } from 'rxjs';
import { filter, map } from 'rxjs/operators';
import { mountWithIntl } from 'test_utils/enzyme_helpers';
import { httpServiceMock, uiSettingsServiceMock } from '../../../../../src/core/public/mocks';
import {
GlobalSearchBatchedResults,
GlobalSearchPluginStart,
GlobalSearchResult,
} from '../../../global_search/public';
import { applicationServiceMock } from '../../../../../src/core/public/mocks';
import { GlobalSearchBatchedResults, GlobalSearchResult } from '../../../global_search/public';
import { globalSearchPluginMock } from '../../../global_search/public/mocks';
import { SearchBar } from '../components/search_bar';
import { SearchBar } from './search_bar';

jest.mock('@elastic/eui/lib/services/accessibility/html_id_generator', () => ({
htmlIdGenerator: () => () => 'mockId',
}));

type Result = { id: string; type: string } | string;

Expand All @@ -38,30 +40,46 @@ const createBatch = (...results: Result[]): GlobalSearchBatchedResults => ({
results: results.map(createResult),
});

jest.mock('@elastic/eui/lib/services/accessibility/html_id_generator', () => ({
htmlIdGenerator: () => () => 'mockId',
}));

const getSelectableProps: any = (component: any) => component.find('EuiSelectable').props();
const getSearchProps: any = (component: any) => component.find('EuiFieldSearch').props();

describe('SearchBar', () => {
let searchService: GlobalSearchPluginStart;
let findSpy: jest.SpyInstance;
const http = httpServiceMock.createSetupContract({ basePath: '/test' });
const basePathUrl = http.basePath.prepend('/plugins/globalSearchBar/assets/');
const uiSettings = uiSettingsServiceMock.createStartContract();
const darkMode = uiSettings.get('theme:darkMode');
let searchService: ReturnType<typeof globalSearchPluginMock.createStartContract>;
let applications: ReturnType<typeof applicationServiceMock.createStartContract>;
const basePathUrl = '/plugins/globalSearchBar/assets/';
const darkMode = false;

let component: ReactWrapper<any>;

beforeEach(() => {
applications = applicationServiceMock.createStartContract();
searchService = globalSearchPluginMock.createStartContract();
findSpy = jest.spyOn(searchService, 'find');
jest.useFakeTimers();
});

const triggerFocus = () => {
component.find('input[data-test-subj="header-search"]').simulate('focus');
};

const update = () => {
act(() => {
jest.runAllTimers();
});
component.update();
};

const simulateTypeChar = async (text: string) => {
await waitFor(() =>
getSearchProps(component).onKeyUpCapture({ currentTarget: { value: text } })
);
};

const getDisplayedOptionsLabel = () => {
return getSelectableProps(component).options.map((option: any) => option.label);
};

it('correctly filters and sorts results', async () => {
const navigate = jest.fn();
findSpy
searchService.find
.mockReturnValueOnce(
of(
createBatch('Discover', 'Canvas'),
Expand All @@ -70,35 +88,37 @@ describe('SearchBar', () => {
)
.mockReturnValueOnce(of(createBatch('Discover', { id: 'My Dashboard', type: 'test' })));

const component = mountWithIntl(
component = mountWithIntl(
<SearchBar
globalSearch={searchService.find}
navigateToUrl={navigate}
navigateToUrl={applications.navigateToUrl}
basePathUrl={basePathUrl}
darkMode={darkMode}
/>
);

expect(findSpy).toHaveBeenCalledTimes(0);
component.find('input[data-test-subj="header-search"]').simulate('focus');
jest.runAllTimers();
component.update();
expect(findSpy).toHaveBeenCalledTimes(1);
expect(findSpy).toHaveBeenCalledWith('', {});
expect(searchService.find).toHaveBeenCalledTimes(0);

triggerFocus();
update();

expect(searchService.find).toHaveBeenCalledTimes(1);
expect(searchService.find).toHaveBeenCalledWith('', {});
expect(getSelectableProps(component).options).toMatchSnapshot();
await wait(() => getSearchProps(component).onKeyUpCapture({ currentTarget: { value: 'd' } }));
jest.runAllTimers();
component.update();

await simulateTypeChar('d');
update();

expect(getSelectableProps(component).options).toMatchSnapshot();
expect(findSpy).toHaveBeenCalledTimes(2);
expect(findSpy).toHaveBeenCalledWith('d', {});
expect(searchService.find).toHaveBeenCalledTimes(2);
expect(searchService.find).toHaveBeenCalledWith('d', {});
});

it('supports keyboard shortcuts', () => {
mountWithIntl(
<SearchBar
globalSearch={searchService.find}
navigateToUrl={jest.fn()}
navigateToUrl={applications.navigateToUrl}
basePathUrl={basePathUrl}
darkMode={darkMode}
/>
Expand All @@ -113,4 +133,44 @@ describe('SearchBar', () => {

expect(document.activeElement).toMatchSnapshot();
});

it('only display results from the last search', async () => {
const firstSearchTrigger = new BehaviorSubject<boolean>(false);
const firstSearch = firstSearchTrigger.pipe(
filter((event) => event),
map(() => {
return createBatch('Discover', 'Canvas');
})
);
const secondSearch = of(createBatch('Visualize', 'Map'));

searchService.find.mockReturnValueOnce(firstSearch).mockReturnValueOnce(secondSearch);

component = mountWithIntl(
<SearchBar
globalSearch={searchService.find}
navigateToUrl={applications.navigateToUrl}
basePathUrl={basePathUrl}
darkMode={darkMode}
/>
);

triggerFocus();
update();

expect(searchService.find).toHaveBeenCalledTimes(1);

await simulateTypeChar('d');
update();

expect(getDisplayedOptionsLabel().length).toBe(2);
expect(getDisplayedOptionsLabel()).toEqual(expect.arrayContaining(['Visualize', 'Map']));

firstSearchTrigger.next(true);

update();

expect(getDisplayedOptionsLabel().length).toBe(2);
expect(getDisplayedOptionsLabel()).toEqual(expect.arrayContaining(['Visualize', 'Map']));
});
});
82 changes: 51 additions & 31 deletions x-pack/plugins/global_search_bar/public/components/search_bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ import {
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import { Subscription } from 'rxjs';
import { ApplicationStart } from 'kibana/public';
import React, { useCallback, useState } from 'react';
import React, { useCallback, useState, useRef } from 'react';
import useDebounce from 'react-use/lib/useDebounce';
import useEvent from 'react-use/lib/useEvent';
import useMountedState from 'react-use/lib/useMountedState';
Expand All @@ -45,62 +46,81 @@ const clearField = (field: HTMLInputElement) => {
const cleanMeta = (str: string) => (str.charAt(0).toUpperCase() + str.slice(1)).replace(/-/g, ' ');
const blurEvent = new FocusEvent('blur');

const sortByScore = (a: GlobalSearchResult, b: GlobalSearchResult): number => {
if (a.score < b.score) return 1;
if (a.score > b.score) return -1;
return 0;
};

const sortByTitle = (a: GlobalSearchResult, b: GlobalSearchResult): number => {
const titleA = a.title.toUpperCase(); // ignore upper and lowercase
const titleB = b.title.toUpperCase(); // ignore upper and lowercase
if (titleA < titleB) return -1;
if (titleA > titleB) return 1;
return 0;
};

const resultToOption = (result: GlobalSearchResult): EuiSelectableTemplateSitewideOption => {
const { id, title, url, icon, type, meta } = result;
const option: EuiSelectableTemplateSitewideOption = {
key: id,
label: title,
url,
};

if (icon) {
option.icon = { type: icon };
}

if (type === 'application') {
option.meta = [{ text: meta?.categoryLabel as string }];
} else {
option.meta = [{ text: cleanMeta(type) }];
}

return option;
};

export function SearchBar({ globalSearch, navigateToUrl, basePathUrl, darkMode }: Props) {
const isMounted = useMountedState();
const [searchValue, setSearchValue] = useState<string>('');
const [searchRef, setSearchRef] = useState<HTMLInputElement | null>(null);
const searchSubscription = useRef<Subscription | null>(null);
const [options, _setOptions] = useState([] as EuiSelectableTemplateSitewideOption[]);
const isMac = navigator.platform.toLowerCase().indexOf('mac') >= 0;

const setOptions = useCallback(
(_options: GlobalSearchResult[]) => {
if (!isMounted()) return;

_setOptions([
..._options.map(({ id, title, url, icon, type, meta }) => {
const option: EuiSelectableTemplateSitewideOption = {
key: id,
label: title,
url,
};

if (icon) option.icon = { type: icon };

if (type === 'application') option.meta = [{ text: meta?.categoryLabel as string }];
else option.meta = [{ text: cleanMeta(type) }];
if (!isMounted()) {
return;
}

return option;
}),
]);
_setOptions(_options.map(resultToOption));
},
[isMounted, _setOptions]
);

useDebounce(
() => {
// cancel pending search if not completed yet
if (searchSubscription.current) {
searchSubscription.current.unsubscribe();
searchSubscription.current = null;
}

let arr: GlobalSearchResult[] = [];
globalSearch(searchValue, {}).subscribe({
searchSubscription.current = globalSearch(searchValue, {}).subscribe({
next: ({ results }) => {
if (searchValue.length > 0) {
arr = [...results, ...arr].sort((a, b) => {
if (a.score < b.score) return 1;
if (a.score > b.score) return -1;
return 0;
});
arr = [...results, ...arr].sort(sortByScore);
setOptions(arr);
return;
}

// if searchbar is empty, filter to only applications and sort alphabetically
results = results.filter(({ type }: GlobalSearchResult) => type === 'application');

arr = [...results, ...arr].sort((a, b) => {
const titleA = a.title.toUpperCase(); // ignore upper and lowercase
const titleB = b.title.toUpperCase(); // ignore upper and lowercase
if (titleA < titleB) return -1;
if (titleA > titleB) return 1;
return 0;
});
arr = [...results, ...arr].sort(sortByTitle);

setOptions(arr);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import { EMPTY } from 'rxjs';
import { toArray } from 'rxjs/operators';
import { TestScheduler } from 'rxjs/testing';
import {
SavedObjectsFindResponse,
Expand Down Expand Up @@ -114,8 +115,8 @@ describe('savedObjectsResultProvider', () => {
expect(provider.id).toBe('savedObjects');
});

it('calls `savedObjectClient.find` with the correct parameters', () => {
provider.find('term', defaultOption, context);
it('calls `savedObjectClient.find` with the correct parameters', async () => {
await provider.find('term', defaultOption, context).toPromise();

expect(context.core.savedObjects.client.find).toHaveBeenCalledTimes(1);
expect(context.core.savedObjects.client.find).toHaveBeenCalledWith({
Expand All @@ -128,6 +129,13 @@ describe('savedObjectsResultProvider', () => {
});
});

it('does not call `savedObjectClient.find` if `term` is empty', async () => {
const results = await provider.find('', defaultOption, context).pipe(toArray()).toPromise();

expect(context.core.savedObjects.client.find).not.toHaveBeenCalled();
expect(results).toEqual([[]]);
});

it('converts the saved objects to results', async () => {
context.core.savedObjects.client.find.mockResolvedValue(
createFindResponse([
Expand Down
Loading

0 comments on commit 2ace108

Please sign in to comment.