Merged
Conversation
… for eui markdown
…ansfer service (#216232) Fixes #213153 ## Summary This PR ensures that the embeddable state transfer service **exclusively** uases serialized state. Previously, the transfer service accepted **either** runtime state or serialized state - but we are in the process of removing the concept of "runtime state" for embeddables (at least as far as the Dashboard is concerned), and this is one step towards that goal. Note that this PR is built off the subset of "embeddable state transfer service related" changes from #215416 - all I did was clean up these changes, finalized some of the cases that were missed in the original draft, and updated tests as necessary. ### 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)
…ized-state' into embeddable-serialized-state
…anges initializer.
Also updates state manager API
e4e735e to
777ee00
Compare
_This PR does not need to be reviewed by external teams. This PR merges into a feature branch that Kibana presentation team is working on to convert the embeddable framework to only expose serialized state. Your team will be pinged for review once the work is complete and the final PR opens that merges the feature branch into main._ ## Summary Updates the Field list example embeddable to serialized state only. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
…ized state only (#218270) _This PR does not need to be reviewed by external teams. This PR merges into a feature branch that Kibana presentation team is working on to convert the embeddable framework to only expose serialized state. Your team will be pinged for review once the work is complete and the final PR opens that merges the feature branch into main._ Converts the SLO embeddable to serialized state only. ## Testing this PR Create an SLO using the "How to Test" section in the description of [this PR](#179147). --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
_This PR does not need to be reviewed by external teams. This PR merges into a feature branch that Kibana presentation team is working on to convert the embeddable framework to only expose serialized state. Your team will be pinged for review once the work is complete and the final PR opens that merges the feature branch into main._ Converts the SLO alerts embeddable to serialized state only. ## Testing this PR Create an SLO using the "How to Test" section in the description of [this PR](#179147). --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
…te only (#218461) _This PR does not need to be reviewed by external teams. This PR merges into a feature branch that Kibana presentation team is working on to convert the embeddable framework to only expose serialized state. Your team will be pinged for review once the work is complete and the final PR opens that merges the feature branch into main._ ## Summary Convert SLO burn rate to serialized state only ## Testing this PR Create an SLO using the "How to Test" section in the description of [this PR](#179147).
## Summary This PR removes runtime state from the example data table embeddable --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Contributor
|
@elasticmachine merge upstream |
Contributor
|
@elasticmachine merge upstream |
Contributor
|
@elasticmachine merge upstream |
Contributor
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
nreese
pushed a commit
to nreese/kibana
that referenced
this pull request
May 6, 2025
Closes elastic#205531 Closes elastic#219877. Closes elastic#213153 Closes elastic#150920 Closes elastic#203130 ### Overview The embeddable framework has two types of state: `SerializedState` and `RuntimeState`. `SerializedState` is the form of the state when saved into a Dashboard saved object. I.e. the References are extracted, and state saved externally (by reference) is removed. In contrast `RuntimeState` is an exact snapshot of the state used by the embeddable to render. <b>Exposing SerializedState and RuntimeState was a mistake</b> that caused numerous regressions and architectural complexities. This PR simplifies the embeddable framework by only exposing `SerializedState`. `RuntimeState` stays localized to the embeddable implementation and is never leaked to the embeddable framework. ### Whats changed * `ReactEmbeddableFactory<SerializedState, RuntimeState, Api>` => `EmbeddableFactory<SerializedState, Api>` * `deserializeState` removed from embeddable factory. Instead, `SerializedState` is passed directly into `buildEmbeddable`. * `buildEmbeddable` parameter `buildApi` replaced with `finalizeApi`. `buildApi({ api, comparators })` => `finalizeApi(api)`. * The embeddable framework previously used its knowledge of `RuntimeState` to setup and monitor unsaved changes. Now, unsaved changes setup is pushed down to the embeddable implementation since the embeddable framework no longer has knowledge of embeddable RuntimeState. ### Reviewer instructions <b>Please prioritize reviews.</b> This is a large effort from our team and is blocking many other initiatives. Getting this merged is a top priority. This is a large change that would best be reviewed by manually testing the changes * adding/editing your embeddable types * Ensuring dashboard shows unsaved changes as expected * Ensuring dashboard resets unsaved changes as expected * Ensuring dashboard does not show unsaved changes after save and reset * Returning to a dashboard with unsaved changes renders embeddables with those unsaved changes --------- Co-authored-by: Hannah Mudge <Heenawter@users.noreply.github.com> Co-authored-by: Nathan Reese <reese.nathan@elastic.co> Co-authored-by: Nick Peihl <nick.peihl@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Catherine Liu <catherine.liu@elastic.co> Co-authored-by: Ola Pawlus <98127445+olapawlus@users.noreply.github.com> (cherry picked from commit 3e882d8) # Conflicts: # packages/kbn-optimizer/limits.yml # src/platform/plugins/shared/dashboard/public/dashboard_app/top_nav/share/show_share_modal.tsx # src/platform/plugins/shared/discover/public/embeddable/get_search_embeddable_factory.tsx # src/platform/plugins/shared/unified_doc_viewer/public/components/observability/traces/components/trace.tsx # x-pack/platform/plugins/shared/aiops/public/embeddables/change_point_chart/embeddable_change_point_chart_factory.tsx # x-pack/platform/plugins/shared/aiops/public/embeddables/log_rate_analysis/embeddable_log_rate_analysis_factory.tsx # x-pack/platform/plugins/shared/aiops/public/embeddables/pattern_analysis/embeddable_pattern_analysis_factory.tsx # x-pack/platform/plugins/shared/lens/public/plugin.ts # x-pack/platform/plugins/shared/ml/public/plugin.ts # x-pack/solutions/observability/plugins/apm/public/embeddable/trace_waterfall/react_embeddable_factory.tsx # x-pack/solutions/observability/plugins/apm/tsconfig.json # x-pack/solutions/observability/plugins/infra/public/components/log_stream/log_stream_react_embeddable.tsx
nreese
added a commit
that referenced
this pull request
May 7, 2025
# Backport This will backport the following commits from `main` to `8.19`: - [[Embeddables] Serialized State Only (#215947)](#215947) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Devon Thomson","email":"devon.thomson@elastic.co"},"sourceCommit":{"committedDate":"2025-05-06T21:08:34Z","message":"[Embeddables] Serialized State Only (#215947)\n\nCloses https://github.com/elastic/kibana/issues/205531\nCloses #219877.\nCloses https://github.com/elastic/kibana/issues/213153\nCloses https://github.com/elastic/kibana/issues/150920\nCloses https://github.com/elastic/kibana/issues/203130\n \n### Overview\nThe embeddable framework has two types of state: `SerializedState` and\n`RuntimeState`.\n\n`SerializedState` is the form of the state when saved into a Dashboard\nsaved object. I.e. the References are extracted, and state saved\nexternally (by reference) is removed. In contrast `RuntimeState` is an\nexact snapshot of the state used by the embeddable to render.\n\n<b>Exposing SerializedState and RuntimeState was a mistake</b> that\ncaused numerous regressions and architectural complexities.\n\nThis PR simplifies the embeddable framework by only exposing\n`SerializedState`. `RuntimeState` stays localized to the embeddable\nimplementation and is never leaked to the embeddable framework.\n\n### Whats changed\n* `ReactEmbeddableFactory<SerializedState, RuntimeState, Api>` =>\n`EmbeddableFactory<SerializedState, Api>`\n* `deserializeState` removed from embeddable factory. Instead,\n`SerializedState` is passed directly into `buildEmbeddable`.\n* `buildEmbeddable` parameter `buildApi` replaced with `finalizeApi`.\n`buildApi({ api, comparators })` => `finalizeApi(api)`.\n* The embeddable framework previously used its knowledge of\n`RuntimeState` to setup and monitor unsaved changes. Now, unsaved\nchanges setup is pushed down to the embeddable implementation since the\nembeddable framework no longer has knowledge of embeddable RuntimeState.\n\n### Reviewer instructions\n<b>Please prioritize reviews.</b> This is a large effort from our team\nand is blocking many other initiatives. Getting this merged is a top\npriority.\n\nThis is a large change that would best be reviewed by manually testing\nthe changes\n* adding/editing your embeddable types\n* Ensuring dashboard shows unsaved changes as expected\n* Ensuring dashboard resets unsaved changes as expected\n* Ensuring dashboard does not show unsaved changes after save and reset\n* Returning to a dashboard with unsaved changes renders embeddables with\nthose unsaved changes\n\n---------\n\nCo-authored-by: Hannah Mudge <Heenawter@users.noreply.github.com>\nCo-authored-by: Nathan Reese <reese.nathan@elastic.co>\nCo-authored-by: Nick Peihl <nick.peihl@elastic.co>\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>\nCo-authored-by: Catherine Liu <catherine.liu@elastic.co>\nCo-authored-by: Ola Pawlus <98127445+olapawlus@users.noreply.github.com>","sha":"3e882d8cd9b784dc73961dbdfa79285761eab7ba","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Embedding","Team:Presentation","release_note:skip","Team:obs-ux-infra_services","Team:obs-ux-management","backport:version","v9.1.0","v8.19.0"],"title":"[Embeddables] Serialized State Only","number":215947,"url":"https://github.com/elastic/kibana/pull/215947","mergeCommit":{"message":"[Embeddables] Serialized State Only (#215947)\n\nCloses https://github.com/elastic/kibana/issues/205531\nCloses #219877.\nCloses https://github.com/elastic/kibana/issues/213153\nCloses https://github.com/elastic/kibana/issues/150920\nCloses https://github.com/elastic/kibana/issues/203130\n \n### Overview\nThe embeddable framework has two types of state: `SerializedState` and\n`RuntimeState`.\n\n`SerializedState` is the form of the state when saved into a Dashboard\nsaved object. I.e. the References are extracted, and state saved\nexternally (by reference) is removed. In contrast `RuntimeState` is an\nexact snapshot of the state used by the embeddable to render.\n\n<b>Exposing SerializedState and RuntimeState was a mistake</b> that\ncaused numerous regressions and architectural complexities.\n\nThis PR simplifies the embeddable framework by only exposing\n`SerializedState`. `RuntimeState` stays localized to the embeddable\nimplementation and is never leaked to the embeddable framework.\n\n### Whats changed\n* `ReactEmbeddableFactory<SerializedState, RuntimeState, Api>` =>\n`EmbeddableFactory<SerializedState, Api>`\n* `deserializeState` removed from embeddable factory. Instead,\n`SerializedState` is passed directly into `buildEmbeddable`.\n* `buildEmbeddable` parameter `buildApi` replaced with `finalizeApi`.\n`buildApi({ api, comparators })` => `finalizeApi(api)`.\n* The embeddable framework previously used its knowledge of\n`RuntimeState` to setup and monitor unsaved changes. Now, unsaved\nchanges setup is pushed down to the embeddable implementation since the\nembeddable framework no longer has knowledge of embeddable RuntimeState.\n\n### Reviewer instructions\n<b>Please prioritize reviews.</b> This is a large effort from our team\nand is blocking many other initiatives. Getting this merged is a top\npriority.\n\nThis is a large change that would best be reviewed by manually testing\nthe changes\n* adding/editing your embeddable types\n* Ensuring dashboard shows unsaved changes as expected\n* Ensuring dashboard resets unsaved changes as expected\n* Ensuring dashboard does not show unsaved changes after save and reset\n* Returning to a dashboard with unsaved changes renders embeddables with\nthose unsaved changes\n\n---------\n\nCo-authored-by: Hannah Mudge <Heenawter@users.noreply.github.com>\nCo-authored-by: Nathan Reese <reese.nathan@elastic.co>\nCo-authored-by: Nick Peihl <nick.peihl@elastic.co>\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>\nCo-authored-by: Catherine Liu <catherine.liu@elastic.co>\nCo-authored-by: Ola Pawlus <98127445+olapawlus@users.noreply.github.com>","sha":"3e882d8cd9b784dc73961dbdfa79285761eab7ba"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/215947","number":215947,"mergeCommit":{"message":"[Embeddables] Serialized State Only (#215947)\n\nCloses https://github.com/elastic/kibana/issues/205531\nCloses #219877.\nCloses https://github.com/elastic/kibana/issues/213153\nCloses https://github.com/elastic/kibana/issues/150920\nCloses https://github.com/elastic/kibana/issues/203130\n \n### Overview\nThe embeddable framework has two types of state: `SerializedState` and\n`RuntimeState`.\n\n`SerializedState` is the form of the state when saved into a Dashboard\nsaved object. I.e. the References are extracted, and state saved\nexternally (by reference) is removed. In contrast `RuntimeState` is an\nexact snapshot of the state used by the embeddable to render.\n\n<b>Exposing SerializedState and RuntimeState was a mistake</b> that\ncaused numerous regressions and architectural complexities.\n\nThis PR simplifies the embeddable framework by only exposing\n`SerializedState`. `RuntimeState` stays localized to the embeddable\nimplementation and is never leaked to the embeddable framework.\n\n### Whats changed\n* `ReactEmbeddableFactory<SerializedState, RuntimeState, Api>` =>\n`EmbeddableFactory<SerializedState, Api>`\n* `deserializeState` removed from embeddable factory. Instead,\n`SerializedState` is passed directly into `buildEmbeddable`.\n* `buildEmbeddable` parameter `buildApi` replaced with `finalizeApi`.\n`buildApi({ api, comparators })` => `finalizeApi(api)`.\n* The embeddable framework previously used its knowledge of\n`RuntimeState` to setup and monitor unsaved changes. Now, unsaved\nchanges setup is pushed down to the embeddable implementation since the\nembeddable framework no longer has knowledge of embeddable RuntimeState.\n\n### Reviewer instructions\n<b>Please prioritize reviews.</b> This is a large effort from our team\nand is blocking many other initiatives. Getting this merged is a top\npriority.\n\nThis is a large change that would best be reviewed by manually testing\nthe changes\n* adding/editing your embeddable types\n* Ensuring dashboard shows unsaved changes as expected\n* Ensuring dashboard resets unsaved changes as expected\n* Ensuring dashboard does not show unsaved changes after save and reset\n* Returning to a dashboard with unsaved changes renders embeddables with\nthose unsaved changes\n\n---------\n\nCo-authored-by: Hannah Mudge <Heenawter@users.noreply.github.com>\nCo-authored-by: Nathan Reese <reese.nathan@elastic.co>\nCo-authored-by: Nick Peihl <nick.peihl@elastic.co>\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>\nCo-authored-by: Catherine Liu <catherine.liu@elastic.co>\nCo-authored-by: Ola Pawlus <98127445+olapawlus@users.noreply.github.com>","sha":"3e882d8cd9b784dc73961dbdfa79285761eab7ba"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> --------- Co-authored-by: Devon Thomson <devon.thomson@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
nreese
added a commit
that referenced
this pull request
May 28, 2025
Fixes #221545 ### Background #215947 refactored [ControlGroupRenderer](https://github.com/elastic/kibana/blob/main/src/platform/plugins/shared/controls/public/control_group/control_group_renderer/control_group_renderer.tsx) The regression is caused by this line changing `await getCreationOptions?.(getDefaultControlGroupRuntimeState(), controlGroupStateBuilder)` to `getCreationOptions(defaultRuntimeState, controlGroupStateBuilder)`. Opening the SLO view caused https://github.com/elastic/kibana/blob/main/x-pack/solutions/observability/plugins/slo/public/pages/slos/components/common/quick_filters.tsx#L72 to mutate `defaultRuntimeState`, and populate `initialChildControlState` with `slo-status-filter` and `slo-tags-filter`. Then when returning to Alerts view, `defaultRuntimeState.initialChildControlState` contained the controls from SLO page. ### Fix This PR resolves the issue by passing a new object to `getCreationOptions` so that `defaultRuntimeState` can not be polluted by consumers. ### Test steps * Start kibana with `yarn start` * Go license page and install 30 day trial license * In seperate window, run ``` node x-pack/scripts/data_forge.js \ --events-per-cycle 100 \ --lookback now-7d \ --dataset fake_stack \ --install-kibana-assets \ --kibana-url http://localhost:5601/<basePath> ``` * In SLO, create an SLO with the following ([more details](https://github.com/elastic/kibana/blob/main/x-pack/solutions/observability/plugins/slo/dev_docs/slo.md)) * Type: Custom Query * Index: Admin Console * Good (query): http.response.status_code < 500 * Total (query): http.response.status_code : * * toggle between SLO and alerts page and verify controls in alerts page do not grow after each visit to SLO page
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
May 28, 2025
Fixes elastic#221545 ### Background elastic#215947 refactored [ControlGroupRenderer](https://github.com/elastic/kibana/blob/main/src/platform/plugins/shared/controls/public/control_group/control_group_renderer/control_group_renderer.tsx) The regression is caused by this line changing `await getCreationOptions?.(getDefaultControlGroupRuntimeState(), controlGroupStateBuilder)` to `getCreationOptions(defaultRuntimeState, controlGroupStateBuilder)`. Opening the SLO view caused https://github.com/elastic/kibana/blob/main/x-pack/solutions/observability/plugins/slo/public/pages/slos/components/common/quick_filters.tsx#L72 to mutate `defaultRuntimeState`, and populate `initialChildControlState` with `slo-status-filter` and `slo-tags-filter`. Then when returning to Alerts view, `defaultRuntimeState.initialChildControlState` contained the controls from SLO page. ### Fix This PR resolves the issue by passing a new object to `getCreationOptions` so that `defaultRuntimeState` can not be polluted by consumers. ### Test steps * Start kibana with `yarn start` * Go license page and install 30 day trial license * In seperate window, run ``` node x-pack/scripts/data_forge.js \ --events-per-cycle 100 \ --lookback now-7d \ --dataset fake_stack \ --install-kibana-assets \ --kibana-url http://localhost:5601/<basePath> ``` * In SLO, create an SLO with the following ([more details](https://github.com/elastic/kibana/blob/main/x-pack/solutions/observability/plugins/slo/dev_docs/slo.md)) * Type: Custom Query * Index: Admin Console * Good (query): http.response.status_code < 500 * Total (query): http.response.status_code : * * toggle between SLO and alerts page and verify controls in alerts page do not grow after each visit to SLO page (cherry picked from commit 3398573)
kibanamachine
added a commit
that referenced
this pull request
May 28, 2025
# Backport This will backport the following commits from `main` to `8.19`: - [[Alerts] fix Duplicated filters are shown (#221678)](#221678) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Nathan Reese","email":"reese.nathan@elastic.co"},"sourceCommit":{"committedDate":"2025-05-28T00:06:11Z","message":"[Alerts] fix Duplicated filters are shown (#221678)\n\nFixes https://github.com/elastic/kibana/issues/221545\n\n### Background\nhttps://github.com//pull/215947 refactored\n[ControlGroupRenderer](https://github.com/elastic/kibana/blob/main/src/platform/plugins/shared/controls/public/control_group/control_group_renderer/control_group_renderer.tsx)\n\nThe regression is caused by this line changing `await\ngetCreationOptions?.(getDefaultControlGroupRuntimeState(),\ncontrolGroupStateBuilder)` to `getCreationOptions(defaultRuntimeState,\ncontrolGroupStateBuilder)`.\n\nOpening the SLO view caused\nhttps://github.com/elastic/kibana/blob/main/x-pack/solutions/observability/plugins/slo/public/pages/slos/components/common/quick_filters.tsx#L72\nto mutate `defaultRuntimeState`, and populate `initialChildControlState`\nwith `slo-status-filter` and `slo-tags-filter`. Then when returning to\nAlerts view, `defaultRuntimeState.initialChildControlState` contained\nthe controls from SLO page.\n\n### Fix\nThis PR resolves the issue by passing a new object to\n`getCreationOptions` so that `defaultRuntimeState` can not be polluted\nby consumers.\n\n### Test steps\n* Start kibana with `yarn start`\n* Go license page and install 30 day trial license\n* In seperate window, run\n ```\n node x-pack/scripts/data_forge.js \\\n\t--events-per-cycle 100 \\\n\t--lookback now-7d \\\n\t--dataset fake_stack \\\n\t--install-kibana-assets \\\n\t--kibana-url http://localhost:5601/<basePath>\n ```\n* In SLO, create an SLO with the following ([more\ndetails](https://github.com/elastic/kibana/blob/main/x-pack/solutions/observability/plugins/slo/dev_docs/slo.md))\n * Type: Custom Query\n * Index: Admin Console\n * Good (query): http.response.status_code < 500\n * Total (query): http.response.status_code : *\n* toggle between SLO and alerts page and verify controls in alerts page\ndo not grow after each visit to SLO page","sha":"33985735fd15b55fe2cff47dca8501ea6c2c96aa","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","release_note:skip","backport:version","v9.1.0","v8.19.0"],"title":"[Alerts] fix Duplicated filters are shown","number":221678,"url":"https://github.com/elastic/kibana/pull/221678","mergeCommit":{"message":"[Alerts] fix Duplicated filters are shown (#221678)\n\nFixes https://github.com/elastic/kibana/issues/221545\n\n### Background\nhttps://github.com//pull/215947 refactored\n[ControlGroupRenderer](https://github.com/elastic/kibana/blob/main/src/platform/plugins/shared/controls/public/control_group/control_group_renderer/control_group_renderer.tsx)\n\nThe regression is caused by this line changing `await\ngetCreationOptions?.(getDefaultControlGroupRuntimeState(),\ncontrolGroupStateBuilder)` to `getCreationOptions(defaultRuntimeState,\ncontrolGroupStateBuilder)`.\n\nOpening the SLO view caused\nhttps://github.com/elastic/kibana/blob/main/x-pack/solutions/observability/plugins/slo/public/pages/slos/components/common/quick_filters.tsx#L72\nto mutate `defaultRuntimeState`, and populate `initialChildControlState`\nwith `slo-status-filter` and `slo-tags-filter`. Then when returning to\nAlerts view, `defaultRuntimeState.initialChildControlState` contained\nthe controls from SLO page.\n\n### Fix\nThis PR resolves the issue by passing a new object to\n`getCreationOptions` so that `defaultRuntimeState` can not be polluted\nby consumers.\n\n### Test steps\n* Start kibana with `yarn start`\n* Go license page and install 30 day trial license\n* In seperate window, run\n ```\n node x-pack/scripts/data_forge.js \\\n\t--events-per-cycle 100 \\\n\t--lookback now-7d \\\n\t--dataset fake_stack \\\n\t--install-kibana-assets \\\n\t--kibana-url http://localhost:5601/<basePath>\n ```\n* In SLO, create an SLO with the following ([more\ndetails](https://github.com/elastic/kibana/blob/main/x-pack/solutions/observability/plugins/slo/dev_docs/slo.md))\n * Type: Custom Query\n * Index: Admin Console\n * Good (query): http.response.status_code < 500\n * Total (query): http.response.status_code : *\n* toggle between SLO and alerts page and verify controls in alerts page\ndo not grow after each visit to SLO page","sha":"33985735fd15b55fe2cff47dca8501ea6c2c96aa"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/221678","number":221678,"mergeCommit":{"message":"[Alerts] fix Duplicated filters are shown (#221678)\n\nFixes https://github.com/elastic/kibana/issues/221545\n\n### Background\nhttps://github.com//pull/215947 refactored\n[ControlGroupRenderer](https://github.com/elastic/kibana/blob/main/src/platform/plugins/shared/controls/public/control_group/control_group_renderer/control_group_renderer.tsx)\n\nThe regression is caused by this line changing `await\ngetCreationOptions?.(getDefaultControlGroupRuntimeState(),\ncontrolGroupStateBuilder)` to `getCreationOptions(defaultRuntimeState,\ncontrolGroupStateBuilder)`.\n\nOpening the SLO view caused\nhttps://github.com/elastic/kibana/blob/main/x-pack/solutions/observability/plugins/slo/public/pages/slos/components/common/quick_filters.tsx#L72\nto mutate `defaultRuntimeState`, and populate `initialChildControlState`\nwith `slo-status-filter` and `slo-tags-filter`. Then when returning to\nAlerts view, `defaultRuntimeState.initialChildControlState` contained\nthe controls from SLO page.\n\n### Fix\nThis PR resolves the issue by passing a new object to\n`getCreationOptions` so that `defaultRuntimeState` can not be polluted\nby consumers.\n\n### Test steps\n* Start kibana with `yarn start`\n* Go license page and install 30 day trial license\n* In seperate window, run\n ```\n node x-pack/scripts/data_forge.js \\\n\t--events-per-cycle 100 \\\n\t--lookback now-7d \\\n\t--dataset fake_stack \\\n\t--install-kibana-assets \\\n\t--kibana-url http://localhost:5601/<basePath>\n ```\n* In SLO, create an SLO with the following ([more\ndetails](https://github.com/elastic/kibana/blob/main/x-pack/solutions/observability/plugins/slo/dev_docs/slo.md))\n * Type: Custom Query\n * Index: Admin Console\n * Good (query): http.response.status_code < 500\n * Total (query): http.response.status_code : *\n* toggle between SLO and alerts page and verify controls in alerts page\ndo not grow after each visit to SLO page","sha":"33985735fd15b55fe2cff47dca8501ea6c2c96aa"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Nathan Reese <reese.nathan@elastic.co>
akowalska622
pushed a commit
to akowalska622/kibana
that referenced
this pull request
May 29, 2025
Closes elastic#205531 Closes elastic#219877. Closes elastic#213153 Closes elastic#150920 Closes elastic#203130 ### Overview The embeddable framework has two types of state: `SerializedState` and `RuntimeState`. `SerializedState` is the form of the state when saved into a Dashboard saved object. I.e. the References are extracted, and state saved externally (by reference) is removed. In contrast `RuntimeState` is an exact snapshot of the state used by the embeddable to render. <b>Exposing SerializedState and RuntimeState was a mistake</b> that caused numerous regressions and architectural complexities. This PR simplifies the embeddable framework by only exposing `SerializedState`. `RuntimeState` stays localized to the embeddable implementation and is never leaked to the embeddable framework. ### Whats changed * `ReactEmbeddableFactory<SerializedState, RuntimeState, Api>` => `EmbeddableFactory<SerializedState, Api>` * `deserializeState` removed from embeddable factory. Instead, `SerializedState` is passed directly into `buildEmbeddable`. * `buildEmbeddable` parameter `buildApi` replaced with `finalizeApi`. `buildApi({ api, comparators })` => `finalizeApi(api)`. * The embeddable framework previously used its knowledge of `RuntimeState` to setup and monitor unsaved changes. Now, unsaved changes setup is pushed down to the embeddable implementation since the embeddable framework no longer has knowledge of embeddable RuntimeState. ### Reviewer instructions <b>Please prioritize reviews.</b> This is a large effort from our team and is blocking many other initiatives. Getting this merged is a top priority. This is a large change that would best be reviewed by manually testing the changes * adding/editing your embeddable types * Ensuring dashboard shows unsaved changes as expected * Ensuring dashboard resets unsaved changes as expected * Ensuring dashboard does not show unsaved changes after save and reset * Returning to a dashboard with unsaved changes renders embeddables with those unsaved changes --------- Co-authored-by: Hannah Mudge <Heenawter@users.noreply.github.com> Co-authored-by: Nathan Reese <reese.nathan@elastic.co> Co-authored-by: Nick Peihl <nick.peihl@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Catherine Liu <catherine.liu@elastic.co> Co-authored-by: Ola Pawlus <98127445+olapawlus@users.noreply.github.com>
akowalska622
pushed a commit
to akowalska622/kibana
that referenced
this pull request
May 29, 2025
Fixes elastic#221545 ### Background elastic#215947 refactored [ControlGroupRenderer](https://github.com/elastic/kibana/blob/main/src/platform/plugins/shared/controls/public/control_group/control_group_renderer/control_group_renderer.tsx) The regression is caused by this line changing `await getCreationOptions?.(getDefaultControlGroupRuntimeState(), controlGroupStateBuilder)` to `getCreationOptions(defaultRuntimeState, controlGroupStateBuilder)`. Opening the SLO view caused https://github.com/elastic/kibana/blob/main/x-pack/solutions/observability/plugins/slo/public/pages/slos/components/common/quick_filters.tsx#L72 to mutate `defaultRuntimeState`, and populate `initialChildControlState` with `slo-status-filter` and `slo-tags-filter`. Then when returning to Alerts view, `defaultRuntimeState.initialChildControlState` contained the controls from SLO page. ### Fix This PR resolves the issue by passing a new object to `getCreationOptions` so that `defaultRuntimeState` can not be polluted by consumers. ### Test steps * Start kibana with `yarn start` * Go license page and install 30 day trial license * In seperate window, run ``` node x-pack/scripts/data_forge.js \ --events-per-cycle 100 \ --lookback now-7d \ --dataset fake_stack \ --install-kibana-assets \ --kibana-url http://localhost:5601/<basePath> ``` * In SLO, create an SLO with the following ([more details](https://github.com/elastic/kibana/blob/main/x-pack/solutions/observability/plugins/slo/dev_docs/slo.md)) * Type: Custom Query * Index: Admin Console * Good (query): http.response.status_code < 500 * Total (query): http.response.status_code : * * toggle between SLO and alerts page and verify controls in alerts page do not grow after each visit to SLO page
qn895
pushed a commit
to qn895/kibana
that referenced
this pull request
Jun 3, 2025
Closes elastic#205531 Closes elastic#219877. Closes elastic#213153 Closes elastic#150920 Closes elastic#203130 ### Overview The embeddable framework has two types of state: `SerializedState` and `RuntimeState`. `SerializedState` is the form of the state when saved into a Dashboard saved object. I.e. the References are extracted, and state saved externally (by reference) is removed. In contrast `RuntimeState` is an exact snapshot of the state used by the embeddable to render. <b>Exposing SerializedState and RuntimeState was a mistake</b> that caused numerous regressions and architectural complexities. This PR simplifies the embeddable framework by only exposing `SerializedState`. `RuntimeState` stays localized to the embeddable implementation and is never leaked to the embeddable framework. ### Whats changed * `ReactEmbeddableFactory<SerializedState, RuntimeState, Api>` => `EmbeddableFactory<SerializedState, Api>` * `deserializeState` removed from embeddable factory. Instead, `SerializedState` is passed directly into `buildEmbeddable`. * `buildEmbeddable` parameter `buildApi` replaced with `finalizeApi`. `buildApi({ api, comparators })` => `finalizeApi(api)`. * The embeddable framework previously used its knowledge of `RuntimeState` to setup and monitor unsaved changes. Now, unsaved changes setup is pushed down to the embeddable implementation since the embeddable framework no longer has knowledge of embeddable RuntimeState. ### Reviewer instructions <b>Please prioritize reviews.</b> This is a large effort from our team and is blocking many other initiatives. Getting this merged is a top priority. This is a large change that would best be reviewed by manually testing the changes * adding/editing your embeddable types * Ensuring dashboard shows unsaved changes as expected * Ensuring dashboard resets unsaved changes as expected * Ensuring dashboard does not show unsaved changes after save and reset * Returning to a dashboard with unsaved changes renders embeddables with those unsaved changes --------- Co-authored-by: Hannah Mudge <Heenawter@users.noreply.github.com> Co-authored-by: Nathan Reese <reese.nathan@elastic.co> Co-authored-by: Nick Peihl <nick.peihl@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Catherine Liu <catherine.liu@elastic.co> Co-authored-by: Ola Pawlus <98127445+olapawlus@users.noreply.github.com>
9 tasks
cqliu1
added a commit
that referenced
this pull request
Jun 24, 2025
## Summary Cherry-picked c528ca0 from #215947. This registers the ML embeddable synchronously to avoid a racing condition where the dashboard could try to render an ML embeddable before the embeddable is available. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] 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) ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. - [ ] [See some risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) - [ ] ... --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This was referenced Jul 14, 2025
kibanamachine
added a commit
to kibanamachine/kibana
that referenced
this pull request
Jul 18, 2025
## Summary The issue was a regression caused by elastic#215947 where the wrong serialized state was used when cloning the panel fix elastic#227831 fixes elastic#226970 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 120bb3d)
kibanamachine
added a commit
to kibanamachine/kibana
that referenced
this pull request
Jul 18, 2025
## Summary The issue was a regression caused by elastic#215947 where the wrong serialized state was used when cloning the panel fix elastic#227831 fixes elastic#226970 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 120bb3d)
kibanamachine
added a commit
that referenced
this pull request
Jul 18, 2025
# Backport This will backport the following commits from `main` to `9.1`: - [[TSVB] Serialise the actual vis on clone (#227882)](#227882) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Marco Vettorello","email":"marco.vettorello@elastic.co"},"sourceCommit":{"committedDate":"2025-07-18T08:11:33Z","message":"[TSVB] Serialise the actual vis on clone (#227882)\n\n## Summary\n\nThe issue was a regression caused by\nhttps://github.com//pull/215947 where the wrong serialized\nstate was used when cloning the panel\n\nfix https://github.com/elastic/kibana/issues/227831\nfixes https://github.com/elastic/kibana/issues/226970\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"120bb3d18ecf32769baaf6c38e70772fa2cffeaa","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Team:Visualizations","release_note:skip","backport:version","v9.1.0","v8.19.0","v9.2.0"],"title":"[TSVB] Serialise the actual vis on clone","number":227882,"url":"https://github.com/elastic/kibana/pull/227882","mergeCommit":{"message":"[TSVB] Serialise the actual vis on clone (#227882)\n\n## Summary\n\nThe issue was a regression caused by\nhttps://github.com//pull/215947 where the wrong serialized\nstate was used when cloning the panel\n\nfix https://github.com/elastic/kibana/issues/227831\nfixes https://github.com/elastic/kibana/issues/226970\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"120bb3d18ecf32769baaf6c38e70772fa2cffeaa"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19"],"targetPullRequestStates":[{"branch":"9.1","label":"v9.1.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/227882","number":227882,"mergeCommit":{"message":"[TSVB] Serialise the actual vis on clone (#227882)\n\n## Summary\n\nThe issue was a regression caused by\nhttps://github.com//pull/215947 where the wrong serialized\nstate was used when cloning the panel\n\nfix https://github.com/elastic/kibana/issues/227831\nfixes https://github.com/elastic/kibana/issues/226970\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"120bb3d18ecf32769baaf6c38e70772fa2cffeaa"}}]}] BACKPORT--> Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
kibanamachine
added a commit
that referenced
this pull request
Jul 18, 2025
# Backport This will backport the following commits from `main` to `8.19`: - [[TSVB] Serialise the actual vis on clone (#227882)](#227882) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Marco Vettorello","email":"marco.vettorello@elastic.co"},"sourceCommit":{"committedDate":"2025-07-18T08:11:33Z","message":"[TSVB] Serialise the actual vis on clone (#227882)\n\n## Summary\n\nThe issue was a regression caused by\nhttps://github.com//pull/215947 where the wrong serialized\nstate was used when cloning the panel\n\nfix https://github.com/elastic/kibana/issues/227831\nfixes https://github.com/elastic/kibana/issues/226970\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"120bb3d18ecf32769baaf6c38e70772fa2cffeaa","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Team:Visualizations","release_note:skip","backport:version","v9.1.0","v8.19.0","v9.2.0"],"title":"[TSVB] Serialise the actual vis on clone","number":227882,"url":"https://github.com/elastic/kibana/pull/227882","mergeCommit":{"message":"[TSVB] Serialise the actual vis on clone (#227882)\n\n## Summary\n\nThe issue was a regression caused by\nhttps://github.com//pull/215947 where the wrong serialized\nstate was used when cloning the panel\n\nfix https://github.com/elastic/kibana/issues/227831\nfixes https://github.com/elastic/kibana/issues/226970\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"120bb3d18ecf32769baaf6c38e70772fa2cffeaa"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19"],"targetPullRequestStates":[{"branch":"9.1","label":"v9.1.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/227882","number":227882,"mergeCommit":{"message":"[TSVB] Serialise the actual vis on clone (#227882)\n\n## Summary\n\nThe issue was a regression caused by\nhttps://github.com//pull/215947 where the wrong serialized\nstate was used when cloning the panel\n\nfix https://github.com/elastic/kibana/issues/227831\nfixes https://github.com/elastic/kibana/issues/226970\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"120bb3d18ecf32769baaf6c38e70772fa2cffeaa"}}]}] BACKPORT--> Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
This was referenced Jul 19, 2025
Bluefinger
pushed a commit
to Bluefinger/kibana
that referenced
this pull request
Jul 22, 2025
## Summary The issue was a regression caused by elastic#215947 where the wrong serialized state was used when cloning the panel fix elastic#227831 fixes elastic#226970 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
kertal
pushed a commit
to kertal/kibana
that referenced
this pull request
Jul 25, 2025
## Summary The issue was a regression caused by elastic#215947 where the wrong serialized state was used when cloning the panel fix elastic#227831 fixes elastic#226970 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
markov00
added a commit
that referenced
this pull request
Aug 5, 2025
## Summary This PR tries to apply a patch to a logic that is not very clear about what a serialized state is and how this is used to evaluate if a dashboard has unsaved changes or not. The current implementation leverage the type `SerializedPanelState` that has a `rawState` property and a `references` array. https://github.com/elastic/kibana/blob/2eb9972ea52589fc2aab16f17bbc99cfedf76783/src/platform/packages/shared/presentation/presentation_publishing/interfaces/has_serializable_state.ts#L12-L19 This type is the result of the call to the passed method called `serializeState`, an as described in the type, the references are extracted. To my understanding, extracted refers to the fact that the `rawState` should only have a link/name to an external reference, and should not contain any trace of the original referenced object id. If this is the right way to understand that type definition, the associated method and the operation of `reference extraction` then I'm inclined to say that the current implementation of `initializeUnsavedChanges` doesn't take in consideration the `references` array in the equality check, because the only property passed and used with the comparators is the `rawState` as seen here: https://github.com/elastic/kibana/blob/c9b441ec50dc8dc3c9180e1412019c695199909f/src/platform/packages/shared/presentation/presentation_containers/interfaces/unsaved_changes/initialize_unsaved_changes.ts#L48-L59 This can create unwanted situation where a change to the used references can be undetected by the "unsaved changes" method. One example is coming from the linked [issue](#228203) where TSVB was extracting the references as requested, replacing, on the serializedState, the `index_pattern: {id: "123abc"}` with a `index_pattern_ref_name: "metric_0_index_pattern"` and moving the actual index pattern references in the array outside the `rawState`. This generates the issue where switching an index pattern doesn't result in an unsaved change detected at the dashboard level because both the current rawState and the latest saved rawState are in fact the same (but not he references array). An AggBased visualization, that uses the same embeddable as TSVB, suffers from the same problem. If you the switch the data view the change is not picked up. TSVB and AggBased issues are both regression coming from #215947 (I tested before and after that PR) Lens probably has the same issue, but due to a wrong implementation, the `rawState` also has it's own copy of the `references` array that is replaced at the time of the references injection, but that is not removed at the time of the reference extractions (yes it is saved in the SO as part of the attributes and could differ from the dashboard reference array if a reference id has been changed in the dashboard). (I had also a discussion with @nickpeihl about this wrong behaviour). So basically Lens, for a wrong implementation, doesn't suffer of this problem, but if we are going to implement the reference extraction correctly will suffer of this issue. The implementation is simple, but can be definitely improved. One thing that is not clear now is the following: The dashboard is saved after the first click to the `Save` button, but it will keep showing the `usaved changes` until: - you click save again - or you refresh the page So basically the changes are correctly saved to ES but the latestSavedState is not updated immediately. I believe this is an issue with the current `Spaghetti Observable` used here. fix #228203 ## Release Note Fixes a bug that prevented saving linked TSVB visualizations when change data view. --------- Co-authored-by: Nick Partridge <nicholas.partridge@elastic.co> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
nickofthyme
pushed a commit
to nickofthyme/kibana
that referenced
this pull request
Aug 5, 2025
…228685) ## Summary This PR tries to apply a patch to a logic that is not very clear about what a serialized state is and how this is used to evaluate if a dashboard has unsaved changes or not. The current implementation leverage the type `SerializedPanelState` that has a `rawState` property and a `references` array. https://github.com/elastic/kibana/blob/2eb9972ea52589fc2aab16f17bbc99cfedf76783/src/platform/packages/shared/presentation/presentation_publishing/interfaces/has_serializable_state.ts#L12-L19 This type is the result of the call to the passed method called `serializeState`, an as described in the type, the references are extracted. To my understanding, extracted refers to the fact that the `rawState` should only have a link/name to an external reference, and should not contain any trace of the original referenced object id. If this is the right way to understand that type definition, the associated method and the operation of `reference extraction` then I'm inclined to say that the current implementation of `initializeUnsavedChanges` doesn't take in consideration the `references` array in the equality check, because the only property passed and used with the comparators is the `rawState` as seen here: https://github.com/elastic/kibana/blob/c9b441ec50dc8dc3c9180e1412019c695199909f/src/platform/packages/shared/presentation/presentation_containers/interfaces/unsaved_changes/initialize_unsaved_changes.ts#L48-L59 This can create unwanted situation where a change to the used references can be undetected by the "unsaved changes" method. One example is coming from the linked [issue](elastic#228203) where TSVB was extracting the references as requested, replacing, on the serializedState, the `index_pattern: {id: "123abc"}` with a `index_pattern_ref_name: "metric_0_index_pattern"` and moving the actual index pattern references in the array outside the `rawState`. This generates the issue where switching an index pattern doesn't result in an unsaved change detected at the dashboard level because both the current rawState and the latest saved rawState are in fact the same (but not he references array). An AggBased visualization, that uses the same embeddable as TSVB, suffers from the same problem. If you the switch the data view the change is not picked up. TSVB and AggBased issues are both regression coming from elastic#215947 (I tested before and after that PR) Lens probably has the same issue, but due to a wrong implementation, the `rawState` also has it's own copy of the `references` array that is replaced at the time of the references injection, but that is not removed at the time of the reference extractions (yes it is saved in the SO as part of the attributes and could differ from the dashboard reference array if a reference id has been changed in the dashboard). (I had also a discussion with @nickpeihl about this wrong behaviour). So basically Lens, for a wrong implementation, doesn't suffer of this problem, but if we are going to implement the reference extraction correctly will suffer of this issue. The implementation is simple, but can be definitely improved. One thing that is not clear now is the following: The dashboard is saved after the first click to the `Save` button, but it will keep showing the `usaved changes` until: - you click save again - or you refresh the page So basically the changes are correctly saved to ES but the latestSavedState is not updated immediately. I believe this is an issue with the current `Spaghetti Observable` used here. fix elastic#228203 ## Release Note Fixes a bug that prevented saving linked TSVB visualizations when change data view. --------- Co-authored-by: Nick Partridge <nicholas.partridge@elastic.co> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com> (cherry picked from commit 2747360) # Conflicts: # src/platform/plugins/shared/dashboard/public/dashboard_api/get_dashboard_api.ts
delanni
pushed a commit
to delanni/kibana
that referenced
this pull request
Aug 5, 2025
…228685) ## Summary This PR tries to apply a patch to a logic that is not very clear about what a serialized state is and how this is used to evaluate if a dashboard has unsaved changes or not. The current implementation leverage the type `SerializedPanelState` that has a `rawState` property and a `references` array. https://github.com/elastic/kibana/blob/2eb9972ea52589fc2aab16f17bbc99cfedf76783/src/platform/packages/shared/presentation/presentation_publishing/interfaces/has_serializable_state.ts#L12-L19 This type is the result of the call to the passed method called `serializeState`, an as described in the type, the references are extracted. To my understanding, extracted refers to the fact that the `rawState` should only have a link/name to an external reference, and should not contain any trace of the original referenced object id. If this is the right way to understand that type definition, the associated method and the operation of `reference extraction` then I'm inclined to say that the current implementation of `initializeUnsavedChanges` doesn't take in consideration the `references` array in the equality check, because the only property passed and used with the comparators is the `rawState` as seen here: https://github.com/elastic/kibana/blob/c9b441ec50dc8dc3c9180e1412019c695199909f/src/platform/packages/shared/presentation/presentation_containers/interfaces/unsaved_changes/initialize_unsaved_changes.ts#L48-L59 This can create unwanted situation where a change to the used references can be undetected by the "unsaved changes" method. One example is coming from the linked [issue](elastic#228203) where TSVB was extracting the references as requested, replacing, on the serializedState, the `index_pattern: {id: "123abc"}` with a `index_pattern_ref_name: "metric_0_index_pattern"` and moving the actual index pattern references in the array outside the `rawState`. This generates the issue where switching an index pattern doesn't result in an unsaved change detected at the dashboard level because both the current rawState and the latest saved rawState are in fact the same (but not he references array). An AggBased visualization, that uses the same embeddable as TSVB, suffers from the same problem. If you the switch the data view the change is not picked up. TSVB and AggBased issues are both regression coming from elastic#215947 (I tested before and after that PR) Lens probably has the same issue, but due to a wrong implementation, the `rawState` also has it's own copy of the `references` array that is replaced at the time of the references injection, but that is not removed at the time of the reference extractions (yes it is saved in the SO as part of the attributes and could differ from the dashboard reference array if a reference id has been changed in the dashboard). (I had also a discussion with @nickpeihl about this wrong behaviour). So basically Lens, for a wrong implementation, doesn't suffer of this problem, but if we are going to implement the reference extraction correctly will suffer of this issue. The implementation is simple, but can be definitely improved. One thing that is not clear now is the following: The dashboard is saved after the first click to the `Save` button, but it will keep showing the `usaved changes` until: - you click save again - or you refresh the page So basically the changes are correctly saved to ES but the latestSavedState is not updated immediately. I believe this is an issue with the current `Spaghetti Observable` used here. fix elastic#228203 ## Release Note Fixes a bug that prevented saving linked TSVB visualizations when change data view. --------- Co-authored-by: Nick Partridge <nicholas.partridge@elastic.co> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
NicholasPeretti
pushed a commit
to NicholasPeretti/kibana
that referenced
this pull request
Aug 18, 2025
…228685) ## Summary This PR tries to apply a patch to a logic that is not very clear about what a serialized state is and how this is used to evaluate if a dashboard has unsaved changes or not. The current implementation leverage the type `SerializedPanelState` that has a `rawState` property and a `references` array. https://github.com/elastic/kibana/blob/2eb9972ea52589fc2aab16f17bbc99cfedf76783/src/platform/packages/shared/presentation/presentation_publishing/interfaces/has_serializable_state.ts#L12-L19 This type is the result of the call to the passed method called `serializeState`, an as described in the type, the references are extracted. To my understanding, extracted refers to the fact that the `rawState` should only have a link/name to an external reference, and should not contain any trace of the original referenced object id. If this is the right way to understand that type definition, the associated method and the operation of `reference extraction` then I'm inclined to say that the current implementation of `initializeUnsavedChanges` doesn't take in consideration the `references` array in the equality check, because the only property passed and used with the comparators is the `rawState` as seen here: https://github.com/elastic/kibana/blob/c9b441ec50dc8dc3c9180e1412019c695199909f/src/platform/packages/shared/presentation/presentation_containers/interfaces/unsaved_changes/initialize_unsaved_changes.ts#L48-L59 This can create unwanted situation where a change to the used references can be undetected by the "unsaved changes" method. One example is coming from the linked [issue](elastic#228203) where TSVB was extracting the references as requested, replacing, on the serializedState, the `index_pattern: {id: "123abc"}` with a `index_pattern_ref_name: "metric_0_index_pattern"` and moving the actual index pattern references in the array outside the `rawState`. This generates the issue where switching an index pattern doesn't result in an unsaved change detected at the dashboard level because both the current rawState and the latest saved rawState are in fact the same (but not he references array). An AggBased visualization, that uses the same embeddable as TSVB, suffers from the same problem. If you the switch the data view the change is not picked up. TSVB and AggBased issues are both regression coming from elastic#215947 (I tested before and after that PR) Lens probably has the same issue, but due to a wrong implementation, the `rawState` also has it's own copy of the `references` array that is replaced at the time of the references injection, but that is not removed at the time of the reference extractions (yes it is saved in the SO as part of the attributes and could differ from the dashboard reference array if a reference id has been changed in the dashboard). (I had also a discussion with @nickpeihl about this wrong behaviour). So basically Lens, for a wrong implementation, doesn't suffer of this problem, but if we are going to implement the reference extraction correctly will suffer of this issue. The implementation is simple, but can be definitely improved. One thing that is not clear now is the following: The dashboard is saved after the first click to the `Save` button, but it will keep showing the `usaved changes` until: - you click save again - or you refresh the page So basically the changes are correctly saved to ES but the latestSavedState is not updated immediately. I believe this is an issue with the current `Spaghetti Observable` used here. fix elastic#228203 ## Release Note Fixes a bug that prevented saving linked TSVB visualizations when change data view. --------- Co-authored-by: Nick Partridge <nicholas.partridge@elastic.co> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
markov00
added a commit
to markov00/kibana
that referenced
this pull request
Sep 15, 2025
…228685) ## Summary This PR tries to apply a patch to a logic that is not very clear about what a serialized state is and how this is used to evaluate if a dashboard has unsaved changes or not. The current implementation leverage the type `SerializedPanelState` that has a `rawState` property and a `references` array. https://github.com/elastic/kibana/blob/2eb9972ea52589fc2aab16f17bbc99cfedf76783/src/platform/packages/shared/presentation/presentation_publishing/interfaces/has_serializable_state.ts#L12-L19 This type is the result of the call to the passed method called `serializeState`, an as described in the type, the references are extracted. To my understanding, extracted refers to the fact that the `rawState` should only have a link/name to an external reference, and should not contain any trace of the original referenced object id. If this is the right way to understand that type definition, the associated method and the operation of `reference extraction` then I'm inclined to say that the current implementation of `initializeUnsavedChanges` doesn't take in consideration the `references` array in the equality check, because the only property passed and used with the comparators is the `rawState` as seen here: https://github.com/elastic/kibana/blob/c9b441ec50dc8dc3c9180e1412019c695199909f/src/platform/packages/shared/presentation/presentation_containers/interfaces/unsaved_changes/initialize_unsaved_changes.ts#L48-L59 This can create unwanted situation where a change to the used references can be undetected by the "unsaved changes" method. One example is coming from the linked [issue](elastic#228203) where TSVB was extracting the references as requested, replacing, on the serializedState, the `index_pattern: {id: "123abc"}` with a `index_pattern_ref_name: "metric_0_index_pattern"` and moving the actual index pattern references in the array outside the `rawState`. This generates the issue where switching an index pattern doesn't result in an unsaved change detected at the dashboard level because both the current rawState and the latest saved rawState are in fact the same (but not he references array). An AggBased visualization, that uses the same embeddable as TSVB, suffers from the same problem. If you the switch the data view the change is not picked up. TSVB and AggBased issues are both regression coming from elastic#215947 (I tested before and after that PR) Lens probably has the same issue, but due to a wrong implementation, the `rawState` also has it's own copy of the `references` array that is replaced at the time of the references injection, but that is not removed at the time of the reference extractions (yes it is saved in the SO as part of the attributes and could differ from the dashboard reference array if a reference id has been changed in the dashboard). (I had also a discussion with @nickpeihl about this wrong behaviour). So basically Lens, for a wrong implementation, doesn't suffer of this problem, but if we are going to implement the reference extraction correctly will suffer of this issue. The implementation is simple, but can be definitely improved. One thing that is not clear now is the following: The dashboard is saved after the first click to the `Save` button, but it will keep showing the `usaved changes` until: - you click save again - or you refresh the page So basically the changes are correctly saved to ES but the latestSavedState is not updated immediately. I believe this is an issue with the current `Spaghetti Observable` used here. fix elastic#228203 ## Release Note Fixes a bug that prevented saving linked TSVB visualizations when change data view. --------- Co-authored-by: Nick Partridge <nicholas.partridge@elastic.co> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com> (cherry picked from commit 2747360) # Conflicts: # src/platform/plugins/shared/dashboard/public/dashboard_api/get_dashboard_api.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #205531
Closes #219877.
Closes #213153
Closes #150920
Closes #203130
Overview
The embeddable framework has two types of state:
SerializedStateandRuntimeState.SerializedStateis the form of the state when saved into a Dashboard saved object. I.e. the References are extracted, and state saved externally (by reference) is removed. In contrastRuntimeStateis an exact snapshot of the state used by the embeddable to render.Exposing SerializedState and RuntimeState was a mistake that caused numerous regressions and architectural complexities.
This PR simplifies the embeddable framework by only exposing
SerializedState.RuntimeStatestays localized to the embeddable implementation and is never leaked to the embeddable framework.Whats changed
ReactEmbeddableFactory<SerializedState, RuntimeState, Api>=>EmbeddableFactory<SerializedState, Api>deserializeStateremoved from embeddable factory. Instead,SerializedStateis passed directly intobuildEmbeddable.buildEmbeddableparameterbuildApireplaced withfinalizeApi.buildApi({ api, comparators })=>finalizeApi(api).RuntimeStateto setup and monitor unsaved changes. Now, unsaved changes setup is pushed down to the embeddable implementation since the embeddable framework no longer has knowledge of embeddable RuntimeState.Reviewer instructions
Please prioritize reviews. This is a large effort from our team and is blocking many other initiatives. Getting this merged is a top priority.
This is a large change that would best be reviewed by manually testing the changes