diff --git a/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx b/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx index bbe8a65ec5b1a..72d313d5a1888 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx @@ -181,12 +181,15 @@ export function DiscoverLayout({ const contentCentered = resultState === 'uninitialized' || resultState === 'none'; const onDataViewCreated = useCallback( (nextDataView: DataView) => { + if (!nextDataView.isPersisted()) { + stateContainer.actions.appendAdHocDataView(nextDataView); + } if (nextDataView.id) { onChangeDataView(nextDataView.id); } savedSearchRefetch$.next('reset'); }, - [onChangeDataView, savedSearchRefetch$] + [onChangeDataView, savedSearchRefetch$, stateContainer] ); const savedSearchTitle = useRef(null); diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx index 9356a1d51e4df..67199ec191782 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx @@ -128,13 +128,16 @@ export const DiscoverTopNav = ({ const createNewDataView = useCallback(() => { closeDataViewEditor.current = dataViewEditor.openEditor({ onSave: async (dataViewToSave) => { + if (!dataViewToSave.isPersisted()) { + stateContainer.actions.appendAdHocDataView(dataViewToSave); + } if (dataViewToSave.id) { onChangeDataView(dataViewToSave.id); } }, allowAdHocDataView: true, }); - }, [dataViewEditor, onChangeDataView]); + }, [dataViewEditor, onChangeDataView, stateContainer.actions]); const onCreateDefaultAdHocDataView = useCallback( async (pattern: string) => { @@ -144,9 +147,11 @@ export const DiscoverTopNav = ({ if (newDataView.fields.getByName('@timestamp')?.type === 'date') { newDataView.timeFieldName = '@timestamp'; } + + stateContainer.actions.appendAdHocDataView(newDataView); onChangeDataView(newDataView.id!); }, - [dataViews, onChangeDataView] + [dataViews, onChangeDataView, stateContainer.actions] ); const topNavMenu = useMemo( diff --git a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx index c561cb13211a5..b015085c8cf2c 100644 --- a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx +++ b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx @@ -130,9 +130,7 @@ describe('useAdHocDataViews', () => { updatedDataView = await hook.result.current.updateAdHocDataViewId(mockDataView); }); - expect(dataViewsCreateMock).toHaveBeenCalledWith({ - id: undefined, - }); + expect(mockDiscoverServices.dataViews.clearInstanceCache).toHaveBeenCalledWith(mockDataView.id); expect(updatedDataView!.id).toEqual('updated-mock-id'); }); }); 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 8f4cfce6f903e..63004bdee47e6 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 @@ -7,19 +7,15 @@ */ import { useCallback } from 'react'; +import uuid from 'uuid/v4'; import type { DataView, DataViewsContract } from '@kbn/data-views-plugin/public'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; -import { - UPDATE_FILTER_REFERENCES_ACTION, - UPDATE_FILTER_REFERENCES_TRIGGER, -} from '@kbn/unified-search-plugin/public'; -import { ActionExecutionContext } from '@kbn/ui-actions-plugin/public'; import type { FilterManager } from '@kbn/data-plugin/public'; import type { ToastsStart } from '@kbn/core-notifications-browser'; -import { getUiActions } from '../../../kibana_services'; import { useConfirmPersistencePrompt } from '../../../hooks/use_confirm_persistence_prompt'; import { DiscoverStateContainer } from '../services/discover_state'; import { useFiltersValidation } from './use_filters_validation'; +import { updateFiltersReferences } from '../utils/update_filter_references'; export const useAdHocDataViews = ({ savedSearch, @@ -46,50 +42,40 @@ export const useAdHocDataViews = ({ * This is to prevent duplicate ids messing with our system */ const updateAdHocDataViewId = useCallback( - async (dataViewToUpdate: DataView) => { - const adHocDataViewList = stateContainer.internalState.getState().dataViewsAdHoc; - const newDataView = await dataViews.create({ ...dataViewToUpdate.toSpec(), id: undefined }); + async (prevDataView: DataView) => { + const newDataView = await dataViews.create({ ...prevDataView.toSpec(), id: uuid() }); + dataViews.clearInstanceCache(prevDataView.id); - dataViews.clearInstanceCache(dataViewToUpdate.id); - const nextAdHocDataViewList = adHocDataViewList.filter( - (d: DataView) => d.id && dataViewToUpdate.id && d.id !== dataViewToUpdate.id - ); - stateContainer.internalState.transitions.setDataViewsAdHoc(nextAdHocDataViewList); + updateFiltersReferences(prevDataView, newDataView); - const uiActions = await getUiActions(); - const trigger = uiActions.getTrigger(UPDATE_FILTER_REFERENCES_TRIGGER); - const action = uiActions.getAction(UPDATE_FILTER_REFERENCES_ACTION); - - // execute shouldn't be awaited, this is important for pending history push cancellation - action?.execute({ - trigger, - fromDataView: dataViewToUpdate.id, - toDataView: newDataView.id, - usedDataViews: [], - } as ActionExecutionContext); - stateContainer.actions.setDataView(newDataView); + stateContainer.actions.replaceAdHocDataViewWithId(prevDataView.id!, newDataView); await stateContainer.replaceUrlAppState({ index: newDataView.id }); + setUrlTracking(newDataView); return newDataView; }, [dataViews, setUrlTracking, stateContainer] ); - const { openConfirmSavePrompt, updateSavedSearch } = - useConfirmPersistencePrompt(updateAdHocDataViewId); + const { openConfirmSavePrompt, updateSavedSearch } = useConfirmPersistencePrompt(stateContainer); const persistDataView = useCallback(async () => { const currentDataView = savedSearch.searchSource.getField('index')!; - if (currentDataView && !currentDataView.isPersisted()) { - const createdDataView = await openConfirmSavePrompt(currentDataView); + if (!currentDataView || currentDataView.isPersisted()) { + return currentDataView; + } + const createdDataView = await openConfirmSavePrompt(currentDataView); + if (!createdDataView) { + return currentDataView; // persistance cancelled + } + + if (savedSearch.id) { // update saved search with saved data view - if (createdDataView && savedSearch.id) { - const currentState = stateContainer.appState.getState(); - await updateSavedSearch({ savedSearch, dataView: createdDataView, state: currentState }); - } - return createdDataView; + const currentState = stateContainer.appState.getState(); + await updateSavedSearch({ savedSearch, dataView: createdDataView, state: currentState }); } - return currentDataView; + + return createdDataView; }, [stateContainer, openConfirmSavePrompt, savedSearch, updateSavedSearch]); return { persistDataView, updateAdHocDataViewId }; 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 49a7d7aa3254c..37935167d9d8b 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 @@ -63,7 +63,8 @@ export function useDiscoverState({ }); container.actions.setDataView(dataView); return container; - }, [history, savedSearch, services, dataView]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [history, savedSearch, services]); const { setUrlTracking } = useUrlTracking(savedSearch, dataView); diff --git a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts index 6e42b1792fefa..6ac294cbccabd 100644 --- a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts @@ -20,7 +20,11 @@ export interface InternalState { interface InternalStateTransitions { setDataView: (state: InternalState) => (dataView: DataView) => InternalState; - setDataViewsAdHoc: (state: InternalState) => (dataViews: DataView[]) => InternalState; + appendAdHocDataView: (state: InternalState) => (dataView: DataView) => InternalState; + removeAdHocDataViewById: (state: InternalState) => (id: string) => InternalState; + replaceAdHocDataViewWithId: ( + state: InternalState + ) => (id: string, dataView: DataView) => InternalState; } export type InternalStateContainer = ReduxLikeStateContainer< @@ -42,10 +46,21 @@ export function getInternalStateContainer() { ...prevState, dataView: nextDataView, }), - setDataViewsAdHoc: (prevState: InternalState) => (dataViewsAdHoc: DataView[]) => ({ + appendAdHocDataView: (prevState: InternalState) => (dataViewAdHoc: DataView) => ({ ...prevState, - dataViewsAdHoc, + dataViewsAdHoc: prevState.dataViewsAdHoc.concat(dataViewAdHoc), }), + removeAdHocDataViewById: (prevState: InternalState) => (id: string) => ({ + ...prevState, + dataViewsAdHoc: prevState.dataViewsAdHoc.filter((dataView) => dataView.id !== id), + }), + replaceAdHocDataViewWithId: + (prevState: InternalState) => (prevId: string, newDataView: DataView) => ({ + ...prevState, + dataViewsAdHoc: prevState.dataViewsAdHoc.map((dataView) => + dataView.id === prevId ? newDataView : dataView + ), + }), }, {}, { freeze: (state) => state } 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 27bde15953e20..558f18c0cc1e7 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.ts @@ -128,6 +128,12 @@ export interface DiscoverStateContainer { * Set the currently selected data view */ setDataView: (dataView: DataView) => void; + /** + * Manage AdHoc data views + */ + appendAdHocDataView: (dataView: DataView) => void; + removeAdHocDataViewById: (id: string) => void; + replaceAdHocDataViewWithId: (id: string, dataView: DataView) => void; }; } @@ -210,16 +216,14 @@ export function getDiscoverStateContainer({ } }; - const setDataView = (dataView: DataView) => { + const setDataView = (dataView: DataView) => internalStateContainer.transitions.setDataView(dataView); - if (!dataView.isPersisted()) { - const adHocDataViewList = internalStateContainer.getState().dataViewsAdHoc; - const existing = adHocDataViewList.find((prevDataView) => prevDataView.id === dataView.id); - if (!existing) { - internalStateContainer.transitions.setDataViewsAdHoc([...adHocDataViewList, dataView]); - } - } - }; + const appendAdHocDataView = (dataView: DataView) => + internalStateContainer.transitions.appendAdHocDataView(dataView); + const replaceAdHocDataViewWithId = (id: string, dataView: DataView) => + internalStateContainer.transitions.replaceAdHocDataViewWithId(id, dataView); + const removeAdHocDataViewById = (id: string) => + internalStateContainer.transitions.removeAdHocDataViewById(id); return { kbnUrlStateStorage: stateStorage, @@ -302,6 +306,9 @@ export function getDiscoverStateContainer({ }, actions: { setDataView, + appendAdHocDataView, + replaceAdHocDataViewWithId, + removeAdHocDataViewById, }, }; } diff --git a/src/plugins/discover/public/application/main/utils/update_filter_references.ts b/src/plugins/discover/public/application/main/utils/update_filter_references.ts new file mode 100644 index 0000000000000..8ab92d37a76f5 --- /dev/null +++ b/src/plugins/discover/public/application/main/utils/update_filter_references.ts @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { + UPDATE_FILTER_REFERENCES_ACTION, + UPDATE_FILTER_REFERENCES_TRIGGER, +} from '@kbn/unified-search-plugin/public'; +import { ActionExecutionContext } from '@kbn/ui-actions-plugin/public'; +import type { DataView } from '@kbn/data-views-plugin/public'; +import { getUiActions } from '../../../kibana_services'; + +export const updateFiltersReferences = (prevDataView: DataView, nextDataView: DataView) => { + const uiActions = getUiActions(); + const trigger = uiActions.getTrigger(UPDATE_FILTER_REFERENCES_TRIGGER); + const action = uiActions.getAction(UPDATE_FILTER_REFERENCES_ACTION); + action?.execute({ + trigger, + fromDataView: prevDataView.id, + toDataView: nextDataView.id, + usedDataViews: [], + } as ActionExecutionContext); +}; diff --git a/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.test.tsx b/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.test.tsx index 33e2435644305..127d0b90d71c9 100644 --- a/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.test.tsx +++ b/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.test.tsx @@ -10,8 +10,11 @@ import React from 'react'; import type { DataView } from '@kbn/data-views-plugin/public'; import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public'; import { renderHook } from '@testing-library/react-hooks'; +import type { UiActionsStart } from '@kbn/ui-actions-plugin/public'; import { discoverServiceMock as mockDiscoverServices } from '../__mocks__/services'; import { useConfirmPersistencePrompt } from './use_confirm_persistence_prompt'; +import { getDiscoverStateMock } from '../__mocks__/discover_state.mock'; +import { setUiActions } from '../kibana_services'; jest.mock('./show_confirm_panel', () => { return { @@ -28,11 +31,16 @@ const mockDataView = { toSpec: () => ({}), } as DataView; +setUiActions({ + getTrigger: jest.fn(() => {}), + getAction: jest.fn(() => ({ execute: jest.fn() })), +} as unknown as UiActionsStart); + describe('useConfirmPersistencePrompt', () => { it('should save data view correctly', async () => { mockDiscoverServices.dataViews.createAndSave = jest.fn().mockResolvedValue(mockDataView); const hook = renderHook( - (d: DataView) => useConfirmPersistencePrompt(() => Promise.resolve(mockDataView)), + (d: DataView) => useConfirmPersistencePrompt(getDiscoverStateMock({})), { initialProps: mockDataView, wrapper: ({ children }) => ( @@ -52,7 +60,7 @@ describe('useConfirmPersistencePrompt', () => { .fn() .mockRejectedValue(new Error('failed to save')); const hook = renderHook( - (d: DataView) => useConfirmPersistencePrompt(() => Promise.resolve(mockDataView)), + (d: DataView) => useConfirmPersistencePrompt(getDiscoverStateMock({})), { initialProps: mockDataView, wrapper: ({ children }) => ( 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 3884bfd085a48..5332ffac6e0be 100644 --- a/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.ts +++ b/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.ts @@ -7,31 +7,39 @@ */ import { useCallback } from 'react'; +import uuid from 'uuid/v4'; import { i18n } from '@kbn/i18n'; import type { DataView } from '@kbn/data-views-plugin/public'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; import { useDiscoverServices } from './use_discover_services'; import { showConfirmPanel } from './show_confirm_panel'; import { persistSavedSearch } from '../application/main/utils/persist_saved_search'; +import { DiscoverStateContainer } from '../application/main/services/discover_state'; +import { updateFiltersReferences } from '../application/main/utils/update_filter_references'; -export const useConfirmPersistencePrompt = ( - updateAdHocDataViewId: (dataView: DataView) => Promise -) => { +export const useConfirmPersistencePrompt = (stateContainer: DiscoverStateContainer) => { const services = useDiscoverServices(); - const persistDataView: (dataView: DataView) => Promise = useCallback( - async (dataView) => { + const persistDataView: (adHocDataView: DataView) => Promise = useCallback( + async (adHocDataView) => { try { - const updatedDataView = await updateAdHocDataViewId(dataView); + const persistedDataView = await services.dataViews.createAndSave({ + ...adHocDataView.toSpec(), + id: uuid(), + }); + services.dataViews.clearInstanceCache(adHocDataView.id); + + updateFiltersReferences(adHocDataView, persistedDataView); - const response = await services.dataViews.createAndSave(updatedDataView.toSpec()); + stateContainer.actions.removeAdHocDataViewById(adHocDataView.id!); + await stateContainer.replaceUrlAppState({ index: persistedDataView.id }); const message = i18n.translate('discover.dataViewPersist.message', { defaultMessage: "Saved '{dataViewName}'", - values: { dataViewName: response.getName() }, + values: { dataViewName: persistedDataView.getName() }, }); services.toastNotifications.addSuccess(message); - return response; + return persistedDataView; } catch (error) { services.toastNotifications.addDanger({ title: i18n.translate('discover.dataViewPersistError.title', { @@ -42,7 +50,7 @@ export const useConfirmPersistencePrompt = ( throw new Error(error); } }, - [services.dataViews, services.toastNotifications, updateAdHocDataViewId] + [services.dataViews, services.toastNotifications, stateContainer] ); const openConfirmSavePrompt: (dataView: DataView) => Promise = useCallback( @@ -61,7 +69,7 @@ export const useConfirmPersistencePrompt = ( ); const onUpdateSuccess = useCallback( - (id: string, savedSearch: SavedSearch) => { + (savedSearch: SavedSearch) => { services.toastNotifications.addSuccess({ title: i18n.translate('discover.notifications.updateSavedSearchTitle', { defaultMessage: `Search '{savedSearchTitle}' updated with saved data view`, @@ -94,7 +102,7 @@ export const useConfirmPersistencePrompt = ( ({ savedSearch, dataView, state }) => { return persistSavedSearch(savedSearch, { dataView, - onSuccess: (id) => onUpdateSuccess(id, savedSearch), + onSuccess: () => onUpdateSuccess(savedSearch), onError: (error) => onUpdateError(error, savedSearch), state, saveOptions: {},