Skip to content

[visualize] fix Save to library action from a by value panel breaks the chart panel#210125

Merged
nreese merged 8 commits intoelastic:mainfrom
nreese:issue_206921
Feb 13, 2025
Merged

[visualize] fix Save to library action from a by value panel breaks the chart panel#210125
nreese merged 8 commits intoelastic:mainfrom
nreese:issue_206921

Conversation

@nreese
Copy link
Copy Markdown
Contributor

@nreese nreese commented Feb 7, 2025 •
edited by kibanamachine
Loading

Fixes #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.

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Feb 7, 2025

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Feb 7, 2025

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Feb 7, 2025

/ci

@nreese nreese marked this pull request as ready for review February 7, 2025 18:52
@nreese nreese requested a review from a team as a code owner February 7, 2025 18:52
@nreese nreese added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// label Feb 7, 2025
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Feb 7, 2025

Pinging @elastic/kibana-presentation (Team:Presentation)

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Feb 10, 2025

@elasticmachine merge upstream

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Feb 12, 2025

@elasticmachine merge upstream

dej611
dej611 approved these changes Feb 12, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@dej611 dej611 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review looks 👍

Tested in dashboard with the following scenarios and all passed:

  • Started with a by-value agg-based panel
    • Click edit => Cancel ✅
    • Click edit => Save and Return ✅
    • Click edit => Save to library ✅
    • Click edit => Changed => Reset ✅
    • Save to library => Click edit => Cancel => Unlink from library ✅
    • Save to library => Click edit => Save and return => Unlink from library ✅
    • Save to library => Click edit => Reset
    • Save to library => Unlink from library => Click edit ✅
    • Save to library => Unlink from library => Click edit => Save as ✅
    • Save to library => Unlink from library => Cancel ✅

Loaded then a by-reference panel and tested again all scenarios above (Save by library initial step was skipped in this case).

Tried also to change title, inspect and perform other actions before/after unlinking or Reset and I could not break it.

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Feb 12, 2025

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
visualizations 507 506 -1

Async chunks

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

id before after diff
visualizations 366.6KB 369.7KB +3.1KB

Page load bundle

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

id before after diff
visualizations 38.5KB 38.7KB +138.0B
Unknown metric groups

async chunk count

id before after diff
visualizations 19 20 +1

History

@nreese nreese merged commit 109dcce into elastic:main Feb 13, 2025
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 13, 2025

Starting backport for target branches: 8.16, 8.17, 8.18, 8.x, 9.0

https://github.com/elastic/kibana/actions/runs/13313171547

nreese added a commit to nreese/kibana that referenced this pull request Feb 13, 2025
@nreese
[visualize] fix Save to library action from a by value panel breaks t…
bb0836f 
…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
@nreese nreese mentioned this pull request Feb 13, 2025
Merged
nreese added a commit to nreese/kibana that referenced this pull request Feb 13, 2025
@nreese
[visualize] fix Save to library action from a by value panel breaks t…
640c832 
…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
@nreese nreese mentioned this pull request Feb 13, 2025
Merged
@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Feb 13, 2025

💚 All backports created successfully

Status Branch Result
✅ 8.17
✅ 8.16

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Feb 13, 2025
@kibanamachine @nreese
[9.0] [visualize] fix Save to library action from a by value panel br…
2b69ea0 
…eaks the chart panel (#210125) (#211052)

# Backport

This will backport the following commits from `main` to `9.0`:
- [[visualize] fix Save to library action from a by value panel breaks
the chart panel
(#210125)](#210125)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/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","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":["9.0","8.18","8.x","8.17","8.16"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"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,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.16","label":"v8.16.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Nathan Reese <reese.nathan@elastic.co>
kibanamachine added a commit that referenced this pull request Feb 13, 2025
@kibanamachine @nreese
[8.x] [visualize] fix Save to library action from a by value panel br…
1b7c334 
…eaks the chart panel (#210125) (#211051)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[visualize] fix Save to library action from a by value panel breaks
the chart panel
(#210125)](#210125)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/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","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":["9.0","8.18","8.x","8.17","8.16"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"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,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.16","label":"v8.16.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Nathan Reese <reese.nathan@elastic.co>
This was referenced Feb 13, 2025
Merged
Merged
Merged
@nreese nreese mentioned this pull request Feb 14, 2025
Merged
nreese added a commit that referenced this pull request Feb 14, 2025
@nreese @kibanamachine
[8.16] [visualize] fix Save to library action from a by value panel b…
0f28744 
…reaks the chart panel (#210125) (#211057)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[visualize] fix Save to library action from a by value panel breaks
the chart panel
(#210125)](#210125)

<!--- Backport version: 9.6.4 -->

### 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","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":["8.17","8.16"],"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":"OPEN"},{"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":"OPEN"},{"branch":"8.17","label":"v8.17.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.16","label":"v8.16.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
nreese added a commit that referenced this pull request Feb 14, 2025
@nreese @kibanamachine
[8.17] [visualize] fix Save to library action from a by value panel b…
ed6b357 
…reaks the chart panel (#210125) (#211054)

# Backport

This will backport the following commits from `main` to `8.17`:
- [[visualize] fix Save to library action from a by value panel breaks
the chart panel
(#210125)](#210125)

<!--- Backport version: 9.6.4 -->

### 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","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":["8.17","8.16"],"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":"OPEN"},{"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":"OPEN"},{"branch":"8.17","label":"v8.17.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.16","label":"v8.16.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 17, 2025
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 17, 2025

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

nreese added a commit that referenced this pull request Feb 18, 2025
@nreese @kibanamachine @elasticmachine
[visualize] fix unsaved state when adding by-value visualize embeddab…
6789c94 
…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
@nreese @kibanamachine @elasticmachine
[visualize] fix unsaved state when adding by-value visualize embeddab…
72bccb5 
…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
@nreese @kibanamachine @elasticmachine
[visualize] fix unsaved state when adding by-value visualize embeddab…
f6d164c 
…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
@nreese @kibanamachine @elasticmachine
[visualize] fix unsaved state when adding by-value visualize embeddab…
dc645c9 
…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
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 18, 2025

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

kibanamachine added a commit that referenced this pull request Feb 18, 2025
@kibanamachine @nreese
[8.x] [visualize] fix unsaved state when adding by-value visualize em…
6231149 
…beddable to dashboard (#211264) (#211611)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[visualize] fix unsaved state when adding by-value visualize
embeddable to dashboard
(#211264)](#211264)

<!--- 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-18T16:17:42Z","message":"[visualize]
fix unsaved state when adding by-value visualize embeddable to dashboard
(#211264)\n\nFollow up to
https://github.com/elastic/kibana/pull/210125\r\n\r\n[8.16](https://github.com/elastic/kibana/pull/211057)
and\r\n[8.17](#211054) backports
for\r\nhttps://github.com//pull/210125 were failing
functional\r\ntest\r\nhttps://github.com/elastic/kibana/blob/8.17/test/functional/apps/dashboard/group1/dashboard_unsaved_listing.ts#L142.\r\nThe
functional test adds a by-value and by-reference legacy\r\nvisualization
to a new dashboard. Upon saving the dashboard, the\r\ndashboard still
showed unsaved changes.\r\n\r\nThe reason this test did not fail main
and other branches is
that\r\nhttps://github.com//pull/208116 removed the
\"by-value\"\r\npart of the test (since its no longer possible to add a
by-value legacy\r\nvisualization from within a dashboard). It is still
possible to recreate\r\nthe issue in main with the following steps\r\n1)
Click \"Visualize Library\" in left nav\r\n2) Click \"Create
visualization\" button. \r\n3) Click \"Legacy\" tab\r\n4) Click
\"Aggregation based\"\r\n5) Click \"Area\"\r\n6) Click web logs sample
data view\r\n7) Click \"Save\"\r\n8) Set title\r\n9) Under \"Add to
dashboard\", click \"New\", click save\r\n10) save dashboard. Notice how
dashboard still has unsaved changes.\r\n\r\n8.16 and 8.17 required a
[new\r\ncommit](https://github.com/elastic/kibana/pull/211054/commits/1fd631c5a30046b5ab2f63948174ec29bac6fd84)\r\nto
resolve the issue by updating the `linkedToLibrary` to
ignore\r\nundefined values.\r\n\r\nThis PR fixes the issue for the other
branches that have already
been\r\nmerged.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"6789c946885fe0b6a2e57e056831f1ff25156f73","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Visualizations","release_note:skip","v9.0.0","project:embeddableRebuild","backport:version","v8.18.0","v9.1.0","v8.19.0"],"title":"[visualize]
fix unsaved state when adding by-value visualize embeddable to
dashboard","number":211264,"url":"https://github.com/elastic/kibana/pull/211264","mergeCommit":{"message":"[visualize]
fix unsaved state when adding by-value visualize embeddable to dashboard
(#211264)\n\nFollow up to
https://github.com/elastic/kibana/pull/210125\r\n\r\n[8.16](https://github.com/elastic/kibana/pull/211057)
and\r\n[8.17](#211054) backports
for\r\nhttps://github.com//pull/210125 were failing
functional\r\ntest\r\nhttps://github.com/elastic/kibana/blob/8.17/test/functional/apps/dashboard/group1/dashboard_unsaved_listing.ts#L142.\r\nThe
functional test adds a by-value and by-reference legacy\r\nvisualization
to a new dashboard. Upon saving the dashboard, the\r\ndashboard still
showed unsaved changes.\r\n\r\nThe reason this test did not fail main
and other branches is
that\r\nhttps://github.com//pull/208116 removed the
\"by-value\"\r\npart of the test (since its no longer possible to add a
by-value legacy\r\nvisualization from within a dashboard). It is still
possible to recreate\r\nthe issue in main with the following steps\r\n1)
Click \"Visualize Library\" in left nav\r\n2) Click \"Create
visualization\" button. \r\n3) Click \"Legacy\" tab\r\n4) Click
\"Aggregation based\"\r\n5) Click \"Area\"\r\n6) Click web logs sample
data view\r\n7) Click \"Save\"\r\n8) Set title\r\n9) Under \"Add to
dashboard\", click \"New\", click save\r\n10) save dashboard. Notice how
dashboard still has unsaved changes.\r\n\r\n8.16 and 8.17 required a
[new\r\ncommit](https://github.com/elastic/kibana/pull/211054/commits/1fd631c5a30046b5ab2f63948174ec29bac6fd84)\r\nto
resolve the issue by updating the `linkedToLibrary` to
ignore\r\nundefined values.\r\n\r\nThis PR fixes the issue for the other
branches that have already
been\r\nmerged.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"6789c946885fe0b6a2e57e056831f1ff25156f73"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18","8.x"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/211264","number":211264,"mergeCommit":{"message":"[visualize]
fix unsaved state when adding by-value visualize embeddable to dashboard
(#211264)\n\nFollow up to
https://github.com/elastic/kibana/pull/210125\r\n\r\n[8.16](https://github.com/elastic/kibana/pull/211057)
and\r\n[8.17](#211054) backports
for\r\nhttps://github.com//pull/210125 were failing
functional\r\ntest\r\nhttps://github.com/elastic/kibana/blob/8.17/test/functional/apps/dashboard/group1/dashboard_unsaved_listing.ts#L142.\r\nThe
functional test adds a by-value and by-reference legacy\r\nvisualization
to a new dashboard. Upon saving the dashboard, the\r\ndashboard still
showed unsaved changes.\r\n\r\nThe reason this test did not fail main
and other branches is
that\r\nhttps://github.com//pull/208116 removed the
\"by-value\"\r\npart of the test (since its no longer possible to add a
by-value legacy\r\nvisualization from within a dashboard). It is still
possible to recreate\r\nthe issue in main with the following steps\r\n1)
Click \"Visualize Library\" in left nav\r\n2) Click \"Create
visualization\" button. \r\n3) Click \"Legacy\" tab\r\n4) Click
\"Aggregation based\"\r\n5) Click \"Area\"\r\n6) Click web logs sample
data view\r\n7) Click \"Save\"\r\n8) Set title\r\n9) Under \"Add to
dashboard\", click \"New\", click save\r\n10) save dashboard. Notice how
dashboard still has unsaved changes.\r\n\r\n8.16 and 8.17 required a
[new\r\ncommit](https://github.com/elastic/kibana/pull/211054/commits/1fd631c5a30046b5ab2f63948174ec29bac6fd84)\r\nto
resolve the issue by updating the `linkedToLibrary` to
ignore\r\nundefined values.\r\n\r\nThis PR fixes the issue for the other
branches that have already
been\r\nmerged.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"6789c946885fe0b6a2e57e056831f1ff25156f73"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Nathan Reese <reese.nathan@elastic.co>
@nreese nreese mentioned this pull request Feb 18, 2025
Merged
@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Feb 18, 2025

💚 All backports created successfully

Status Branch Result
✅ 8.18

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

nreese added a commit to nreese/kibana that referenced this pull request Feb 18, 2025
@nreese @elasticmachine
[visualize] fix Save to library action from a by value panel breaks t…
743123d 
…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)
nreese added a commit that referenced this pull request Feb 19, 2025
@nreese
[8.18] [visualize] fix Save to library action from a by value panel b…
ae1aab1 
…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-->
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 19, 2025
kibanamachine added a commit that referenced this pull request Feb 20, 2025
@kibanamachine @nreese @elasticmachine
[9.0] [visualize] fix unsaved state when adding by-value visualize em…
f7f8976 
…beddable to dashboard (#211264) (#211612)

# Backport

This will backport the following commits from `main` to `9.0`:
- [[visualize] fix unsaved state when adding by-value visualize
embeddable to dashboard
(#211264)](#211264)

<!--- 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-18T16:17:42Z","message":"[visualize]
fix unsaved state when adding by-value visualize embeddable to dashboard
(#211264)\n\nFollow up to
https://github.com/elastic/kibana/pull/210125\r\n\r\n[8.16](https://github.com/elastic/kibana/pull/211057)
and\r\n[8.17](#211054) backports
for\r\nhttps://github.com//pull/210125 were failing
functional\r\ntest\r\nhttps://github.com/elastic/kibana/blob/8.17/test/functional/apps/dashboard/group1/dashboard_unsaved_listing.ts#L142.\r\nThe
functional test adds a by-value and by-reference legacy\r\nvisualization
to a new dashboard. Upon saving the dashboard, the\r\ndashboard still
showed unsaved changes.\r\n\r\nThe reason this test did not fail main
and other branches is
that\r\nhttps://github.com//pull/208116 removed the
\"by-value\"\r\npart of the test (since its no longer possible to add a
by-value legacy\r\nvisualization from within a dashboard). It is still
possible to recreate\r\nthe issue in main with the following steps\r\n1)
Click \"Visualize Library\" in left nav\r\n2) Click \"Create
visualization\" button. \r\n3) Click \"Legacy\" tab\r\n4) Click
\"Aggregation based\"\r\n5) Click \"Area\"\r\n6) Click web logs sample
data view\r\n7) Click \"Save\"\r\n8) Set title\r\n9) Under \"Add to
dashboard\", click \"New\", click save\r\n10) save dashboard. Notice how
dashboard still has unsaved changes.\r\n\r\n8.16 and 8.17 required a
[new\r\ncommit](https://github.com/elastic/kibana/pull/211054/commits/1fd631c5a30046b5ab2f63948174ec29bac6fd84)\r\nto
resolve the issue by updating the `linkedToLibrary` to
ignore\r\nundefined values.\r\n\r\nThis PR fixes the issue for the other
branches that have already
been\r\nmerged.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"6789c946885fe0b6a2e57e056831f1ff25156f73","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Visualizations","release_note:skip","v9.0.0","project:embeddableRebuild","backport:version","v8.18.0","v9.1.0","v8.19.0"],"title":"[visualize]
fix unsaved state when adding by-value visualize embeddable to
dashboard","number":211264,"url":"https://github.com/elastic/kibana/pull/211264","mergeCommit":{"message":"[visualize]
fix unsaved state when adding by-value visualize embeddable to dashboard
(#211264)\n\nFollow up to
https://github.com/elastic/kibana/pull/210125\r\n\r\n[8.16](https://github.com/elastic/kibana/pull/211057)
and\r\n[8.17](#211054) backports
for\r\nhttps://github.com//pull/210125 were failing
functional\r\ntest\r\nhttps://github.com/elastic/kibana/blob/8.17/test/functional/apps/dashboard/group1/dashboard_unsaved_listing.ts#L142.\r\nThe
functional test adds a by-value and by-reference legacy\r\nvisualization
to a new dashboard. Upon saving the dashboard, the\r\ndashboard still
showed unsaved changes.\r\n\r\nThe reason this test did not fail main
and other branches is
that\r\nhttps://github.com//pull/208116 removed the
\"by-value\"\r\npart of the test (since its no longer possible to add a
by-value legacy\r\nvisualization from within a dashboard). It is still
possible to recreate\r\nthe issue in main with the following steps\r\n1)
Click \"Visualize Library\" in left nav\r\n2) Click \"Create
visualization\" button. \r\n3) Click \"Legacy\" tab\r\n4) Click
\"Aggregation based\"\r\n5) Click \"Area\"\r\n6) Click web logs sample
data view\r\n7) Click \"Save\"\r\n8) Set title\r\n9) Under \"Add to
dashboard\", click \"New\", click save\r\n10) save dashboard. Notice how
dashboard still has unsaved changes.\r\n\r\n8.16 and 8.17 required a
[new\r\ncommit](https://github.com/elastic/kibana/pull/211054/commits/1fd631c5a30046b5ab2f63948174ec29bac6fd84)\r\nto
resolve the issue by updating the `linkedToLibrary` to
ignore\r\nundefined values.\r\n\r\nThis PR fixes the issue for the other
branches that have already
been\r\nmerged.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"6789c946885fe0b6a2e57e056831f1ff25156f73"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18","8.x"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/211264","number":211264,"mergeCommit":{"message":"[visualize]
fix unsaved state when adding by-value visualize embeddable to dashboard
(#211264)\n\nFollow up to
https://github.com/elastic/kibana/pull/210125\r\n\r\n[8.16](https://github.com/elastic/kibana/pull/211057)
and\r\n[8.17](#211054) backports
for\r\nhttps://github.com//pull/210125 were failing
functional\r\ntest\r\nhttps://github.com/elastic/kibana/blob/8.17/test/functional/apps/dashboard/group1/dashboard_unsaved_listing.ts#L142.\r\nThe
functional test adds a by-value and by-reference legacy\r\nvisualization
to a new dashboard. Upon saving the dashboard, the\r\ndashboard still
showed unsaved changes.\r\n\r\nThe reason this test did not fail main
and other branches is
that\r\nhttps://github.com//pull/208116 removed the
\"by-value\"\r\npart of the test (since its no longer possible to add a
by-value legacy\r\nvisualization from within a dashboard). It is still
possible to recreate\r\nthe issue in main with the following steps\r\n1)
Click \"Visualize Library\" in left nav\r\n2) Click \"Create
visualization\" button. \r\n3) Click \"Legacy\" tab\r\n4) Click
\"Aggregation based\"\r\n5) Click \"Area\"\r\n6) Click web logs sample
data view\r\n7) Click \"Save\"\r\n8) Set title\r\n9) Under \"Add to
dashboard\", click \"New\", click save\r\n10) save dashboard. Notice how
dashboard still has unsaved changes.\r\n\r\n8.16 and 8.17 required a
[new\r\ncommit](https://github.com/elastic/kibana/pull/211054/commits/1fd631c5a30046b5ab2f63948174ec29bac6fd84)\r\nto
resolve the issue by updating the `linkedToLibrary` to
ignore\r\nundefined values.\r\n\r\nThis PR fixes the issue for the other
branches that have already
been\r\nmerged.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"6789c946885fe0b6a2e57e056831f1ff25156f73"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Nathan Reese <reese.nathan@elastic.co>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@nreese nreese mentioned this pull request Mar 4, 2025
Closed
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Mar 22, 2025
@nreese @elasticmachine @CAWilson94
[visualize] fix Save to library action from a by value panel breaks t…
9df2f33 
…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>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Mar 22, 2025
@nreese @kibanamachine
@elasticmachine @CAWilson94
[visualize] fix unsaved state when adding by-value visualize embeddab…
dd47416 
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dej611 dej611 dej611 approved these changes

Assignees

No one assigned

Labels

backport:version Backport to applied version labels project:embeddableRebuild release_note:fix Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v8.16.5 v8.17.3 v8.18.0 v8.19.0 v9.0.0 v9.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Visualize] Save to library action from a by value panel breaks the chart panel

4 participants

@nreese @elasticmachine @kibanamachine @dej611