[Discover][Embeddable] Add New Discover session for Dashboards#256293
[Discover][Embeddable] Add New Discover session for Dashboards#256293Bluefinger merged 14 commits intoelastic:mainfrom
Conversation
|
/ci |
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
nreese
left a comment
There was a problem hiding this comment.
kibana-presentation changes LGTM
code review only
|
Just chiming in to say that this has been a long time coming and I'm really happy to see this change. |
davismcphee
left a comment
There was a problem hiding this comment.
Code changes look good and it works well overall! Left a bit of minor feedback, but otherwise all good code wise. I did encounter one error case I think we should try to fix before merging.
MiloszRadzynski
left a comment
There was a problem hiding this comment.
LGTM 🚀, I just left a minor nit. I won't approve it because there are still pending requests from @davismcphee
Flaky Test Runner Stats🟠 Some tests failed. - kibana-flaky-test-suite-runner#11014[❌] src/platform/test/functional/apps/discover/embeddable/config.ts: 0/30 tests passed. |
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#11016[✅] src/platform/test/functional/apps/discover/embeddable/config.ts: 30/30 tests passed. |
davismcphee
left a comment
There was a problem hiding this comment.
Thanks for the updates! Seems close overall, but I tested again and found a couple more issues to look into.
…oard save (#256997) ## Summary Resolves #256943. Fixes by-value Classic mode Discover Session panels failing to load after saving and reloading a dashboard (error: `Could not find reference for kibanaSavedObjectMeta.searchSourceJSON.index`). This was a bug introduced in #252786. ### How to verify This problem really only surfaces in the UX after the changes in #256293. 1. Create a new dashboard → Add panel → Discover Session. 2. Save a default Classic mode session and return to the dashboard. 3. Save the dashboard and reload the page. 4. The Discover session panel should load without the "Could not find reference" error. Co-authored-by: Gonçalo Rica Pais da Silva <goncalo.rica@elastic.co>
💔 Build Failed
Failed CI StepsTest Failures
Metrics [docs]Module Count
Async chunks
Page load bundle
History
cc @Bluefinger |
|
I tested this with the rebased main branch now that the fixes are in, and confirm this works. |
davismcphee
left a comment
There was a problem hiding this comment.
Latest changes look good, and I tested the two remaining scenarios and both are working as expected now. Glad we were able to get the other fix merged first. Thanks!
| /** | ||
| * Specifies the action to be taken for navigating back to an editor. | ||
| */ | ||
| export enum TransferAction { | ||
| /** | ||
| * A Cancel operation. Returns to the editor without modifying the original state. | ||
| */ | ||
| Cancel, | ||
| /** | ||
| * A Save Session operation. Updates the saved session and doesn't pass back any serialised state. | ||
| */ | ||
| SaveSession, | ||
| /** | ||
| * A Save By Value operation. Sends back to the editor the serialised updated state for the embeddable. | ||
| */ | ||
| SaveByValue, | ||
| } |
There was a problem hiding this comment.
I like this approach 👌
| public transferBackToEditor(action: TransferAction.Cancel | TransferAction.SaveSession): void; | ||
| public transferBackToEditor( | ||
| action: TransferAction.SaveByValue, | ||
| state: SavedSearchByValueAttributes | ||
| ): void; | ||
| public transferBackToEditor(action: TransferAction, state?: SavedSearchByValueAttributes): void; |
There was a problem hiding this comment.
You're one of few to make good use of function overloading in TS! 😄
There was a problem hiding this comment.
All this would be unnecessary if TS had Rust-like enums and pattern-matching 😅
I'm so rust-brained...
…257835) ## Summary Related to #256293 (comment). Synced with @florent-leborgne offline, and we agreed not to use "by-value" in the UI copy. Instead just going with "New Discover session" in the breadcrumb when adding a new Discover panel to a dashboard: <img src="https://github.com/user-attachments/assets/c051ea00-d721-47d1-a88b-063020e2a092" /> ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [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 - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [x] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [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.
…oard save (#256997) ## Summary Resolves #256943. Fixes by-value Classic mode Discover Session panels failing to load after saving and reloading a dashboard (error: `Could not find reference for kibanaSavedObjectMeta.searchSourceJSON.index`). This was a bug introduced in #252786. ### How to verify This problem really only surfaces in the UX after the changes in #256293. 1. Create a new dashboard → Add panel → Discover Session. 2. Save a default Classic mode session and return to the dashboard. 3. Save the dashboard and reload the page. 4. The Discover session panel should load without the "Could not find reference" error. Co-authored-by: Gonçalo Rica Pais da Silva <goncalo.rica@elastic.co>
## Summary This PR adds a new button in the New Panel options of the Dashboard to embed a Discover By-Value session directly. This opens Discover in embeddable editor mode, and creates the panel with the By-value session saved. https://github.com/user-attachments/assets/9efa383f-7dd5-4bd8-8112-5dd140d68635 Closes #249104 ## How to test - Go to Dashboards and create a new Dashboard - Click on the Add button and then click New Panel - Click on Discover session and edit the session, saving it once done. - Should return to Dashboards with a Discover panel created with the same view/configuration as during the editing. ## TODOs - [x] Initial working code - [x] Polish & refinement - [x] Functional tests
…257835) ## Summary Related to #256293 (comment). Synced with @florent-leborgne offline, and we agreed not to use "by-value" in the UI copy. Instead just going with "New Discover session" in the breadcrumb when adding a new Discover panel to a dashboard: <img src="https://github.com/user-attachments/assets/c051ea00-d721-47d1-a88b-063020e2a092" /> ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [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 - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [x] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [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.
…oard save (elastic#256997) ## Summary Resolves elastic#256943. Fixes by-value Classic mode Discover Session panels failing to load after saving and reloading a dashboard (error: `Could not find reference for kibanaSavedObjectMeta.searchSourceJSON.index`). This was a bug introduced in elastic#252786. ### How to verify This problem really only surfaces in the UX after the changes in elastic#256293. 1. Create a new dashboard → Add panel → Discover Session. 2. Save a default Classic mode session and return to the dashboard. 3. Save the dashboard and reload the page. 4. The Discover session panel should load without the "Could not find reference" error. Co-authored-by: Gonçalo Rica Pais da Silva <goncalo.rica@elastic.co>
…ic#256293) ## Summary This PR adds a new button in the New Panel options of the Dashboard to embed a Discover By-Value session directly. This opens Discover in embeddable editor mode, and creates the panel with the By-value session saved. https://github.com/user-attachments/assets/9efa383f-7dd5-4bd8-8112-5dd140d68635 Closes elastic#249104 ## How to test - Go to Dashboards and create a new Dashboard - Click on the Add button and then click New Panel - Click on Discover session and edit the session, saving it once done. - Should return to Dashboards with a Discover panel created with the same view/configuration as during the editing. ## TODOs - [x] Initial working code - [x] Polish & refinement - [x] Functional tests
…lastic#257835) ## Summary Related to elastic#256293 (comment). Synced with @florent-leborgne offline, and we agreed not to use "by-value" in the UI copy. Instead just going with "New Discover session" in the breadcrumb when adding a new Discover panel to a dashboard: <img src="https://github.com/user-attachments/assets/c051ea00-d721-47d1-a88b-063020e2a092" /> ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [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 - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [x] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [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.
Summary
This PR adds a new button in the New Panel options of the Dashboard to embed a Discover By-Value session directly. This opens Discover in embeddable editor mode, and creates the panel with the By-value session saved.
Screen.Recording.2026-03-06.151406.mp4
Closes #249104
How to test
TODOs