Skip to content
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
efc8993
[Discover Tabs] Extend locator with `tabId`
jughosta Sep 12, 2025
708f08e
[Discover Tabs] Start opening search session links in a new tab
jughosta Sep 12, 2025
42f403d
[Discover Tabs] Handle the case when it was added to an existing save…
jughosta Sep 12, 2025
ea68b9e
Revert "[Discover Tabs] Handle the case when it was added to an exist…
jughosta Sep 14, 2025
449a634
[Discover Tabs] Add a new action
jughosta Sep 14, 2025
da0f12f
[Discover Tabs] Fix the label
jughosta Sep 14, 2025
cfb3ed6
[Discover Tabs] Fix the label
jughosta Sep 14, 2025
a909cc8
[Discover Tabs] Allow to override the label
jughosta Sep 14, 2025
73da781
[Discover Tabs] Update param
jughosta Sep 14, 2025
be766b1
[Discover Tabs] Improve the redux action
jughosta Sep 15, 2025
96a2dc8
[Discover Tabs] Improve the test
jughosta Sep 15, 2025
17f6970
[Discover Tabs] Fix URL history stack
jughosta Sep 15, 2025
00add5c
[Discover Tabs] Add feature flag checks
jughosta Sep 15, 2025
3936eb8
Merge branch 'main' into 230829-locator-new-tab
jughosta Sep 15, 2025
4785f72
Update src/platform/plugins/shared/discover/public/application/main/s…
jughosta Sep 15, 2025
6bdab7e
Update src/platform/plugins/shared/discover/public/application/main/s…
jughosta Sep 15, 2025
bdaf825
[Discover Tabs] Fix the test
jughosta Sep 15, 2025
6256d4d
Merge branch 'main' into 230829-locator-new-tab
jughosta Sep 15, 2025
1f9be33
Merge branch 'main' into 230829-locator-new-tab
jughosta Sep 17, 2025
1d268cc
[Discover] Rename tab param
jughosta Sep 17, 2025
33a1964
[Discover] Update router params
jughosta Sep 17, 2025
27766ef
[Discover] Add a comment
jughosta Sep 17, 2025
1f6aa3d
[Discover] Check only tabs feature flag
jughosta Sep 17, 2025
38d9c42
[Discover] Add a test
jughosta Sep 17, 2025
0dec342
Merge branch 'main' into 230829-locator-new-tab
jughosta Sep 17, 2025
1808017
[Discover] More renaming
jughosta Sep 17, 2025
db8bafe
Merge remote-tracking branch 'origin/230829-locator-new-tab' into 230…
jughosta Sep 17, 2025
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 @@ -75,6 +75,10 @@ export function SearchSessionsMgmtTable({
() => moment.duration(config.management.refreshInterval).asMilliseconds(),
[config.management.refreshInterval]
);
const enableOpeningInNewTab = useMemo(
() => core.featureFlags.getBooleanValue('discover.tabsEnabled', false),
[core.featureFlags]
);

const { pageSize, sorting, onTableChange } = useEuiTablePersist<UISession>({
tableId: 'searchSessionsMgmt',
Expand Down Expand Up @@ -111,7 +115,12 @@ export function SearchSessionsMgmtTable({
try {
const { savedObjects, statuses } = await api.fetchTableData({ appId });
results = savedObjects.map((savedObject) =>
mapToUISession({ savedObject, locators, sessionStatuses: statuses })
mapToUISession({
savedObject,
locators,
sessionStatuses: statuses,
enableOpeningInNewTab,
})
);
} catch (e) {} // eslint-disable-line no-empty

Expand All @@ -125,7 +134,7 @@ export function SearchSessionsMgmtTable({
if (refreshTimeoutRef.current) clearTimeout(refreshTimeoutRef.current);
refreshTimeoutRef.current = window.setTimeout(doRefresh, refreshInterval);
}
}, [api, refreshInterval, locators, appId]);
}, [api, refreshInterval, locators, appId, enableOpeningInNewTab]);

// initial data load
useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@ export const mapToUISession = ({
locators,
sessionStatuses,
actions: filteredActions,
enableOpeningInNewTab,
}: {
savedObject: SearchSessionSavedObject;
locators: LocatorsStart;
sessionStatuses: SearchSessionsFindResponse['statuses'];
actions?: ACTION[];
enableOpeningInNewTab?: boolean;
}): UISession => {
const {
name,
Expand All @@ -59,7 +61,16 @@ export const mapToUISession = ({
if (initialState) delete initialState.searchSessionId;
// derive the URL and add it in
const reloadUrl = getUrlFromState(locators, locatorId, initialState);
const restoreUrl = getUrlFromState(locators, locatorId, restoreState);
const restoreUrl = getUrlFromState(
locators,
locatorId,
enableOpeningInNewTab
? {
...restoreState,
tab: { id: 'new', label: name },
}
: restoreState
);

return {
id: savedObject.id,
Expand Down
24 changes: 24 additions & 0 deletions src/platform/plugins/shared/discover/common/app_locator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,30 @@ describe('Discover url generator', () => {
expect(path).toContain('__test__');
});

test('can specify a tab id', async () => {
const { locator } = await setup();
const { path } = await locator.getLocation({
tab: { id: '__test__' },
});

expect(path).toMatchInlineSnapshot(`"#/?_tab=(tabId:__test__)"`);
});

test('can specify to open in a new tab', async () => {
const { locator } = await setup();
const { path } = await locator.getLocation({
tab: { id: 'new', label: 'My new tab' },
query: {
esql: 'SELECT * FROM test',
},
searchSessionId: '__test__',
});

expect(path).toMatchInlineSnapshot(
`"#/?searchSessionId=__test__&_a=(dataSource:(type:esql),query:(esql:'SELECT%20*%20FROM%20test'))&_tab=(tabId:new,tabLabel:'My%20new%20tab')"`
);
});

test('can specify columns, grid, interval, sort and savedQuery', async () => {
const { locator } = await setup();
const { path } = await locator.getLocation({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { RefreshInterval } from '@kbn/data-plugin/public';
import type { LocatorDefinition, LocatorPublic } from '@kbn/share-plugin/public';
import type { DiscoverGridSettings } from '@kbn/saved-search-plugin/common';
import type { DataViewSpec } from '@kbn/data-views-plugin/common';
import type { VIEW_MODE } from './constants';
import type { VIEW_MODE, NEW_TAB_ID } from './constants';

export const DISCOVER_APP_LOCATOR = 'DISCOVER_APP_LOCATOR';

Expand Down Expand Up @@ -65,6 +65,13 @@ export interface DiscoverAppLocatorParams extends SerializableRecord {
*/
searchSessionId?: string;

/**
* Optionally set Discover tab state.
* Use `new` as value for `id` to indicate that a new tab should be created.
* Once created, the new tab will have a unique id which can be referenced too if necessary.
*/
tab?: { id: typeof NEW_TAB_ID; label?: string } | { id: string };

/**
* Columns displayed in the table
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ import type { setStateToKbnUrl as setStateToKbnUrlCommon } from '@kbn/kibana-uti
import type { DiscoverAppLocatorGetLocation, MainHistoryLocationState } from './app_locator';
import type { DiscoverAppState } from '../public';
import { createDataViewDataSource, createEsqlDataSource } from './data_sources';
import { APP_STATE_URL_KEY } from './constants';
import {
APP_STATE_URL_KEY,
GLOBAL_STATE_URL_KEY,
NEW_TAB_ID,
TABS_STATE_URL_KEY,
} from './constants';

export const appLocatorGetLocationCommon = async (
{
Expand Down Expand Up @@ -45,6 +50,7 @@ export const appLocatorGetLocationCommon = async (
hideAggregatedPreview,
breakdownField,
isAlertResults,
tab,
} = params;
const savedSearchPath = savedSearchId ? `view/${encodeURIComponent(savedSearchId)}` : '';
const appState: Partial<DiscoverAppState> = {};
Expand Down Expand Up @@ -79,13 +85,27 @@ export const appLocatorGetLocationCommon = async (
}

if (Object.keys(queryState).length) {
path = setStateToKbnUrl<GlobalQueryStateFromUrl>('_g', queryState, { useHash }, path);
path = setStateToKbnUrl<GlobalQueryStateFromUrl>(
GLOBAL_STATE_URL_KEY,
queryState,
{ useHash },
path
);
}

if (Object.keys(appState).length) {
path = setStateToKbnUrl(APP_STATE_URL_KEY, appState, { useHash }, path);
}

if (tab?.id) {
path = setStateToKbnUrl(
TABS_STATE_URL_KEY,
{ tabId: tab.id, tabLabel: tab.id === NEW_TAB_ID && 'label' in tab ? tab.label : undefined },
{ useHash },
path
);
}

return {
app: 'discover',
path,
Expand Down
9 changes: 8 additions & 1 deletion src/platform/plugins/shared/discover/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,19 @@ export const getDefaultRowsPerPage = (uiSettings: IUiSettingsClient): number =>
// local storage key for the ES|QL to Dataviews transition modal
export const ESQL_TRANSITION_MODAL_KEY = 'data.textLangTransitionModal';

/**
* The id value used to indicate that a link should open in a new Discover tab.
* It will be used in the `_tab` URL param to indicate that a new tab should be created.
* Once created, the new tab will have a unique id.
*/
export const NEW_TAB_ID = 'new' as const;

/**
* The query param key used to store the Discover app state in the URL
*/
export const APP_STATE_URL_KEY = '_a';
export const GLOBAL_STATE_URL_KEY = '_g';
export const TABS_STATE_URL_KEY = '_t';
export const TABS_STATE_URL_KEY = '_tab'; // `_t` is already used by Kibana for time, so we use `_tab` here

/**
* Product feature IDs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ export const getShareAppMenuItem = ({
timeRange,
refreshInterval,
};

// TODO: for a persisted saved search, add the current tab ID to the params
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably worth creating an issue for this one if we don't do it in this PR, since we should do it for 9.2. Could be a good smaller task for one of the newer team members.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #235320

const relativeUrl = locator.getRedirectUrl(params);

// This logic is duplicated from `relativeToAbsolute` (for bundle size reasons). Ultimately, this should be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const useStateManagers = ({
TABS_ENABLED_FEATURE_FLAG_KEY,
false
);
// syncing with the _t part URL
// syncing with the _tab part URL
const [tabsStorageManager] = useState(() =>
createTabsStorageManager({
urlStateStorage,
Expand All @@ -63,13 +63,13 @@ export const useStateManagers = ({

useEffect(() => {
const stopUrlSync = tabsStorageManager.startUrlSync({
// if `_t` in URL changes (for example via browser history), try to restore the previous state
// if `_tab` in URL changes (for example via browser history), try to restore the previous state
onChanged: (urlState) => {
const { tabId: restoreTabId } = urlState;
if (restoreTabId) {
internalState.dispatch(internalStateActions.restoreTab({ restoreTabId }));
} else {
// if tabId is not present in `_t`, clear all tabs
// if tabId is not present in `_tab`, clear all tabs
internalState.dispatch(internalStateActions.clearAllTabs());
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ import {
selectInitialUnifiedHistogramLayoutPropsMap,
selectTabRuntimeInternalState,
} from '../runtime_state';
import { APP_STATE_URL_KEY, GLOBAL_STATE_URL_KEY } from '../../../../../../common/constants';
import {
APP_STATE_URL_KEY,
GLOBAL_STATE_URL_KEY,
NEW_TAB_ID,
} from '../../../../../../common/constants';
import type { DiscoverAppState } from '../../discover_app_state_container';
import { createInternalStateAsyncThunk, createTabItem } from '../utils';
import { setBreadcrumbs } from '../../../../../utils/breadcrumbs';
Expand Down Expand Up @@ -113,6 +117,20 @@ export const updateTabs: InternalStateThunkActionCreator<[TabbedContentState], P
};

if (!existingTab) {
// the following assignments for initialAppState, globalState, and dataRequestParams are for supporting `openInNewTab` action
tab.initialAppState =
'initialAppState' in item
? cloneDeep(item.initialAppState as TabState['initialAppState'])
: tab.initialAppState;
tab.globalState =
'globalState' in item
? cloneDeep(item.globalState as TabState['globalState'])
: tab.globalState;
tab.dataRequestParams =
'dataRequestParams' in item
? (item.dataRequestParams as TabState['dataRequestParams'])
: tab.dataRequestParams;

if (item.duplicatedFromId) {
// the new tab was created by duplicating an existing tab
const existingTabToDuplicateFrom = selectTab(currentState, item.duplicatedFromId);
Expand Down Expand Up @@ -288,7 +306,9 @@ export const restoreTab: InternalStateThunkActionCreator<[{ restoreTabId: string
(dispatch, getState) => {
const currentState = getState();

if (restoreTabId === currentState.tabs.unsafeCurrentId) {
// Restoring the 'new' tab ID is a no-op because it represents a placeholder for creating new tabs,
// not an actual tab that can be restored.
if (restoreTabId === currentState.tabs.unsafeCurrentId || restoreTabId === NEW_TAB_ID) {
return;
}

Expand Down Expand Up @@ -318,6 +338,46 @@ export const restoreTab: InternalStateThunkActionCreator<[{ restoreTabId: string
);
};

export const openInNewTab: InternalStateThunkActionCreator<
[
{
tabLabel?: string;
appState?: TabState['initialAppState'];
globalState?: TabState['globalState'];
searchSessionId?: string;
}
]
> =
({ tabLabel, appState, globalState, searchSessionId }) =>
(dispatch, getState) => {
const initialAppState = appState ? cloneDeep(appState) : undefined;
const initialGlobalState = globalState ? cloneDeep(globalState) : {};
const currentState = getState();
const currentTabs = selectAllTabs(currentState);

const newDefaultTab: TabState = {
...DEFAULT_TAB_STATE,
...createTabItem(currentTabs),
initialAppState,
globalState: initialGlobalState,
};

if (tabLabel) {
newDefaultTab.label = tabLabel;
}

if (searchSessionId) {
newDefaultTab.dataRequestParams = {
...newDefaultTab.dataRequestParams,
searchSessionId,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would still need additional handling to sync it to the search session manager.

};
}

return dispatch(
updateTabs({ items: [...currentTabs, newDefaultTab], selectedItem: newDefaultTab })
);
};

export const disconnectTab: InternalStateThunkActionCreator<[TabActionPayload]> =
({ tabId }) =>
(_, __, { runtimeStateManager }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
updateTabs,
disconnectTab,
restoreTab,
openInNewTab,
clearAllTabs,
initializeTabs,
saveDiscoverSession,
Expand Down Expand Up @@ -56,6 +57,7 @@ export const internalStateActions = {
initializeSingleTab,
syncLocallyPersistedTabState,
restoreTab,
openInNewTab,
clearAllTabs,
initializeTabs,
saveDiscoverSession,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { createKbnUrlStateStorage } from '@kbn/kibana-utils-plugin/public';
import { createTabsStorageManager } from '../tabs_storage_manager';

describe('InternalStateStore', () => {
it('should set data view', async () => {
const createTestStore = async () => {
const services = createDiscoverServicesMock();
const urlStateStorage = createKbnUrlStateStorage();
const runtimeStateManager = createRuntimeStateManager();
Expand All @@ -36,6 +36,12 @@ describe('InternalStateStore', () => {
tabsStorageManager,
});
await store.dispatch(internalStateActions.initializeTabs({ discoverSessionId: undefined }));

return { store, runtimeStateManager };
};

it('should set data view', async () => {
const { store, runtimeStateManager } = await createTestStore();
const tabId = store.getState().tabs.unsafeCurrentId;
expect(
selectTabRuntimeState(runtimeStateManager, tabId).currentDataView$.value
Expand All @@ -45,4 +51,37 @@ describe('InternalStateStore', () => {
dataViewMock
);
});

it('should append a new tab to the tabs list', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding a unit test here! Would have liked to add more of these initially if I wasn't rushing through so much of it early on 🙂

const { store } = await createTestStore();
const initialTabId = store.getState().tabs.unsafeCurrentId;
expect(store.getState().tabs.allIds).toHaveLength(1);
expect(store.getState().tabs.unsafeCurrentId).toBe(initialTabId);
const params = {
tabLabel: 'New tab',
searchSessionId: 'session_123',
appState: {
query: { query: 'test this', language: 'kuery' },
},
globalState: {
timeRange: {
from: '2024-01-01T00:00:00.000Z',
to: '2024-01-02T00:00:00.000Z',
},
},
};
await store.dispatch(internalStateActions.openInNewTab(params));
const tabsState = store.getState().tabs;
expect(tabsState.allIds).toHaveLength(2);
expect(tabsState.unsafeCurrentId).not.toBe(initialTabId);
expect(tabsState.unsafeCurrentId).toBe(tabsState.allIds[1]);
expect(tabsState.byId[tabsState.unsafeCurrentId].label).toBe(params.tabLabel);
expect(tabsState.byId[tabsState.unsafeCurrentId].initialAppState).toEqual(params.appState);
expect(tabsState.byId[tabsState.unsafeCurrentId].globalState).toEqual(params.globalState);
expect(tabsState.byId[tabsState.unsafeCurrentId].dataRequestParams).toEqual({
searchSessionId: params.searchSessionId,
timeRangeAbsolute: undefined,
timeRangeRelative: undefined,
});
});
});
Loading