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


##########
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:
   `requestStub.mockRestore()` here and the 
`getChartMetadataRegistry().remove(vizType)` just below only run if every 
assertion above passes, and the file's `afterEach` restores only post/get/put. 
If one assertion fails, the `SupersetClient.request` stub and the 
`interactive_test_chart` registration stay in place for the later tests in this 
file, so a single failure can show up as unrelated failures further down. Could 
the teardown move into a `finally` or the `afterEach`?



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