Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] [Discover] Fix flaky cell actions tests (#202097) #202805

Merged
merged 1 commit into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ describe('test fetchAll', () => {
customFilters: [],
overriddenVisContextAfterInvalidation: undefined,
resetDefaultProfileState: {
resetId: 'test',
columns: false,
rowHeight: false,
breakdownField: false,
Expand Down Expand Up @@ -269,6 +270,7 @@ describe('test fetchAll', () => {
customFilters: [],
overriddenVisContextAfterInvalidation: undefined,
resetDefaultProfileState: {
resetId: 'test',
columns: false,
rowHeight: false,
breakdownField: false,
Expand Down Expand Up @@ -392,6 +394,7 @@ describe('test fetchAll', () => {
customFilters: [],
overriddenVisContextAfterInvalidation: undefined,
resetDefaultProfileState: {
resetId: 'test',
columns: false,
rowHeight: false,
breakdownField: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: [] }),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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<
Expand All @@ -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) => ({
Expand Down Expand Up @@ -151,9 +164,12 @@ export function getInternalStateContainer() {
}),
setResetDefaultProfileState:
(prevState: InternalState) =>
(resetDefaultProfileState: InternalState['resetDefaultProfileState']) => ({
(resetDefaultProfileState: Omit<InternalState['resetDefaultProfileState'], 'resetId'>) => ({
...prevState,
resetDefaultProfileState,
resetDefaultProfileState: {
...resetDefaultProfileState,
resetId: uuidv4(),
},
}),
},
{},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('getDefaultProfileState', () => {
let appState = getDefaultProfileState({
profilesManager: profilesManagerMock,
resetDefaultProfileState: {
resetId: 'test',
columns: false,
rowHeight: false,
breakdownField: true,
Expand All @@ -39,6 +40,7 @@ describe('getDefaultProfileState', () => {
appState = getDefaultProfileState({
profilesManager: profilesManagerMock,
resetDefaultProfileState: {
resetId: 'test',
columns: false,
rowHeight: false,
breakdownField: true,
Expand All @@ -54,6 +56,7 @@ describe('getDefaultProfileState', () => {
let appState = getDefaultProfileState({
profilesManager: profilesManagerMock,
resetDefaultProfileState: {
resetId: 'test',
columns: true,
rowHeight: false,
breakdownField: false,
Expand All @@ -79,6 +82,7 @@ describe('getDefaultProfileState', () => {
appState = getDefaultProfileState({
profilesManager: profilesManagerMock,
resetDefaultProfileState: {
resetId: 'test',
columns: true,
rowHeight: false,
breakdownField: false,
Expand Down Expand Up @@ -107,6 +111,7 @@ describe('getDefaultProfileState', () => {
const appState = getDefaultProfileState({
profilesManager: profilesManagerMock,
resetDefaultProfileState: {
resetId: 'test',
columns: false,
rowHeight: true,
breakdownField: false,
Expand All @@ -125,6 +130,7 @@ describe('getDefaultProfileState', () => {
const appState = getDefaultProfileState({
profilesManager: profilesManagerMock,
resetDefaultProfileState: {
resetId: 'test',
columns: false,
rowHeight: false,
breakdownField: false,
Expand Down
Loading