[Embeddable Rebuild] [Saved Search] Migrate saved search embeddable to new embeddable framework by Heenawter · Pull Request #180536 · elastic/kibana

Heenawter · 2024-04-10T20:31:11Z

Summary

This PR converts the Saved Search embeddable to the new React embeddable framework. There should not be any changes in user-facing behaviour (except for the intentional change described here) - therefore, testing of this PR should be focused on ensuring that no behaviour is changed and/or broken with this refactor.

Warning

The saved search embeddable takes many forms and so, while I tried my best to test everything thoroughly, it is very, very likely that I missed some test cases due to not being the expert in this area. It is important that @elastic/kibana-data-discovery in particular approaches this PR review with a fine-tooth comb 🙇 Thanks so much.

Notes about the embeddable state:

As part of this refactor, I made three significant changes to how the state is managed:

Once the embeddable is being built in buildEmbeddable, the only difference between the runtime state of a by reference and a by value panel is that the by reference one will have three saved object-specific keys: savedObjectId, savedObjectDescription, and savedObjectTitle.
Number 1 made it possible for me to "flatten out" the runtime state of the embeddable by removing the attributes key, which makes it easier to access the pieces of state that you need.
Previously, the savedSearch element of the Saved Search embeddable object was never modified; instead, changes made to the columns, sort, sample size, etc. from the dashboard were stored in explicitInput. This essentially created two sources of truth.

With the new embeddable system, we only ever want one source of truth - so, the saved search is now modified directly when making changes from the dashboard. However, in order to keep behaviour consistent with the old embeddable, changes made from the dashboard to a by reference saved search should not modify the underlying saved object (this behaviour will have to change if we ever want inline editing for saved searches, but that is another discussion) - therefore, when serializing the runtime state (which happens when the dashboard is saved), we only serialize state that has changed from the initial state; then, on deserialization, we take this "changed" state and overwrite the state of the saved search with it.

Note that this only applies to by reference saved searches - with by value saved searches, we don't have to worry about this and can freely modify the state.

I also had to make changes to how the search source is stored in runtime state. Previously, when initializing the embeddable, fetching the saved search saved object also created and returned an unserializable search source object. However, in the new system, runtime state most always be serializable (see #186052) - therefore, I've had to instead use the serialized search source in my runtime state saved search - therefore, I've had to make changes to toSavedSearch method to allow for a serializable saved search to be returned.

	Runtime state (`input`) before	Runtime state after
By value
By reference

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

Heenawter · 2024-04-17T22:34:21Z

/ci

Heenawter · 2024-04-18T14:17:21Z

/ci

Heenawter · 2024-07-22T21:05:38Z

It looks like the Discover page load bundle increased 20% from this PR. Is there a way we could knock this down some before merging, maybe with dynamic imports? We try our best to keep the page load bundle as small as possible when we can.

Sorry yeah, this is 100% something we are aware of. The +8.8KB increase is specifically coming from the @kbn/presentation-containers package - at one point, we were looking at making this a shared package so that each plugin wouldn't inherit this bloat, but at the time it was deemed not worth it (@ThomThomson would know more about this, since he is the one that looked into it).

This is definitely something on our mind (since all React embeddables are dealing with this), so stay tuned for changes here - but unfortunately will have to merge this PR as-is because that will be a larger discussion with the team 🙈 Hopefully that is okay!!

cc @nreese

davismcphee · 2024-07-23T00:15:19Z

@Heenawter No problem! Thanks for the explanation and glad to hear it's on your radar already 🙂

## Summary This PR is a follow up to #180536, where `8.8KB` was added to the Discover bundle size - now, after async importing from the `presentation-publishing` package where necessary, I've reduced that increase to `3.4KB` (a decrease of `5.4KB` from the previous PR). ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

…189717) - Follow up for #180536 ## Summary This PR fixes an issue with `rowsPerPage` Advanced Setting: after the refactoring it was ignored. To test: Change `rowsPerPage` on Advanced Setting page, navigate to Dashboard and add a saved search panel. It should use the configured value by default. The default value can be overwritten by custom panel settings or saved search own settings. ### 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

…11678) ## Summary This PR re-adds drilldown support to the Discover session embeddable after it was accidentally removed during the refactoring in #180536 (related PR where drilldowns / dynamic actions were refactored: #178896). A new functional test has also been added to prevent future regressions. Fixes #211677. ### Checklist - [ ] 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) - [ ] 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 - [ ] 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) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>

…astic#211678) ## Summary This PR re-adds drilldown support to the Discover session embeddable after it was accidentally removed during the refactoring in elastic#180536 (related PR where drilldowns / dynamic actions were refactored: elastic#178896). A new functional test has also been added to prevent future regressions. Fixes elastic#211677. ### Checklist - [ ] 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) - [ ] 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 - [ ] 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) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co> (cherry picked from commit 257971d)

…astic#211678) ## Summary This PR re-adds drilldown support to the Discover session embeddable after it was accidentally removed during the refactoring in elastic#180536 (related PR where drilldowns / dynamic actions were refactored: elastic#178896). A new functional test has also been added to prevent future regressions. Fixes elastic#211677. ### Checklist - [ ] 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) - [ ] 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 - [ ] 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) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co> (cherry picked from commit 257971d) # Conflicts: # src/platform/plugins/shared/discover/public/types.ts # src/platform/plugins/shared/discover/tsconfig.json

…astic#211678) ## Summary This PR re-adds drilldown support to the Discover session embeddable after it was accidentally removed during the refactoring in elastic#180536 (related PR where drilldowns / dynamic actions were refactored: elastic#178896). A new functional test has also been added to prevent future regressions. Fixes elastic#211677. ### Checklist - [ ] 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) - [ ] 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 - [ ] 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) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co> (cherry picked from commit 257971d) # Conflicts: # src/plugins/discover/public/build_services.ts # src/plugins/discover/public/embeddable/get_search_embeddable_factory.tsx # src/plugins/discover/public/embeddable/types.ts # src/plugins/discover/public/embeddable/utils/serialization_utils.test.ts # src/plugins/discover/public/types.ts # src/plugins/discover/tsconfig.json

…wn (#211678) (#212143) # Backport This will backport the following commits from `main` to `9.0`: - [[Discover] [Embeddable] Fix Discover session embeddable drilldown (#211678)](#211678)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)

…own (#211678) (#212142) # Backport This will backport the following commits from `main` to `8.17`: - [[Discover] [Embeddable] Fix Discover session embeddable drilldown (#211678)](#211678)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)

…own (#211678) (#212147) # Backport This will backport the following commits from `main` to `8.18`: - [[Discover] [Embeddable] Fix Discover session embeddable drilldown (#211678)](#211678)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)

…own (#211678) (#212153) # Backport This will backport the following commits from `main` to `8.16`: - [[Discover] [Embeddable] Fix Discover session embeddable drilldown (#211678)](#211678)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

…wn (#211678) (#212144) # Backport This will backport the following commits from `main` to `8.x`: - [[Discover] [Embeddable] Fix Discover session embeddable drilldown (#211678)](#211678)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

…astic#211678) ## Summary This PR re-adds drilldown support to the Discover session embeddable after it was accidentally removed during the refactoring in elastic#180536 (related PR where drilldowns / dynamic actions were refactored: elastic#178896). A new functional test has also been added to prevent future regressions. Fixes elastic#211677. ### Checklist - [ ] 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) - [ ] 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 - [ ] 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) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>

…wn (elastic#211678) (elastic#212144) # Backport This will backport the following commits from `main` to `8.x`: - [[Discover] [Embeddable] Fix Discover session embeddable drilldown (elastic#211678)](elastic#211678)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

…astic#211678) ## Summary This PR re-adds drilldown support to the Discover session embeddable after it was accidentally removed during the refactoring in elastic#180536 (related PR where drilldowns / dynamic actions were refactored: elastic#178896). A new functional test has also been added to prevent future regressions. Fixes elastic#211677. ### Checklist - [ ] 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) - [ ] 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 - [ ] 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) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>

Heenawter self-assigned this Apr 10, 2024

Heenawter added the Team:DataDiscovery Discover, search (data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. t// label Apr 10, 2024

Heenawter force-pushed the refactor-saved-search_2024-04-05 branch 2 times, most recently from 575a44b to e58fe5a Compare April 15, 2024 18:41

Heenawter mentioned this pull request Apr 16, 2024

[Embeddables Rebuild] Decouple other actions from Embeddable framework #175138

Closed

Heenawter force-pushed the refactor-saved-search_2024-04-05 branch 2 times, most recently from 7f18c4c to 15a0544 Compare April 23, 2024 16:10

Heenawter force-pushed the refactor-saved-search_2024-04-05 branch 4 times, most recently from d74f53a to 0b3fc47 Compare April 30, 2024 22:30

Heenawter added 11 commits May 29, 2024 14:15

Going to move embeddable registry back to Discover plugin

401a86e

Move back to Discover plugin

ed20b74

Fix unlink bug

bb088e1

Fix some typing

2fcf18d

Move embeddable registration to saved search plugin

d4e70b3

Move fixing after moving embeddable

6d66c47

Going to move embeddable back to Discover LOL - too much shared code

135735d

Move back to Discover

9fa6d18

Starting to figure it out... maybe

136d656

Working on getting fetching to work

fb2398c

Switch to new fetch subscription

438bfa0

Heenawter merged commit 5c4eae1 into elastic:main Jul 22, 2024

kibanamachine added v8.16.0 backport:skip This PR does not require backporting labels Jul 22, 2024

Heenawter mentioned this pull request Jul 25, 2024

[Embeddable Rebuild] [Discover] Fix bundle size #189206

Merged

1 task

jughosta mentioned this pull request Aug 1, 2024

[Discover][SavedSearch] Fix default rowsPerPage for Dashboard panels #189717

Merged

1 task

Heenawter deleted the refactor-saved-search_2024-04-05 branch August 8, 2024 17:50

thomasneirynck mentioned this pull request Sep 17, 2024

[embeddable rebuild] integrate react control group embeddable into dashboard container - reloaded #192221

Merged

This was referenced Feb 19, 2025

[Discover] [Embeddable] Discover session embeddable is missing drilldown support #211677

Closed

[Discover] [Embeddable] Fix Discover session embeddable drilldown #211678

Merged

This was referenced Feb 6, 2026

[Discover] Move visContext and controlGroupJson management from savedSearchContainer to redux store #250902

Merged

[Discover] [Embeddable] Missing props when converting from a by value -> by reference Discover session #252194

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Embeddable Rebuild] [Saved Search] Migrate saved search embeddable to new embeddable framework#180536

[Embeddable Rebuild] [Saved Search] Migrate saved search embeddable to new embeddable framework#180536
Heenawter merged 144 commits intoelastic:mainfrom
Heenawter:refactor-saved-search_2024-04-05

Heenawter commented Apr 10, 2024 •

edited

Loading

Uh oh!

Heenawter commented Apr 17, 2024

Uh oh!

Heenawter commented Apr 18, 2024

Uh oh!

Heenawter commented Jul 22, 2024 •

edited

Loading

Uh oh!

davismcphee commented Jul 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

Conversation

Heenawter commented Apr 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Notes about the embeddable state:

Checklist

For maintainers

Uh oh!

Heenawter commented Apr 17, 2024

Uh oh!

Heenawter commented Apr 18, 2024

Uh oh!

Heenawter commented Jul 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davismcphee commented Jul 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

Heenawter commented Apr 10, 2024 •

edited

Loading

Heenawter commented Jul 22, 2024 •

edited

Loading