Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[React embeddable] Viewing dashboards with unsaved changes can display empty panels for new by-reference embeddable panels #198853

Open
nreese opened this issue Nov 4, 2024 · 2 comments
Assignees
Labels
bug Fixes for quality problems that affect the customer experience impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:large Large Level of Effort project:embeddableRebuild Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@nreese
Copy link
Contributor

nreese commented Nov 4, 2024

See #198846 as a live example.

Problem

Below is the logic used to build embeddable runtime state.

const factory = await getReactEmbeddableFactory<SerializedState, RuntimeState, Api>(type);
const serializedState = parentApi.getSerializedStateForChild(uuid);
const lastSavedRuntimeState = serializedState
  ? await factory.deserializeState(serializedState)
  : ({} as RuntimeState);

// If the parent provides runtime state for the child (usually as a state backup or cache),
// we merge it with the last saved runtime state.
const partialRuntimeState = apiHasRuntimeChildState<RuntimeState>(parentApi)
  ? parentApi.getRuntimeStateForChild(uuid) ?? ({} as Partial<RuntimeState>)
  : ({} as Partial<RuntimeState>);

const initialRuntimeState = { ...lastSavedRuntimeState, ...partialRuntimeState };

Lets compare this logic for 2 cases with a by-reference links embeddable

Case 1 - By-reference links embeddable with no unsaved changes

  1. parentApi.getSerializedStateForChild returns { savedObjectId: '1' }
  2. factory.deserializeState loads links from saved object and returns { links: [...]}
  3. initialRuntimeState is { links: [...]}
  4. Links embeddable renders correctly because its passed runtime state.

Case 2 - Empty dashboard with unsaved changes to by-reference links embeddable

  1. parentApi.getSerializedStateForChild returns undefined because there is no saved state for the panel
  2. factory.deserializeState is not run
  3. parentApi.getRuntimeStateForChild returns { savedObjectId: '1' }
  4. initialRuntimeState is { savedObjectId: '1' }
  5. Links embeddable does not render any links because its passed a savedObjectId that has not been loaded.

Proposed solution #1

Embeddables should store all by-reference library state as unsaved changes.

Case 2 - Empty dashboard with unsaved changes to by-reference links embeddable

  1. parentApi.getSerializedStateForChild returns undefined because there is no saved state for the panel
  2. factory.deserializeState is not run
  3. parentApi.getRuntimeStateForChild returns { savedObjectId: '1', links: [...] }
  4. initialRuntimeState is { savedObjectId: '1', links: [...] }
  5. Links embeddable renders correctly because its passed runtime state.

The problem with this solution is that you are now storing library state in 2 places

  1. dashboard unsaved changes
  2. library state

Users could edit the library item and then return to the unsaved dashboard. They would rightfully be confused if the dashboard showed the unsaved changes version of the library state instead of the newly edited library state.

Proposed solution #2

Embeddables should not be expected to include all library state as unsaved changes for by-reference state. Instead, embeddables should be given the opportunity to hydrate unsaved changes state. Below is the logic that calls hydrateUnsavedChanges when provided by the factory.

const factory = await getReactEmbeddableFactory<SerializedState, RuntimeState, Api>(type);
const serializedState = parentApi.getSerializedStateForChild(uuid);
const lastSavedRuntimeState = serializedState
  ? await factory.deserializeState(serializedState)
  : ({} as RuntimeState);

// If the parent provides runtime state for the child (usually as a state backup or cache),
// we merge it with the last saved runtime state.
const partialRuntimeState = apiHasRuntimeChildState<RuntimeState>(parentApi)
  ? parentApi.getRuntimeStateForChild(uuid) ?? ({} as Partial<RuntimeState>)
  : ({} as Partial<RuntimeState>);

const initialRuntimeState = {
  ...lastSavedRuntimeState,
  ...(factory.isByReference?.(partialRuntimeState) && factory.hydrateByReferenceUnsavedChanges 
    ? await factory.hydrateByReferenceUnsavedChanges(partialRuntimeState, serializedState.references) 
    : partialRuntimeState)
};

Case 1 - By-reference links embeddable with no unsaved changes

No changes

Case 2 - Empty dashboard with unsaved changes to by-reference links embeddable

  1. parentApi.getSerializedStateForChild returns undefined because there is no saved state for the panel
  2. factory.deserializeState is not run
  3. parentApi.getRuntimeStateForChild returns { savedObjectId: '1' }
  4. factory.hydrateByReferenceUnsavedChanges loads links from saved object and returns { links: [...]}
  5. initialRuntimeState is { links: [...]}
  6. Links embeddable renders correctly because its passed runtime state.

Proposed solution #3

Update comparator tuple with a 4 element that specifies runtimeState or serializedState. Then unsaved changes can be return to help build serializedState and runtimeState. Below is an example

// links comparators
{
  savedObjectId: [
    savedObjectId$,
    (val) => savedObjectId$.next(val),
    (a, b) => a === b,
    'serializedState'
  ]
}


// ReactEmbeddableRenderer
const factory = await getReactEmbeddableFactory<SerializedState, RuntimeState, Api>(type);
const savedSerializedState = parentApi.getSavedSerializedStateForChild(uuid);
const unsavedSerializedState = parentApi.getUnsavedSerializedStateForChild(uuid);
const serializedState = savedSerializedState || unsavedSerializedState
  ? 
    {
      ...(savedSerializedState ?? {}),
      ...(unsavedSerializedState ?? {})
    }
  : undefined
const lastSavedRuntimeState = serializedState
  ? await factory.deserializeState(serializedState)
  : ({} as RuntimeState);

// If the parent provides runtime state for the child (usually as a state backup or cache),
// we merge it with the last saved runtime state.
const partialRuntimeState = apiHasRuntimeChildState<RuntimeState>(parentApi)
  ? parentApi.getUnsavedRuntimeStateForChild(uuid) ?? ({} as Partial<RuntimeState>)
  : ({} as Partial<RuntimeState>);

const initialRuntimeState = {
  ...lastSavedRuntimeState,
  ...partialRuntimeState
};

Case 1 - By-reference links embeddable with no unsaved changes

No changes

Case 2 - Empty dashboard with unsaved changes to by-reference links embeddable

  1. parentApi.getSavedSerializedStateForChild returns undefined because there is no saved state for the panel
  2. parentApi.getUnsavedSerializedStateForChild returns { savedObjectId: '1' }
  3. factory.deserializeState loads links from saved object and returns { links: [...]}
  4. parentApi.getUnsavedRuntimeStateForChild returns {...}
  5. initialRuntimeState is { links: [...]}
  6. Links embeddable renders correctly because its passed all runtime state.
@nreese nreese added bug Fixes for quality problems that affect the customer experience Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas project:embeddableRebuild labels Nov 4, 2024
@elasticmachine
Copy link
Contributor

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

@nreese
Copy link
Contributor Author

nreese commented Dec 4, 2024

Breaking out solution into 2 separate fixes based on branch

  • 9.0/8.18: architecture change - large effort
  • 8.17, 8.16, 8.15: hack fix in ReactEmbeddableRenderer to pass runtime state savedObjectId to factory.deserializeState. Do not want to backport a large architecture change to patch releases to minimize risk. Small effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:large Large Level of Effort project:embeddableRebuild Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
3 participants