schizophrenicmaniac commented on code in PR #45220:
URL: https://github.com/apache/superset/pull/45220#discussion_r4403191554


##########
superset-frontend/src/dashboard/actions/dashboardState.test.ts:
##########
@@ -496,6 +500,119 @@ describe('dashboardState actions', () => {
       );
     });
 
+    test('Save As sends the cross-filter config with the copy and leaves the 
source dashboard alone', async () => {
+      const sourceDashboardId = 1;
+      const vizType = 'interactive_test_chart';
+      getChartMetadataRegistry().registerValue(
+        vizType,
+        new ChartMetadata({
+          name: 'interactive test chart',
+          thumbnail: '.png',
+          behaviors: [Behavior.InteractiveChart],
+        }),
+      );
+      const chartLayoutItem = (chartId: number) => ({
+        children: [],
+        id: `CHART-${chartId}`,
+        meta: { chartId, width: 4, height: 50 },
+        parents: ['ROOT_ID', 'GRID_ID'],
+        type: 'CHART',
+      });
+      const chart = (id: number) => ({ id, form_data: { viz_type: vizType } });
+      // The stored chartsInScope values are stale on purpose, so the copy
+      // only matches if the configuration is recomputed for the current state.
+      const state = {
+        ...mockState,
+        dashboardInfo: {
+          id: sourceDashboardId,
+          metadata: {
+            color_scheme: 'supersetColors',
+            chart_configuration: {
+              '1': {
+                id: 1,
+                crossFilters: {
+                  scope: { rootPath: ['ROOT_ID'], excluded: [2] },
+                  chartsInScope: [2],
+                },
+              },
+              '2': {
+                id: 2,
+                crossFilters: { scope: 'global', chartsInScope: [] },
+              },
+            },
+            global_chart_configuration: {
+              scope: { rootPath: ['ROOT_ID'], excluded: [] },
+              chartsInScope: [],
+            },
+          },
+        },
+        dashboardLayout: {
+          past: [],
+          present: {
+            'CHART-1': chartLayoutItem(1),
+            'CHART-2': chartLayoutItem(2),
+          },
+          future: {},
+        },
+        charts: { 1: chart(1), 2: chart(2) },
+      };
+      const getState = jest.fn(() => state) as unknown as () => any;
+      const dispatch: jest.Mock = jest.fn(action =>
+        typeof action === 'function' ? action(dispatch, getState) : action,
+      );
+      const requestStub = jest
+        .spyOn(SupersetClient, 'request')
+        .mockResolvedValue({ json: { result: {} } } as any);
+      postStub.mockRestore();
+      postStub = jest.spyOn(SupersetClient, 'post').mockResolvedValue({
+        json: { result: { id: 999, last_modified_time: 0 } },
+      } as any);
+
+      await saveDashboardRequest(
+        { ...newDashboardData, duplicate_slices: true },
+        sourceDashboardId,
+        SAVE_TYPE_NEWDASHBOARD,
+      )(dispatch, getState);
+
+      await waitFor(() => expect(mockNavigateTo).toHaveBeenCalled());
+      expect(
+        dispatch.mock.calls.some(
+          ([action]) => action?.type === 'SAVE_CHART_CONFIG_BEGIN',
+        ),
+      ).toBe(false);
+      expect(requestStub).not.toHaveBeenCalled();
+      expect(putStub).not.toHaveBeenCalled();
+      const { endpoint, jsonPayload } = postStub.mock.calls[0][0];
+      expect(endpoint).toBe(`/api/v1/dashboard/${sourceDashboardId}/copy/`);
+      const metadata = JSON.parse(jsonPayload.json_metadata);
+      const { chartConfiguration, globalChartConfiguration } =
+        getCrossFiltersConfiguration(
+          state.dashboardLayout.present as any,
+          state.dashboardInfo.metadata as any,
+          state.charts as any,
+        );
+      expect(metadata.chart_configuration).toEqual(chartConfiguration);
+      expect(metadata.global_chart_configuration).toEqual(
+        globalChartConfiguration,
+      );
+      expect(metadata.chart_configuration).toEqual({
+        '1': {
+          id: 1,
+          crossFilters: {
+            scope: { rootPath: ['ROOT_ID'], excluded: [2] },
+            chartsInScope: [],
+          },
+        },
+        '2': {
+          id: 2,
+          crossFilters: { scope: 'global', chartsInScope: [1] },
+        },
+      });
+      expect(metadata.global_chart_configuration.chartsInScope).toEqual([1, 
2]);
+      requestStub.mockRestore();

Review Comment:
   Good catch. I moved the cleanup into a finally, so the request stub is 
restored and the test viz type is removed from the registry even if an 
assertion fails. The registration now happens right before the stub so both are 
set up and torn down together.



-- 
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]

Reply via email to