diff --git a/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.tsx b/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.tsx index 0036b517c0eb9..59723e5b1225f 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.tsx @@ -181,7 +181,7 @@ export const getTopNavLinks = ({ defaultMessage: 'Untitled discover search', }), }, - isDirty: !savedSearch.id || state.isAppStateDirty(), + isDirty: !savedSearch.id || state.appState.hasChanged(), showPublicUrlSwitch, onClose: () => { anchorElement?.focus(); diff --git a/src/plugins/discover/public/application/main/components/top_nav/on_save_search.test.tsx b/src/plugins/discover/public/application/main/components/top_nav/on_save_search.test.tsx index f8199fadd2e61..a7efab7d984a9 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/on_save_search.test.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/on_save_search.test.tsx @@ -14,35 +14,35 @@ jest.mock('../../utils/persist_saved_search', () => ({ import { onSaveSearch } from './on_save_search'; import { dataViewMock } from '../../../../__mocks__/data_view'; import { savedSearchMock } from '../../../../__mocks__/saved_search'; -import { DiscoverServices } from '../../../../build_services'; -import { DiscoverStateContainer } from '../../services/discover_state'; -import { i18nServiceMock } from '@kbn/core/public/mocks'; +import { getDiscoverStateContainer } from '../../services/discover_state'; import { ReactElement } from 'react'; import { discoverServiceMock } from '../../../../__mocks__/services'; import * as persistSavedSearchUtils from '../../utils/persist_saved_search'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; +import { createBrowserHistory } from 'history'; + +function getStateContainer() { + const savedSearch = savedSearchMock; + const history = createBrowserHistory(); + const stateContainer = getDiscoverStateContainer({ + savedSearch, + services: discoverServiceMock, + history, + }); + stateContainer.appState.getState = jest.fn(() => ({ + rowsPerPage: 250, + })); + return stateContainer; +} describe('onSaveSearch', () => { it('should call showSaveModal', async () => { - const serviceMock = { - core: { - i18n: i18nServiceMock.create(), - }, - } as unknown as DiscoverServices; - const stateMock = { - appState: { - getState: () => ({ - rowsPerPage: 250, - }), - }, - } as unknown as DiscoverStateContainer; - await onSaveSearch({ dataView: dataViewMock, navigateTo: jest.fn(), savedSearch: savedSearchMock, - services: serviceMock, - state: stateMock, + services: discoverServiceMock, + state: getStateContainer(), updateAdHocDataViewId: jest.fn(), }); @@ -50,14 +50,6 @@ describe('onSaveSearch', () => { }); it('should pass tags to the save modal', async () => { - const serviceMock = discoverServiceMock; - const stateMock = { - appState: { - getState: () => ({ - rowsPerPage: 250, - }), - }, - } as unknown as DiscoverStateContainer; let saveModal: ReactElement | undefined; jest.spyOn(savedObjectsPlugin, 'showSaveModal').mockImplementationOnce((modal) => { saveModal = modal; @@ -69,23 +61,14 @@ describe('onSaveSearch', () => { ...savedSearchMock, tags: ['tag1', 'tag2'], }, - services: serviceMock, - state: stateMock, + services: discoverServiceMock, + state: getStateContainer(), updateAdHocDataViewId: jest.fn(), }); expect(saveModal?.props.tags).toEqual(['tag1', 'tag2']); }); it('should update the saved search tags', async () => { - const serviceMock = discoverServiceMock; - const stateMock = { - appState: { - getState: () => ({ - rowsPerPage: 250, - }), - }, - resetInitialAppState: jest.fn(), - } as unknown as DiscoverStateContainer; let saveModal: ReactElement | undefined; jest.spyOn(savedObjectsPlugin, 'showSaveModal').mockImplementationOnce((modal) => { saveModal = modal; @@ -98,8 +81,8 @@ describe('onSaveSearch', () => { dataView: dataViewMock, navigateTo: jest.fn(), savedSearch, - services: serviceMock, - state: stateMock, + services: discoverServiceMock, + state: getStateContainer(), updateAdHocDataViewId: jest.fn(), }); expect(savedSearch.tags).toEqual(['tag1', 'tag2']); @@ -122,14 +105,6 @@ describe('onSaveSearch', () => { it('should not update tags if savedObjectsTagging is undefined', async () => { const serviceMock = discoverServiceMock; - const stateMock = { - appState: { - getState: () => ({ - rowsPerPage: 250, - }), - }, - resetInitialAppState: jest.fn(), - } as unknown as DiscoverStateContainer; let saveModal: ReactElement | undefined; jest.spyOn(savedObjectsPlugin, 'showSaveModal').mockImplementationOnce((modal) => { saveModal = modal; @@ -146,7 +121,7 @@ describe('onSaveSearch', () => { ...serviceMock, savedObjectsTagging: undefined, }, - state: stateMock, + state: getStateContainer(), updateAdHocDataViewId: jest.fn(), }); expect(savedSearch.tags).toEqual(['tag1', 'tag2']); diff --git a/src/plugins/discover/public/application/main/components/top_nav/on_save_search.tsx b/src/plugins/discover/public/application/main/components/top_nav/on_save_search.tsx index 237fc2b4ab574..4d88e48b8adf5 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/on_save_search.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/on_save_search.tsx @@ -53,7 +53,7 @@ async function saveDataSource({ navigateTo(`/view/${encodeURIComponent(id)}`); } else { // Update defaults so that "reload saved query" functions correctly - state.resetAppState(savedSearch); + state.appState.resetWithSavedSearch(savedSearch); services.chrome.docTitle.change(savedSearch.title!); setBreadcrumbsTitle( @@ -169,7 +169,7 @@ export async function onSaveSearch({ savedSearch.tags = currentTags; } } else { - state.resetInitialAppState(); + state.appState.resetInitialState(); } onSaveCb?.(); return response; diff --git a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts index c87c13b0e9fb7..31e96f01a2b74 100644 --- a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts +++ b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts @@ -63,7 +63,7 @@ export const useAdHocDataViews = ({ updateFiltersReferences(prevDataView, newDataView); stateContainer.actions.replaceAdHocDataViewWithId(prevDataView.id!, newDataView); - await stateContainer.replaceUrlAppState({ index: newDataView.id }); + await stateContainer.appState.update({ index: newDataView.id }, true); setUrlTracking(newDataView); return newDataView; diff --git a/src/plugins/discover/public/application/main/hooks/use_discover_state.ts b/src/plugins/discover/public/application/main/hooks/use_discover_state.ts index 29b6f0151361f..1170629142dae 100644 --- a/src/plugins/discover/public/application/main/hooks/use_discover_state.ts +++ b/src/plugins/discover/public/application/main/hooks/use_discover_state.ts @@ -179,7 +179,7 @@ export function useDiscoverState({ timefilter: services.timefilter, }); - await stateContainer.replaceUrlAppState(newAppState); + await stateContainer.appState.update(newAppState, true); setState(newAppState); }, [services, dataView, stateContainer] diff --git a/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts b/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts index 0f1b6488f3681..b596e2bc06c40 100644 --- a/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts +++ b/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts @@ -19,16 +19,15 @@ import { AggregateQuery, Query } from '@kbn/es-query'; import { dataViewMock } from '../../../__mocks__/data_view'; import { DataViewListItem } from '@kbn/data-views-plugin/common'; import { savedSearchMock } from '../../../__mocks__/saved_search'; -import { AppState } from '../services/discover_app_state_container'; import { getDiscoverStateMock } from '../../../__mocks__/discover_state.mock'; function getHookProps( - replaceUrlAppState: (newState: Partial) => Promise, query: AggregateQuery | Query | undefined, dataViewsService?: DataViewsContract ) { + const replaceUrlState = jest.fn(); const stateContainer = getDiscoverStateMock({ isTimeBased: true }); - stateContainer.replaceUrlAppState = replaceUrlAppState; + stateContainer.appState.replaceUrlState = replaceUrlState; stateContainer.setAppState({ columns: [] }); stateContainer.internalState.transitions.setSavedDataViews([dataViewMock as DataViewListItem]); @@ -45,6 +44,7 @@ function getHookProps( dataViews: dataViewsService ?? discoverServiceMock.dataViews, stateContainer, savedSearch: savedSearchMock, + replaceUrlState, }; } const query = { sql: 'SELECT * from the-data-view-title' }; @@ -63,37 +63,35 @@ const msgComplete = { describe('useTextBasedQueryLanguage', () => { test('a text based query should change state when loading and finished', async () => { - const replaceUrlAppState = jest.fn(); - const props = getHookProps(replaceUrlAppState, query); - const { documents$ } = props; + const props = getHookProps(query); + const { documents$, replaceUrlState } = props; renderHook(() => useTextBasedQueryLanguage(props)); - await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(1)); - expect(replaceUrlAppState).toHaveBeenCalledWith({ index: 'the-data-view-id' }); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1)); + expect(replaceUrlState).toHaveBeenCalledWith({ index: 'the-data-view-id' }); - replaceUrlAppState.mockReset(); + replaceUrlState.mockReset(); documents$.next(msgComplete); - await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1)); await waitFor(() => { - expect(replaceUrlAppState).toHaveBeenCalledWith({ + expect(replaceUrlState).toHaveBeenCalledWith({ index: 'the-data-view-id', columns: ['field1', 'field2'], }); }); }); test('changing a text based query with different result columns should change state when loading and finished', async () => { - const replaceUrlAppState = jest.fn(); - const props = getHookProps(replaceUrlAppState, query); - const { documents$ } = props; + const props = getHookProps(query); + const { documents$, replaceUrlState } = props; renderHook(() => useTextBasedQueryLanguage(props)); documents$.next(msgComplete); - await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(2)); - replaceUrlAppState.mockReset(); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(2)); + replaceUrlState.mockReset(); documents$.next({ recordRawType: RecordRawType.PLAIN, @@ -107,25 +105,24 @@ describe('useTextBasedQueryLanguage', () => { ], query: { sql: 'SELECT field1 from the-data-view-title' }, }); - await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1)); await waitFor(() => { - expect(replaceUrlAppState).toHaveBeenCalledWith({ + expect(replaceUrlState).toHaveBeenCalledWith({ index: 'the-data-view-id', columns: ['field1'], }); }); }); test('only changing a text based query with same result columns should not change columns', async () => { - const replaceUrlAppState = jest.fn(); - const props = getHookProps(replaceUrlAppState, query); - const { documents$ } = props; + const props = getHookProps(query); + const { documents$, replaceUrlState } = props; renderHook(() => useTextBasedQueryLanguage(props)); documents$.next(msgComplete); - await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(2)); - replaceUrlAppState.mockReset(); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(2)); + replaceUrlState.mockReset(); documents$.next({ recordRawType: RecordRawType.PLAIN, @@ -139,8 +136,8 @@ describe('useTextBasedQueryLanguage', () => { ], query: { sql: 'SELECT field1 from the-data-view-title' }, }); - await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(1)); - replaceUrlAppState.mockReset(); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1)); + replaceUrlState.mockReset(); documents$.next({ recordRawType: RecordRawType.PLAIN, @@ -156,21 +153,20 @@ describe('useTextBasedQueryLanguage', () => { }); await waitFor(() => { - expect(replaceUrlAppState).toHaveBeenCalledWith({ + expect(replaceUrlState).toHaveBeenCalledWith({ index: 'the-data-view-id', }); }); }); test('if its not a text based query coming along, it should be ignored', async () => { - const replaceUrlAppState = jest.fn(); - const props = getHookProps(replaceUrlAppState, query); - const { documents$ } = props; + const props = getHookProps(query); + const { documents$, replaceUrlState } = props; renderHook(() => useTextBasedQueryLanguage(props)); documents$.next(msgComplete); - await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(2)); - replaceUrlAppState.mockReset(); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(2)); + replaceUrlState.mockReset(); documents$.next({ recordRawType: RecordRawType.DOCUMENT, @@ -198,7 +194,7 @@ describe('useTextBasedQueryLanguage', () => { }); await waitFor(() => { - expect(replaceUrlAppState).toHaveBeenCalledWith({ + expect(replaceUrlState).toHaveBeenCalledWith({ index: 'the-data-view-id', columns: ['field1'], }); @@ -206,12 +202,11 @@ describe('useTextBasedQueryLanguage', () => { }); test('it should not overwrite existing state columns on initial fetch', async () => { - const replaceUrlAppState = jest.fn(); - const props = getHookProps(replaceUrlAppState, query); + const props = getHookProps(query); props.stateContainer.appState.getState = jest.fn(() => { return { columns: ['field1'], index: 'the-data-view-id' }; }); - const { documents$ } = props; + const { documents$, replaceUrlState } = props; renderHook(() => useTextBasedQueryLanguage(props)); documents$.next({ @@ -239,19 +234,18 @@ describe('useTextBasedQueryLanguage', () => { ], query: { sql: 'SELECT field1 from the-data-view-title' }, }); - await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(1)); - expect(replaceUrlAppState).toHaveBeenCalledWith({ + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1)); + expect(replaceUrlState).toHaveBeenCalledWith({ columns: ['field1'], }); }); test('it should not overwrite state column when successfully fetching after an error fetch', async () => { - const replaceUrlAppState = jest.fn(); - const props = getHookProps(replaceUrlAppState, query); + const props = getHookProps(query); props.stateContainer.appState.getState = jest.fn(() => { return { columns: [], index: 'the-data-view-id' }; }); - const { documents$ } = props; + const { documents$, replaceUrlState } = props; renderHook(() => useTextBasedQueryLanguage(props)); documents$.next({ @@ -259,7 +253,7 @@ describe('useTextBasedQueryLanguage', () => { fetchStatus: FetchStatus.LOADING, query: { sql: 'SELECT * from the-data-view-title WHERE field1=2' }, }); - await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(0)); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(0)); documents$.next({ recordRawType: RecordRawType.PLAIN, fetchStatus: FetchStatus.COMPLETE, @@ -272,11 +266,11 @@ describe('useTextBasedQueryLanguage', () => { ], query: { sql: 'SELECT * from the-data-view-title WHERE field1=2' }, }); - await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1)); props.stateContainer.appState.getState = jest.fn(() => { return { columns: ['field1', 'field2'], index: 'the-data-view-id' }; }); - replaceUrlAppState.mockReset(); + replaceUrlState.mockReset(); documents$.next({ recordRawType: RecordRawType.PLAIN, @@ -308,14 +302,13 @@ describe('useTextBasedQueryLanguage', () => { query: { sql: 'SELECT field1 from the-data-view-title' }, }); - await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(1)); - expect(replaceUrlAppState).toHaveBeenCalledWith({ + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1)); + expect(replaceUrlState).toHaveBeenCalledWith({ columns: ['field1'], }); }); test('changing a text based query with an index pattern that not corresponds to a dataview should return results', async () => { - const replaceUrlAppState = jest.fn(); const dataViewsCreateMock = discoverServiceMock.dataViews.create as jest.Mock; dataViewsCreateMock.mockImplementation(() => ({ ...dataViewMock, @@ -324,14 +317,14 @@ describe('useTextBasedQueryLanguage', () => { ...discoverServiceMock.dataViews, create: dataViewsCreateMock, }; - const props = getHookProps(replaceUrlAppState, query, dataViewsService); - const { documents$ } = props; + const props = getHookProps(query, dataViewsService); + const { documents$, replaceUrlState } = props; renderHook(() => useTextBasedQueryLanguage(props)); documents$.next(msgComplete); - await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(2)); - replaceUrlAppState.mockReset(); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(2)); + replaceUrlState.mockReset(); documents$.next({ recordRawType: RecordRawType.PLAIN, @@ -345,10 +338,10 @@ describe('useTextBasedQueryLanguage', () => { ], query: { sql: 'SELECT field1 from the-data-view-*' }, }); - await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1)); await waitFor(() => { - expect(replaceUrlAppState).toHaveBeenCalledWith({ + expect(replaceUrlState).toHaveBeenCalledWith({ index: 'the-data-view-id', columns: ['field1'], }); diff --git a/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts b/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts index b47c7145904bf..5e2eb33efdbe9 100644 --- a/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts +++ b/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts @@ -116,7 +116,7 @@ export function useTextBasedQueryLanguage({ ...(addDataViewToState && { index: dataViewObj.id }), ...(addColumnsToState && { columns: nextColumns }), }; - stateContainer.replaceUrlAppState(nextState); + stateContainer.appState.replaceUrlState(nextState); } else { // cleanup for a "regular" query cleanup(); diff --git a/src/plugins/discover/public/application/main/hooks/utils/build_state_subscribe.test.ts b/src/plugins/discover/public/application/main/hooks/utils/build_state_subscribe.test.ts index d8070ab97d6de..2ea8275dea013 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/build_state_subscribe.test.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/build_state_subscribe.test.ts @@ -103,15 +103,32 @@ describe('buildStateSubscribe', () => { }); it('should not execute setState function if initialFetchStatus is UNINITIALIZED', async () => { - const stateSubcribeFn = await buildStateSubscribe({ + const stateSubscribeFn = await buildStateSubscribe({ stateContainer, savedSearch, setState, }); stateContainer.dataState.initialFetchStatus = FetchStatus.UNINITIALIZED; - await stateSubcribeFn({ index: dataViewComplexMock.id }); + await stateSubscribeFn({ index: dataViewComplexMock.id }); expect(stateContainer.dataState.reset).toHaveBeenCalled(); expect(setState).not.toHaveBeenCalled(); }); + it('should not execute setState twice if the identical data view change is propagated twice', async () => { + const stateSubscribeFn = await buildStateSubscribe({ + stateContainer, + savedSearch, + setState, + }); + await stateSubscribeFn({ index: dataViewComplexMock.id }); + + expect(setState).toBeCalledTimes(0); + expect(stateContainer.dataState.reset).toBeCalledTimes(1); + + stateContainer.appState.getPrevious = jest.fn(() => ({ index: dataViewComplexMock.id })); + + await stateSubscribeFn({ index: dataViewComplexMock.id }); + expect(setState).toBeCalledTimes(0); + expect(stateContainer.dataState.reset).toBeCalledTimes(1); + }); }); diff --git a/src/plugins/discover/public/application/main/hooks/utils/build_state_subscribe.ts b/src/plugins/discover/public/application/main/hooks/utils/build_state_subscribe.ts index 5c86d8c5b20f9..81bed8eeeaddc 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/build_state_subscribe.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/build_state_subscribe.ts @@ -5,10 +5,10 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ -import { isEqual } from 'lodash'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; +import { isEqual } from 'lodash'; import { DiscoverStateContainer } from '../../services/discover_state'; -import { AppState } from '../../services/discover_app_state_container'; +import { AppState, isEqualState } from '../../services/discover_app_state_container'; import { addLog } from '../../../../utils/add_log'; import { FetchStatus } from '../../../types'; @@ -30,6 +30,11 @@ export const buildStateSubscribe = setState: (state: AppState) => void; }) => async (nextState: AppState) => { + const prevState = stateContainer.appState.getPrevious(); + if (isEqualState(prevState, nextState)) { + addLog('[appstate] subscribe update ignored due to no changes'); + return; + } addLog('[appstate] subscribe triggered', nextState); const { hideChart, interval, breakdownField, sort, index } = stateContainer.appState.getPrevious(); diff --git a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts index a63f46ac312ae..04afdac829f97 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts @@ -25,7 +25,7 @@ const setupTestParams = (dataView: DataView | undefined) => { }); discoverState.internalState.transitions.setDataView(savedSearch.searchSource.getField('index')!); services.dataViews.get = jest.fn(() => Promise.resolve(dataView as DataView)); - discoverState.replaceUrlAppState = jest.fn(); + discoverState.appState.update = jest.fn(); return { services, discoverState, setUrlTracking: jest.fn() }; }; @@ -33,7 +33,7 @@ describe('changeDataView', () => { it('should set the right app state when a valid data view to switch to is given', async () => { const params = setupTestParams(dataViewComplexMock as DataView); await changeDataView('data-view-with-various-field-types', params); - expect(params.discoverState.replaceUrlAppState).toHaveBeenCalledWith({ + expect(params.discoverState.appState.update).toHaveBeenCalledWith({ columns: ['default_column'], index: 'data-view-with-various-field-types-id', sort: [['data', 'desc']], @@ -43,6 +43,6 @@ describe('changeDataView', () => { it('should not set the app state when an invalid data view to switch to is given', async () => { const params = setupTestParams(undefined); await changeDataView('data-view-with-various-field-types', params); - expect(params.discoverState.replaceUrlAppState).not.toHaveBeenCalled(); + expect(params.discoverState.appState.update).not.toHaveBeenCalled(); }); }); diff --git a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts index a9f976c72794f..0ceb3c984a165 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts @@ -50,6 +50,6 @@ export async function changeDataView( ); setUrlTracking(nextDataView); - discoverState.replaceUrlAppState(nextAppState); + discoverState.appState.update(nextAppState); } } diff --git a/src/plugins/discover/public/application/main/services/discover_app_state_container.ts b/src/plugins/discover/public/application/main/services/discover_app_state_container.ts index bc899def87a39..9225d9c8a05e1 100644 --- a/src/plugins/discover/public/application/main/services/discover_app_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_app_state_container.ts @@ -11,7 +11,14 @@ import { createStateContainerReactHelpers, ReduxLikeStateContainer, } from '@kbn/kibana-utils-plugin/common'; -import { AggregateQuery, Filter, FilterStateStore, Query } from '@kbn/es-query'; +import { + AggregateQuery, + COMPARE_ALL_OPTIONS, + compareFilters, + Filter, + FilterStateStore, + Query, +} from '@kbn/es-query'; import { SavedSearch, VIEW_MODE } from '@kbn/saved-search-plugin/public'; import { IKbnUrlStateStorage, ISyncStateRef, syncState } from '@kbn/kibana-utils-plugin/public'; import { cloneDeep, isEqual } from 'lodash'; @@ -22,7 +29,7 @@ import { getValidFilters } from '../../../utils/get_valid_filters'; import { cleanupUrlState } from '../utils/cleanup_url_state'; import { getStateDefaults } from '../utils/get_state_defaults'; import { handleSourceColumnState } from '../../../utils/state_helpers'; -import { APP_STATE_URL_KEY, AppStateUrl, isEqualState, setState } from './discover_state'; +import { APP_STATE_URL_KEY, AppStateUrl } from './discover_state'; import { DiscoverGridSettings } from '../../../components/discover_grid/types'; export interface DiscoverAppStateContainer extends ReduxLikeStateContainer { @@ -39,6 +46,12 @@ export interface DiscoverAppStateContainer extends ReduxLikeStateContainer () => void; + /** + * Replaces the current state in URL with the given state + * @param newState + * @param merge if true, the given state is merged with the current state + */ + replaceUrlState: (newPartial: AppState, merge?: boolean) => void; /** * Resets the state by the given saved search * @param savedSearch @@ -253,6 +266,7 @@ export const getDiscoverAppStateContainer = ({ initAndSync: initializeAndSync, resetWithSavedSearch, resetInitialState, + replaceUrlState, syncState: startAppStateUrlSync, update, }; @@ -276,3 +290,43 @@ function getInitialState( services.uiSettings ); } + +/** + * Helper function to merge a given new state with the existing state and to set the given state + * container + */ +export function setState(stateContainer: ReduxLikeStateContainer, newState: AppState) { + addLog('[appstate] setState', { newState }); + const oldState = stateContainer.getState(); + const mergedState = { ...oldState, ...newState }; + if (!isEqualState(oldState, mergedState)) { + stateContainer.set(mergedState); + } +} + +/** + * Helper function to compare 2 different filter states + */ +export function isEqualFilters(filtersA?: Filter[] | Filter, filtersB?: Filter[] | Filter) { + if (!filtersA && !filtersB) { + return true; + } else if (!filtersA || !filtersB) { + return false; + } + return compareFilters(filtersA, filtersB, COMPARE_ALL_OPTIONS); +} + +/** + * Helper function to compare 2 different state, is needed since comparing filters + * works differently + */ +export function isEqualState(stateA: AppState, stateB: AppState) { + if (!stateA && !stateB) { + return true; + } else if (!stateA || !stateB) { + return false; + } + const { filters: stateAFilters = [], ...stateAPartial } = stateA; + const { filters: stateBFilters = [], ...stateBPartial } = stateB; + return isEqual(stateAPartial, stateBPartial) && isEqualFilters(stateAFilters, stateBFilters); +} diff --git a/src/plugins/discover/public/application/main/services/discover_state.test.ts b/src/plugins/discover/public/application/main/services/discover_state.test.ts index f3ed93a3394df..8bc64f83920ee 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.test.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.test.ts @@ -34,7 +34,7 @@ describe('Test discover state', () => { services: discoverServiceMock, history, }); - await state.replaceUrlAppState({}); + await state.appState.update({}, true); stopSync = state.startSync(); }); afterEach(() => { @@ -67,16 +67,16 @@ describe('Test discover state', () => { test('isAppStateDirty returns whether the current state has changed', async () => { state.setAppState({ index: 'modified' }); - expect(state.isAppStateDirty()).toBeTruthy(); - state.resetInitialAppState(); - expect(state.isAppStateDirty()).toBeFalsy(); + expect(state.appState.hasChanged()).toBeTruthy(); + state.appState.resetInitialState(); + expect(state.appState.hasChanged()).toBeFalsy(); }); test('getPreviousAppState returns the state before the current', async () => { state.setAppState({ index: 'first' }); const stateA = state.appState.getState(); state.setAppState({ index: 'second' }); - expect(state.getPreviousAppState()).toEqual(stateA); + expect(state.appState.getPrevious()).toEqual(stateA); }); test('pauseAutoRefreshInterval sets refreshInterval.pause to true', async () => { @@ -96,7 +96,7 @@ describe('Test discover initial state sort handling', () => { services: discoverServiceMock, history, }); - await state.replaceUrlAppState({}); + await state.appState.update({}, true); const stopSync = state.startSync(); expect(state.appState.getState().sort).toEqual([['order_date', 'desc']]); stopSync(); @@ -110,7 +110,7 @@ describe('Test discover initial state sort handling', () => { services: discoverServiceMock, history, }); - await state.replaceUrlAppState({}); + await state.appState.update({}, true); const stopSync = state.startSync(); expect(state.appState.getState().sort).toEqual([['bytes', 'desc']]); stopSync(); @@ -123,7 +123,7 @@ describe('Test discover initial state sort handling', () => { services: discoverServiceMock, history, }); - await state.replaceUrlAppState({}); + await state.appState.update({}, true); const stopSync = state.startSync(); expect(state.appState.getState().sort).toEqual([['timestamp', 'desc']]); stopSync(); diff --git a/src/plugins/discover/public/application/main/services/discover_state.ts b/src/plugins/discover/public/application/main/services/discover_state.ts index 94c0bbb9d77ee..1286deb191ecf 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.ts @@ -6,14 +6,11 @@ * Side Public License, v 1. */ -import { isEqual } from 'lodash'; import { i18n } from '@kbn/i18n'; import { History } from 'history'; -import { COMPARE_ALL_OPTIONS, compareFilters, Filter } from '@kbn/es-query'; import { createKbnUrlStateStorage, IKbnUrlStateStorage, - ReduxLikeStateContainer, StateContainer, withNotifyOnErrors, } from '@kbn/kibana-utils-plugin/public'; @@ -25,7 +22,6 @@ import { } from '@kbn/data-plugin/public'; import { DataView } from '@kbn/data-views-plugin/public'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; -import { addLog } from '../../../utils/add_log'; import { loadDataView, resolveDataView } from '../utils/resolve_data_view'; import { DataStateContainer, getDataStateContainer } from './discover_data_state_container'; import { DiscoverSearchSessionManager } from './discover_search_session'; @@ -100,30 +96,10 @@ export interface DiscoverStateContainer { * Set app state to with a partial new app state */ setAppState: (newState: Partial) => void; - /** - * Set state in Url using history.replace - */ - replaceUrlAppState: (newState: Partial) => Promise; /** * Sync state to URL, used for testing */ flushToUrl: () => void; - /** - * Reset initial state to the current app state - */ - resetInitialAppState: () => void; - /** - * Return the Appstate before the current app state, useful for diffing changes - */ - getPreviousAppState: () => AppState; - /** - * Returns whether the current app state is different to the initial state - */ - isAppStateDirty: () => boolean; - /** - * Reset AppState by the given savedSearch discarding all changes - */ - resetAppState: (nextSavedSearch: SavedSearch) => void; /** * Pause the auto refresh interval without pushing an entry to history */ @@ -206,10 +182,6 @@ export function getDiscoverStateContainer({ */ const appStateContainer = getDiscoverAppStateContainer({ stateStorage, savedSearch, services }); - const replaceUrlAppState = async (newPartial: AppState = {}) => { - await appStateContainer.update(newPartial, true); - }; - const internalStateContainer = getInternalStateContainer(); const pauseAutoRefreshInterval = async () => { @@ -272,14 +244,8 @@ export function getDiscoverStateContainer({ start(); return stop; }, - setAppState: (newPartial: AppState) => setState(appStateContainer, newPartial), - replaceUrlAppState, - resetInitialAppState: () => appStateContainer.resetInitialState(), - resetAppState: (nextSavedSearch: SavedSearch) => - appStateContainer.resetWithSavedSearch(nextSavedSearch), - getPreviousAppState: () => appStateContainer.getPrevious(), + setAppState: (newPartial: AppState) => appStateContainer.update(newPartial), flushToUrl: () => stateStorage.kbnUrlControls.flush(), - isAppStateDirty: () => appStateContainer.hasChanged(), pauseAutoRefreshInterval, initializeAndSync: () => appStateContainer.initAndSync(savedSearch), actions: { @@ -294,46 +260,6 @@ export function getDiscoverStateContainer({ }; } -/** - * Helper function to merge a given new state with the existing state and to set the given state - * container - */ -export function setState(stateContainer: ReduxLikeStateContainer, newState: AppState) { - addLog('[appstate] setState', { newState }); - const oldState = stateContainer.getState(); - const mergedState = { ...oldState, ...newState }; - if (!isEqualState(oldState, mergedState)) { - stateContainer.set(mergedState); - } -} - -/** - * Helper function to compare 2 different filter states - */ -export function isEqualFilters(filtersA?: Filter[] | Filter, filtersB?: Filter[] | Filter) { - if (!filtersA && !filtersB) { - return true; - } else if (!filtersA || !filtersB) { - return false; - } - return compareFilters(filtersA, filtersB, COMPARE_ALL_OPTIONS); -} - -/** - * Helper function to compare 2 different state, is needed since comparing filters - * works differently - */ -export function isEqualState(stateA: AppState, stateB: AppState) { - if (!stateA && !stateB) { - return true; - } else if (!stateA || !stateB) { - return false; - } - const { filters: stateAFilters = [], ...stateAPartial } = stateA; - const { filters: stateBFilters = [], ...stateBPartial } = stateB; - return isEqual(stateAPartial, stateBPartial) && isEqualFilters(stateAFilters, stateBFilters); -} - export function createSearchSessionRestorationDataProvider(deps: { appStateContainer: StateContainer; data: DataPublicPluginStart; diff --git a/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.ts b/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.ts index 1f98837a8eb25..d5ea19c4316b3 100644 --- a/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.ts +++ b/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.ts @@ -32,7 +32,7 @@ export const useConfirmPersistencePrompt = (stateContainer: DiscoverStateContain updateFiltersReferences(adHocDataView, persistedDataView); stateContainer.actions.removeAdHocDataViewById(adHocDataView.id!); - await stateContainer.replaceUrlAppState({ index: persistedDataView.id }); + await stateContainer.appState.update({ index: persistedDataView.id }, true); const message = i18n.translate('discover.dataViewPersist.message', { defaultMessage: "Saved '{dataViewName}'", diff --git a/src/plugins/discover/public/utils/get_sharing_data.ts b/src/plugins/discover/public/utils/get_sharing_data.ts index 9a32a2431649b..d1c07b0c13c6c 100644 --- a/src/plugins/discover/public/utils/get_sharing_data.ts +++ b/src/plugins/discover/public/utils/get_sharing_data.ts @@ -15,14 +15,16 @@ import type { } from '@kbn/data-plugin/public'; import type { Filter } from '@kbn/es-query'; import type { SavedSearch, SortOrder } from '@kbn/saved-search-plugin/public'; -import { AppState } from '../application/main/services/discover_app_state_container'; +import { + AppState, + isEqualFilters, +} from '../application/main/services/discover_app_state_container'; import { getSortForSearchSource } from './sorting'; import { DOC_HIDE_TIME_COLUMN_SETTING, SEARCH_FIELDS_FROM_SOURCE, SORT_DEFAULT_ORDER_SETTING, } from '../../common'; -import { isEqualFilters } from '../application/main/services/discover_state'; /** * Preparing data to share the current state as link or CSV/Report