EnxDev commented on code in PR #45235:
URL: https://github.com/apache/superset/pull/45235#discussion_r4255950135
##########
superset-frontend/src/dashboard/actions/dashboardState.test.ts:
##########
@@ -178,6 +180,51 @@ describe('dashboardState actions', () => {
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
describe('saveDashboardRequest', () => {
+ test.each([false, true])(
+ 'Save As preserves scoping without updating the source
(duplicate_slices=%s)',
+ async duplicateSlices => {
+ const sourceId = 192;
+ const copyId = 193;
+ const { getState, state } = setup({
+ dashboardInfo: { ...mockState.dashboardInfo, id: sourceId },
+ });
+ const dispatch: jest.Mock = jest.fn(action =>
+ typeof action === 'function' ? action(dispatch, getState) : action,
+ );
+ postStub.mockResolvedValue({
+ json: { result: { id: copyId, last_modified_time: 123 } },
+ } as unknown as JsonResponse);
+ const { chartConfiguration, globalChartConfiguration } =
+ getCrossFiltersConfiguration(
+ state.dashboardLayout.present,
Review Comment:
`tsc` rejects this argument (TS2345 at 199:13), and that's what is failing
lint-frontend. The mock layout's `meta: {}` doesn't satisfy `DashboardLayout`.
A cast like the one on the metadata argument just below should clear it.
##########
superset-frontend/src/dashboard/actions/dashboardState.ts:
##########
@@ -766,11 +757,17 @@ export function saveDashboardRequest(
(cleanedData.metadata as JsonObject).default_filters =
safeStringify(serializedFilters);
(cleanedData.metadata as JsonObject).filter_scopes =
serializedFilterScopes;
+ const { chartConfiguration, globalChartConfiguration } =
+ handleChartConfiguration();
const copyPayload = {
dashboard_title: cleanedData.dashboard_title,
css: cleanedData.css,
duplicate_slices: cleanedData.duplicate_slices,
- json_metadata: JSON.stringify(cleanedData.metadata),
+ json_metadata: JSON.stringify({
+ ...cleanedData.metadata,
+ chart_configuration: chartConfiguration,
Review Comment:
With `duplicate_slices` on, this config is keyed by the source chart ids.
`copy_dashboard` remaps `positions` and `filter_scopes` through
`old_to_new_slice_ids`, but `chart_configuration` passes straight through
`set_dash_metadata`, so custom per-chart scoping is lost when the copy loads.
Not new, the copy already inherited the same ids from
`original_dash.params`. But the `duplicate_slices=true` test case is called
"preserves scoping" and asserts the source-keyed config. Remap it in the DAO,
or rename the case so it doesn't claim that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]