From 9ece3ce1f2b274f0ce5a179eea1c4004002125d0 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Thu, 25 Jan 2024 16:55:34 +0100 Subject: [PATCH] [Discover] Return suggestion to histogram main state as it triggers refetch in discover. Try to align currnent externalVisContext prop with current query. --- .../layout/discover_histogram_layout.tsx | 16 +------------- .../layout/use_discover_histogram.ts | 22 ++++++++++++++++--- .../container/hooks/use_state_props.test.ts | 16 ++++++++++++++ .../public/container/hooks/use_state_props.ts | 8 +++++++ .../container/services/state_service.test.ts | 2 ++ .../container/services/state_service.ts | 18 +++++++++++++++ .../public/layout/layout.tsx | 16 ++++++++++++++ .../public/services/lens_vis_service.ts | 22 +++++++++++++++++-- 8 files changed, 100 insertions(+), 20 deletions(-) diff --git a/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.tsx b/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.tsx index 01a3bc1e804b4..b7a4a346cbd31 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.tsx @@ -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'; @@ -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, @@ -75,8 +63,6 @@ export const DiscoverHistogramLayout = ({ container={container} css={histogramLayoutCss} renderCustomChartToggleActions={renderCustomChartToggleActions} - externalVisContext={visContext} - onVisContextChanged={onVisContextChanged} > { + 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); }); @@ -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, @@ -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, }; }; @@ -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 ?? [], })) ); diff --git a/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts b/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts index eab21cec010ef..e836cd80b9c65 100644 --- a/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts +++ b/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts @@ -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'; @@ -31,6 +32,7 @@ describe('useStateProps', () => { topPanelHeight: 100, totalHitsStatus: UnifiedHistogramFetchStatus.uninitialized, totalHitsResult: undefined, + currentSuggestionContext: undefined, }; const getStateService = (options: Omit) => { @@ -44,6 +46,7 @@ describe('useStateProps', () => { jest.spyOn(stateService, 'setTimeInterval'); jest.spyOn(stateService, 'setLensRequestAdapter'); jest.spyOn(stateService, 'setTotalHits'); + jest.spyOn(stateService, 'setCurrentSuggestionContext'); return stateService; }; @@ -118,6 +121,7 @@ describe('useStateProps', () => { "onBreakdownFieldChange": [Function], "onChartHiddenChange": [Function], "onChartLoad": [Function], + "onSuggestionContextChange": [Function], "onTimeIntervalChange": [Function], "onTopPanelHeightChange": [Function], "onTotalHitsChange": [Function], @@ -197,6 +201,7 @@ describe('useStateProps', () => { "onBreakdownFieldChange": [Function], "onChartHiddenChange": [Function], "onChartLoad": [Function], + "onSuggestionContextChange": [Function], "onTimeIntervalChange": [Function], "onTopPanelHeightChange": [Function], "onTotalHitsChange": [Function], @@ -218,6 +223,7 @@ describe('useStateProps', () => { const stateService = getStateService({ initialState: { ...initialState, + currentSuggestionContext: undefined, }, }); const { result } = renderHook(() => @@ -296,6 +302,7 @@ describe('useStateProps', () => { "onBreakdownFieldChange": [Function], "onChartHiddenChange": [Function], "onChartLoad": [Function], + "onSuggestionContextChange": [Function], "onTimeIntervalChange": [Function], "onTopPanelHeightChange": [Function], "onTotalHitsChange": [Function], @@ -372,6 +379,7 @@ describe('useStateProps', () => { "onBreakdownFieldChange": [Function], "onChartHiddenChange": [Function], "onChartLoad": [Function], + "onSuggestionContextChange": [Function], "onTimeIntervalChange": [Function], "onTopPanelHeightChange": [Function], "onTotalHitsChange": [Function], @@ -407,6 +415,7 @@ describe('useStateProps', () => { onChartHiddenChange, onChartLoad, onBreakdownFieldChange, + onSuggestionContextChange, } = result.current; act(() => { onTopPanelHeightChange(200); @@ -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', () => { diff --git a/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts b/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts index 125f6a4b85fef..7afdb029fd3cc 100644 --- a/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts +++ b/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts @@ -158,6 +158,13 @@ export const useStateProps = ({ [stateService] ); + const onSuggestionContextChange = useCallback( + (suggestionContext) => { + stateService?.setCurrentSuggestionContext(suggestionContext); + }, + [stateService] + ); + /** * Effects */ @@ -183,5 +190,6 @@ export const useStateProps = ({ onChartHiddenChange, onChartLoad, onBreakdownFieldChange, + onSuggestionContextChange, }; }; diff --git a/src/plugins/unified_histogram/public/container/services/state_service.test.ts b/src/plugins/unified_histogram/public/container/services/state_service.test.ts index adfd03bd0bf43..6249c3e423877 100644 --- a/src/plugins/unified_histogram/public/container/services/state_service.test.ts +++ b/src/plugins/unified_histogram/public/container/services/state_service.test.ts @@ -52,6 +52,7 @@ describe('UnifiedHistogramStateService', () => { topPanelHeight: 100, totalHitsStatus: UnifiedHistogramFetchStatus.uninitialized, totalHitsResult: undefined, + currentSuggestionContext: undefined, }; it('should initialize state with default values', () => { @@ -66,6 +67,7 @@ describe('UnifiedHistogramStateService', () => { topPanelHeight: undefined, totalHitsResult: undefined, totalHitsStatus: UnifiedHistogramFetchStatus.uninitialized, + currentSuggestionContext: undefined, }); }); diff --git a/src/plugins/unified_histogram/public/container/services/state_service.ts b/src/plugins/unified_histogram/public/container/services/state_service.ts index 339050527ba08..dd70dc646c9fb 100644 --- a/src/plugins/unified_histogram/public/container/services/state_service.ts +++ b/src/plugins/unified_histogram/public/container/services/state_service.ts @@ -19,6 +19,7 @@ import { setChartHidden, setTopPanelHeight, } from '../utils/local_storage_utils'; +import type { UnifiedHistogramSuggestionContext } from '../../types'; /** * The current state of the container @@ -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 */ @@ -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 */ @@ -142,6 +153,7 @@ export const createStateService = ( const state$ = new BehaviorSubject({ breakdownField: initialBreakdownField, chartHidden: initialChartHidden, + currentSuggestionContext: undefined, lensRequestAdapter: undefined, timeInterval: 'auto', topPanelHeight: initialTopPanelHeight, @@ -184,6 +196,12 @@ export const createStateService = ( updateState({ breakdownField }); }, + setCurrentSuggestionContext: ( + suggestionContext: UnifiedHistogramSuggestionContext | undefined + ) => { + updateState({ currentSuggestionContext: suggestionContext }); + }, + setTimeInterval: (timeInterval: string) => { updateState({ timeInterval }); }, diff --git a/src/plugins/unified_histogram/public/layout/layout.tsx b/src/plugins/unified_histogram/public/layout/layout.tsx index c496eacd5b78b..de451ac0b5302 100644 --- a/src/plugins/unified_histogram/public/layout/layout.tsx +++ b/src/plugins/unified_histogram/public/layout/layout.tsx @@ -37,6 +37,7 @@ import type { UnifiedHistogramInput$, UnifiedHistogramRequestContext, UnifiedHistogramServices, + UnifiedHistogramSuggestionContext, } from '../types'; import { UnifiedHistogramSuggestionType } from '../types'; import { LensVisService } from '../services/lens_vis_service'; @@ -157,6 +158,12 @@ export interface UnifiedHistogramLayoutProps extends PropsWithChildren * 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 */ @@ -214,6 +221,7 @@ export const UnifiedHistogramLayout = ({ onChartHiddenChange, onTimeIntervalChange, onBreakdownFieldChange, + onSuggestionContextChange, onVisContextChanged, onTotalHitsChange, onChartLoad, @@ -235,6 +243,11 @@ export const UnifiedHistogramLayout = ({ ); useEffect(() => { + if (isChartLoading) { + console.log('chart is loading', requestParams.query, externalVisContext); + return; + } + lensVisService.update({ externalVisContext, queryParams: { @@ -248,6 +261,7 @@ export const UnifiedHistogramLayout = ({ chartTitle: originalChart?.title, timeInterval: originalChart?.timeInterval, breakdownField: breakdown?.field, + onSuggestionContextChange, onVisContextChanged: isPlainRecord ? onVisContextChanged : undefined, }); }, [ @@ -261,7 +275,9 @@ export const UnifiedHistogramLayout = ({ columns, breakdown, externalVisContext, + onSuggestionContextChange, onVisContextChanged, + isChartLoading, ]); const chart = diff --git a/src/plugins/unified_histogram/public/services/lens_vis_service.ts b/src/plugins/unified_histogram/public/services/lens_vis_service.ts index 1c65a08f028b9..7d1dda22a13ad 100644 --- a/src/plugins/unified_histogram/public/services/lens_vis_service.ts +++ b/src/plugins/unified_histogram/public/services/lens_vis_service.ts @@ -91,6 +91,9 @@ export class LensVisService { chartTitle: string | undefined; timeInterval: string | undefined; breakdownField: DataViewField | undefined; + onSuggestionContextChange?: ( + suggestionContext: UnifiedHistogramSuggestionContext | undefined + ) => void; onVisContextChanged?: (visContext: ExternalVisContext | undefined) => void; } | undefined; @@ -127,6 +130,7 @@ export class LensVisService { chartTitle, timeInterval, breakdownField, + onSuggestionContextChange, onVisContextChanged, }: { externalVisContext: ExternalVisContext | undefined; @@ -134,6 +138,9 @@ export class LensVisService { chartTitle: string | undefined; timeInterval: string | undefined; breakdownField: DataViewField | undefined; + onSuggestionContextChange?: ( + suggestionContext: UnifiedHistogramSuggestionContext | undefined + ) => void; onVisContextChanged?: (visContext: ExternalVisContext | undefined) => void; }) => { const suggestionContextSelectedPreviously = this.state$.getValue().currentSuggestionContext; @@ -166,6 +173,10 @@ export class LensVisService { console.log('service lensAttributesState', lensAttributesState); + if (suggestionState.shouldUpdateSelectedSuggestionDueToDepsChange) { + onSuggestionContextChange?.(suggestionState.currentSuggestionContext); + } + if ( externalVisContext?.attributes && (suggestionState.shouldUpdateSelectedSuggestionDueToDepsChange || @@ -204,8 +215,14 @@ export class LensVisService { return; } - const { queryParams, chartTitle, timeInterval, breakdownField, onVisContextChanged } = - this.prevUpdateContext; + const { + queryParams, + chartTitle, + timeInterval, + breakdownField, + onSuggestionContextChange, + onVisContextChanged, + } = this.prevUpdateContext; const lensAttributesState = this.getLensAttributesState({ currentSuggestionContext: editedSuggestionContext, @@ -222,6 +239,7 @@ export class LensVisService { lensAttributesContext: lensAttributesState.lensAttributesContext, }); + onSuggestionContextChange?.(editedSuggestionContext); onVisContextChanged?.(lensAttributesState.lensAttributesContext); };