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.

Sign up for GitHub

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

[Embeddables Rebuild] Fix sharing unchanged panels #184873

Merged
merged 5 commits into from
Jun 7, 2024
Merged

[Embeddables Rebuild] Fix sharing unchanged panels #184873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
621dbd9
Fix sharing unchanged panels
ThomThomson Jun 5, 2024
417e5d2
refactor for readability + not to include panels when not necessary
ThomThomson Jun 5, 2024
a323be8
review feedback
ThomThomson Jun 6, 2024
56049f4
add comment
ThomThomson Jun 6, 2024
441bd81
Merge branch 'main' into embeddablesRebuild/fixSharingUnchangedPanels
kibanamachine Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/plugins/dashboard/public/dashboard_app/top_nav/share/show_share_modal.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,20 @@ describe('ShowShareModal', () => {
query: { query: 'bye', language: 'kuery' },
} as unknown as DashboardContainerInput;
const showModalProps = getPropsAndShare(unsavedDashboardState);
showModalProps.getDashboardState = () => {
return {
panels: {
panel_1: {
type: 'panel_type',
gridData: { w: 0, h: 0, x: 0, y: 0, i: '0' },
panelRefName: 'superPanel',
explicitInput: {
id: 'superPanel',
},
},
},
} as unknown as DashboardContainerInput;
};
ShowShareModal(showModalProps);
expect(toggleShareMenuSpy).toHaveBeenCalledTimes(1);
const shareLocatorParams = (
Expand Down Expand Up @@ -175,6 +189,14 @@ describe('ShowShareModal', () => {
changedKey2: 'definitely NOT changed',
},
},
panel_3: {
gridData: { w: 0, h: 0, x: 0, y: 0, i: '0' },
type: 'superType',
explicitInput: {
id: 'whatever2',
changedKey3: 'should still exist',
},
},
},
} as unknown as DashboardContainerInput;
};
Expand All @@ -197,5 +219,6 @@ describe('ShowShareModal', () => {
expect(shareLocatorParams.panels).toBeDefined();
expect(shareLocatorParams.panels![0].embeddableConfig.changedKey1).toBe('changed');
expect(shareLocatorParams.panels![1].embeddableConfig.changedKey2).toBe('definitely changed');
expect(shareLocatorParams.panels![2].embeddableConfig.changedKey3).toBe('should still exist');
});
});
62 changes: 36 additions & 26 deletions src/plugins/dashboard/public/dashboard_app/top_nav/share/show_share_modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,40 +127,50 @@ export function ShowShareModal({

let unsavedStateForLocator: DashboardLocatorParams = {};

const { dashboardState: unsavedDashboardState, panels: unsavedPanelStates } =
const { dashboardState: unsavedDashboardState, panels: panelModifications } =
dashboardBackup.getState(savedObjectId) ?? {};

const latestPanels = getDashboardState().panels;
const unsavedPanelsMap = unsavedPanelStates
? Object.entries(unsavedPanelStates).reduce((acc, [panelId, unsavedPanel]) => {
if (unsavedPanel && latestPanels?.[panelId]) {
acc[panelId] = {
...latestPanels[panelId],
explicitInput: {
...latestPanels?.[panelId].explicitInput,
...unsavedPanel,
id: panelId,
},
};
}
return acc;
}, {} as DashboardPanelMap)
: {};

const allPanels: DashboardPanelMap = {
...(unsavedDashboardState?.panels ?? {}),
...unsavedPanelsMap,
};
const allUnsavedPanels = (() => {
if (
Object.keys(unsavedDashboardState?.panels ?? {}).length === 0 &&
Object.keys(panelModifications ?? {}).length === 0
) {
// if this dashboard has no modifications or unsaved panels return early. No overrides needed.
return;
}

const latestPanels = getDashboardState().panels;
// apply modifications to panels.
const modifiedPanels = panelModifications
? Object.entries(panelModifications).reduce((acc, [panelId, unsavedPanel]) => {
if (unsavedPanel && latestPanels?.[panelId]) {
acc[panelId] = {
...latestPanels[panelId],
explicitInput: {
...latestPanels?.[panelId].explicitInput,
...unsavedPanel,
id: panelId,
},
};
}
return acc;
}, {} as DashboardPanelMap)
: {};

// The latest state of panels to share. This will overwrite panels from the saved object on Dashboard load.
const allUnsavedPanelsMap = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should allUnsavedPanelsMap be renamed since it contains saved and unsaved panel state?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a rename is not needed - just a comment explaining what this is and why/when its used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 56049f4

...latestPanels,
...modifiedPanels,
};
return convertPanelMapToSavedPanels(allUnsavedPanelsMap);
})();

if (unsavedDashboardState) {
unsavedStateForLocator = {
query: unsavedDashboardState.query,
filters: unsavedDashboardState.filters,
controlGroupInput: unsavedDashboardState.controlGroupInput as SerializableControlGroupInput,
panels:
allPanels && Object.keys(allPanels).length > 0
? (convertPanelMapToSavedPanels(allPanels) as DashboardLocatorParams['panels'])
: undefined,
panels: allUnsavedPanels as DashboardLocatorParams['panels'],

// options
useMargins: unsavedDashboardState?.useMargins,
Expand Down