Skip to content

Commit bf7de0e

Browse files
[Discover] Support state updates across tabs (#215620)
## Summary This PR adjusts the approach introduced in #214861 to ensure state updates work consistently across tabs, even after switching tabs during async operations. The `currentTabId` prop has been removed from the central state since it can't be relied on in actions, and instead tab IDs are injected using a `CurrentTabProvider`. This allows selectors to work the same as they did before, and tab specific actions have been updated to use a standard `TabAction` interface that accepts a tab ID and prevents leaking state changes. This approach is safer but adds some complexity, so for actions dispatched from React components, a `useCurrentTabAction` hook has been added to handle injecting the current tab ID. We also still need to access tab state within `DiscoverStateContainer` for now, so two utility methods (`injectCurrentTab` and `getCurrentTab`) have been added to make this easier. Since `DiscoverStateContainer` is scoped to a single tab, this should be safe, and ideally temporary until we get rid of it completely. Resolves #215398. ### 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/src/platform/packages/shared/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 - [ ] 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 was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]>
1 parent 63575a8 commit bf7de0e

39 files changed

+813
-593
lines changed

examples/discover_customization_examples/public/plugin.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ export class DiscoverCustomizationExamplesPlugin implements Plugin {
240240
ControlGroupRendererApi | undefined
241241
>();
242242
const stateStorage = stateContainer.stateStorage;
243-
const currentTabId = stateContainer.internalState.getState().tabs.currentId;
243+
const currentTabId = stateContainer.getCurrentTab().id;
244244
const dataView = useObservable(
245245
stateContainer.runtimeStateManager.tabs.byId[currentTabId].currentDataView$,
246246
stateContainer.runtimeStateManager.tabs.byId[currentTabId].currentDataView$.getValue()

src/platform/plugins/shared/discover/public/__mocks__/discover_state.mock.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,16 +55,18 @@ export function getDiscoverStateMock({
5555
...(toasts && withNotifyOnErrors(toasts)),
5656
});
5757
runtimeStateManager = runtimeStateManager ?? createRuntimeStateManager();
58+
const internalState = createInternalStateStore({
59+
services,
60+
customizationContext,
61+
runtimeStateManager,
62+
urlStateStorage: stateStorageContainer,
63+
});
5864
const container = getDiscoverStateContainer({
65+
tabId: internalState.getState().tabs.allIds[0],
5966
services,
6067
customizationContext,
6168
stateStorageContainer,
62-
internalState: createInternalStateStore({
63-
services,
64-
customizationContext,
65-
runtimeStateManager,
66-
urlStateStorage: stateStorageContainer,
67-
}),
69+
internalState,
6870
runtimeStateManager,
6971
});
7072
if (savedSearch !== false) {

src/platform/plugins/shared/discover/public/application/main/components/layout/discover_documents.test.tsx

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import { createCustomizationService } from '../../../../customizations/customiza
3030
import { DiscoverGrid } from '../../../../components/discover_grid';
3131
import { createDataViewDataSource } from '../../../../../common/data_sources';
3232
import type { ProfilesManager } from '../../../../context_awareness';
33-
import { internalStateActions } from '../../state_management/redux';
33+
import { CurrentTabProvider, internalStateActions } from '../../state_management/redux';
3434

3535
const customisationService = createCustomizationService();
3636

@@ -54,14 +54,16 @@ async function mountComponent(
5454
dataSource: createDataViewDataSource({ dataViewId: dataViewMock.id! }),
5555
});
5656
stateContainer.internalState.dispatch(
57-
internalStateActions.setDataRequestParams({
58-
timeRangeRelative: {
59-
from: '2020-05-14T11:05:13.590',
60-
to: '2020-05-14T11:20:13.590',
61-
},
62-
timeRangeAbsolute: {
63-
from: '2020-05-14T11:05:13.590',
64-
to: '2020-05-14T11:20:13.590',
57+
stateContainer.injectCurrentTab(internalStateActions.setDataRequestParams)({
58+
dataRequestParams: {
59+
timeRangeRelative: {
60+
from: '2020-05-14T11:05:13.590',
61+
to: '2020-05-14T11:20:13.590',
62+
},
63+
timeRangeAbsolute: {
64+
from: '2020-05-14T11:05:13.590',
65+
to: '2020-05-14T11:20:13.590',
66+
},
6567
},
6668
})
6769
);
@@ -81,11 +83,13 @@ async function mountComponent(
8183
services={{ ...services, profilesManager: profilesManager ?? services.profilesManager }}
8284
>
8385
<DiscoverCustomizationProvider value={customisationService}>
84-
<DiscoverMainProvider value={stateContainer}>
85-
<EuiProvider highContrastMode={false}>
86-
<DiscoverDocuments {...props} />
87-
</EuiProvider>
88-
</DiscoverMainProvider>
86+
<CurrentTabProvider currentTabId={stateContainer.getCurrentTab().id}>
87+
<DiscoverMainProvider value={stateContainer}>
88+
<EuiProvider highContrastMode={false}>
89+
<DiscoverDocuments {...props} />
90+
</EuiProvider>
91+
</DiscoverMainProvider>
92+
</CurrentTabProvider>
8993
</DiscoverCustomizationProvider>
9094
</KibanaContextProvider>
9195
);

src/platform/plugins/shared/discover/public/application/main/components/layout/discover_documents.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,10 @@ import {
7676
} from '../../../../context_awareness';
7777
import {
7878
internalStateActions,
79+
useCurrentTabSelector,
7980
useInternalStateDispatch,
8081
useInternalStateSelector,
8182
} from '../../state_management/redux';
82-
import { useCurrentTabSelector } from '../../state_management/redux/hooks';
8383

8484
const DiscoverGridMemoized = React.memo(DiscoverGrid);
8585

src/platform/plugins/shared/discover/public/application/main/components/layout/discover_histogram_layout.test.tsx

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@ import { DiscoverMainProvider } from '../../state_management/discover_state_prov
3737
import { act } from 'react-dom/test-utils';
3838
import { PanelsToggle } from '../../../../components/panels_toggle';
3939
import { createDataViewDataSource } from '../../../../../common/data_sources';
40-
import { RuntimeStateProvider, internalStateActions } from '../../state_management/redux';
40+
import {
41+
CurrentTabProvider,
42+
RuntimeStateProvider,
43+
internalStateActions,
44+
} from '../../state_management/redux';
4145

4246
function getStateContainer(savedSearch?: SavedSearch) {
4347
const stateContainer = getDiscoverStateMock({ isTimeBased: true, savedSearch });
@@ -50,16 +54,20 @@ function getStateContainer(savedSearch?: SavedSearch) {
5054

5155
stateContainer.appState.update(appState);
5256

53-
stateContainer.internalState.dispatch(internalStateActions.setDataView(dataView));
5457
stateContainer.internalState.dispatch(
55-
internalStateActions.setDataRequestParams({
56-
timeRangeAbsolute: {
57-
from: '2020-05-14T11:05:13.590',
58-
to: '2020-05-14T11:20:13.590',
59-
},
60-
timeRangeRelative: {
61-
from: '2020-05-14T11:05:13.590',
62-
to: '2020-05-14T11:20:13.590',
58+
stateContainer.injectCurrentTab(internalStateActions.setDataView)({ dataView })
59+
);
60+
stateContainer.internalState.dispatch(
61+
stateContainer.injectCurrentTab(internalStateActions.setDataRequestParams)({
62+
dataRequestParams: {
63+
timeRangeAbsolute: {
64+
from: '2020-05-14T11:05:13.590',
65+
to: '2020-05-14T11:20:13.590',
66+
},
67+
timeRangeRelative: {
68+
from: '2020-05-14T11:05:13.590',
69+
to: '2020-05-14T11:20:13.590',
70+
},
6371
},
6472
})
6573
);
@@ -147,11 +155,13 @@ const mountComponent = async ({
147155
const component = mountWithIntl(
148156
<KibanaRenderContextProvider {...services.core}>
149157
<KibanaContextProvider services={services}>
150-
<DiscoverMainProvider value={stateContainer}>
151-
<RuntimeStateProvider currentDataView={dataView} adHocDataViews={[]}>
152-
<DiscoverHistogramLayout {...props} />
153-
</RuntimeStateProvider>
154-
</DiscoverMainProvider>
158+
<CurrentTabProvider currentTabId={stateContainer.getCurrentTab().id}>
159+
<DiscoverMainProvider value={stateContainer}>
160+
<RuntimeStateProvider currentDataView={dataView} adHocDataViews={[]}>
161+
<DiscoverHistogramLayout {...props} />
162+
</RuntimeStateProvider>
163+
</DiscoverMainProvider>
164+
</CurrentTabProvider>
155165
</KibanaContextProvider>
156166
</KibanaRenderContextProvider>
157167
);

src/platform/plugins/shared/discover/public/application/main/components/layout/discover_layout.test.tsx

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,11 @@ import { act } from 'react-dom/test-utils';
4040
import { ErrorCallout } from '../../../../components/common/error_callout';
4141
import { PanelsToggle } from '../../../../components/panels_toggle';
4242
import { createDataViewDataSource } from '../../../../../common/data_sources';
43-
import { RuntimeStateProvider, internalStateActions } from '../../state_management/redux';
43+
import {
44+
CurrentTabProvider,
45+
RuntimeStateProvider,
46+
internalStateActions,
47+
} from '../../state_management/redux';
4448

4549
jest.mock('@elastic/eui', () => ({
4650
...jest.requireActual('@elastic/eui'),
@@ -105,9 +109,13 @@ async function mountComponent(
105109
interval: 'auto',
106110
query,
107111
});
108-
stateContainer.internalState.dispatch(internalStateActions.setDataView(dataView));
109112
stateContainer.internalState.dispatch(
110-
internalStateActions.setDataRequestParams({ timeRangeAbsolute: time, timeRangeRelative: time })
113+
stateContainer.injectCurrentTab(internalStateActions.setDataView)({ dataView })
114+
);
115+
stateContainer.internalState.dispatch(
116+
stateContainer.injectCurrentTab(internalStateActions.setDataRequestParams)({
117+
dataRequestParams: { timeRangeAbsolute: time, timeRangeRelative: time },
118+
})
111119
);
112120

113121
const props = {
@@ -127,13 +135,15 @@ async function mountComponent(
127135

128136
const component = mountWithIntl(
129137
<KibanaContextProvider services={services}>
130-
<DiscoverMainProvider value={stateContainer}>
131-
<RuntimeStateProvider currentDataView={dataView} adHocDataViews={[]}>
132-
<EuiProvider highContrastMode={false}>
133-
<DiscoverLayout {...props} />
134-
</EuiProvider>
135-
</RuntimeStateProvider>
136-
</DiscoverMainProvider>
138+
<CurrentTabProvider currentTabId={stateContainer.getCurrentTab().id}>
139+
<DiscoverMainProvider value={stateContainer}>
140+
<RuntimeStateProvider currentDataView={dataView} adHocDataViews={[]}>
141+
<EuiProvider highContrastMode={false}>
142+
<DiscoverLayout {...props} />
143+
</EuiProvider>
144+
</RuntimeStateProvider>
145+
</DiscoverMainProvider>
146+
</CurrentTabProvider>
137147
</KibanaContextProvider>,
138148
mountOptions
139149
);

src/platform/plugins/shared/discover/public/application/main/components/layout/discover_layout.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,7 @@ import type { PanelsToggleProps } from '../../../../components/panels_toggle';
5757
import { PanelsToggle } from '../../../../components/panels_toggle';
5858
import { sendErrorMsg } from '../../hooks/use_saved_search_messages';
5959
import { useIsEsqlMode } from '../../hooks/use_is_esql_mode';
60-
import { useCurrentDataView } from '../../state_management/redux';
61-
import { useCurrentTabSelector } from '../../state_management/redux/hooks';
60+
import { useCurrentDataView, useCurrentTabSelector } from '../../state_management/redux';
6261

6362
const SidebarMemoized = React.memo(DiscoverSidebarResponsive);
6463
const TopNavMemoized = React.memo(DiscoverTopNav);

src/platform/plugins/shared/discover/public/application/main/components/layout/discover_main_content.test.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock'
3939
import { PanelsToggle } from '../../../../components/panels_toggle';
4040
import type { Storage } from '@kbn/kibana-utils-plugin/public';
4141
import { createDataViewDataSource } from '../../../../../common/data_sources';
42+
import { CurrentTabProvider } from '../../state_management/redux';
4243

4344
const mountComponent = async ({
4445
hideChart = false,
@@ -130,9 +131,11 @@ const mountComponent = async ({
130131
const component = mountWithIntl(
131132
<KibanaRenderContextProvider {...services.core}>
132133
<KibanaContextProvider services={services}>
133-
<DiscoverMainProvider value={stateContainer}>
134-
<DiscoverMainContent {...props} />
135-
</DiscoverMainProvider>
134+
<CurrentTabProvider currentTabId={stateContainer.getCurrentTab().id}>
135+
<DiscoverMainProvider value={stateContainer}>
136+
<DiscoverMainContent {...props} />
137+
</DiscoverMainProvider>
138+
</CurrentTabProvider>
136139
</KibanaContextProvider>
137140
</KibanaRenderContextProvider>
138141
);

src/platform/plugins/shared/discover/public/application/main/components/layout/use_discover_histogram.test.tsx

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ import type { InspectorAdapters } from '../../hooks/use_inspector';
2929
import type { UnifiedHistogramCustomization } from '../../../../customizations/customization_types/histogram_customization';
3030
import { useDiscoverCustomization } from '../../../../customizations';
3131
import type { DiscoverCustomizationId } from '../../../../customizations/customization_service';
32-
import { RuntimeStateProvider, internalStateActions } from '../../state_management/redux';
32+
import {
33+
CurrentTabProvider,
34+
RuntimeStateProvider,
35+
internalStateActions,
36+
} from '../../state_management/redux';
3337
import { dataViewMockWithTimeField } from '@kbn/discover-utils/src/__mocks__';
3438

3539
const mockData = dataPluginMock.createStartContract();
@@ -123,11 +127,13 @@ describe('useDiscoverHistogram', () => {
123127
};
124128

125129
const Wrapper = ({ children }: React.PropsWithChildren<unknown>) => (
126-
<DiscoverMainProvider value={stateContainer}>
127-
<RuntimeStateProvider currentDataView={dataViewMockWithTimeField} adHocDataViews={[]}>
128-
{children as ReactElement}
129-
</RuntimeStateProvider>
130-
</DiscoverMainProvider>
130+
<CurrentTabProvider currentTabId={stateContainer.getCurrentTab().id}>
131+
<DiscoverMainProvider value={stateContainer}>
132+
<RuntimeStateProvider currentDataView={dataViewMockWithTimeField} adHocDataViews={[]}>
133+
{children as ReactElement}
134+
</RuntimeStateProvider>
135+
</DiscoverMainProvider>
136+
</CurrentTabProvider>
131137
);
132138

133139
const hook = renderHook(
@@ -391,9 +397,11 @@ describe('useDiscoverHistogram', () => {
391397
const timeRangeAbs = { from: '2021-05-01T20:00:00Z', to: '2021-05-02T20:00:00Z' };
392398
const timeRangeRel = { from: 'now-15m', to: 'now' };
393399
stateContainer.internalState.dispatch(
394-
internalStateActions.setDataRequestParams({
395-
timeRangeAbsolute: timeRangeAbs,
396-
timeRangeRelative: timeRangeRel,
400+
stateContainer.injectCurrentTab(internalStateActions.setDataRequestParams)({
401+
dataRequestParams: {
402+
timeRangeAbsolute: timeRangeAbs,
403+
timeRangeRelative: timeRangeRel,
404+
},
397405
})
398406
);
399407
const { hook } = await renderUseDiscoverHistogram({ stateContainer });

src/platform/plugins/shared/discover/public/application/main/components/layout/use_discover_histogram.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,10 @@ import { useIsEsqlMode } from '../../hooks/use_is_esql_mode';
5757
import {
5858
internalStateActions,
5959
useCurrentDataView,
60+
useCurrentTabAction,
61+
useCurrentTabSelector,
6062
useInternalStateDispatch,
6163
} from '../../state_management/redux';
62-
import { useCurrentTabSelector } from '../../state_management/redux/hooks';
6364

6465
const EMPTY_ESQL_COLUMNS: DatatableColumn[] = [];
6566
const EMPTY_FILTERS: Filter[] = [];
@@ -321,6 +322,9 @@ export const useDiscoverHistogram = ({
321322

322323
// eslint-disable-next-line react-hooks/exhaustive-deps
323324
const timeRangeMemoized = useMemo(() => timeRange, [timeRange?.from, timeRange?.to]);
325+
const setOverriddenVisContextAfterInvalidation = useCurrentTabAction(
326+
internalStateActions.setOverriddenVisContextAfterInvalidation
327+
);
324328
const dispatch = useInternalStateDispatch();
325329

326330
const onVisContextChanged = useCallback(
@@ -335,25 +339,41 @@ export const useDiscoverHistogram = ({
335339
stateContainer.savedSearchState.updateVisContext({
336340
nextVisContext,
337341
});
338-
dispatch(internalStateActions.setOverriddenVisContextAfterInvalidation(undefined));
342+
dispatch(
343+
setOverriddenVisContextAfterInvalidation({
344+
overriddenVisContextAfterInvalidation: undefined,
345+
})
346+
);
339347
break;
340348
case UnifiedHistogramExternalVisContextStatus.automaticallyOverridden:
341349
// if the visualization was invalidated as incompatible and rebuilt
342350
// (it will be used later for saving the visualization via Save button)
343-
dispatch(internalStateActions.setOverriddenVisContextAfterInvalidation(nextVisContext));
351+
dispatch(
352+
setOverriddenVisContextAfterInvalidation({
353+
overriddenVisContextAfterInvalidation: nextVisContext,
354+
})
355+
);
344356
break;
345357
case UnifiedHistogramExternalVisContextStatus.automaticallyCreated:
346358
case UnifiedHistogramExternalVisContextStatus.applied:
347359
// clearing the value in the internal state so we don't use it during saved search saving
348-
dispatch(internalStateActions.setOverriddenVisContextAfterInvalidation(undefined));
360+
dispatch(
361+
setOverriddenVisContextAfterInvalidation({
362+
overriddenVisContextAfterInvalidation: undefined,
363+
})
364+
);
349365
break;
350366
case UnifiedHistogramExternalVisContextStatus.unknown:
351367
// using `{}` to overwrite the value inside the saved search SO during saving
352-
dispatch(internalStateActions.setOverriddenVisContextAfterInvalidation({}));
368+
dispatch(
369+
setOverriddenVisContextAfterInvalidation({
370+
overriddenVisContextAfterInvalidation: {},
371+
})
372+
);
353373
break;
354374
}
355375
},
356-
[dispatch, stateContainer.savedSearchState]
376+
[dispatch, setOverriddenVisContextAfterInvalidation, stateContainer.savedSearchState]
357377
);
358378

359379
const breakdownField = useAppStateSelector((state) => state.breakdownField);

0 commit comments

Comments
 (0)