[Discover Tabs] Extend the discover locator to allow opening in a new tab #234912

jughosta · 2025-09-12T13:36:57Z

Closes [Discover Tabs] Extend the discover locator to allow opening in a new tab #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

kibana/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts

Lines 393 to 396 in 17f6970

    
           const openTabs = 
        
             persistedDiscoverSession?.id === storedTabsState.discoverSessionId 
        
               ? storedTabsState.openTabs.map((tab) => toTabState(tab, defaultTabState)) 
        
               : persistedTabs ?? [];

(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

Unit or functional tests were updated or added to match the most common scenarios
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines
Review the backport guidelines and apply applicable backport:* labels.

…d search

…ing saved search" This reverts commit 42f403d.

jughosta · 2025-09-15T12:14:51Z

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

+    if (searchSessionId) {
+      newDefaultTab.dataRequestParams = {
+        ...newDefaultTab.dataRequestParams,
+        searchSessionId,


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

Copilot

Pull Request Overview

This PR extends the Discover locator to support opening searches in new tabs by adding tabId and tabLabel parameters. When tabId is set to "new", it signals to create a new Discover tab with the specified configuration.

Key changes:

Added NEW_TAB_ID constant and tab-related parameters to the locator interface
Implemented logic to handle new tab creation via URL parameters
Added openInNewTab Redux action for programmatic tab creation
Updated search session management to optionally open restored sessions in new tabs

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`src/platform/plugins/shared/discover/common/constants.ts`	Added `NEW_TAB_ID` constant and updated `TABS_STATE_URL_KEY`
`src/platform/plugins/shared/discover/common/app_locator.ts`	Extended locator interface with `tabId` and `tabLabel` parameters
`src/platform/plugins/shared/discover/common/app_locator_get_location.ts`	Added URL state handling for tab parameters
`src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts`	Implemented new tab creation logic and URL state management
`src/platform/plugins/shared/discover/public/application/main/state_management/redux/actions/tabs.ts`	Added `openInNewTab` action and updated tab restoration logic
`src/platform/plugins/shared/discover/public/application/main/state_management/redux/index.ts`	Exported new `openInNewTab` action
`src/platform/plugins/shared/discover/public/application/main/state_management/redux/internal_state.test.ts`	Added test coverage for the new tab functionality
`src/platform/plugins/shared/discover/public/application/main/state_management/hooks/use_state_managers.ts`	Updated comments to reflect correct URL parameter name
`src/platform/plugins/shared/discover/public/application/main/components/top_nav/app_menu_actions/get_share.tsx`	Added TODO comment for future tab ID integration
`src/platform/plugins/shared/discover/common/app_locator.test.ts`	Added test cases for new tab functionality
`src/platform/plugins/shared/data/public/search/session/sessions_mgmt/components/table/utils/map_to_ui_session.ts`	Added support for opening search sessions in new tabs
`src/platform/plugins/shared/data/public/search/session/sessions_mgmt/components/table/table.tsx`	Added feature flag check for new tab functionality

...orm/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts

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

…tate_management/tabs_storage_manager.ts Co-authored-by: Copilot <[email protected]>

…tate_management/redux/actions/tabs.ts Co-authored-by: Copilot <[email protected]>

elasticmachine · 2025-09-15T16:37:19Z

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

AlexGPlay · 2025-09-16T09:03:48Z

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

  );
+  const enableOpeningInNewTab = useMemo(
+    () =>
+      core.featureFlags.getBooleanValue(BACKGROUND_SEARCH_FEATURE_FLAG_KEY, false) &&


nit: do we need to check both feature flags for this?

@AlexGPlay Good question! How would you recommend to change it?

i was thinking that maybe the tabs one is enough since it's the feature this is truly targeting, wdyt?

Updated in 1f6aa3d

AlexGPlay

tested from a bgs perspective and lgtm 🚀

dej611

@elastic/kibana-visualizations code review 👍

davismcphee

Code changes LGTM, also tested locally and it seems to work as expected! Left some minor feedback, but nothing blocking.

Thanks for adding the expected outcome matrix to the summary, it helped a lot with testing. I agree it would be best to use the Redux action for restoring background searches within the flyout (not sure it'll make it for 9.2 though).

The expected outcomes are fairly complex, but I'm not sure what else you could have done here either given the current background search implementation. We'll at least have the unsaved changes modal and recently closed tabs to limit the impact for 9.2.

We should continue discussing this either async or in the office hours this Thursday, but I think we're good to merge the current implementation for now. If we figure out a way to resolve the conflicts better, IMO it could be treated as a fix and backported to 9.2 post FF.

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

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

davismcphee · 2025-09-16T23:34:31Z

...ns/shared/discover/public/application/main/components/top_nav/app_menu_actions/get_share.tsx

      refreshInterval,
    };
+
+    // TODO: for a persisted saved search, add the current tab ID to the params


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.

Created #235320

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

davismcphee · 2025-09-16T23:39:20Z

...lugins/shared/discover/public/application/main/state_management/redux/internal_state.test.ts

    );
  });
+
+  it('should append a new tab to the tabs list', async () => {


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 🙂

...orm/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts

jughosta

Thanks for the reviews, @davismcphee and @AlexGPlay!
Updated the code.

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

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

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

jughosta · 2025-09-17T08:12:42Z

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

  );
+  const enableOpeningInNewTab = useMemo(
+    () =>
+      core.featureFlags.getBooleanValue(BACKGROUND_SEARCH_FEATURE_FLAG_KEY, false) &&


Updated in 1f6aa3d

...orm/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts

jughosta · 2025-09-17T08:16:40Z

...ns/shared/discover/public/application/main/components/top_nav/app_menu_actions/get_share.tsx

      refreshInterval,
    };
+
+    // TODO: for a persisted saved search, add the current tab ID to the params


Created #235320

…829-locator-new-tab

elasticmachine · 2025-09-17T11:28:09Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: db8bafe

Failed CI Steps

FTR Configs #96

Test Failures

[job] [logs] FTR Configs #96 / Cloud Security Posture Test adding Cloud Security Posture Integrations CSPM AWS CIS_AWS Organization Manual Direct Access CIS_AWS Organization Manual Direct Access Workflow

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`discover`	1.1MB	1.1MB	+941.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`data`	421.5KB	421.7KB	+180.0B
`discover`	23.3KB	23.3KB	+19.0B
total			+199.0B

Unknown metric groups

API count

id	before	after	diff
`discover`	209	210	+1

History

💚 Build #339498 succeeded 6256d4d
💔 Build #339332 failed 6bdab7e
💛 Build #339071 was flaky 73da781
💚 Build #338881 succeeded 708f08e

cc @jughosta

… tab (elastic#234912) - Closes elastic#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]>

… 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]>

… tab (elastic#234912) - Closes elastic#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]>

jughosta added 2 commits September 12, 2025 14:03

[Discover Tabs] Extend locator with tabId

efc8993

[Discover Tabs] Start opening search session links in a new tab

708f08e

jughosta self-assigned this Sep 12, 2025

jughosta added 12 commits September 12, 2025 17:33

[Discover Tabs] Handle the case when it was added to an existing save…

42f403d

…d search

Revert "[Discover Tabs] Handle the case when it was added to an exist…

ea68b9e

…ing saved search" This reverts commit 42f403d.

[Discover Tabs] Add a new action

449a634

[Discover Tabs] Fix the label

da0f12f

[Discover Tabs] Fix the label

cfb3ed6

[Discover Tabs] Allow to override the label

a909cc8

[Discover Tabs] Update param

73da781

[Discover Tabs] Improve the redux action

be766b1

[Discover Tabs] Improve the test

96a2dc8

[Discover Tabs] Fix URL history stack

17f6970

[Discover Tabs] Add feature flag checks

00add5c

Merge branch 'main' into 230829-locator-new-tab

3936eb8

jughosta commented Sep 15, 2025

View reviewed changes

jughosta requested a review from Copilot September 15, 2025 12:20

Copilot AI reviewed Sep 15, 2025

View reviewed changes

...orm/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts Show resolved Hide resolved

...tform/plugins/shared/discover/public/application/main/state_management/redux/actions/tabs.ts Show resolved Hide resolved

jughosta and others added 3 commits September 15, 2025 14:25

Update src/platform/plugins/shared/discover/public/application/main/s…

4785f72

…tate_management/tabs_storage_manager.ts Co-authored-by: Copilot <[email protected]>

Update src/platform/plugins/shared/discover/public/application/main/s…

6bdab7e

…tate_management/redux/actions/tabs.ts Co-authored-by: Copilot <[email protected]>

[Discover Tabs] Fix the test

bdaf825

jughosta marked this pull request as ready for review September 15, 2025 16:37

jughosta requested review from a team as code owners September 15, 2025 16:37

Merge branch 'main' into 230829-locator-new-tab

6256d4d

AlexGPlay reviewed Sep 16, 2025

View reviewed changes

AlexGPlay approved these changes Sep 16, 2025

View reviewed changes

jughosta mentioned this pull request Sep 16, 2025

[Discover Tabs] Fix recently closed tabs #234750

Merged

4 tasks

dej611 approved these changes Sep 16, 2025

View reviewed changes

davismcphee approved these changes Sep 17, 2025

View reviewed changes

kertal mentioned this pull request Sep 17, 2025

[Discover Tabs] Use openInNewTab in Background search flyout #235303

Closed

jughosta added 6 commits September 17, 2025 09:35

Merge branch 'main' into 230829-locator-new-tab

1f9be33

[Discover] Rename tab param

1d268cc

[Discover] Update router params

33a1964

[Discover] Add a comment

27766ef

[Discover] Check only tabs feature flag

1f6aa3d

[Discover] Add a test

38d9c42

jughosta commented Sep 17, 2025

View reviewed changes

jughosta added 3 commits September 17, 2025 10:21

Merge branch 'main' into 230829-locator-new-tab

0dec342

[Discover] More renaming

1808017

Merge remote-tracking branch 'origin/230829-locator-new-tab' into 230…

db8bafe

…829-locator-new-tab

jughosta merged commit f3495de into elastic:main Sep 17, 2025
12 checks passed

jughosta deleted the 230829-locator-new-tab branch September 17, 2025 13:55

kibanamachine added the v9.2.0 label Sep 17, 2025

	const openTabs =
	persistedDiscoverSession?.id === storedTabsState.discoverSessionId
	? storedTabsState.openTabs.map((tab) => toTabState(tab, defaultTabState))
	: persistedTabs ?? [];

[Discover Tabs] Extend the discover locator to allow opening in a new tab #234912

[Discover Tabs] Extend the discover locator to allow opening in a new tab #234912

Uh oh!

Conversation

jughosta commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented Sep 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexGPlay left a comment

Choose a reason for hiding this comment

Uh oh!

dej611 left a comment

Choose a reason for hiding this comment

Uh oh!

davismcphee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jughosta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Sep 17, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Async chunks

Page load bundle

API count

History

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jughosta commented Sep 12, 2025 •

edited

Loading