Skip to content

Commit 8b2d988

Browse files
jughostaCopilot
authored andcommitted
[Discover Tabs] Extend the discover locator to allow opening in a new tab (#234912)
- Closes #230829 ## Summary This PR extends the Discover locator with a new param `tabId` (and `tabLabel`). Passing `new` value to it will signal to open the search in a new Discover tab. | Was a persisted Discover Session (A) open when a search session was triggered? | Was user previously viewing a persisted Discover session (B) in Discover | Outcome after opening a search session link from Stack Management | | ------------- | ------------- | ------------- | | no | no | non-persisted Discover session | | no | yes | non-persisted Discover session (previous tabs cleared) | | yes | no | the persisted Discover session A (previous tabs cleared) | | yes | yes (A discover session Id is the same as B) | the persisted Discover session A | | yes | yes (A discover session Id is different from B) | the persisted Discover session A (previous tabs cleared) | Let me know if it's too complex or you have ideas on how to append to a persisted session tabs without reseting previous tabs. The main problem comes from the fact that it also depends from where you saved a search session: it might include the notion of `savedSearchId` and it might not => this result in getting either `/view/<saved search id>?searchSessionId=<id>` route or `/?searchSessionId=<id>`. And Discover reacts to saved search id value in URL. From the other side tabs gets cleared if ids don't match https://github.com/elastic/kibana/blob/17f697055f699438ca84eae3c0230a54d4ad8170/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts#L393-L396 (non-persisted Discover session means no id). Additionally, this PR adds `openInNewTab` redux action. I think we should use it from Background Search flyout as it would append to tabs without the mentioned problems. ### 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) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. --------- Co-authored-by: Copilot <[email protected]>
1 parent 6469835 commit 8b2d988

File tree

13 files changed

+308
-42
lines changed

13 files changed

+308
-42
lines changed

src/platform/plugins/shared/data/public/search/session/sessions_mgmt/components/table/table.tsx

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ export function SearchSessionsMgmtTable({
7575
() => moment.duration(config.management.refreshInterval).asMilliseconds(),
7676
[config.management.refreshInterval]
7777
);
78+
const enableOpeningInNewTab = useMemo(
79+
() => core.featureFlags.getBooleanValue('discover.tabsEnabled', false),
80+
[core.featureFlags]
81+
);
7882

7983
const { pageSize, sorting, onTableChange } = useEuiTablePersist<UISession>({
8084
tableId: 'searchSessionsMgmt',
@@ -111,7 +115,12 @@ export function SearchSessionsMgmtTable({
111115
try {
112116
const { savedObjects, statuses } = await api.fetchTableData({ appId });
113117
results = savedObjects.map((savedObject) =>
114-
mapToUISession({ savedObject, locators, sessionStatuses: statuses })
118+
mapToUISession({
119+
savedObject,
120+
locators,
121+
sessionStatuses: statuses,
122+
enableOpeningInNewTab,
123+
})
115124
);
116125
} catch (e) {} // eslint-disable-line no-empty
117126

@@ -125,7 +134,7 @@ export function SearchSessionsMgmtTable({
125134
if (refreshTimeoutRef.current) clearTimeout(refreshTimeoutRef.current);
126135
refreshTimeoutRef.current = window.setTimeout(doRefresh, refreshInterval);
127136
}
128-
}, [api, refreshInterval, locators, appId]);
137+
}, [api, refreshInterval, locators, appId, enableOpeningInNewTab]);
129138

130139
// initial data load
131140
useEffect(() => {

src/platform/plugins/shared/data/public/search/session/sessions_mgmt/components/table/utils/map_to_ui_session.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,13 @@ export const mapToUISession = ({
3030
locators,
3131
sessionStatuses,
3232
actions: filteredActions,
33+
enableOpeningInNewTab,
3334
}: {
3435
savedObject: SearchSessionSavedObject;
3536
locators: LocatorsStart;
3637
sessionStatuses: SearchSessionsFindResponse['statuses'];
3738
actions?: ACTION[];
39+
enableOpeningInNewTab?: boolean;
3840
}): UISession => {
3941
const {
4042
name,
@@ -59,7 +61,16 @@ export const mapToUISession = ({
5961
if (initialState) delete initialState.searchSessionId;
6062
// derive the URL and add it in
6163
const reloadUrl = getUrlFromState(locators, locatorId, initialState);
62-
const restoreUrl = getUrlFromState(locators, locatorId, restoreState);
64+
const restoreUrl = getUrlFromState(
65+
locators,
66+
locatorId,
67+
enableOpeningInNewTab
68+
? {
69+
...restoreState,
70+
tab: { id: 'new', label: name },
71+
}
72+
: restoreState
73+
);
6374

6475
return {
6576
id: savedObject.id,

src/platform/plugins/shared/discover/common/app_locator.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,30 @@ describe('Discover url generator', () => {
236236
expect(path).toContain('__test__');
237237
});
238238

239+
test('can specify a tab id', async () => {
240+
const { locator } = await setup();
241+
const { path } = await locator.getLocation({
242+
tab: { id: '__test__' },
243+
});
244+
245+
expect(path).toMatchInlineSnapshot(`"#/?_tab=(tabId:__test__)"`);
246+
});
247+
248+
test('can specify to open in a new tab', async () => {
249+
const { locator } = await setup();
250+
const { path } = await locator.getLocation({
251+
tab: { id: 'new', label: 'My new tab' },
252+
query: {
253+
esql: 'SELECT * FROM test',
254+
},
255+
searchSessionId: '__test__',
256+
});
257+
258+
expect(path).toMatchInlineSnapshot(
259+
`"#/?searchSessionId=__test__&_a=(dataSource:(type:esql),query:(esql:'SELECT%20*%20FROM%20test'))&_tab=(tabId:new,tabLabel:'My%20new%20tab')"`
260+
);
261+
});
262+
239263
test('can specify columns, grid, interval, sort and savedQuery', async () => {
240264
const { locator } = await setup();
241265
const { path } = await locator.getLocation({

src/platform/plugins/shared/discover/common/app_locator.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import type { RefreshInterval } from '@kbn/data-plugin/public';
1313
import type { LocatorDefinition, LocatorPublic } from '@kbn/share-plugin/public';
1414
import type { DiscoverGridSettings } from '@kbn/saved-search-plugin/common';
1515
import type { DataViewSpec } from '@kbn/data-views-plugin/common';
16-
import type { VIEW_MODE } from './constants';
16+
import type { VIEW_MODE, NEW_TAB_ID } from './constants';
1717

1818
export const DISCOVER_APP_LOCATOR = 'DISCOVER_APP_LOCATOR';
1919

@@ -65,6 +65,13 @@ export interface DiscoverAppLocatorParams extends SerializableRecord {
6565
*/
6666
searchSessionId?: string;
6767

68+
/**
69+
* Optionally set Discover tab state.
70+
* Use `new` as value for `id` to indicate that a new tab should be created.
71+
* Once created, the new tab will have a unique id which can be referenced too if necessary.
72+
*/
73+
tab?: { id: typeof NEW_TAB_ID; label?: string } | { id: string };
74+
6875
/**
6976
* Columns displayed in the table
7077
*/

src/platform/plugins/shared/discover/common/app_locator_get_location.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,12 @@ import type { setStateToKbnUrl as setStateToKbnUrlCommon } from '@kbn/kibana-uti
1313
import type { DiscoverAppLocatorGetLocation, MainHistoryLocationState } from './app_locator';
1414
import type { DiscoverAppState } from '../public';
1515
import { createDataViewDataSource, createEsqlDataSource } from './data_sources';
16-
import { APP_STATE_URL_KEY } from './constants';
16+
import {
17+
APP_STATE_URL_KEY,
18+
GLOBAL_STATE_URL_KEY,
19+
NEW_TAB_ID,
20+
TAB_STATE_URL_KEY,
21+
} from './constants';
1722

1823
export const appLocatorGetLocationCommon = async (
1924
{
@@ -45,6 +50,7 @@ export const appLocatorGetLocationCommon = async (
4550
hideAggregatedPreview,
4651
breakdownField,
4752
isAlertResults,
53+
tab,
4854
} = params;
4955
const savedSearchPath = savedSearchId ? `view/${encodeURIComponent(savedSearchId)}` : '';
5056
const appState: Partial<DiscoverAppState> = {};
@@ -79,13 +85,27 @@ export const appLocatorGetLocationCommon = async (
7985
}
8086

8187
if (Object.keys(queryState).length) {
82-
path = setStateToKbnUrl<GlobalQueryStateFromUrl>('_g', queryState, { useHash }, path);
88+
path = setStateToKbnUrl<GlobalQueryStateFromUrl>(
89+
GLOBAL_STATE_URL_KEY,
90+
queryState,
91+
{ useHash },
92+
path
93+
);
8394
}
8495

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

100+
if (tab?.id) {
101+
path = setStateToKbnUrl(
102+
TAB_STATE_URL_KEY,
103+
{ tabId: tab.id, tabLabel: tab.id === NEW_TAB_ID && 'label' in tab ? tab.label : undefined },
104+
{ useHash },
105+
path
106+
);
107+
}
108+
89109
return {
90110
app: 'discover',
91111
path,

src/platform/plugins/shared/discover/common/constants.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,19 @@ export const getDefaultRowsPerPage = (uiSettings: IUiSettingsClient): number =>
2626
// local storage key for the ES|QL to Dataviews transition modal
2727
export const ESQL_TRANSITION_MODAL_KEY = 'data.textLangTransitionModal';
2828

29+
/**
30+
* The id value used to indicate that a link should open in a new Discover tab.
31+
* It will be used in the `_tab` URL param to indicate that a new tab should be created.
32+
* Once created, the new tab will have a unique id.
33+
*/
34+
export const NEW_TAB_ID = 'new' as const;
35+
2936
/**
3037
* The query param key used to store the Discover app state in the URL
3138
*/
3239
export const APP_STATE_URL_KEY = '_a';
3340
export const GLOBAL_STATE_URL_KEY = '_g';
34-
export const TABS_STATE_URL_KEY = '_t';
41+
export const TAB_STATE_URL_KEY = '_tab'; // `_t` is already used by Kibana for time, so we use `_tab` here
3542

3643
/**
3744
* Product feature IDs

src/platform/plugins/shared/discover/public/application/main/components/top_nav/app_menu_actions/get_share.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ export const getShareAppMenuItem = ({
7070
timeRange,
7171
refreshInterval,
7272
};
73+
74+
// TODO: for a persisted saved search, add the current tab ID to the params
7375
const relativeUrl = locator.getRedirectUrl(params);
7476

7577
// This logic is duplicated from `relativeToAbsolute` (for bundle size reasons). Ultimately, this should be

src/platform/plugins/shared/discover/public/application/main/state_management/hooks/use_state_managers.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export const useStateManagers = ({
4141
TABS_ENABLED_FEATURE_FLAG_KEY,
4242
false
4343
);
44-
// syncing with the _t part URL
44+
// syncing with the _tab part URL
4545
const [tabsStorageManager] = useState(() =>
4646
createTabsStorageManager({
4747
urlStateStorage,
@@ -63,13 +63,13 @@ export const useStateManagers = ({
6363

6464
useEffect(() => {
6565
const stopUrlSync = tabsStorageManager.startUrlSync({
66-
// if `_t` in URL changes (for example via browser history), try to restore the previous state
66+
// if `_tab` in URL changes (for example via browser history), try to restore the previous state
6767
onChanged: (urlState) => {
6868
const { tabId: restoreTabId } = urlState;
6969
if (restoreTabId) {
7070
internalState.dispatch(internalStateActions.restoreTab({ restoreTabId }));
7171
} else {
72-
// if tabId is not present in `_t`, clear all tabs
72+
// if tabId is not present in `_tab`, clear all tabs
7373
internalState.dispatch(internalStateActions.clearAllTabs());
7474
}
7575
},

src/platform/plugins/shared/discover/public/application/main/state_management/redux/actions/tabs.ts

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ import {
2929
selectInitialUnifiedHistogramLayoutPropsMap,
3030
selectTabRuntimeInternalState,
3131
} from '../runtime_state';
32-
import { APP_STATE_URL_KEY, GLOBAL_STATE_URL_KEY } from '../../../../../../common/constants';
32+
import {
33+
APP_STATE_URL_KEY,
34+
GLOBAL_STATE_URL_KEY,
35+
NEW_TAB_ID,
36+
} from '../../../../../../common/constants';
3337
import type { DiscoverAppState } from '../../discover_app_state_container';
3438
import { createInternalStateAsyncThunk, createTabItem } from '../utils';
3539
import { setBreadcrumbs } from '../../../../../utils/breadcrumbs';
@@ -113,6 +117,20 @@ export const updateTabs: InternalStateThunkActionCreator<[TabbedContentState], P
113117
};
114118

115119
if (!existingTab) {
120+
// the following assignments for initialAppState, globalState, and dataRequestParams are for supporting `openInNewTab` action
121+
tab.initialAppState =
122+
'initialAppState' in item
123+
? cloneDeep(item.initialAppState as TabState['initialAppState'])
124+
: tab.initialAppState;
125+
tab.globalState =
126+
'globalState' in item
127+
? cloneDeep(item.globalState as TabState['globalState'])
128+
: tab.globalState;
129+
tab.dataRequestParams =
130+
'dataRequestParams' in item
131+
? (item.dataRequestParams as TabState['dataRequestParams'])
132+
: tab.dataRequestParams;
133+
116134
if (item.duplicatedFromId) {
117135
// the new tab was created by duplicating an existing tab
118136
const existingTabToDuplicateFrom = selectTab(currentState, item.duplicatedFromId);
@@ -288,7 +306,9 @@ export const restoreTab: InternalStateThunkActionCreator<[{ restoreTabId: string
288306
(dispatch, getState) => {
289307
const currentState = getState();
290308

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

@@ -318,6 +338,46 @@ export const restoreTab: InternalStateThunkActionCreator<[{ restoreTabId: string
318338
);
319339
};
320340

341+
export const openInNewTab: InternalStateThunkActionCreator<
342+
[
343+
{
344+
tabLabel?: string;
345+
appState?: TabState['initialAppState'];
346+
globalState?: TabState['globalState'];
347+
searchSessionId?: string;
348+
}
349+
]
350+
> =
351+
({ tabLabel, appState, globalState, searchSessionId }) =>
352+
(dispatch, getState) => {
353+
const initialAppState = appState ? cloneDeep(appState) : undefined;
354+
const initialGlobalState = globalState ? cloneDeep(globalState) : {};
355+
const currentState = getState();
356+
const currentTabs = selectAllTabs(currentState);
357+
358+
const newDefaultTab: TabState = {
359+
...DEFAULT_TAB_STATE,
360+
...createTabItem(currentTabs),
361+
initialAppState,
362+
globalState: initialGlobalState,
363+
};
364+
365+
if (tabLabel) {
366+
newDefaultTab.label = tabLabel;
367+
}
368+
369+
if (searchSessionId) {
370+
newDefaultTab.dataRequestParams = {
371+
...newDefaultTab.dataRequestParams,
372+
searchSessionId,
373+
};
374+
}
375+
376+
return dispatch(
377+
updateTabs({ items: [...currentTabs, newDefaultTab], selectedItem: newDefaultTab })
378+
);
379+
};
380+
321381
export const disconnectTab: InternalStateThunkActionCreator<[TabActionPayload]> =
322382
({ tabId }) =>
323383
(_, __, { runtimeStateManager }) => {

src/platform/plugins/shared/discover/public/application/main/state_management/redux/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
updateTabs,
2222
disconnectTab,
2323
restoreTab,
24+
openInNewTab,
2425
clearAllTabs,
2526
initializeTabs,
2627
saveDiscoverSession,
@@ -56,6 +57,7 @@ export const internalStateActions = {
5657
initializeSingleTab,
5758
syncLocallyPersistedTabState,
5859
restoreTab,
60+
openInNewTab,
5961
clearAllTabs,
6062
initializeTabs,
6163
saveDiscoverSession,

0 commit comments

Comments
 (0)