From 2f42e162ba1828a906be3219febc2e236cbe5cd8 Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 4 Dec 2024 08:21:19 +1100 Subject: [PATCH] [8.x] [Discover] Fix flaky cell actions tests (#202097) (#202805) # Backport This will backport the following commits from `main` to `8.x`: - [[Discover] Fix flaky cell actions tests (#202097)](https://github.com/elastic/kibana/pull/202097) ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) Co-authored-by: Davis McPhee --- .../main/data_fetching/fetch_all.test.ts | 3 +++ .../main/hooks/use_esql_mode.test.tsx | 15 ++++++----- .../discover_app_state_container.test.ts | 17 ++++++------ .../discover_app_state_container.ts | 3 +++ .../discover_data_state_container.test.ts | 5 ++-- .../discover_data_state_container.ts | 7 +++++ .../discover_internal_state_container.ts | 26 +++++++++++++++---- .../utils/change_data_view.ts | 12 +++++---- .../utils/get_default_profile_state.test.ts | 6 +++++ 9 files changed, 67 insertions(+), 27 deletions(-) diff --git a/src/plugins/discover/public/application/main/data_fetching/fetch_all.test.ts b/src/plugins/discover/public/application/main/data_fetching/fetch_all.test.ts index c7b80181867e0..bbf893a937171 100644 --- a/src/plugins/discover/public/application/main/data_fetching/fetch_all.test.ts +++ b/src/plugins/discover/public/application/main/data_fetching/fetch_all.test.ts @@ -76,6 +76,7 @@ describe('test fetchAll', () => { customFilters: [], overriddenVisContextAfterInvalidation: undefined, resetDefaultProfileState: { + resetId: 'test', columns: false, rowHeight: false, breakdownField: false, @@ -269,6 +270,7 @@ describe('test fetchAll', () => { customFilters: [], overriddenVisContextAfterInvalidation: undefined, resetDefaultProfileState: { + resetId: 'test', columns: false, rowHeight: false, breakdownField: false, @@ -392,6 +394,7 @@ describe('test fetchAll', () => { customFilters: [], overriddenVisContextAfterInvalidation: undefined, resetDefaultProfileState: { + resetId: 'test', columns: false, rowHeight: false, breakdownField: false, diff --git a/src/plugins/discover/public/application/main/hooks/use_esql_mode.test.tsx b/src/plugins/discover/public/application/main/hooks/use_esql_mode.test.tsx index 410406b7eb2d7..f50b8135e2a9f 100644 --- a/src/plugins/discover/public/application/main/hooks/use_esql_mode.test.tsx +++ b/src/plugins/discover/public/application/main/hooks/use_esql_mode.test.tsx @@ -25,6 +25,7 @@ import { DiscoverStateContainer } from '../state_management/discover_state'; import { VIEW_MODE } from '@kbn/saved-search-plugin/public'; import { dataViewAdHoc } from '../../../__mocks__/data_view_complex'; import { buildDataTableRecord, EsHitRecord } from '@kbn/discover-utils'; +import { omit } from 'lodash'; function getHookProps( query: AggregateQuery | Query | undefined, @@ -501,7 +502,7 @@ describe('useEsqlMode', () => { FetchStatus.LOADING ); const documents$ = stateContainer.dataState.data$.documents$; - expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(stateContainer.internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: false, rowHeight: false, breakdownField: false, @@ -516,7 +517,7 @@ describe('useEsqlMode', () => { query: { esql: 'from pattern1' }, }); await waitFor(() => - expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(stateContainer.internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: true, rowHeight: true, breakdownField: true, @@ -537,7 +538,7 @@ describe('useEsqlMode', () => { query: { esql: 'from pattern1' }, }); await waitFor(() => - expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(stateContainer.internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: false, rowHeight: false, breakdownField: false, @@ -553,7 +554,7 @@ describe('useEsqlMode', () => { query: { esql: 'from pattern2' }, }); await waitFor(() => - expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(stateContainer.internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: true, rowHeight: true, breakdownField: true, @@ -570,7 +571,7 @@ describe('useEsqlMode', () => { const documents$ = stateContainer.dataState.data$.documents$; const result1 = [buildDataTableRecord({ message: 'foo' } as EsHitRecord)]; const result2 = [buildDataTableRecord({ message: 'foo', extension: 'bar' } as EsHitRecord)]; - expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(stateContainer.internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: false, rowHeight: false, breakdownField: false, @@ -581,7 +582,7 @@ describe('useEsqlMode', () => { result: result1, }); await waitFor(() => - expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(stateContainer.internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: false, rowHeight: false, breakdownField: false, @@ -593,7 +594,7 @@ describe('useEsqlMode', () => { result: result2, }); await waitFor(() => - expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(stateContainer.internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: true, rowHeight: false, breakdownField: false, diff --git a/src/plugins/discover/public/application/main/state_management/discover_app_state_container.test.ts b/src/plugins/discover/public/application/main/state_management/discover_app_state_container.test.ts index 361688f10c393..41a4569a37683 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_app_state_container.test.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_app_state_container.test.ts @@ -26,6 +26,7 @@ import { getSavedSearchContainer, } from './discover_saved_search_container'; import { getDiscoverGlobalStateContainer } from './discover_global_state_container'; +import { omit } from 'lodash'; let history: History; let stateStorage: IKbnUrlStateStorage; @@ -269,13 +270,13 @@ describe('Test discover app state container', () => { describe('initAndSync', () => { it('should call setResetDefaultProfileState correctly with no initial state', () => { const state = getStateContainer(); - expect(internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: false, rowHeight: false, breakdownField: false, }); state.initAndSync(); - expect(internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: true, rowHeight: true, breakdownField: true, @@ -286,13 +287,13 @@ describe('Test discover app state container', () => { const stateStorageGetSpy = jest.spyOn(stateStorage, 'get'); stateStorageGetSpy.mockReturnValue({ columns: ['test'] }); const state = getStateContainer(); - expect(internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: false, rowHeight: false, breakdownField: false, }); state.initAndSync(); - expect(internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: false, rowHeight: true, breakdownField: true, @@ -303,13 +304,13 @@ describe('Test discover app state container', () => { const stateStorageGetSpy = jest.spyOn(stateStorage, 'get'); stateStorageGetSpy.mockReturnValue({ rowHeight: 5 }); const state = getStateContainer(); - expect(internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: false, rowHeight: false, breakdownField: false, }); state.initAndSync(); - expect(internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: true, rowHeight: false, breakdownField: true, @@ -326,13 +327,13 @@ describe('Test discover app state container', () => { managed: false, }); const state = getStateContainer(); - expect(internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: false, rowHeight: false, breakdownField: false, }); state.initAndSync(); - expect(internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: false, rowHeight: false, breakdownField: false, diff --git a/src/plugins/discover/public/application/main/state_management/discover_app_state_container.ts b/src/plugins/discover/public/application/main/state_management/discover_app_state_container.ts index 2e2fd0c6be084..af3541d89c376 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_app_state_container.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_app_state_container.ts @@ -262,9 +262,12 @@ export const getDiscoverAppStateContainer = ({ addLog('[appState] initialize state and sync with URL', currentSavedSearch); + // Set the default profile state only if not loading a saved search, + // to avoid overwriting saved search state if (!currentSavedSearch.id) { const { breakdownField, columns, rowHeight } = getCurrentUrlState(stateStorage, services); + // Only set default state which is not already set in the URL internalStateContainer.transitions.setResetDefaultProfileState({ columns: columns === undefined, rowHeight: rowHeight === undefined, diff --git a/src/plugins/discover/public/application/main/state_management/discover_data_state_container.test.ts b/src/plugins/discover/public/application/main/state_management/discover_data_state_container.test.ts index 40b688c0d2152..810bc7ffe9766 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_data_state_container.test.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_data_state_container.test.ts @@ -16,6 +16,7 @@ import { FetchStatus } from '../../types'; import { DataDocuments$ } from './discover_data_state_container'; import { getDiscoverStateMock } from '../../../__mocks__/discover_state.mock'; import { fetchDocuments } from '../data_fetching/fetch_documents'; +import { omit } from 'lodash'; jest.mock('../data_fetching/fetch_documents', () => ({ fetchDocuments: jest.fn().mockResolvedValue({ records: [] }), @@ -190,7 +191,7 @@ describe('test getDataStateContainer', () => { await waitFor(() => { expect(dataState.data$.main$.value.fetchStatus).toBe(FetchStatus.COMPLETE); }); - expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(stateContainer.internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: false, rowHeight: false, breakdownField: false, @@ -221,7 +222,7 @@ describe('test getDataStateContainer', () => { await waitFor(() => { expect(dataState.data$.main$.value.fetchStatus).toBe(FetchStatus.COMPLETE); }); - expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ + expect(omit(stateContainer.internalState.get().resetDefaultProfileState, 'resetId')).toEqual({ columns: false, rowHeight: false, breakdownField: false, diff --git a/src/plugins/discover/public/application/main/state_management/discover_data_state_container.ts b/src/plugins/discover/public/application/main/state_management/discover_data_state_container.ts index 5bc27fdb45a60..478d0ca06ea1c 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_data_state_container.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_data_state_container.ts @@ -293,6 +293,13 @@ export function getDataStateContainer({ ...commonFetchDeps, }, async () => { + const { resetDefaultProfileState: currentResetDefaultProfileState } = + internalStateContainer.getState(); + + if (currentResetDefaultProfileState.resetId !== resetDefaultProfileState.resetId) { + return; + } + const { esqlQueryColumns } = dataSubjects.documents$.getValue(); const defaultColumns = uiSettings.get(DEFAULT_COLUMNS_SETTING, []); const postFetchStateUpdate = defaultProfileState?.getPostFetchState({ diff --git a/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts index 67b6551e985f4..8aad8eb91738b 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts @@ -7,6 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ +import { v4 as uuidv4 } from 'uuid'; import { createStateContainer, createStateContainerReactHelpers, @@ -26,7 +27,12 @@ export interface InternalState { customFilters: Filter[]; overriddenVisContextAfterInvalidation: UnifiedHistogramVisContext | {} | undefined; // it will be used during saved search saving isESQLToDataViewTransitionModalVisible?: boolean; - resetDefaultProfileState: { columns: boolean; rowHeight: boolean; breakdownField: boolean }; + resetDefaultProfileState: { + resetId: string; + columns: boolean; + rowHeight: boolean; + breakdownField: boolean; + }; } export interface InternalStateTransitions { @@ -56,7 +62,9 @@ export interface InternalStateTransitions { ) => (isVisible: boolean) => InternalState; setResetDefaultProfileState: ( state: InternalState - ) => (resetDefaultProfileState: InternalState['resetDefaultProfileState']) => InternalState; + ) => ( + resetDefaultProfileState: Omit + ) => InternalState; } export type DiscoverInternalStateContainer = ReduxLikeStateContainer< @@ -77,7 +85,12 @@ export function getInternalStateContainer() { expandedDoc: undefined, customFilters: [], overriddenVisContextAfterInvalidation: undefined, - resetDefaultProfileState: { columns: false, rowHeight: false, breakdownField: false }, + resetDefaultProfileState: { + resetId: '', + columns: false, + rowHeight: false, + breakdownField: false, + }, }, { setDataView: (prevState: InternalState) => (nextDataView: DataView) => ({ @@ -151,9 +164,12 @@ export function getInternalStateContainer() { }), setResetDefaultProfileState: (prevState: InternalState) => - (resetDefaultProfileState: InternalState['resetDefaultProfileState']) => ({ + (resetDefaultProfileState: Omit) => ({ ...prevState, - resetDefaultProfileState, + resetDefaultProfileState: { + ...resetDefaultProfileState, + resetId: uuidv4(), + }, }), }, {}, diff --git a/src/plugins/discover/public/application/main/state_management/utils/change_data_view.ts b/src/plugins/discover/public/application/main/state_management/utils/change_data_view.ts index d2815800fa9c6..8d5b429c43dd7 100644 --- a/src/plugins/discover/public/application/main/state_management/utils/change_data_view.ts +++ b/src/plugins/discover/public/application/main/state_management/utils/change_data_view.ts @@ -43,11 +43,6 @@ export async function changeDataView( let nextDataView: DataView | null = null; internalState.transitions.setIsDataViewLoading(true); - internalState.transitions.setResetDefaultProfileState({ - columns: true, - rowHeight: true, - breakdownField: true, - }); try { nextDataView = typeof id === 'string' ? await dataViews.get(id, false) : id; @@ -56,6 +51,13 @@ export async function changeDataView( } if (nextDataView && dataView) { + // Reset the default profile state if we are switching to a different data view + internalState.transitions.setResetDefaultProfileState({ + columns: true, + rowHeight: true, + breakdownField: true, + }); + const nextAppState = getDataViewAppState( dataView, nextDataView, diff --git a/src/plugins/discover/public/application/main/state_management/utils/get_default_profile_state.test.ts b/src/plugins/discover/public/application/main/state_management/utils/get_default_profile_state.test.ts index 6ba709bdd04ec..a3a904260f2f5 100644 --- a/src/plugins/discover/public/application/main/state_management/utils/get_default_profile_state.test.ts +++ b/src/plugins/discover/public/application/main/state_management/utils/get_default_profile_state.test.ts @@ -27,6 +27,7 @@ describe('getDefaultProfileState', () => { let appState = getDefaultProfileState({ profilesManager: profilesManagerMock, resetDefaultProfileState: { + resetId: 'test', columns: false, rowHeight: false, breakdownField: true, @@ -39,6 +40,7 @@ describe('getDefaultProfileState', () => { appState = getDefaultProfileState({ profilesManager: profilesManagerMock, resetDefaultProfileState: { + resetId: 'test', columns: false, rowHeight: false, breakdownField: true, @@ -54,6 +56,7 @@ describe('getDefaultProfileState', () => { let appState = getDefaultProfileState({ profilesManager: profilesManagerMock, resetDefaultProfileState: { + resetId: 'test', columns: true, rowHeight: false, breakdownField: false, @@ -79,6 +82,7 @@ describe('getDefaultProfileState', () => { appState = getDefaultProfileState({ profilesManager: profilesManagerMock, resetDefaultProfileState: { + resetId: 'test', columns: true, rowHeight: false, breakdownField: false, @@ -107,6 +111,7 @@ describe('getDefaultProfileState', () => { const appState = getDefaultProfileState({ profilesManager: profilesManagerMock, resetDefaultProfileState: { + resetId: 'test', columns: false, rowHeight: true, breakdownField: false, @@ -125,6 +130,7 @@ describe('getDefaultProfileState', () => { const appState = getDefaultProfileState({ profilesManager: profilesManagerMock, resetDefaultProfileState: { + resetId: 'test', columns: false, rowHeight: false, breakdownField: false,