[Embeddables Rebuild] Fix sharing unchanged panels #184873
Conversation
| // this dashboard has unsaved panels. | ||
| const allUnsavedPanelsMap = { | ||
| ...latestPanels, | ||
| ...(unsavedDashboardState?.panels ?? {}), |
There was a problem hiding this comment.
Why are you spreading unsavedDashboardState?.panels? How would unsavedDashboardState?.panels be different from panelModifications?
There was a problem hiding this comment.
This is pretty messy right now. Likely we can clean this up majorly once the legacy embeddable system is removed.
latestPanelsis the latest panels directly frominput.unsavedDashboardState?.panelsis the full panels listing if there are any unsaved changes to panels. This is from the legacy system.panelModificationsis a map of all react embeddables with unsaved changes, where each key has been modified.
We can determine if there are any unsaved panels at all by checking if panelModifications is populated or if unsavedDashboardState.panels is populated.
Then if either is populated we return the complete panels. When spreading, we can probably omit unsavedDashboardState?.panels - not because it's covered in panelModifications, but because it would be covered by latestPanels.
Let me test this.
In the future, when the old system is removed we will serialize panelModifications here directly because that will contain everything. Then on load time we can zip those with the panels loaded from the saved object rather than replacing them wholesale.
There was a problem hiding this comment.
Update, yes I could technically remove this, just had to update a test to better match the actual behaviour. Done in a323be8
| : {}; | ||
|
|
||
| // this dashboard has unsaved panels. | ||
| const allUnsavedPanelsMap = { |
There was a problem hiding this comment.
Should allUnsavedPanelsMap be renamed since it contains saved and unsaved panel state?
There was a problem hiding this comment.
I can rename it sure. The reason it's still called unsaved is because it's only populated when there are some unsaved changes to panels. Otherwise it's undefined. I.e. fall back to the panels from the SO.
There was a problem hiding this comment.
Maybe a rename is not needed - just a comment explaining what this is and why/when its used
ThomThomson
added
Feature:Dashboard
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
|
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
To update your PR or re-run it, just comment with: |
kibanamachine
added
v8.15.0
backport:skip
Summary
Fixes #184870
Fixes a regression introduced in #184264 where if there were any unsaved changes to a react embeddable, any panels without unsaved changes would be missing.