[8.16] [visualize] fix Save to library action from a by value panel breaks the chart panel (#210125)#211057
Merged
nreese merged 4 commits intoelastic:8.16from Feb 14, 2025
Merged
Conversation
…he chart panel (elastic#210125) Fixes elastic#206921 ### Problem The visualize embeddable is inconstant when passing runtime state to `buildEmbeddable`. Sometimes, only `{ savedObjectId }` is provided. The embeddable tried to work around this by calling `deserializeState` in `buildEmbeddable`. There was a different bug with the `deserializeState` guard in `buildEmbeddable` where state like `{ savedObjectId, savedVis: {} }` would not pass the guard. Dashboard adds runtime state so `savedVis` was getting added to `initialState` and thus failing the guard This resulted in the visualize embeddable trying to initialize with state `{ savedObjectId }` instead of state in the shape `{ savedObjectId, serializedVis: {} }`. This resulted in error message like "Could not read properties of undefined" when the embeddable tried to read from `state.serializedVis.type`. ### Solution The solution is to ensure that `buildEmbeddable` is always passed runtime state containing `serializedVis`. This pattern is in line with the lens embeddable. ### Test instructions * install sample web logs * create agg based visualization * create new dashboard, add agg based visualization. Open context menu of vis and select "Unlink from library". (Side note, removing legacy visualizations from add panel makes it hard to add by-value agg based visualizations to a dashboard) * save dashboard * edit agg based vis * Click "Save to library" and fill out form * Verify visualization is rendered in dashboard. --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 109dcce) # Conflicts: # src/plugins/visualizations/public/plugin.ts
Contributor
💔 Build Failed
Failed CI StepsTest Failures
Metrics [docs]Module Count
Async chunks
Page load bundle
History
|
Heenawter
approved these changes
Feb 13, 2025
nreese
added a commit
that referenced
this pull request
Feb 18, 2025
…le to dashboard (#211264) Follow up to #210125 [8.16](#211057) and [8.17](#211054) backports for #210125 were failing functional test https://github.com/elastic/kibana/blob/8.17/test/functional/apps/dashboard/group1/dashboard_unsaved_listing.ts#L142. The functional test adds a by-value and by-reference legacy visualization to a new dashboard. Upon saving the dashboard, the dashboard still showed unsaved changes. The reason this test did not fail main and other branches is that #208116 removed the "by-value" part of the test (since its no longer possible to add a by-value legacy visualization from within a dashboard). It is still possible to recreate the issue in main with the following steps 1) Click "Visualize Library" in left nav 2) Click "Create visualization" button. 3) Click "Legacy" tab 4) Click "Aggregation based" 5) Click "Area" 6) Click web logs sample data view 7) Click "Save" 8) Set title 9) Under "Add to dashboard", click "New", click save 10) save dashboard. Notice how dashboard still has unsaved changes. 8.16 and 8.17 required a [new commit](1fd631c) to resolve the issue by updating the `linkedToLibrary` to ignore undefined values. This PR fixes the issue for the other branches that have already been merged. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
kibanamachine
added a commit
to kibanamachine/kibana
that referenced
this pull request
Feb 18, 2025
…le to dashboard (elastic#211264) Follow up to elastic#210125 [8.16](elastic#211057) and [8.17](elastic#211054) backports for elastic#210125 were failing functional test https://github.com/elastic/kibana/blob/8.17/test/functional/apps/dashboard/group1/dashboard_unsaved_listing.ts#L142. The functional test adds a by-value and by-reference legacy visualization to a new dashboard. Upon saving the dashboard, the dashboard still showed unsaved changes. The reason this test did not fail main and other branches is that elastic#208116 removed the "by-value" part of the test (since its no longer possible to add a by-value legacy visualization from within a dashboard). It is still possible to recreate the issue in main with the following steps 1) Click "Visualize Library" in left nav 2) Click "Create visualization" button. 3) Click "Legacy" tab 4) Click "Aggregation based" 5) Click "Area" 6) Click web logs sample data view 7) Click "Save" 8) Set title 9) Under "Add to dashboard", click "New", click save 10) save dashboard. Notice how dashboard still has unsaved changes. 8.16 and 8.17 required a [new commit](elastic@1fd631c) to resolve the issue by updating the `linkedToLibrary` to ignore undefined values. This PR fixes the issue for the other branches that have already been merged. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 6789c94)
kibanamachine
added a commit
to kibanamachine/kibana
that referenced
this pull request
Feb 18, 2025
…le to dashboard (elastic#211264) Follow up to elastic#210125 [8.16](elastic#211057) and [8.17](elastic#211054) backports for elastic#210125 were failing functional test https://github.com/elastic/kibana/blob/8.17/test/functional/apps/dashboard/group1/dashboard_unsaved_listing.ts#L142. The functional test adds a by-value and by-reference legacy visualization to a new dashboard. Upon saving the dashboard, the dashboard still showed unsaved changes. The reason this test did not fail main and other branches is that elastic#208116 removed the "by-value" part of the test (since its no longer possible to add a by-value legacy visualization from within a dashboard). It is still possible to recreate the issue in main with the following steps 1) Click "Visualize Library" in left nav 2) Click "Create visualization" button. 3) Click "Legacy" tab 4) Click "Aggregation based" 5) Click "Area" 6) Click web logs sample data view 7) Click "Save" 8) Set title 9) Under "Add to dashboard", click "New", click save 10) save dashboard. Notice how dashboard still has unsaved changes. 8.16 and 8.17 required a [new commit](elastic@1fd631c) to resolve the issue by updating the `linkedToLibrary` to ignore undefined values. This PR fixes the issue for the other branches that have already been merged. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 6789c94)
kibanamachine
added a commit
to kibanamachine/kibana
that referenced
this pull request
Feb 18, 2025
…le to dashboard (elastic#211264) Follow up to elastic#210125 [8.16](elastic#211057) and [8.17](elastic#211054) backports for elastic#210125 were failing functional test https://github.com/elastic/kibana/blob/8.17/test/functional/apps/dashboard/group1/dashboard_unsaved_listing.ts#L142. The functional test adds a by-value and by-reference legacy visualization to a new dashboard. Upon saving the dashboard, the dashboard still showed unsaved changes. The reason this test did not fail main and other branches is that elastic#208116 removed the "by-value" part of the test (since its no longer possible to add a by-value legacy visualization from within a dashboard). It is still possible to recreate the issue in main with the following steps 1) Click "Visualize Library" in left nav 2) Click "Create visualization" button. 3) Click "Legacy" tab 4) Click "Aggregation based" 5) Click "Area" 6) Click web logs sample data view 7) Click "Save" 8) Set title 9) Under "Add to dashboard", click "New", click save 10) save dashboard. Notice how dashboard still has unsaved changes. 8.16 and 8.17 required a [new commit](elastic@1fd631c) to resolve the issue by updating the `linkedToLibrary` to ignore undefined values. This PR fixes the issue for the other branches that have already been merged. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 6789c94)
nreese
added a commit
that referenced
this pull request
Feb 19, 2025
…reaks the chart panel (#210125) (#211663) # Backport This will backport the following commits from `main` to `8.18`: - [[visualize] fix Save to library action from a by value panel breaks the chart panel (#210125)](#210125) <!--- 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-02-13T17:09:29Z","message":"[visualize] fix Save to library action from a by value panel breaks the chart panel (#210125)\n\nFixes https://github.com/elastic/kibana/issues/206921\r\n\r\n### Problem\r\nThe visualize embeddable is inconstant when passing runtime state to\r\n`buildEmbeddable`. Sometimes, only `{ savedObjectId }` is provided. The\r\nembeddable tried to work around this by calling `deserializeState` in\r\n`buildEmbeddable`.\r\n\r\nThere was a different bug with the `deserializeState` guard in\r\n`buildEmbeddable` where state like `{ savedObjectId, savedVis: {} }`\r\nwould not pass the guard. Dashboard adds runtime state so `savedVis` was\r\ngetting added to `initialState` and thus failing the guard\r\n\r\nThis resulted in the visualize embeddable trying to initialize with\r\nstate `{ savedObjectId }` instead of state in the shape `{\r\nsavedObjectId, serializedVis: {} }`. This resulted in error message like\r\n\"Could not read properties of undefined\" when the embeddable tried to\r\nread from `state.serializedVis.type`.\r\n\r\n### Solution\r\nThe solution is to ensure that `buildEmbeddable` is always passed\r\nruntime state containing `serializedVis`. This pattern is in line with\r\nthe lens embeddable.\r\n\r\n### Test instructions\r\n* install sample web logs\r\n* create agg based visualization\r\n* create new dashboard, add agg based visualization. Open context menu\r\nof vis and select \"Unlink from library\". (Side note, removing legacy\r\nvisualizations from add panel makes it hard to add by-value agg based\r\nvisualizations to a dashboard)\r\n* save dashboard\r\n* edit agg based vis\r\n* Click \"Save to library\" and fill out form\r\n* Verify visualization is rendered in dashboard.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"109dcce33864a4d8be2e5dc6ac088d8a9976afb5","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Presentation","backport missing","v9.0.0","project:embeddableRebuild","backport:version","v8.18.0","v9.1.0","v8.19.0","v8.17.3","v8.16.5"],"title":"[visualize] fix Save to library action from a by value panel breaks the chart panel","number":210125,"url":"https://github.com/elastic/kibana/pull/210125","mergeCommit":{"message":"[visualize] fix Save to library action from a by value panel breaks the chart panel (#210125)\n\nFixes https://github.com/elastic/kibana/issues/206921\r\n\r\n### Problem\r\nThe visualize embeddable is inconstant when passing runtime state to\r\n`buildEmbeddable`. Sometimes, only `{ savedObjectId }` is provided. The\r\nembeddable tried to work around this by calling `deserializeState` in\r\n`buildEmbeddable`.\r\n\r\nThere was a different bug with the `deserializeState` guard in\r\n`buildEmbeddable` where state like `{ savedObjectId, savedVis: {} }`\r\nwould not pass the guard. Dashboard adds runtime state so `savedVis` was\r\ngetting added to `initialState` and thus failing the guard\r\n\r\nThis resulted in the visualize embeddable trying to initialize with\r\nstate `{ savedObjectId }` instead of state in the shape `{\r\nsavedObjectId, serializedVis: {} }`. This resulted in error message like\r\n\"Could not read properties of undefined\" when the embeddable tried to\r\nread from `state.serializedVis.type`.\r\n\r\n### Solution\r\nThe solution is to ensure that `buildEmbeddable` is always passed\r\nruntime state containing `serializedVis`. This pattern is in line with\r\nthe lens embeddable.\r\n\r\n### Test instructions\r\n* install sample web logs\r\n* create agg based visualization\r\n* create new dashboard, add agg based visualization. Open context menu\r\nof vis and select \"Unlink from library\". (Side note, removing legacy\r\nvisualizations from add panel makes it hard to add by-value agg based\r\nvisualizations to a dashboard)\r\n* save dashboard\r\n* edit agg based vis\r\n* Click \"Save to library\" and fill out form\r\n* Verify visualization is rendered in dashboard.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"109dcce33864a4d8be2e5dc6ac088d8a9976afb5"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/211052","number":211052,"state":"MERGED","mergeCommit":{"sha":"2b69ea053f47e0b698884bd5d549042a5c5ad9bc","message":"[9.0] [visualize] fix Save to library action from a by value panel breaks the chart panel (#210125) (#211052)\n\n# Backport\n\nThis will backport the following commits from `main` to `9.0`:\n- [[visualize] fix Save to library action from a by value panel breaks\nthe chart panel\n(#210125)](https://github.com/elastic/kibana/pull/210125)\n\n\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sqren/backport)\n\n\n\nCo-authored-by: Nathan Reese <reese.nathan@elastic.co>"}},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/211050","number":211050,"state":"OPEN"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/210125","number":210125,"mergeCommit":{"message":"[visualize] fix Save to library action from a by value panel breaks the chart panel (#210125)\n\nFixes https://github.com/elastic/kibana/issues/206921\r\n\r\n### Problem\r\nThe visualize embeddable is inconstant when passing runtime state to\r\n`buildEmbeddable`. Sometimes, only `{ savedObjectId }` is provided. The\r\nembeddable tried to work around this by calling `deserializeState` in\r\n`buildEmbeddable`.\r\n\r\nThere was a different bug with the `deserializeState` guard in\r\n`buildEmbeddable` where state like `{ savedObjectId, savedVis: {} }`\r\nwould not pass the guard. Dashboard adds runtime state so `savedVis` was\r\ngetting added to `initialState` and thus failing the guard\r\n\r\nThis resulted in the visualize embeddable trying to initialize with\r\nstate `{ savedObjectId }` instead of state in the shape `{\r\nsavedObjectId, serializedVis: {} }`. This resulted in error message like\r\n\"Could not read properties of undefined\" when the embeddable tried to\r\nread from `state.serializedVis.type`.\r\n\r\n### Solution\r\nThe solution is to ensure that `buildEmbeddable` is always passed\r\nruntime state containing `serializedVis`. This pattern is in line with\r\nthe lens embeddable.\r\n\r\n### Test instructions\r\n* install sample web logs\r\n* create agg based visualization\r\n* create new dashboard, add agg based visualization. Open context menu\r\nof vis and select \"Unlink from library\". (Side note, removing legacy\r\nvisualizations from add panel makes it hard to add by-value agg based\r\nvisualizations to a dashboard)\r\n* save dashboard\r\n* edit agg based vis\r\n* Click \"Save to library\" and fill out form\r\n* Verify visualization is rendered in dashboard.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"109dcce33864a4d8be2e5dc6ac088d8a9976afb5"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/211051","number":211051,"state":"MERGED","mergeCommit":{"sha":"1b7c334e9035bc93ee47e43b1d739e816a95898a","message":"[8.x] [visualize] fix Save to library action from a by value panel breaks the chart panel (#210125) (#211051)\n\n# Backport\n\nThis will backport the following commits from `main` to `8.x`:\n- [[visualize] fix Save to library action from a by value panel breaks\nthe chart panel\n(#210125)](https://github.com/elastic/kibana/pull/210125)\n\n\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sqren/backport)\n\n\n\nCo-authored-by: Nathan Reese <reese.nathan@elastic.co>"}},{"branch":"8.17","label":"v8.17.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/211054","number":211054,"state":"MERGED","mergeCommit":{"sha":"ed6b3573771032ad3a37ae272be5a3bdcdfad7b0","message":"[8.17] [visualize] fix Save to library action from a by value panel breaks the chart panel (#210125) (#211054)\n\n# Backport\n\nThis will backport the following commits from `main` to `8.17`:\n- [[visualize] fix Save to library action from a by value panel breaks\nthe chart panel\n(#210125)](https://github.com/elastic/kibana/pull/210125)\n\n\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sorenlouv/backport)\n\n\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>"}},{"branch":"8.16","label":"v8.16.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/211057","number":211057,"state":"MERGED","mergeCommit":{"sha":"0f2874417ece3c735e4091f885ad6f4b5d426da1","message":"[8.16] [visualize] fix Save to library action from a by value panel breaks the chart panel (#210125) (#211057)\n\n# Backport\n\nThis will backport the following commits from `main` to `8.16`:\n- [[visualize] fix Save to library action from a by value panel breaks\nthe chart panel\n(#210125)](https://github.com/elastic/kibana/pull/210125)\n\n\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sorenlouv/backport)\n\n\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>"}}]}] BACKPORT-->
CAWilson94
pushed a commit
to CAWilson94/kibana
that referenced
this pull request
Mar 22, 2025
…le to dashboard (elastic#211264) Follow up to elastic#210125 [8.16](elastic#211057) and [8.17](elastic#211054) backports for elastic#210125 were failing functional test https://github.com/elastic/kibana/blob/8.17/test/functional/apps/dashboard/group1/dashboard_unsaved_listing.ts#L142. The functional test adds a by-value and by-reference legacy visualization to a new dashboard. Upon saving the dashboard, the dashboard still showed unsaved changes. The reason this test did not fail main and other branches is that elastic#208116 removed the "by-value" part of the test (since its no longer possible to add a by-value legacy visualization from within a dashboard). It is still possible to recreate the issue in main with the following steps 1) Click "Visualize Library" in left nav 2) Click "Create visualization" button. 3) Click "Legacy" tab 4) Click "Aggregation based" 5) Click "Area" 6) Click web logs sample data view 7) Click "Save" 8) Set title 9) Under "Add to dashboard", click "New", click save 10) save dashboard. Notice how dashboard still has unsaved changes. 8.16 and 8.17 required a [new commit](elastic@1fd631c) to resolve the issue by updating the `linkedToLibrary` to ignore undefined values. This PR fixes the issue for the other branches that have already been merged. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
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
4 participants
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.
Backport
This will backport the following commits from
mainto8.16:Questions ?
Please refer to the Backport tool documentation