diff --git a/src/plugins/discover/public/application/main/state_management/discover_saved_search_container.test.ts b/src/plugins/discover/public/application/main/state_management/discover_saved_search_container.test.ts index 3ce4c969bbd3f..927b9e60baf1e 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_saved_search_container.test.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_saved_search_container.test.ts @@ -14,6 +14,8 @@ import { dataViewMock } from '@kbn/discover-utils/src/__mocks__'; import { dataViewComplexMock } from '../../../__mocks__/data_view_complex'; import { getDiscoverGlobalStateContainer } from './discover_global_state_container'; import { createKbnUrlStateStorage } from '@kbn/kibana-utils-plugin/public'; +import { VIEW_MODE } from '../../../../common/constants'; +import { createSearchSourceMock } from '@kbn/data-plugin/common/search/search_source/mocks'; describe('DiscoverSavedSearchContainer', () => { const savedSearch = savedSearchMock; @@ -232,4 +234,57 @@ describe('DiscoverSavedSearchContainer', () => { expect(savedSearchContainer.getHasChanged$().getValue()).toBe(false); }); }); + + describe('isEqualSavedSearch', () => { + it('should return true for equal saved searches', () => { + const savedSearch1 = savedSearchMock; + const savedSearch2 = { ...savedSearchMock }; + expect(isEqualSavedSearch(savedSearch1, savedSearch2)).toBe(true); + }); + + it('should return false for different saved searches', () => { + const savedSearch1 = savedSearchMock; + const savedSearch2 = { ...savedSearchMock, title: 'New title' }; + expect(isEqualSavedSearch(savedSearch1, savedSearch2)).toBe(false); + }); + + it('should return true for saved searches with equal default values of viewMode and false otherwise', () => { + const savedSearch1 = { ...savedSearchMock, viewMode: undefined }; + const savedSearch2 = { ...savedSearchMock, viewMode: VIEW_MODE.DOCUMENT_LEVEL }; + const savedSearch3 = { ...savedSearchMock, viewMode: VIEW_MODE.AGGREGATED_LEVEL }; + expect(isEqualSavedSearch(savedSearch1, savedSearch2)).toBe(true); + expect(isEqualSavedSearch(savedSearch2, savedSearch1)).toBe(true); + expect(isEqualSavedSearch(savedSearch1, savedSearch3)).toBe(false); + expect(isEqualSavedSearch(savedSearch2, savedSearch3)).toBe(false); + }); + + it('should return true for saved searches with equal default values of breakdownField and false otherwise', () => { + const savedSearch1 = { ...savedSearchMock, breakdownField: undefined }; + const savedSearch2 = { ...savedSearchMock, breakdownField: '' }; + const savedSearch3 = { ...savedSearchMock, breakdownField: 'test' }; + expect(isEqualSavedSearch(savedSearch1, savedSearch2)).toBe(true); + expect(isEqualSavedSearch(savedSearch2, savedSearch1)).toBe(true); + expect(isEqualSavedSearch(savedSearch1, savedSearch3)).toBe(false); + expect(isEqualSavedSearch(savedSearch2, savedSearch3)).toBe(false); + }); + + it('should check searchSource fields', () => { + const savedSearch1 = { + ...savedSearchMock, + searchSource: createSearchSourceMock({ + index: savedSearchMock.searchSource.getField('index'), + }), + }; + const savedSearch2 = { + ...savedSearchMock, + searchSource: createSearchSourceMock({ + index: savedSearchMock.searchSource.getField('index'), + }), + }; + expect(isEqualSavedSearch(savedSearch1, savedSearch1)).toBe(true); + expect(isEqualSavedSearch(savedSearch1, savedSearch2)).toBe(true); + savedSearch2.searchSource.setField('query', { language: 'lucene', query: 'test' }); + expect(isEqualSavedSearch(savedSearch1, savedSearch2)).toBe(false); + }); + }); }); diff --git a/src/plugins/discover/public/application/main/state_management/discover_saved_search_container.ts b/src/plugins/discover/public/application/main/state_management/discover_saved_search_container.ts index c6a380cad98e2..8d3975b5bfef5 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_saved_search_container.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_saved_search_container.ts @@ -341,12 +341,7 @@ export function isEqualSavedSearch(savedSearchPrev: SavedSearch, savedSearchNext const prevValue = getSavedSearchFieldForComparison(prevSavedSearch, key); const nextValue = getSavedSearchFieldForComparison(nextSavedSearchWithoutSearchSource, key); - const isSame = - isEqual(prevValue, nextValue) || - // By default, viewMode: undefined is equivalent to documents view - // So they should be treated as same - (key === 'viewMode' && - (prevValue ?? VIEW_MODE.DOCUMENT_LEVEL) === (nextValue ?? VIEW_MODE.DOCUMENT_LEVEL)); + const isSame = isEqual(prevValue, nextValue); if (!isSame) { addLog('[savedSearch] difference between initial and changed version', { @@ -412,6 +407,12 @@ function getSavedSearchFieldForComparison( return savedSearch.breakdownField || ''; // ignore the difference between an empty string and undefined } + if (fieldName === 'viewMode') { + // By default, viewMode: undefined is equivalent to documents view + // So they should be treated as same + return savedSearch.viewMode ?? VIEW_MODE.DOCUMENT_LEVEL; + } + return savedSearch[fieldName]; } diff --git a/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.test.ts b/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.test.ts index 1a487c5bb3bcb..50ee8b0c74fe7 100644 --- a/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.test.ts +++ b/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.test.ts @@ -11,7 +11,8 @@ import { FetchStatus } from '../../../types'; import { dataViewComplexMock } from '../../../../__mocks__/data_view_complex'; import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock'; import { discoverServiceMock } from '../../../../__mocks__/services'; -import { createDataViewDataSource } from '../../../../../common/data_sources'; +import { createDataViewDataSource, DataSourceType } from '../../../../../common/data_sources'; +import { VIEW_MODE } from '@kbn/saved-search-plugin/common'; describe('buildStateSubscribe', () => { const savedSearch = savedSearchMock; @@ -19,6 +20,7 @@ describe('buildStateSubscribe', () => { stateContainer.dataState.refetch$.next = jest.fn(); stateContainer.dataState.reset = jest.fn(); stateContainer.actions.setDataView = jest.fn(); + stateContainer.savedSearchState.update = jest.fn(); const getSubscribeFn = () => { return buildStateSubscribe({ @@ -51,6 +53,20 @@ describe('buildStateSubscribe', () => { expect(stateContainer.dataState.refetch$.next).not.toHaveBeenCalled(); }); + it('should not call refetch$ if viewMode changes', async () => { + await getSubscribeFn()({ + ...stateContainer.appState.getState(), + dataSource: { + type: DataSourceType.Esql, + }, + viewMode: VIEW_MODE.AGGREGATED_LEVEL, + }); + + expect(stateContainer.dataState.refetch$.next).not.toHaveBeenCalled(); + expect(stateContainer.dataState.reset).not.toHaveBeenCalled(); + expect(stateContainer.savedSearchState.update).toHaveBeenCalled(); + }); + it('should call refetch$ if the chart is hidden', async () => { await getSubscribeFn()({ hideChart: true }); diff --git a/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.ts b/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.ts index 5c71311377595..865992187089a 100644 --- a/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.ts +++ b/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.ts @@ -55,12 +55,14 @@ export const buildStateSubscribe = const isEsqlMode = isDataSourceType(nextState.dataSource, DataSourceType.Esql); const queryChanged = !isEqual(nextQuery, prevQuery) || !isEqual(nextQuery, prevState.query); - if ( - isEsqlMode && - isEqualState(prevState, nextState, ['dataSource', 'viewMode']) && - !queryChanged - ) { - // When there's a switch from data view to es|ql, this just leads to a cleanup of index and viewMode + if (isEsqlMode && prevState.viewMode !== nextState.viewMode && !queryChanged) { + savedSearchState.update({ nextState }); + addLog('[appstate] subscribe $fetch ignored for es|ql', { prevState, nextState }); + return; + } + + if (isEsqlMode && isEqualState(prevState, nextState, ['dataSource']) && !queryChanged) { + // When there's a switch from data view to es|ql, this just leads to a cleanup of index // And there's no subsequent action in this function required addLog('[appstate] subscribe update ignored for es|ql', { prevState, nextState }); return;