[Discover] Improve support for pinned filters in surrounding documents (#135722)

* [Discover] Update surrounding documents to support global filters

* [Discover] Finish adding support for global filters in surrounding documents

* [Discover] Fix broken jest tests

* [Discover] Fixed null initial global state in surrounding documents

* [Discover] Fix issue with null filters when query param gets cleared, and add functional tests for changes

* [Discover] Remove filterManager from useEffect dependencies

* [Discover] Fix 'cannot read properties of undefined' toast when navigating between surrounding documents while filters are present

* [Discover] Fix typo in createInitialGlobalState, and clear current app filters when leaving surrounding documents so they don't appear on other screens

* [Discover] Fix issue where Discover breadcrumb link breaks in surrounding documents after filters have been added or modified

* [Discover] Add support for syncing the current pinned filters in surrounding documents to the Discover breadcrumb link

* Revert "[Discover] Add support for syncing the current pinned filters in surrounding documents to the Discover breadcrumb link"

This reverts commit 651a0fcb4b.

* [Discover] Undo change to clear local filters when leaving Surrounding documents
This commit is contained in:
Davis McPhee 2022-07-14 10:48:47 -03:00 committed by GitHub
parent 274c2c7478
commit 21a9b24fed
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 184 additions and 52 deletions

View file

@ -21,6 +21,7 @@ import { uiSettingsMock } from '../../__mocks__/ui_settings';
import { themeServiceMock } from '@kbn/core/public/mocks';
import { LocalStorageMock } from '../../__mocks__/local_storage_mock';
import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public';
import { dataPluginMock } from '@kbn/data-plugin/public/mocks';
const mockFilterManager = createFilterManagerMock();
const mockNavigationPlugin = { ui: { TopNavMenu: mockTopNavMenu } };
@ -28,6 +29,7 @@ const mockNavigationPlugin = { ui: { TopNavMenu: mockTopNavMenu } };
describe('ContextApp test', () => {
const services = {
data: {
...dataPluginMock.createStartContract(),
search: {
searchSource: {
createEmpty: jest.fn(),

View file

@ -19,7 +19,7 @@ import { i18n } from '@kbn/i18n';
import { DOC_TABLE_LEGACY, SEARCH_FIELDS_FROM_SOURCE } from '../../../common';
import { ContextErrorMessage } from './components/context_error_message';
import { LoadingStatus } from './services/context_query_state';
import { AppState, isEqualFilters } from './services/context_state';
import { AppState, GlobalState, isEqualFilters } from './services/context_state';
import { useColumns } from '../../hooks/use_data_grid_columns';
import { useContextAppState } from './hooks/use_context_app_state';
import { useContextAppFetch } from './hooks/use_context_app_fetch';
@ -52,8 +52,9 @@ export const ContextApp = ({ indexPattern, anchorId }: ContextAppProps) => {
/**
* Context app state
*/
const { appState, setAppState } = useContextAppState({ services });
const { appState, globalState, setAppState } = useContextAppState({ services });
const prevAppState = useRef<AppState>();
const prevGlobalState = useRef<GlobalState>({ filters: [] });
/**
* Context fetched state
@ -85,13 +86,18 @@ export const ContextApp = ({ indexPattern, anchorId }: ContextAppProps) => {
fetchSurroundingRows(SurrDocType.PREDECESSORS);
} else if (prevAppState.current.successorCount !== appState.successorCount) {
fetchSurroundingRows(SurrDocType.SUCCESSORS);
} else if (!isEqualFilters(prevAppState.current.filters, appState.filters)) {
} else if (
!isEqualFilters(prevAppState.current.filters, appState.filters) ||
!isEqualFilters(prevGlobalState.current.filters, globalState.filters)
) {
fetchContextRows();
}
prevAppState.current = cloneDeep(appState);
prevGlobalState.current = cloneDeep(globalState);
}, [
appState,
globalState,
anchorId,
fetchContextRows,
fetchAllRows,

View file

@ -130,17 +130,19 @@ export function useContextAppFetch({
try {
setState({ [statusKey]: { value: LoadingStatus.LOADING } });
const rows = await fetchSurroundingDocs(
type,
indexPattern,
anchor,
tieBreakerField,
SortDirection.desc,
count,
filters,
data,
useNewFieldsApi
);
const rows = anchor.id
? await fetchSurroundingDocs(
type,
indexPattern,
anchor,
tieBreakerField,
SortDirection.desc,
count,
filters,
data,
useNewFieldsApi
)
: [];
setState({ [type]: rows, [statusKey]: { value: LoadingStatus.LOADED } });
} catch (error) {
setState(createError(statusKey, FailureReason.UNKNOWN, error));

View file

@ -7,13 +7,12 @@
*/
import { useEffect, useMemo, useState } from 'react';
import { cloneDeep } from 'lodash';
import { CONTEXT_DEFAULT_SIZE_SETTING } from '../../../../common';
import { DiscoverServices } from '../../../build_services';
import { AppState, getState } from '../services/context_state';
import { AppState, getState, GlobalState } from '../services/context_state';
export function useContextAppState({ services }: { services: DiscoverServices }) {
const { uiSettings: config, history, core, filterManager } = services;
const { uiSettings: config, history, core } = services;
const stateContainer = useMemo(() => {
return getState({
@ -22,10 +21,14 @@ export function useContextAppState({ services }: { services: DiscoverServices })
history: history(),
toasts: core.notifications.toasts,
uiSettings: config,
data: services.data,
});
}, [config, history, core.notifications.toasts]);
}, [config, history, core.notifications.toasts, services.data]);
const [appState, setState] = useState<AppState>(stateContainer.appState.getState());
const [appState, setAppState] = useState<AppState>(stateContainer.appState.getState());
const [globalState, setGlobalState] = useState<GlobalState>(
stateContainer.globalState.getState()
);
/**
* Sync with app state container
@ -38,32 +41,24 @@ export function useContextAppState({ services }: { services: DiscoverServices })
useEffect(() => {
const unsubscribeAppState = stateContainer.appState.subscribe((newState) => {
setState((prevState) => ({ ...prevState, ...newState }));
const newStateEnsureFilter = { ...newState, filters: newState.filters ?? [] };
setAppState((prevState) => ({ ...prevState, ...newStateEnsureFilter }));
});
return () => unsubscribeAppState();
}, [stateContainer, setState]);
/**
* Take care of filters
*/
useEffect(() => {
const filters = stateContainer.appState.getState().filters;
if (filters) {
filterManager.setAppFilters(cloneDeep(filters));
}
const { setFilters } = stateContainer;
const filterObservable = filterManager.getUpdates$().subscribe(() => {
setFilters(filterManager);
const unsubscribeGlobalState = stateContainer.globalState.subscribe((newState) => {
const newStateEnsureFilter = { ...newState, filters: newState.filters ?? [] };
setGlobalState((prevState) => ({ ...prevState, ...newStateEnsureFilter }));
});
return () => filterObservable.unsubscribe();
}, [filterManager, stateContainer]);
return () => {
unsubscribeAppState();
unsubscribeGlobalState();
};
}, [stateContainer, setAppState]);
return {
appState,
stateContainer,
globalState,
setAppState: stateContainer.setAppState,
};
}

View file

@ -13,7 +13,10 @@ import { createBrowserHistory, History } from 'history';
import { FilterManager } from '@kbn/data-plugin/public';
import { coreMock } from '@kbn/core/public/mocks';
import { SEARCH_FIELDS_FROM_SOURCE } from '../../../../common';
import { discoverServiceMock } from '../../../__mocks__/services';
discoverServiceMock.data.query.filterManager.getAppFilters = jest.fn(() => []);
discoverServiceMock.data.query.filterManager.getGlobalFilters = jest.fn(() => []);
const setupMock = coreMock.createSetup();
describe('Test Discover Context State', () => {
@ -30,6 +33,7 @@ describe('Test Discover Context State', () => {
get: <T>(key: string) =>
(key === SEARCH_FIELDS_FROM_SOURCE ? true : ['_source']) as unknown as T,
} as IUiSettingsClient,
data: discoverServiceMock.data,
});
state.startSync();
});
@ -47,7 +51,11 @@ describe('Test Discover Context State', () => {
"successorCount": 4,
}
`);
expect(state.globalState.getState()).toMatchInlineSnapshot(`null`);
expect(state.globalState.getState()).toMatchInlineSnapshot(`
Object {
"filters": Array [],
}
`);
expect(state.startSync).toBeDefined();
expect(state.stopSync).toBeDefined();
expect(state.getFilters()).toStrictEqual([]);

View file

@ -6,10 +6,10 @@
* Side Public License, v 1.
*/
import { isEqual } from 'lodash';
import { cloneDeep, isEqual } from 'lodash';
import { History } from 'history';
import { NotificationsStart, IUiSettingsClient } from '@kbn/core/public';
import { Filter, compareFilters, COMPARE_ALL_OPTIONS } from '@kbn/es-query';
import { Filter, compareFilters, COMPARE_ALL_OPTIONS, FilterStateStore } from '@kbn/es-query';
import {
createStateContainer,
createKbnUrlStateStorage,
@ -18,7 +18,7 @@ import {
ReduxLikeStateContainer,
} from '@kbn/kibana-utils-plugin/public';
import { FilterManager } from '@kbn/data-plugin/public';
import { connectToQueryState, DataPublicPluginStart, FilterManager } from '@kbn/data-plugin/public';
import { handleSourceColumnState } from '../../../utils/state_helpers';
export interface AppState {
@ -46,7 +46,7 @@ export interface AppState {
sort?: string[][];
}
interface GlobalState {
export interface GlobalState {
/**
* Array of filters
*/
@ -78,6 +78,11 @@ export interface GetStateParams {
* core ui settings service
*/
uiSettings: IUiSettingsClient;
/**
* data service
*/
data: DataPublicPluginStart;
}
export interface GetStateReturn {
@ -128,6 +133,7 @@ export function getState({
history,
toasts,
uiSettings,
data,
}: GetStateParams): GetStateReturn {
const stateStorage = createKbnUrlStateStorage({
useHash: storeInSessionStorage,
@ -135,14 +141,20 @@ export function getState({
...(toasts && withNotifyOnErrors(toasts)),
});
const globalStateInitial = stateStorage.get(GLOBAL_STATE_URL_KEY) as GlobalState;
const globalStateFromUrl = stateStorage.get<GlobalState>(GLOBAL_STATE_URL_KEY) as GlobalState;
const globalStateInitial = createInitialGlobalState(globalStateFromUrl);
const globalStateContainer = createStateContainer<GlobalState>(globalStateInitial);
const appStateFromUrl = stateStorage.get(APP_STATE_URL_KEY) as AppState;
const appStateInitial = createInitialAppState(defaultSize, appStateFromUrl, uiSettings);
const appStateContainer = createStateContainer<AppState>(appStateInitial);
const { start, stop } = syncStates([
const getAllFilters = () => [
...getFilters(globalStateContainer.getState()),
...getFilters(appStateContainer.getState()),
];
const { start: startSyncingStates, stop: stopSyncingStates } = syncStates([
{
storageKey: GLOBAL_STATE_URL_KEY,
stateContainer: {
@ -173,11 +185,33 @@ export function getState({
},
]);
let stopSyncingFilters = () => {};
return {
globalState: globalStateContainer,
appState: appStateContainer,
startSync: start,
stopSync: stop,
startSync: () => {
data.query.filterManager.setFilters(cloneDeep(getAllFilters()));
const stopSyncingAppFilters = connectToQueryState(data.query, appStateContainer, {
filters: FilterStateStore.APP_STATE,
});
const stopSyncingGlobalFilters = connectToQueryState(data.query, globalStateContainer, {
filters: FilterStateStore.GLOBAL_STATE,
});
stopSyncingFilters = () => {
stopSyncingAppFilters();
stopSyncingGlobalFilters();
};
startSyncingStates();
},
stopSync: () => {
stopSyncingFilters();
stopSyncingFilters = () => {};
stopSyncingStates();
},
setAppState: (newState: Partial<AppState>) => {
const oldState = appStateContainer.getState();
const mergedState = { ...oldState, ...newState };
@ -186,10 +220,7 @@ export function getState({
stateStorage.set(APP_STATE_URL_KEY, mergedState, { replace: true });
}
},
getFilters: () => [
...getFilters(globalStateContainer.getState()),
...getFilters(appStateContainer.getState()),
],
getFilters: getAllFilters,
setFilters: (filterManager: FilterManager) => {
// global state filters
const globalFilters = filterManager.getGlobalFilters();
@ -282,3 +313,21 @@ function createInitialAppState(
uiSettings
);
}
/**
* Helper function to return the initial global state, which is a merged object of url state and
* default state
*/
function createInitialGlobalState(urlState: GlobalState): GlobalState {
const defaultState: GlobalState = {
filters: [],
};
if (typeof urlState !== 'object') {
return defaultState;
}
return {
...defaultState,
...urlState,
};
}

View file

@ -66,7 +66,14 @@ const getCurrentBreadcrumbs = (
export const useMainRouteBreadcrumb = () => {
// useRef needed to retrieve initial breadcrumb link from the push state without updates
return useRef(useHistory<HistoryState>().location.state?.breadcrumb).current;
const breadcrumb = useRef<string>();
const history = useHistory<HistoryState>();
if (history.location.state?.breadcrumb) {
breadcrumb.current = history.location.state.breadcrumb;
}
return breadcrumb.current;
};
export const useNavigationProps = ({

View file

@ -6,6 +6,7 @@
* Side Public License, v 1.
*/
import expect from '@kbn/expect';
import { FtrProviderContext } from '../../ftr_provider_context';
const TEST_INDEX_PATTERN = 'logstash-*';
@ -19,6 +20,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const filterBar = getService('filterBar');
const testSubjects = getService('testSubjects');
const retry = getService('retry');
const browser = getService('browser');
const PageObjects = getPageObjects(['common', 'context']);
@ -75,5 +77,66 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
return await filterBar.hasFilter(TEST_ANCHOR_FILTER_FIELD, 'exists', true);
});
});
const addPinnedFilter = async () => {
await filterBar.addFilter(TEST_ANCHOR_FILTER_FIELD, 'IS', TEST_ANCHOR_FILTER_VALUE);
await filterBar.toggleFilterPinned(TEST_ANCHOR_FILTER_FIELD);
};
const everyFieldMatches = async (matches: (field: string[]) => boolean) => {
const fields = await dataGrid.getFields();
return fields.every(matches);
};
it('should update the data grid when a pinned filter is modified', async function () {
await addPinnedFilter();
await PageObjects.context.waitUntilContextLoadingHasFinished();
expect(await everyFieldMatches((field) => field[2] === TEST_ANCHOR_FILTER_VALUE)).to.be(true);
await filterBar.toggleFilterNegated(TEST_ANCHOR_FILTER_FIELD);
await PageObjects.context.waitUntilContextLoadingHasFinished();
expect(await everyFieldMatches((field) => field[2] === TEST_ANCHOR_FILTER_VALUE)).to.be(
false
);
});
const expectFiltersToExist = async () => {
expect(await filterBar.getFilterCount()).to.be(2);
expect(
await filterBar.hasFilter(TEST_ANCHOR_FILTER_FIELD, TEST_ANCHOR_FILTER_VALUE, true, true)
).to.be(true);
expect(await filterBar.hasFilter('extension', 'png')).to.be(true);
expect(
await everyFieldMatches(
(field) => field[1] === 'png' && field[2] === TEST_ANCHOR_FILTER_VALUE
)
).to.be(true);
};
it('should preserve filters when the page is refreshed', async function () {
await addPinnedFilter();
await filterBar.addFilter('extension', 'IS', 'png');
await PageObjects.context.waitUntilContextLoadingHasFinished();
await expectFiltersToExist();
await browser.refresh();
await PageObjects.context.waitUntilContextLoadingHasFinished();
await expectFiltersToExist();
});
it('should update filters when navigating forward and backward in history', async () => {
await filterBar.addFilter('extension', 'IS', 'png');
await PageObjects.context.waitUntilContextLoadingHasFinished();
expect(await filterBar.getFilterCount()).to.be(1);
expect(await filterBar.hasFilter('extension', 'png')).to.be(true);
expect(await everyFieldMatches((field) => field[1] === 'png')).to.be(true);
await browser.goBack();
await PageObjects.context.waitUntilContextLoadingHasFinished();
expect(await filterBar.getFilterCount()).to.be(0);
expect(await everyFieldMatches((field) => field[1] === 'png')).to.be(false);
await browser.goForward();
await PageObjects.context.waitUntilContextLoadingHasFinished();
expect(await filterBar.getFilterCount()).to.be(1);
expect(await filterBar.hasFilter('extension', 'png')).to.be(true);
expect(await everyFieldMatches((field) => field[1] === 'png')).to.be(true);
});
});
}