Skip to content

Commit

Permalink
[Discover] Return suggestion to histogram main state as it triggers r…
Browse files Browse the repository at this point in the history
…efetch in discover. Try to align currnent externalVisContext prop with current query.
  • Loading branch information
jughosta committed Jan 25, 2024
1 parent ac9d3fe commit 9ece3ce
Show file tree
Hide file tree
Showing 8 changed files with 100 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@
*/

import React, { useCallback } from 'react';
import {
type ExternalVisContext,
UnifiedHistogramContainer,
} from '@kbn/unified-histogram-plugin/public';
import { UnifiedHistogramContainer } from '@kbn/unified-histogram-plugin/public';
import { css } from '@emotion/react';
import useObservable from 'react-use/lib/useObservable';
import { useDiscoverHistogram } from './use_discover_histogram';
Expand All @@ -36,15 +33,6 @@ export const DiscoverHistogramLayout = ({
const { dataState } = stateContainer;
const searchSessionId = useObservable(stateContainer.searchSessionManager.searchSessionId$);
const hideChart = useAppStateSelector((state) => state.hideChart);
const visContext = useAppStateSelector((state) => state.visContext);

const onVisContextChanged = useCallback(
(newVisContext: ExternalVisContext | undefined) => {
console.log('got new vis context from histogram', newVisContext);
stateContainer.appState.update({ visContext: newVisContext });
},
[stateContainer]
);

const unifiedHistogramProps = useDiscoverHistogram({
stateContainer,
Expand Down Expand Up @@ -75,8 +63,6 @@ export const DiscoverHistogramLayout = ({
container={container}
css={histogramLayoutCss}
renderCustomChartToggleActions={renderCustomChartToggleActions}
externalVisContext={visContext}
onVisContextChanged={onVisContextChanged}
>
<DiscoverMainContent
{...mainContentProps}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import { useQuerySubscriber } from '@kbn/unified-field-list/src/hooks/use_query_subscriber';
import {
ExternalVisContext,
UnifiedHistogramApi,
UnifiedHistogramFetchStatus,
UnifiedHistogramState,
Expand Down Expand Up @@ -221,10 +222,12 @@ export const useDiscoverHistogram = ({
const {
dataView: textBasedDataView,
query: textBasedQuery,
externalVisContext: textBasedExternalVisContext,
columns,
} = useObservable(textBasedFetchComplete$, {
dataView: stateContainer.internalState.getState().dataView!,
query: stateContainer.appState.getState().query,
query: stateContainer.appState.getState().query!,
externalVisContext: stateContainer.appState.getState().visContext,
columns: savedSearchData$.documents$.getValue().textBasedQueryColumns ?? [],
});

Expand All @@ -233,12 +236,14 @@ export const useDiscoverHistogram = ({
return;
}

const fetchStart = stateContainer.dataState.fetch$.subscribe(() => {
const fetchStart = stateContainer.dataState.fetch$.subscribe((value) => {
console.log('fetchStart', value);
if (!skipRefetch.current) {
setIsSuggestionLoading(true);
}
});
const fetchComplete = textBasedFetchComplete$.subscribe(() => {
const fetchComplete = textBasedFetchComplete$.subscribe((value) => {
console.log('fetchComplete', value);
setIsSuggestionLoading(false);
});

Expand Down Expand Up @@ -325,6 +330,14 @@ export const useDiscoverHistogram = ({
// eslint-disable-next-line react-hooks/exhaustive-deps
const timeRangeMemoized = useMemo(() => timeRange, [timeRange?.from, timeRange?.to]);

const onVisContextChanged = useCallback(
(newVisContext: ExternalVisContext | undefined) => {
console.log('got new vis context from histogram', newVisContext);
stateContainer.appState.update({ visContext: newVisContext });
},
[stateContainer]
);

return {
ref,
getCreationOptions,
Expand All @@ -340,6 +353,8 @@ export const useDiscoverHistogram = ({
withDefaultActions: histogramCustomization?.withDefaultActions,
disabledActions: histogramCustomization?.disabledActions,
isChartLoading: isSuggestionLoading,
externalVisContext: isPlainRecord ? textBasedExternalVisContext : undefined, // visContext should be in sync with current query
onVisContextChanged: isPlainRecord ? onVisContextChanged : undefined,
};
};

Expand Down Expand Up @@ -419,6 +434,7 @@ const createFetchCompleteObservable = (stateContainer: DiscoverStateContainer) =
map(({ textBasedQueryColumns }) => ({
dataView: stateContainer.internalState.getState().dataView!,
query: stateContainer.appState.getState().query!,
externalVisContext: stateContainer.appState.getState().visContext,
columns: textBasedQueryColumns ?? [],
}))
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/
import { DataView, DataViewField, DataViewType } from '@kbn/data-views-plugin/common';
import { RequestAdapter } from '@kbn/inspector-plugin/common';
import { Suggestion } from '@kbn/lens-plugin/public';
import { renderHook } from '@testing-library/react-hooks';
import { act } from 'react-test-renderer';
import { UnifiedHistogramFetchStatus } from '../../types';
Expand All @@ -31,6 +32,7 @@ describe('useStateProps', () => {
topPanelHeight: 100,
totalHitsStatus: UnifiedHistogramFetchStatus.uninitialized,
totalHitsResult: undefined,
currentSuggestionContext: undefined,
};

const getStateService = (options: Omit<UnifiedHistogramStateOptions, 'services'>) => {
Expand All @@ -44,6 +46,7 @@ describe('useStateProps', () => {
jest.spyOn(stateService, 'setTimeInterval');
jest.spyOn(stateService, 'setLensRequestAdapter');
jest.spyOn(stateService, 'setTotalHits');
jest.spyOn(stateService, 'setCurrentSuggestionContext');
return stateService;
};

Expand Down Expand Up @@ -118,6 +121,7 @@ describe('useStateProps', () => {
"onBreakdownFieldChange": [Function],
"onChartHiddenChange": [Function],
"onChartLoad": [Function],
"onSuggestionContextChange": [Function],
"onTimeIntervalChange": [Function],
"onTopPanelHeightChange": [Function],
"onTotalHitsChange": [Function],
Expand Down Expand Up @@ -197,6 +201,7 @@ describe('useStateProps', () => {
"onBreakdownFieldChange": [Function],
"onChartHiddenChange": [Function],
"onChartLoad": [Function],
"onSuggestionContextChange": [Function],
"onTimeIntervalChange": [Function],
"onTopPanelHeightChange": [Function],
"onTotalHitsChange": [Function],
Expand All @@ -218,6 +223,7 @@ describe('useStateProps', () => {
const stateService = getStateService({
initialState: {
...initialState,
currentSuggestionContext: undefined,
},
});
const { result } = renderHook(() =>
Expand Down Expand Up @@ -296,6 +302,7 @@ describe('useStateProps', () => {
"onBreakdownFieldChange": [Function],
"onChartHiddenChange": [Function],
"onChartLoad": [Function],
"onSuggestionContextChange": [Function],
"onTimeIntervalChange": [Function],
"onTopPanelHeightChange": [Function],
"onTotalHitsChange": [Function],
Expand Down Expand Up @@ -372,6 +379,7 @@ describe('useStateProps', () => {
"onBreakdownFieldChange": [Function],
"onChartHiddenChange": [Function],
"onChartLoad": [Function],
"onSuggestionContextChange": [Function],
"onTimeIntervalChange": [Function],
"onTopPanelHeightChange": [Function],
"onTotalHitsChange": [Function],
Expand Down Expand Up @@ -407,6 +415,7 @@ describe('useStateProps', () => {
onChartHiddenChange,
onChartLoad,
onBreakdownFieldChange,
onSuggestionContextChange,
} = result.current;
act(() => {
onTopPanelHeightChange(200);
Expand Down Expand Up @@ -436,6 +445,13 @@ describe('useStateProps', () => {
onBreakdownFieldChange({ name: 'field' } as DataViewField);
});
expect(stateService.setBreakdownField).toHaveBeenLastCalledWith('field');

act(() => {
onSuggestionContextChange({ title: 'Stacked Bar' } as Suggestion);
});
expect(stateService.setCurrentSuggestionContext).toHaveBeenLastCalledWith({
title: 'Stacked Bar',
});
});

it('should clear lensRequestAdapter when chart is hidden', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,13 @@ export const useStateProps = ({
[stateService]
);

const onSuggestionContextChange = useCallback(
(suggestionContext) => {
stateService?.setCurrentSuggestionContext(suggestionContext);
},
[stateService]
);

/**
* Effects
*/
Expand All @@ -183,5 +190,6 @@ export const useStateProps = ({
onChartHiddenChange,
onChartLoad,
onBreakdownFieldChange,
onSuggestionContextChange,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ describe('UnifiedHistogramStateService', () => {
topPanelHeight: 100,
totalHitsStatus: UnifiedHistogramFetchStatus.uninitialized,
totalHitsResult: undefined,
currentSuggestionContext: undefined,
};

it('should initialize state with default values', () => {
Expand All @@ -66,6 +67,7 @@ describe('UnifiedHistogramStateService', () => {
topPanelHeight: undefined,
totalHitsResult: undefined,
totalHitsStatus: UnifiedHistogramFetchStatus.uninitialized,
currentSuggestionContext: undefined,
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
setChartHidden,
setTopPanelHeight,
} from '../utils/local_storage_utils';
import type { UnifiedHistogramSuggestionContext } from '../../types';

/**
* The current state of the container
Expand All @@ -28,6 +29,10 @@ export interface UnifiedHistogramState {
* The current field used for the breakdown
*/
breakdownField: string | undefined;
/**
* The current Lens suggestion
*/
currentSuggestionContext: UnifiedHistogramSuggestionContext | undefined;
/**
* Whether or not the chart is hidden
*/
Expand Down Expand Up @@ -92,6 +97,12 @@ export interface UnifiedHistogramStateService {
* Sets the current chart hidden state
*/
setChartHidden: (chartHidden: boolean) => void;
/**
* Sets current Lens suggestion
*/
setCurrentSuggestionContext: (
suggestionContext: UnifiedHistogramSuggestionContext | undefined
) => void;
/**
* Sets the current top panel height
*/
Expand Down Expand Up @@ -142,6 +153,7 @@ export const createStateService = (
const state$ = new BehaviorSubject<UnifiedHistogramState>({
breakdownField: initialBreakdownField,
chartHidden: initialChartHidden,
currentSuggestionContext: undefined,
lensRequestAdapter: undefined,
timeInterval: 'auto',
topPanelHeight: initialTopPanelHeight,
Expand Down Expand Up @@ -184,6 +196,12 @@ export const createStateService = (
updateState({ breakdownField });
},

setCurrentSuggestionContext: (
suggestionContext: UnifiedHistogramSuggestionContext | undefined
) => {
updateState({ currentSuggestionContext: suggestionContext });
},

setTimeInterval: (timeInterval: string) => {
updateState({ timeInterval });
},
Expand Down
16 changes: 16 additions & 0 deletions src/plugins/unified_histogram/public/layout/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import type {
UnifiedHistogramInput$,
UnifiedHistogramRequestContext,
UnifiedHistogramServices,
UnifiedHistogramSuggestionContext,
} from '../types';
import { UnifiedHistogramSuggestionType } from '../types';
import { LensVisService } from '../services/lens_vis_service';
Expand Down Expand Up @@ -157,6 +158,12 @@ export interface UnifiedHistogramLayoutProps extends PropsWithChildren<unknown>
* Callback to update the breakdown field -- should set {@link UnifiedHistogramBreakdownContext.field} to breakdownField
*/
onBreakdownFieldChange?: (breakdownField: DataViewField | undefined) => void;
/**
* Callback to update the suggested chart
*/
onSuggestionContextChange?: (
suggestionContext: UnifiedHistogramSuggestionContext | undefined
) => void;
/**
* Callback to notify about the change in Lens attributes
*/
Expand Down Expand Up @@ -214,6 +221,7 @@ export const UnifiedHistogramLayout = ({
onChartHiddenChange,
onTimeIntervalChange,
onBreakdownFieldChange,
onSuggestionContextChange,
onVisContextChanged,
onTotalHitsChange,
onChartLoad,
Expand All @@ -235,6 +243,11 @@ export const UnifiedHistogramLayout = ({
);

useEffect(() => {
if (isChartLoading) {
console.log('chart is loading', requestParams.query, externalVisContext);
return;
}

lensVisService.update({
externalVisContext,
queryParams: {
Expand All @@ -248,6 +261,7 @@ export const UnifiedHistogramLayout = ({
chartTitle: originalChart?.title,
timeInterval: originalChart?.timeInterval,
breakdownField: breakdown?.field,
onSuggestionContextChange,
onVisContextChanged: isPlainRecord ? onVisContextChanged : undefined,
});
}, [
Expand All @@ -261,7 +275,9 @@ export const UnifiedHistogramLayout = ({
columns,
breakdown,
externalVisContext,
onSuggestionContextChange,
onVisContextChanged,
isChartLoading,
]);

const chart =
Expand Down
Loading

0 comments on commit 9ece3ce

Please sign in to comment.