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] Show all applications when opening the search bar #78741

Merged
merged 6 commits into from
Oct 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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;
}
Comment on lines +105 to +109
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other changes in the file are just cleanup. I just added previous search unsubscription to avoid previous results to be displayed in case of network issue (previous call finishing after next call)

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have a test for this to avoid breaking in the future?


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