[Discover] Implement tab duplication in state management (#224602)

- Closes https://github.com/elastic/kibana/issues/217582 ## Summary This PR implements the tab duplication logic. Previously, app and global states of the original tab were not carried over to the new tab. Now the new tab gets the correct app and global states which are derived from the original tab. ### Checklist - [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 - [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-06-27 18:51:07 -04:00 · 2025-06-23 17:28:27 +02:00 · 2025-06-23 17:28:27 +02:00 · b1d4ec4b3a
commit b1d4ec4b3a
parent 38e781c452
10 changed files with 117 additions and 142 deletions
--- a/src/platform/packages/shared/kbn-unified-tabs/src/components/tabbed_content/tabbed_content.test.tsx
+++ b/src/platform/packages/shared/kbn-unified-tabs/src/components/tabbed_content/tabbed_content.test.tsx
@ -156,7 +156,11 @@ describe('TabbedContent', () => {

    screen.getByTestId('unifiedTabs_tabMenuItem_duplicate').click();

-    const duplicatedTab = { ...NEW_TAB, label: `${firstTab.label} (copy)` };
+    const duplicatedTab = {
+      ...NEW_TAB,
+      label: `${firstTab.label} (copy)`,
+      duplicatedFromId: firstTab.id,
+    };

    await waitFor(() => {
      expect(onChanged).toHaveBeenCalledWith({
--- a/src/platform/packages/shared/kbn-unified-tabs/src/components/tabbed_content/tabbed_content.tsx
+++ b/src/platform/packages/shared/kbn-unified-tabs/src/components/tabbed_content/tabbed_content.tsx
@ -131,6 +131,8 @@ export const TabbedContent: React.FC<TabbedContentProps> = ({
  const onDuplicate = useCallback(
    (item: TabItem) => {
      const newItem = createItem();
+      newItem.duplicatedFromId = item.id;
+
      const copyLabel = i18n.translate('unifiedTabs.copyLabel', { defaultMessage: 'copy' });
      const escapedCopyLabel = escapeRegExp(copyLabel);
      const baseRegex = new RegExp(`\\s*\\(${escapedCopyLabel}\\)( \\d+)?$`);
--- a/src/platform/packages/shared/kbn-unified-tabs/src/types.ts
+++ b/src/platform/packages/shared/kbn-unified-tabs/src/types.ts
@ -13,6 +13,7 @@ import type { CoreStart } from '@kbn/core/public';
 export interface TabItem {
  id: string;
  label: string;
+  duplicatedFromId?: string; // ID of the tab from which this tab was duplicated
 }

 export interface TabsSizeConfig {
--- a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/actions/initialize_session.ts
+++ b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/actions/initialize_session.ts
@ -38,6 +38,7 @@ import { TABS_ENABLED } from '../../../../../constants';
 import { selectTabRuntimeState } from '../runtime_state';
 import type { ConnectedCustomizationService } from '../../../../../customizations';
 import { disconnectTab, clearAllTabs } from './tabs';
+import { selectTab } from '../selectors';

 export interface InitializeSessionParams {
  stateContainer: DiscoverStateContainer;
@ -77,9 +78,15 @@ export const initializeSession: InternalStateThunkActionCreator<

    const discoverSessionLoadTracker =
      services.ebtManager.trackPerformanceEvent('discoverLoadSavedSearch');
+
    const { currentDataView$, stateContainer$, customizationService$, scopedProfilesManager$ } =
      selectTabRuntimeState(runtimeStateManager, tabId);
-    let initialUrlState = defaultUrlState ?? urlStateStorage.get<AppStateUrl>(APP_STATE_URL_KEY);
+    const tabState = selectTab(getState(), tabId);
+
+    let urlState = cleanupUrlState(
+      defaultUrlState ?? urlStateStorage.get<AppStateUrl>(APP_STATE_URL_KEY),
+      services.uiSettings
+    );

    /**
     * New tab initialization with the restored data if available
@ -97,32 +104,26 @@ export const initializeSession: InternalStateThunkActionCreator<
    }

    if (TABS_ENABLED && !wasTabInitialized) {
-      const tabGlobalStateFromLocalStorage =
-        tabsStorageManager.loadTabGlobalStateFromLocalCache(tabId);
+      const tabInitialGlobalState = tabState.initialGlobalState;

-      if (tabGlobalStateFromLocalStorage?.filters) {
-        services.filterManager.setGlobalFilters(cloneDeep(tabGlobalStateFromLocalStorage.filters));
+      if (tabInitialGlobalState?.filters) {
+        services.filterManager.setGlobalFilters(cloneDeep(tabInitialGlobalState.filters));
      }

-      if (tabGlobalStateFromLocalStorage?.timeRange) {
-        services.timefilter.setTime(tabGlobalStateFromLocalStorage.timeRange);
+      if (tabInitialGlobalState?.timeRange) {
+        services.timefilter.setTime(tabInitialGlobalState.timeRange);
      }
-      if (tabGlobalStateFromLocalStorage?.refreshInterval) {
-        services.timefilter.setRefreshInterval(tabGlobalStateFromLocalStorage.refreshInterval);
+      if (tabInitialGlobalState?.refreshInterval) {
+        services.timefilter.setRefreshInterval(tabInitialGlobalState.refreshInterval);
      }

-      const tabAppStateFromLocalStorage = tabsStorageManager.loadTabAppStateFromLocalCache(tabId);
+      const tabInitialAppState = tabState.initialAppState;

-      if (tabAppStateFromLocalStorage) {
-        initialUrlState = tabAppStateFromLocalStorage;
+      if (tabInitialAppState) {
+        urlState = cloneDeep(tabInitialAppState);
      }
    }

-    /**
-     * "No data" checks
-     */
-    const urlState = cleanupUrlState(initialUrlState, services.uiSettings);
-
    const persistedDiscoverSession = discoverSessionId
      ? await services.savedSearch.get(discoverSessionId)
      : undefined;
--- a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/actions/tabs.ts
+++ b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/actions/tabs.ts
@ -70,11 +70,24 @@ export const updateTabs: InternalStateThunkActionCreator<[TabbedContentState], P
    const currentTab = selectTab(currentState, currentState.tabs.unsafeCurrentId);
    const updatedTabs = items.map<TabState>((item) => {
      const existingTab = selectTab(currentState, item.id);
-      return {
+
+      const tab: TabState = {
        ...defaultTabState,
        ...existingTab,
        ...pick(item, 'id', 'label'),
      };
+
+      if (item.duplicatedFromId) {
+        const existingTabToDuplicate = selectTab(currentState, item.duplicatedFromId);
+        tab.initialAppState =
+          selectTabRuntimeAppState(runtimeStateManager, item.duplicatedFromId) ??
+          cloneDeep(existingTabToDuplicate.initialAppState);
+        tab.initialGlobalState =
+          selectTabRuntimeGlobalState(runtimeStateManager, item.duplicatedFromId) ??
+          cloneDeep(existingTabToDuplicate.initialGlobalState);
+      }
+
+      return tab;
    });

    if (selectedItem?.id !== currentTab.id) {
--- a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/index.ts
+++ b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/index.ts
@ -26,7 +26,12 @@ import {
  initializeTabs,
 } from './actions';

-export type { DiscoverInternalState, TabState, InternalStateDataRequestParams } from './types';
+export type {
+  DiscoverInternalState,
+  TabState,
+  TabStateGlobalState,
+  InternalStateDataRequestParams,
+} from './types';

 export { type InternalStateStore, createInternalStateStore } from './internal_state';

--- a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/types.ts
+++ b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/types.ts
@ -13,6 +13,7 @@ import type { DataTableRecord } from '@kbn/discover-utils';
 import type { Filter, TimeRange } from '@kbn/es-query';
 import type { UnifiedHistogramVisContext } from '@kbn/unified-histogram';
 import type { TabItem } from '@kbn/unified-tabs';
+import type { DiscoverAppState } from '../discover_app_state_container';

 export enum LoadingStatus {
  Uninitialized = 'uninitialized',
@ -45,12 +46,19 @@ export interface InternalStateDataRequestParams {
  searchSessionId: string | undefined;
 }

+export interface TabStateGlobalState {
+  timeRange?: TimeRange;
+  refreshInterval?: RefreshInterval;
+  filters?: Filter[];
+}
+
 export interface TabState extends TabItem {
-  lastPersistedGlobalState: {
-    timeRange?: TimeRange;
-    refreshInterval?: RefreshInterval;
-    filters?: Filter[];
-  };
+  // Initial app and global state for the tab (provided before the tab is initialized).
+  initialAppState?: DiscoverAppState;
+  initialGlobalState?: TabStateGlobalState;
+
+  // The following properties are used to manage the tab's state after it has been initialized.
+  lastPersistedGlobalState: TabStateGlobalState;
  dataViewId: string | undefined;
  isDataViewLoading: boolean;
  dataRequestParams: InternalStateDataRequestParams;
--- a/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.test.tsx
+++ b/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.test.tsx
@ -114,6 +114,16 @@ describe('TabsStorageManager', () => {
    ...('closedAt' in tab ? { closedAt: tab.closedAt } : {}),
  });

+  const toRestoredTab = (storedTab: TabState | RecentlyClosedTabState) => ({
+    ...defaultTabState,
+    id: storedTab.id,
+    label: storedTab.label,
+    initialAppState: mockGetAppState(storedTab.id),
+    initialGlobalState: storedTab.lastPersistedGlobalState,
+    lastPersistedGlobalState: storedTab.lastPersistedGlobalState,
+    ...('closedAt' in storedTab ? { closedAt: storedTab.closedAt } : {}),
+  });
+
  it('should persist tabs state to local storage and push to URL', async () => {
    const {
      services: { storage },
@ -145,19 +155,6 @@ describe('TabsStorageManager', () => {
      openTabs: [toStoredTab(mockTab1), toStoredTab(mockTab2)],
      closedTabs: [toStoredTab(mockRecentlyClosedTab)],
    });
-
-    expect(tabsStorageManager.loadTabAppStateFromLocalCache(mockTab1.id)).toEqual(
-      mockGetAppState(mockTab1.id)
-    );
-    expect(tabsStorageManager.loadTabGlobalStateFromLocalCache(mockTab1.id)).toEqual(
-      mockTab1.lastPersistedGlobalState
-    );
-    expect(tabsStorageManager.loadTabAppStateFromLocalCache(mockRecentlyClosedTab.id)).toEqual(
-      mockGetAppState(mockRecentlyClosedTab.id)
-    );
-    expect(tabsStorageManager.loadTabGlobalStateFromLocalCache(mockRecentlyClosedTab.id)).toEqual(
-      mockRecentlyClosedTab.lastPersistedGlobalState
-    );
  });

  it('should load tabs state from local storage and select one of open tabs', () => {
@ -169,12 +166,6 @@ describe('TabsStorageManager', () => {
    jest.spyOn(urlStateStorage, 'get');
    jest.spyOn(storage, 'get');

-    const props: TabsInternalStatePayload = {
-      allTabs: [mockTab1, mockTab2],
-      selectedTabId: 'tab2',
-      recentlyClosedTabs: [mockRecentlyClosedTab],
-    };
-
    storage.set(TABS_LOCAL_STORAGE_KEY, {
      userId: mockUserId,
      spaceId: mockSpaceId,
@ -183,7 +174,7 @@ describe('TabsStorageManager', () => {
    });

    urlStateStorage.set(TABS_STATE_URL_KEY, {
-      tabId: props.selectedTabId,
+      tabId: 'tab2',
    });

    jest.spyOn(urlStateStorage, 'set');
@ -195,7 +186,11 @@ describe('TabsStorageManager', () => {
      defaultTabState,
    });

-    expect(loadedProps).toEqual(props);
+    expect(loadedProps).toEqual({
+      allTabs: [toRestoredTab(mockTab1), toRestoredTab(mockTab2)],
+      selectedTabId: 'tab2',
+      recentlyClosedTabs: [toRestoredTab(mockRecentlyClosedTab)],
+    });
    expect(urlStateStorage.get).toHaveBeenCalledWith(TABS_STATE_URL_KEY);
    expect(storage.get).toHaveBeenCalledWith(TABS_LOCAL_STORAGE_KEY);
    expect(urlStateStorage.set).not.toHaveBeenCalled();
@ -214,18 +209,6 @@ describe('TabsStorageManager', () => {
    const newClosedAt = Date.now() + 1000;
    jest.spyOn(Date, 'now').mockReturnValue(newClosedAt);

-    const props: TabsInternalStatePayload = {
-      allTabs: [omit(mockRecentlyClosedTab, 'closedAt'), omit(mockRecentlyClosedTab2, 'closedAt')],
-      selectedTabId: mockRecentlyClosedTab2.id,
-      recentlyClosedTabs: [
-        { ...mockTab1, closedAt: newClosedAt },
-        { ...mockTab2, closedAt: newClosedAt },
-        mockRecentlyClosedTab3,
-        mockRecentlyClosedTab,
-        mockRecentlyClosedTab2,
-      ],
-    };
-
    storage.set(TABS_LOCAL_STORAGE_KEY, {
      userId: mockUserId,
      spaceId: mockSpaceId,
@ -250,7 +233,20 @@ describe('TabsStorageManager', () => {
      defaultTabState,
    });

-    expect(loadedProps).toEqual(props);
+    expect(loadedProps).toEqual({
+      allTabs: [
+        toRestoredTab(omit(mockRecentlyClosedTab, 'closedAt')),
+        toRestoredTab(omit(mockRecentlyClosedTab2, 'closedAt')),
+      ],
+      selectedTabId: mockRecentlyClosedTab2.id,
+      recentlyClosedTabs: [
+        toRestoredTab({ ...mockTab1, closedAt: newClosedAt }),
+        toRestoredTab({ ...mockTab2, closedAt: newClosedAt }),
+        toRestoredTab(mockRecentlyClosedTab3),
+        toRestoredTab(mockRecentlyClosedTab),
+        toRestoredTab(mockRecentlyClosedTab2),
+      ],
+    });
    expect(urlStateStorage.get).toHaveBeenCalledWith(TABS_STATE_URL_KEY);
    expect(storage.get).toHaveBeenCalledWith(TABS_LOCAL_STORAGE_KEY);
    expect(urlStateStorage.set).not.toHaveBeenCalled();
@ -339,9 +335,9 @@ describe('TabsStorageManager', () => {
    expect(loadedProps).toEqual(
      expect.objectContaining({
        recentlyClosedTabs: [
-          { ...mockTab1, closedAt: newClosedAt },
-          { ...mockTab2, closedAt: newClosedAt },
-          mockRecentlyClosedTab,
+          toRestoredTab({ ...mockTab1, closedAt: newClosedAt }),
+          toRestoredTab({ ...mockTab2, closedAt: newClosedAt }),
+          toRestoredTab(mockRecentlyClosedTab),
        ],
      })
    );
@ -394,13 +390,6 @@ describe('TabsStorageManager', () => {
      ],
      closedTabs: [toStoredTab(mockRecentlyClosedTab)],
    });
-
-    expect(tabsStorageManager.loadTabAppStateFromLocalCache(mockTab1.id)).toEqual(
-      updatedTabState.appState
-    );
-    expect(tabsStorageManager.loadTabGlobalStateFromLocalCache(mockTab1.id)).toEqual(
-      updatedTabState.globalState
-    );
  });

  it('should limit to N recently closed tabs', () => {
--- a/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts
+++ b/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts
@ -55,11 +55,6 @@ export interface TabsUrlState {
  tabId?: string; // syncing the selected tab id with the URL
 }

-type TabsInStorageCache = Map<
-  string,
-  TabStateInLocalStorage | RecentlyClosedTabStateInLocalStorage
->;
-
 export interface TabsStorageManager {
  /**
   * Supports two-way sync of the selected tab id with the URL.
@ -78,8 +73,6 @@ export interface TabsStorageManager {
    spaceId: string;
    defaultTabState: Omit<TabState, keyof TabItem>;
  }) => TabsInternalStatePayload;
-  loadTabAppStateFromLocalCache: (tabId: string) => TabStateInLocalStorage['appState'];
-  loadTabGlobalStateFromLocalCache: (tabId: string) => TabStateInLocalStorage['globalState'];
  getNRecentlyClosedTabs: (
    previousRecentlyClosedTabs: RecentlyClosedTabState[],
    newClosedTabs: TabState[]
@ -97,7 +90,6 @@ export const createTabsStorageManager = ({
 }): TabsStorageManager => {
  const urlStateContainer = createStateContainer<TabsUrlState>({});
  const sessionInfo = { userId: '', spaceId: '' };
-  const tabsInStorageCache: TabsInStorageCache = new Map();

  const startUrlSync: TabsStorageManager['startUrlSync'] = ({
    onChanged, // can be called when selectedTabId changes in URL to trigger app state change if needed
@ -148,17 +140,17 @@ export const createTabsStorageManager = ({
  };

  const toTabStateInStorage = (
-    tabState: Pick<TabState, 'id' | 'label' | 'lastPersistedGlobalState'>,
+    tabState: TabState,
    getAppState: (tabId: string) => DiscoverAppState | undefined
  ): TabStateInLocalStorage => {
    const getAppStateForTabWithoutRuntimeState = (tabId: string) =>
-      getAppState(tabId) || tabsInStorageCache.get(tabId)?.appState;
+      getAppState(tabId) || tabState.initialAppState;

    return {
      id: tabState.id,
      label: tabState.label,
      appState: getAppStateForTabWithoutRuntimeState(tabState.id),
-      globalState: tabState.lastPersistedGlobalState,
+      globalState: tabState.lastPersistedGlobalState || tabState.initialGlobalState,
    };
  };

@ -173,15 +165,29 @@ export const createTabsStorageManager = ({
    };
  };

+  const getDefinedStateOnly = <T>(state: T | undefined): T | undefined => {
+    if (!state || !Object.keys(state).length) {
+      return undefined;
+    }
+    return state;
+  };
+
  const toTabState = (
    tabStateInStorage: TabStateInLocalStorage,
    defaultTabState: Omit<TabState, keyof TabItem>
-  ): TabState => ({
-    ...defaultTabState,
-    ...pick(tabStateInStorage, 'id', 'label'),
-    lastPersistedGlobalState:
-      tabStateInStorage.globalState || defaultTabState.lastPersistedGlobalState,
-  });
+  ): TabState => {
+    const appState = getDefinedStateOnly(tabStateInStorage.appState);
+    const globalState = getDefinedStateOnly(
+      tabStateInStorage.globalState || defaultTabState.lastPersistedGlobalState
+    );
+    return {
+      ...defaultTabState,
+      ...pick(tabStateInStorage, 'id', 'label'),
+      initialAppState: appState,
+      initialGlobalState: globalState,
+      lastPersistedGlobalState: globalState || {},
+    };
+  };

  const toRecentlyClosedTabState = (
    tabStateInStorage: RecentlyClosedTabStateInLocalStorage,
@ -256,22 +262,14 @@ export const createTabsStorageManager = ({
    const openTabs: TabsStateInLocalStorage['openTabs'] = allTabs.map((tab) => {
      const tabStateInStorage = toTabStateInStorage(tab, getAppState);
      keptTabIds[tab.id] = true;
-      tabsInStorageCache.set(tab.id, tabStateInStorage);
      return tabStateInStorage;
    });
    const closedTabs: TabsStateInLocalStorage['closedTabs'] = recentlyClosedTabs.map((tab) => {
      const tabStateInStorage = toRecentlyClosedTabStateInStorage(tab, getAppState);
      keptTabIds[tab.id] = true;
-      tabsInStorageCache.set(tab.id, tabStateInStorage);
      return tabStateInStorage;
    });

-    for (const tabId of tabsInStorageCache.keys()) {
-      if (!keptTabIds[tabId]) {
-        tabsInStorageCache.delete(tabId);
-      }
-    }
-
    const nextTabsInStorage: TabsStateInLocalStorage = {
      userId: sessionInfo.userId,
      spaceId: sessionInfo.spaceId,
@ -296,14 +294,11 @@ export const createTabsStorageManager = ({
      openTabs: storedTabsState.openTabs.map((tab) => {
        if (tab.id === tabId) {
          hasModifications = true;
-          const newTabStateInStorage = {
+          return {
            ...tab,
            appState: tabStatePartial.appState,
            globalState: tabStatePartial.globalState,
          };
-
-          tabsInStorageCache.set(tabId, newTabStateInStorage);
-          return newTabStateInStorage;
        }
        return tab;
      }),
@ -314,36 +309,6 @@ export const createTabsStorageManager = ({
    }
  };

-  const loadTabAppStateFromLocalCache: TabsStorageManager['loadTabAppStateFromLocalCache'] = (
-    tabId
-  ) => {
-    if (!enabled) {
-      return undefined;
-    }
-    const appState = tabsInStorageCache.get(tabId)?.appState;
-
-    if (!appState || !Object.values(appState).filter(Boolean).length) {
-      return undefined;
-    }
-
-    return appState;
-  };
-
-  const loadTabGlobalStateFromLocalCache: TabsStorageManager['loadTabGlobalStateFromLocalCache'] = (
-    tabId
-  ) => {
-    if (!enabled) {
-      return undefined;
-    }
-    const globalState = tabsInStorageCache.get(tabId)?.globalState;
-
-    if (!globalState || !Object.values(globalState).filter(Boolean).length) {
-      return undefined;
-    }
-
-    return globalState;
-  };
-
  const loadLocally: TabsStorageManager['loadLocally'] = ({ userId, spaceId, defaultTabState }) => {
    const selectedTabId = enabled ? getSelectedTabIdFromURL() : undefined;
    let storedTabsState: TabsStateInLocalStorage = enabled
@ -362,13 +327,6 @@ export const createTabsStorageManager = ({
    sessionInfo.userId = userId;
    sessionInfo.spaceId = spaceId;

-    storedTabsState.openTabs.forEach((tab) => {
-      tabsInStorageCache.set(tab.id, tab);
-    });
-    storedTabsState.closedTabs.forEach((tab) => {
-      tabsInStorageCache.set(tab.id, tab);
-    });
-
    const openTabs = storedTabsState.openTabs.map((tab) => toTabState(tab, defaultTabState));
    const closedTabs = storedTabsState.closedTabs.map((tab) =>
      toRecentlyClosedTabState(tab, defaultTabState)
@ -405,11 +363,6 @@ export const createTabsStorageManager = ({
      ...createTabItem([]),
    };

-    tabsInStorageCache.set(
-      defaultTab.id,
-      toTabStateInStorage(defaultTab, () => undefined)
-    );
-
    return {
      allTabs: [defaultTab],
      selectedTabId: defaultTab.id,
@ -422,8 +375,6 @@ export const createTabsStorageManager = ({
    persistLocally,
    updateTabStateLocally,
    loadLocally,
-    loadTabAppStateFromLocalCache,
-    loadTabGlobalStateFromLocalCache,
    getNRecentlyClosedTabs,
  };
 };
--- a/src/platform/plugins/shared/discover/public/application/main/state_management/utils/cleanup_url_state.ts
+++ b/src/platform/plugins/shared/discover/public/application/main/state_management/utils/cleanup_url_state.ts
@ -17,6 +17,7 @@ import { createDataViewDataSource, createEsqlDataSource } from '../../../../../c
 /**
 * Takes care of the given url state, migrates legacy props and cleans up empty props
 * @param appStateFromUrl
+ * @param uiSettings
 */
 export function cleanupUrlState(
  appStateFromUrl: AppStateUrl | null | undefined,