Skip to content

Commit

Permalink
[8.18] [visualize] fix Save to library action from a by value panel b…
Browse files Browse the repository at this point in the history
…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":"[email protected]"},"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
<[email protected]>","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
<[email protected]>","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
<[email protected]>"}},{"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
<[email protected]>","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
<[email protected]>"}},{"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
<[email protected]>"}},{"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 <[email protected]>"}}]}]
BACKPORT-->
  • Loading branch information
nreese authored Feb 19, 2025
1 parent a99c83f commit ae1aab1
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,11 @@ import {
import { getSavedVisualization } from '../utils/saved_visualize_utils';
import type { SerializedVis } from '../vis';
import {
isVisualizeSavedObjectState,
VisualizeSavedObjectInputState,
VisualizeSerializedState,
VisualizeRuntimeState,
VisualizeSavedVisInputState,
ExtraSavedObjectProperties,
isVisualizeRuntimeState,
} from './types';

export const deserializeState = async (
Expand All @@ -49,15 +47,24 @@ export const deserializeState = async (
},
} as VisualizeRuntimeState;
let serializedState = cloneDeep(state.rawState);
if (isVisualizeSavedObjectState(serializedState)) {
serializedState = await deserializeSavedObjectState(serializedState);
} else if (isVisualizeRuntimeState(serializedState)) {
if ((serializedState as VisualizeSavedObjectInputState).savedObjectId) {
serializedState = await deserializeSavedObjectState(
serializedState as VisualizeSavedObjectInputState
);
} else if ((serializedState as VisualizeRuntimeState).serializedVis) {
// TODO remove once embeddable only exposes SerializedState
// Canvas passes incoming embeddable state in getSerializedStateForChild
// without this early return, serializedVis gets replaced in deserializeSavedVisState
// and breaks adding a new by-value embeddable in Canvas
return serializedState as VisualizeRuntimeState;
}

const references: Reference[] = state.references ?? [];

const deserializedSavedVis = deserializeSavedVisState(serializedState, references);
const deserializedSavedVis = deserializeSavedVisState(
serializedState as VisualizeSavedVisInputState,
references
);

return {
...serializedState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,26 +71,6 @@ export type VisualizeOutputState = VisualizeSavedVisInputState &
Required<Omit<SerializedTitles, 'hidePanelTitles'>> &
ExtraSavedObjectProperties;

export const isVisualizeSavedObjectState = (
state: unknown
): state is VisualizeSavedObjectInputState => {
return (
typeof state !== 'undefined' &&
(state as VisualizeSavedObjectInputState).savedObjectId !== undefined &&
!!(state as VisualizeSavedObjectInputState).savedObjectId &&
!('savedVis' in (state as VisualizeSavedObjectInputState)) &&
!('serializedVis' in (state as VisualizeSavedObjectInputState))
);
};

export const isVisualizeRuntimeState = (state: unknown): state is VisualizeRuntimeState => {
return (
!isVisualizeSavedObjectState(state) &&
!('savedVis' in (state as VisualizeRuntimeState)) &&
(state as VisualizeRuntimeState).serializedVis !== undefined
);
};

export type VisualizeApi = Partial<HasEditCapabilities> &
PublishesDataViews &
PublishesDataLoading &
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ import {
VisualizeOutputState,
VisualizeRuntimeState,
VisualizeSerializedState,
isVisualizeSavedObjectState,
} from './types';
import { initializeEditApi } from './initialize_edit_api';

Expand All @@ -67,15 +66,11 @@ export const getVisualizeEmbeddableFactory: (deps: {
}) => ({
type: VISUALIZE_EMBEDDABLE_TYPE,
deserializeState,
buildEmbeddable: async (initialState: unknown, buildApi, uuid, parentApi) => {
// Handle state transfer from legacy visualize editor, which uses the legacy visualize embeddable and doesn't
// produce a snapshot state. If buildEmbeddable is passed only a savedObjectId in the state, this means deserializeState
// was never run, and it needs to be invoked manually
const state = isVisualizeSavedObjectState(initialState)
? await deserializeState({
rawState: initialState,
})
: (initialState as VisualizeRuntimeState);
buildEmbeddable: async (initialState, buildApi, uuid, parentApi) => {
const state = {
...initialState,
linkedToLibrary: Boolean(initialState.savedObjectId),
};

// Initialize dynamic actions
const dynamicActionsApi = embeddableEnhancedStart?.initializeReactEmbeddableDynamicActions(
Expand Down Expand Up @@ -313,7 +308,13 @@ export const getVisualizeEmbeddableFactory: (deps: {
},
],
savedObjectProperties: getUnchangingComparator(),
linkedToLibrary: [linkedToLibrary$, (value) => linkedToLibrary$.next(value)],
linkedToLibrary: [
linkedToLibrary$,
(value) => linkedToLibrary$.next(value),
(a, b) => {
return a === undefined || b === undefined ? true : a === b;
},
],
}
);

Expand Down
9 changes: 7 additions & 2 deletions src/platform/plugins/shared/visualizations/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -406,10 +406,15 @@ export class VisualizationsPlugin
return getVisualizeEmbeddableFactory({ embeddableStart, embeddableEnhancedStart });
});
embeddable.registerAddFromLibraryType<VisualizationSavedObjectAttributes>({
onAdd: (container, savedObject) => {
onAdd: async (container, savedObject) => {
const { deserializeState } = await import('./embeddable/state');
const initialState = await deserializeState({
rawState: { savedObjectId: savedObject.id },
references: savedObject.references,
});
container.addNewPanel<VisualizeSerializedState>({
panelType: VISUALIZE_EMBEDDABLE_TYPE,
initialState: { savedObjectId: savedObject.id },
initialState,
});
},
savedObjectType: VISUALIZE_EMBEDDABLE_TYPE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,10 @@ export const getTopNavConfig = (
stateTransfer.navigateToWithEmbeddablePackage(app, {
state: {
type: VISUALIZE_EMBEDDABLE_TYPE,
input: { savedObjectId: id },
input: {
serializedVis: vis.serialize(),
savedObjectId: id,
},
embeddableId: saveOptions.copyOnSave ? undefined : embeddableId,
searchSessionId: data.search.session.getSessionId(),
},
Expand Down

0 comments on commit ae1aab1

Please sign in to comment.