[Discover] Fix flaky cell actions tests (#202097)

## Summary This PR fixes a tricky race condition when setting default Discover profile state, which was causing functional test failures such as the linked cell actions tests. Resolves #193367. Resolves #193400. ### 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 - [ ] 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. - [x] [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)
2025-04-24 01:38:56 -04:00 · 2024-12-03 15:29:07 -04:00 · 2024-12-03 15:29:07 -04:00 · 01de887060
commit 01de887060
parent 530b4d482c
9 changed files with 67 additions and 27 deletions
--- 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,
--- 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,
--- 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,
--- 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,
--- 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,
--- 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<string[]>(DEFAULT_COLUMNS_SETTING, []);
              const postFetchStateUpdate = defaultProfileState?.getPostFetchState({
--- 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['resetDefaultProfileState'], 'resetId'>
+  ) => 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<InternalState['resetDefaultProfileState'], 'resetId'>) => ({
          ...prevState,
-          resetDefaultProfileState,
+          resetDefaultProfileState: {
+            ...resetDefaultProfileState,
+            resetId: uuidv4(),
+          },
        }),
    },
    {},
--- 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,
--- 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,