[Discover] [Unified Histogram] Fix reset search button not fully resetting Unified Histogram state (#155967)

## Summary

This PR fixes an issue where resetting a saved search would not fully
reset the Unified Histogram state. The issue was caused by multiple
state values changing in quick succession which resulted in some state
getting overwritten, so I've updated `use_discover_state` to batch state
updates and improve diffing when state changes.

Fixes #151395.

### Checklist

- [ ] ~Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~
- [ ]
~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials~
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] ~Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard
accessibility](https://webaim.org/techniques/keyboard/))~
- [ ] ~Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~
- [ ] ~If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~
- [ ] ~This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~
- [ ] ~This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)~

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
This commit is contained in:
Davis McPhee 2023-04-30 19:50:58 -03:00 committed by GitHub
parent fe190722c5
commit 9d023f6a42
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 771 additions and 624 deletions

View file

@ -144,6 +144,7 @@ describe('useDiscoverHistogram', () => {
beforeEach(() => {
mockCheckHitCount.mockClear();
});
it('should subscribe to state changes', async () => {
const { hook } = await renderUseDiscoverHistogram();
const api = createMockUnifiedHistogramApi();
@ -218,6 +219,7 @@ describe('useDiscoverHistogram', () => {
act(() => {
hook.result.current.ref(api);
});
stateContainer.appState.update({ hideChart: true, interval: '1m', breakdownField: 'test' });
expect(api.setTotalHits).toHaveBeenCalled();
expect(api.setChartHidden).toHaveBeenCalled();
expect(api.setTimeInterval).toHaveBeenCalled();
@ -225,15 +227,15 @@ describe('useDiscoverHistogram', () => {
expect(Object.keys(params ?? {})).toEqual([
'totalHitsStatus',
'totalHitsResult',
'chartHidden',
'timeInterval',
'breakdownField',
'timeInterval',
'chartHidden',
]);
});
it('should exclude totalHitsStatus and totalHitsResult from Unified Histogram state updates after the first load', async () => {
const stateContainer = getStateContainer();
const { hook, initialProps } = await renderUseDiscoverHistogram({ stateContainer });
const { hook } = await renderUseDiscoverHistogram({ stateContainer });
const containerState = stateContainer.appState.getState();
const state = {
timeInterval: containerState.interval,
@ -255,13 +257,14 @@ describe('useDiscoverHistogram', () => {
act(() => {
hook.result.current.ref(api);
});
stateContainer.appState.update({ hideChart: true });
expect(Object.keys(params ?? {})).toEqual([
'totalHitsStatus',
'totalHitsResult',
'chartHidden',
]);
params = {};
hook.rerender({ ...initialProps, hideChart: true });
stateContainer.appState.update({ hideChart: false });
act(() => {
subject$.next({
...state,

View file

@ -14,10 +14,11 @@ import {
} from '@kbn/unified-histogram-plugin/public';
import { isEqual } from 'lodash';
import { useCallback, useEffect, useRef, useMemo, useState } from 'react';
import { distinctUntilChanged, filter, map, Observable } from 'rxjs';
import { distinctUntilChanged, filter, map, Observable, pairwise, startWith } from 'rxjs';
import useObservable from 'react-use/lib/useObservable';
import type { Suggestion } from '@kbn/lens-plugin/public';
import useLatest from 'react-use/lib/useLatest';
import type { RequestAdapter } from '@kbn/inspector-plugin/common';
import { useDiscoverServices } from '../../../../hooks/use_discover_services';
import { getUiActions } from '../../../../kibana_services';
import { FetchStatus } from '../../../types';
@ -25,10 +26,10 @@ import { useDataState } from '../../hooks/use_data_state';
import type { InspectorAdapters } from '../../hooks/use_inspector';
import type { DataDocuments$ } from '../../services/discover_data_state_container';
import { checkHitCount, sendErrorTo } from '../../hooks/use_saved_search_messages';
import { useAppStateSelector } from '../../services/discover_app_state_container';
import type { DiscoverStateContainer } from '../../services/discover_state';
import { addLog } from '../../../../utils/add_log';
import { useInternalStateSelector } from '../../services/discover_internal_state_container';
import type { DiscoverAppState } from '../../services/discover_app_state_container';
export interface UseDiscoverHistogramProps {
stateContainer: DiscoverStateContainer;
@ -80,25 +81,26 @@ export const useDiscoverHistogram = ({
*/
useEffect(() => {
const subscription = createStateSyncObservable(unifiedHistogram?.state$)?.subscribe((state) => {
inspectorAdapters.lensRequests = state.lensRequestAdapter;
const subscription = createUnifiedHistogramStateObservable(unifiedHistogram?.state$)?.subscribe(
(changes) => {
const { lensRequestAdapter, ...stateChanges } = changes;
const appState = stateContainer.appState.getState();
const oldState = {
hideChart: appState.hideChart,
interval: appState.interval,
breakdownField: appState.breakdownField,
};
const newState = { ...oldState, ...stateChanges };
const appState = stateContainer.appState.getState();
const oldState = {
hideChart: appState.hideChart,
interval: appState.interval,
breakdownField: appState.breakdownField,
};
const newState = {
hideChart: state.chartHidden,
interval: state.timeInterval,
breakdownField: state.breakdownField,
};
if ('lensRequestAdapter' in changes) {
inspectorAdapters.lensRequests = lensRequestAdapter;
}
if (!isEqual(oldState, newState)) {
stateContainer.appState.update(newState);
if (!isEqual(oldState, newState)) {
stateContainer.appState.update(newState);
}
}
});
);
return () => {
subscription?.unsubscribe();
@ -132,24 +134,26 @@ export const useDiscoverHistogram = ({
*/
useEffect(() => {
if (typeof hideChart === 'boolean') {
unifiedHistogram?.setChartHidden(hideChart);
}
}, [hideChart, unifiedHistogram]);
const subscription = createAppStateObservable(stateContainer.appState.state$).subscribe(
(changes) => {
if ('breakdownField' in changes) {
unifiedHistogram?.setBreakdownField(changes.breakdownField);
}
const timeInterval = useAppStateSelector((state) => state.interval);
if ('timeInterval' in changes && changes.timeInterval) {
unifiedHistogram?.setTimeInterval(changes.timeInterval);
}
useEffect(() => {
if (timeInterval) {
unifiedHistogram?.setTimeInterval(timeInterval);
}
}, [timeInterval, unifiedHistogram]);
if ('chartHidden' in changes && typeof changes.chartHidden === 'boolean') {
unifiedHistogram?.setChartHidden(changes.chartHidden);
}
}
);
const breakdownField = useAppStateSelector((state) => state.breakdownField);
useEffect(() => {
unifiedHistogram?.setBreakdownField(breakdownField);
}, [breakdownField, unifiedHistogram]);
return () => {
subscription?.unsubscribe();
};
}, [stateContainer.appState.state$, unifiedHistogram]);
/**
* Columns
@ -318,20 +322,70 @@ export const useDiscoverHistogram = ({
};
};
const createStateSyncObservable = (state$?: Observable<UnifiedHistogramState>) => {
// Use pairwise to diff the previous and current state (starting with undefined to ensure
// pairwise triggers after a single emission), and return an object containing only the
// changed properties. By only including the changed properties, we avoid accidentally
// overwriting other state properties that may have been updated between the time this
// obersverable was triggered and the time the state changes are applied.
const createUnifiedHistogramStateObservable = (state$?: Observable<UnifiedHistogramState>) => {
return state$?.pipe(
map(({ lensRequestAdapter, chartHidden, timeInterval, breakdownField }) => ({
lensRequestAdapter,
chartHidden,
timeInterval,
breakdownField,
})),
distinctUntilChanged((prev, curr) => {
const { lensRequestAdapter: prevLensRequestAdapter, ...prevRest } = prev;
const { lensRequestAdapter: currLensRequestAdapter, ...currRest } = curr;
startWith(undefined),
pairwise(),
map(([prev, curr]) => {
const changes: Partial<DiscoverAppState> & { lensRequestAdapter?: RequestAdapter } = {};
return prevLensRequestAdapter === currLensRequestAdapter && isEqual(prevRest, currRest);
})
if (!curr) {
return changes;
}
if (prev?.lensRequestAdapter !== curr.lensRequestAdapter) {
changes.lensRequestAdapter = curr.lensRequestAdapter;
}
if (prev?.chartHidden !== curr.chartHidden) {
changes.hideChart = curr.chartHidden;
}
if (prev?.timeInterval !== curr.timeInterval) {
changes.interval = curr.timeInterval;
}
if (prev?.breakdownField !== curr.breakdownField) {
changes.breakdownField = curr.breakdownField;
}
return changes;
}),
filter((changes) => Object.keys(changes).length > 0)
);
};
const createAppStateObservable = (state$: Observable<DiscoverAppState>) => {
return state$.pipe(
startWith(undefined),
pairwise(),
map(([prev, curr]) => {
const changes: Partial<UnifiedHistogramState> = {};
if (!curr) {
return changes;
}
if (prev?.breakdownField !== curr.breakdownField) {
changes.breakdownField = curr.breakdownField;
}
if (prev?.interval !== curr.interval) {
changes.timeInterval = curr.interval;
}
if (prev?.hideChart !== curr.hideChart) {
changes.chartHidden = curr.hideChart;
}
return changes;
}),
filter((changes) => Object.keys(changes).length > 0)
);
};

View file

@ -192,7 +192,7 @@ export function Chart({
chartToolButtonCss,
} = useChartStyles(chartVisible);
const lensAttributes = useMemo(
const lensAttributesContext = useMemo(
() =>
getLensAttributes({
title: chart?.title,
@ -218,7 +218,7 @@ export function Chart({
services,
dataView,
relativeTimeRange: originalRelativeTimeRange ?? relativeTimeRange,
lensAttributes,
lensAttributes: lensAttributesContext.attributes,
});
return (
@ -340,7 +340,7 @@ export function Chart({
chart={chart}
getTimeRange={getTimeRange}
refetch$={refetch$}
lensAttributes={lensAttributes}
lensAttributesContext={lensAttributesContext}
isPlainRecord={isPlainRecord}
disableTriggers={disableTriggers}
disabledActions={disabledActions}

View file

@ -66,7 +66,7 @@ function mountComponent() {
to: '2020-05-14T11:20:13.590',
}),
refetch$,
lensAttributes: getMockLensAttributes(),
lensAttributesContext: getMockLensAttributes(),
onTotalHitsChange: jest.fn(),
onChartLoad: jest.fn(),
};
@ -91,7 +91,7 @@ describe('Histogram', () => {
const originalProps = getLensProps({
searchSessionId: props.request.searchSessionId,
getTimeRange: props.getTimeRange,
attributes: getMockLensAttributes(),
attributes: getMockLensAttributes().attributes,
onLoad: lensProps.onLoad,
});
expect(lensProps).toEqual(originalProps);

View file

@ -14,7 +14,7 @@ import type { DefaultInspectorAdapters } from '@kbn/expressions-plugin/common';
import type { IKibanaSearchResponse } from '@kbn/data-plugin/public';
import type { estypes } from '@elastic/elasticsearch';
import type { TimeRange } from '@kbn/es-query';
import type { LensEmbeddableInput, TypedLensByValueInput } from '@kbn/lens-plugin/public';
import type { LensEmbeddableInput } from '@kbn/lens-plugin/public';
import { RequestStatus } from '@kbn/inspector-plugin/public';
import type { Observable } from 'rxjs';
import {
@ -31,6 +31,7 @@ import { buildBucketInterval } from './utils/build_bucket_interval';
import { useTimeRange } from './hooks/use_time_range';
import { useStableCallback } from './hooks/use_stable_callback';
import { useLensProps } from './hooks/use_lens_props';
import type { LensAttributesContext } from './utils/get_lens_attributes';
export interface HistogramProps {
services: UnifiedHistogramServices;
@ -41,7 +42,7 @@ export interface HistogramProps {
isPlainRecord?: boolean;
getTimeRange: () => TimeRange;
refetch$: Observable<UnifiedHistogramInputMessage>;
lensAttributes: TypedLensByValueInput['attributes'];
lensAttributesContext: LensAttributesContext;
disableTriggers?: LensEmbeddableInput['disableTriggers'];
disabledActions?: LensEmbeddableInput['disabledActions'];
onTotalHitsChange?: (status: UnifiedHistogramFetchStatus, result?: number | Error) => void;
@ -59,7 +60,7 @@ export function Histogram({
isPlainRecord,
getTimeRange,
refetch$,
lensAttributes: attributes,
lensAttributesContext: attributesContext,
disableTriggers,
disabledActions,
onTotalHitsChange,
@ -78,6 +79,8 @@ export function Histogram({
});
const chartRef = useRef<HTMLDivElement | null>(null);
const { height: containerHeight, width: containerWidth } = useResizeObserver(chartRef.current);
const { attributes } = attributesContext;
useEffect(() => {
if (attributes.visualizationType === 'lnsMetric') {
const size = containerHeight < containerWidth ? containerHeight : containerWidth;
@ -131,11 +134,11 @@ export function Histogram({
}
);
const lensProps = useLensProps({
const { lensProps, requestData } = useLensProps({
request,
getTimeRange,
refetch$,
attributes,
attributesContext,
onLoad,
});
@ -172,6 +175,7 @@ export function Histogram({
<div
data-test-subj="unifiedHistogramChart"
data-time-range={timeRangeText}
data-request-data={requestData}
css={chartCss}
ref={chartRef}
>

View file

@ -20,7 +20,7 @@ describe('useLensProps', () => {
const getTimeRange = jest.fn();
const refetch$ = new Subject<UnifiedHistogramInputMessage>();
const onLoad = jest.fn();
const attributes = getLensAttributes({
const attributesContext = getLensAttributes({
title: 'test',
filters: [],
query: {
@ -40,15 +40,15 @@ describe('useLensProps', () => {
},
getTimeRange,
refetch$,
attributes,
attributesContext,
onLoad,
});
});
expect(lensProps.result.current).toEqual(
expect(lensProps.result.current.lensProps).toEqual(
getLensProps({
searchSessionId: 'id',
getTimeRange,
attributes,
attributes: attributesContext.attributes,
onLoad,
})
);
@ -58,7 +58,7 @@ describe('useLensProps', () => {
const getTimeRange = jest.fn();
const refetch$ = new Subject<UnifiedHistogramInputMessage>();
const onLoad = jest.fn();
const attributes = getLensAttributes({
const attributesContext = getLensAttributes({
title: 'test',
filters: [],
query: {
@ -78,15 +78,15 @@ describe('useLensProps', () => {
},
getTimeRange,
refetch$,
attributes,
attributesContext,
onLoad,
});
});
expect(lensProps.result.current).toEqual(
expect(lensProps.result.current.lensProps).toEqual(
getLensProps({
searchSessionId: 'id',
getTimeRange,
attributes,
attributes: attributesContext.attributes,
onLoad,
})
);
@ -103,7 +103,7 @@ describe('useLensProps', () => {
},
getTimeRange,
refetch$,
attributes: getLensAttributes({
attributesContext: getLensAttributes({
title: 'test',
filters: [],
query: {

View file

@ -13,41 +13,44 @@ import type { TypedLensByValueInput } from '@kbn/lens-plugin/public';
import { useCallback, useEffect, useState } from 'react';
import type { Observable } from 'rxjs';
import type { UnifiedHistogramInputMessage, UnifiedHistogramRequestContext } from '../../types';
import type { LensAttributesContext } from '../utils/get_lens_attributes';
import { useStableCallback } from './use_stable_callback';
export const useLensProps = ({
request,
getTimeRange,
refetch$,
attributes,
attributesContext,
onLoad,
}: {
request?: UnifiedHistogramRequestContext;
getTimeRange: () => TimeRange;
refetch$: Observable<UnifiedHistogramInputMessage>;
attributes: TypedLensByValueInput['attributes'];
attributesContext: LensAttributesContext;
onLoad: (isLoading: boolean, adapters: Partial<DefaultInspectorAdapters> | undefined) => void;
}) => {
const buildLensProps = useCallback(
() =>
getLensProps({
const buildLensProps = useCallback(() => {
const { attributes, requestData } = attributesContext;
return {
requestData: JSON.stringify(requestData),
lensProps: getLensProps({
searchSessionId: request?.searchSessionId,
getTimeRange,
attributes,
onLoad,
}),
[attributes, getTimeRange, onLoad, request?.searchSessionId]
);
};
}, [attributesContext, getTimeRange, onLoad, request?.searchSessionId]);
const [lensProps, setLensProps] = useState(buildLensProps());
const updateLensProps = useStableCallback(() => setLensProps(buildLensProps()));
const [lensPropsContext, setLensPropsContext] = useState(buildLensProps());
const updateLensPropsContext = useStableCallback(() => setLensPropsContext(buildLensProps()));
useEffect(() => {
const subscription = refetch$.subscribe(updateLensProps);
const subscription = refetch$.subscribe(updateLensPropsContext);
return () => subscription.unsubscribe();
}, [refetch$, updateLensProps]);
}, [refetch$, updateLensPropsContext]);
return lensProps;
return lensPropsContext;
};
export const getLensProps = ({

View file

@ -19,6 +19,18 @@ import type {
} from '@kbn/lens-plugin/public';
import { fieldSupportsBreakdown } from './field_supports_breakdown';
export interface LensRequestData {
dataViewId?: string;
timeField?: string;
timeInterval?: string;
breakdownField?: string;
}
export interface LensAttributesContext {
attributes: TypedLensByValueInput['attributes'];
requestData: LensRequestData;
}
export const getLensAttributes = ({
title,
filters,
@ -35,7 +47,7 @@ export const getLensAttributes = ({
timeInterval: string | undefined;
breakdownField: DataViewField | undefined;
suggestion: Suggestion | undefined;
}) => {
}): LensAttributesContext => {
const showBreakdown = breakdownField && fieldSupportsBreakdown(breakdownField);
let columnOrder = ['date_column', 'count_column'];
@ -169,8 +181,7 @@ export const getLensAttributes = ({
yRight: false,
},
};
return {
const attributes = {
title:
title ??
i18n.translate('unifiedHistogram.lensTitle', {
@ -196,4 +207,14 @@ export const getLensAttributes = ({
},
visualizationType: suggestion ? suggestion.visualizationId : 'lnsXY',
} as TypedLensByValueInput['attributes'];
return {
attributes,
requestData: {
dataViewId: dataView.id,
timeField: dataView.timeFieldName,
timeInterval,
breakdownField: breakdownField?.name,
},
};
};

View file

@ -292,5 +292,35 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
// check no error state
expect(await PageObjects.discover.isChartVisible()).to.be(true);
});
it('should reset all histogram state when resetting the saved search', async () => {
await PageObjects.common.navigateToApp('discover');
await PageObjects.discover.waitUntilSearchingHasFinished();
await PageObjects.timePicker.setDefaultAbsoluteRange();
const savedSearch = 'histogram state';
await PageObjects.discover.saveSearch(savedSearch);
await PageObjects.discover.chooseBreakdownField('extension.keyword');
await PageObjects.discover.setChartInterval('Second');
let requestData = await testSubjects.getAttribute(
'unifiedHistogramChart',
'data-request-data'
);
expect(JSON.parse(requestData)).to.eql({
dataViewId: 'long-window-logstash-*',
timeField: '@timestamp',
timeInterval: 's',
breakdownField: 'extension.keyword',
});
await PageObjects.discover.toggleChartVisibility();
await PageObjects.discover.waitUntilSearchingHasFinished();
await PageObjects.discover.clickResetSavedSearchButton();
await PageObjects.discover.waitUntilSearchingHasFinished();
requestData = await testSubjects.getAttribute('unifiedHistogramChart', 'data-request-data');
expect(JSON.parse(requestData)).to.eql({
dataViewId: 'long-window-logstash-*',
timeField: '@timestamp',
timeInterval: 'auto',
});
});
});
}