Re: [PR] fix(alerts): fix error toast when editing report with saved tab selection [superset]

["sadpandajoe (via GitHub)" <gi...@apache.org>]

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


##########
superset-frontend/src/features/alerts/AlertReportModal.test.tsx:
##########
@@ -105,8 +107,8 @@ const generateMockPayload = (dashboard = true) => {
 const FETCH_DASHBOARD_ENDPOINT = 'glob:*/api/v1/report/1';
 const FETCH_CHART_ENDPOINT = 'glob:*/api/v1/report/2';
 
-fetchMock.get(FETCH_DASHBOARD_ENDPOINT, { result: generateMockPayload(true) });
-fetchMock.get(FETCH_CHART_ENDPOINT, { result: generateMockPayload(false) });
+fetchMock.get(FETCH_DASHBOARD_ENDPOINT, { result: generateMockPayload(true) }, { name: FETCH_DASHBOARD_ENDPOINT });
+fetchMock.get(FETCH_CHART_ENDPOINT, { result: generateMockPayload(false) }, { name: FETCH_CHART_ENDPOINT });

Review Comment:
   Thanks for the catch! The code is already formatted in multi-line style — the diff hunk may have been from an earlier revision before the merge commit.



##########
superset-frontend/src/features/alerts/AlertReportModal.test.tsx:
##########
@@ -793,3 +799,437 @@ test('filter reappears in dropdown after clearing with X icon', async () => {
     ).toBeInTheDocument();
   });
 });
+
+const setupAnchorMocks = (
+  nativeFilters: Record<string, unknown>,
+  anchor = 'TAB-abc',
+  tabsOverride?: {
+    all_tabs: Record<string, string>;
+    tab_tree: { title: string; value: string }[];
+  },
+) => {
+  const payloadWithAnchor = {
+    ...generateMockPayload(true),
+    extra: { dashboard: { anchor } },
+  };
+
+  const defaultTabs = {
+    all_tabs: { [anchor]: `Tab ${anchor}` },
+    tab_tree: [{ title: `Tab ${anchor}`, value: anchor }],
+  };
+  const tabs = tabsOverride ?? defaultTabs;
+
+  // Only replace the named routes that need anchor-specific overrides;
+  // unnamed related-endpoint routes (owners, database, etc.) stay intact.
+  fetchMock.removeRoute(FETCH_DASHBOARD_ENDPOINT);
+  fetchMock.removeRoute(FETCH_CHART_ENDPOINT);
+  fetchMock.removeRoute(tabsEndpoint);
+
+  fetchMock.get(
+    FETCH_DASHBOARD_ENDPOINT,
+    { result: payloadWithAnchor },
+    { name: FETCH_DASHBOARD_ENDPOINT },
+  );
+  fetchMock.get(
+    FETCH_CHART_ENDPOINT,
+    { result: generateMockPayload(false) },
+    { name: FETCH_CHART_ENDPOINT },
+  );
+  fetchMock.get(
+    tabsEndpoint,
+    {
+      result: {
+        ...tabs,
+        native_filters: nativeFilters,
+      },
+    },
+    { name: tabsEndpoint },
+  );
+};
+
+const restoreAnchorMocks = () => {
+  fetchMock.removeRoute(FETCH_DASHBOARD_ENDPOINT);
+  fetchMock.get(
+    FETCH_DASHBOARD_ENDPOINT,
+    { result: generateMockPayload(true) },
+    { name: FETCH_DASHBOARD_ENDPOINT },
+  );
+  fetchMock.removeRoute(FETCH_CHART_ENDPOINT);
+  fetchMock.get(
+    FETCH_CHART_ENDPOINT,
+    { result: generateMockPayload(false) },
+    { name: FETCH_CHART_ENDPOINT },
+  );
+  fetchMock.removeRoute(tabsEndpoint);
+  fetchMock.get(
+    tabsEndpoint,
+    { result: { all_tabs: {}, tab_tree: [] } },
+    { name: tabsEndpoint },
+  );
+};
+
+test('no error toast when anchor tab has no scoped native filters', async () => {
+  setupAnchorMocks({
+    all: [
+      {
+        id: 'NATIVE_FILTER-1',
+        name: 'Filter 1',
+        filterType: 'filter_select',
+        targets: [{ column: { name: 'col' } }],
+        adhoc_filters: [],
+      },
+    ],
+  });
+
+  const store = createStore({}, reducerIndex);
+
+  try {
+    render(<AlertReportModal {...generateMockedProps(true, true)} />, {
+      store,
+    });
+
+    userEvent.click(screen.getByTestId('contents-panel'));
+    await screen.findByText(/test dashboard/i);
+
+    await waitFor(() => {
+      expect(
+        fetchMock.callHistory.calls().some(c =>
+          c.url.includes('/dashboard/1/tabs'),
+        ),
+      ).toBe(true);
+    });

Review Comment:
   Good eye! The `setupAnchorMocks` helper already calls `fetchMock.callHistory.clear()` at the top (line 935), which prevents false positives from prior tests' call history. The diff hunk in the review was truncated before that line.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org