Skip to content

Commit

Permalink
[dashboard] Do not reset panel to undefined or empty last saved state (
Browse files Browse the repository at this point in the history
…#203158)

Part of #201627

This is a short term fix for serverless and 8.x branches (long term fix
is an architectural change that will only be merged into 9.0 and 8.18
branches).

Fix prevents users from reseting a panel edited via embeddable transfer
service. This prevents panel from getting into an invalid state.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
3 people authored Dec 10, 2024
1 parent 418e50b commit e103a25
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ export function getPageApi() {
children$.next(children);
}
newPanels = {};
return true;
},
timeRange$,
unsavedChanges: unsavedChanges$ as PublishingSubject<object | undefined>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ import { waitFor } from '@testing-library/react';
describe('childrenUnsavedChanges$', () => {
const child1Api = {
unsavedChanges: new BehaviorSubject<object | undefined>(undefined),
resetUnsavedChanges: () => undefined,
resetUnsavedChanges: () => true,
};
const child2Api = {
unsavedChanges: new BehaviorSubject<object | undefined>(undefined),
resetUnsavedChanges: () => undefined,
resetUnsavedChanges: () => true,
};
const children$ = new BehaviorSubject<{ [key: string]: unknown }>({});
const onFireMock = jest.fn();
Expand Down Expand Up @@ -99,7 +99,7 @@ describe('childrenUnsavedChanges$', () => {
...children$.value,
child3: {
unsavedChanges: new BehaviorSubject<object | undefined>({ key1: 'modified value' }),
resetUnsavedChanges: () => undefined,
resetUnsavedChanges: () => true,
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,19 @@ export const initializeUnsavedChanges = <RuntimeState extends {} = {}>(
unsavedChanges,
resetUnsavedChanges: () => {
const lastSaved = lastSavedState$.getValue();

// Do not reset to undefined or empty last saved state
// Temporary fix for https://github.com/elastic/kibana/issues/201627
// TODO remove when architecture fix resolves issue.
if (comparatorKeys.length && (!lastSaved || Object.keys(lastSaved).length === 0)) {
return false;
}

for (const key of comparatorKeys) {
const setter = comparators[key][1]; // setter function is the 1st element of the tuple
setter(lastSaved?.[key] as RuntimeState[typeof key]);
}
return true;
},
snapshotRuntimeState,
} as PublishesUnsavedChanges<RuntimeState> & HasSnapshottableState<RuntimeState>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { PublishingSubject } from '../publishing_subject';

export interface PublishesUnsavedChanges<Runtime extends object = object> {
unsavedChanges: PublishingSubject<Partial<Runtime> | undefined>;
resetUnsavedChanges: () => void;
resetUnsavedChanges: () => boolean;
}

export const apiPublishesUnsavedChanges = (api: unknown): api is PublishesUnsavedChanges => {
Expand Down
4 changes: 3 additions & 1 deletion src/plugins/controls/public/controls/mocks/control_mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ export const getMockedBuildApi =
uuid,
parentApi: controlGroupApi ?? getMockedControlGroupApi(),
unsavedChanges: new BehaviorSubject<Partial<StateType> | undefined>(undefined),
resetUnsavedChanges: () => {},
resetUnsavedChanges: () => {
return true;
},
type: factory.type,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ describe('TimesliderControlApi', () => {
uuid,
parentApi: controlGroupApi,
unsavedChanges: new BehaviorSubject<Partial<TimesliderControlState> | undefined>(undefined),
resetUnsavedChanges: () => {},
resetUnsavedChanges: () => {
return true;
},
type: factory.type,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import { KibanaRenderContextProvider } from '@kbn/react-kibana-context-render';
import { LocatorPublic } from '@kbn/share-plugin/common';
import { ExitFullScreenButtonKibanaProvider } from '@kbn/shared-ux-button-exit-full-screen';

import { i18n } from '@kbn/i18n';
import { DASHBOARD_CONTAINER_TYPE, DashboardApi, DashboardLocatorParams } from '../..';
import type { DashboardAttributes } from '../../../server/content_management';
import { DashboardContainerInput, DashboardPanelMap, DashboardPanelState } from '../../../common';
Expand Down Expand Up @@ -957,7 +958,16 @@ export class DashboardContainer
for (const panelId of Object.keys(currentChildren)) {
if (this.getInput().panels[panelId]) {
const child = currentChildren[panelId];
if (apiPublishesUnsavedChanges(child)) child.resetUnsavedChanges();
if (apiPublishesUnsavedChanges(child)) {
const success = child.resetUnsavedChanges();
if (!success) {
coreServices.notifications.toasts.addWarning(
i18n.translate('dashboard.reset.panelError', {
defaultMessage: 'Unable to reset panel changes',
})
);
}
}
} else {
// if reset resulted in panel removal, we need to update the list of children
delete currentChildren[panelId];
Expand Down

0 comments on commit e103a25

Please sign in to comment.