This is an automated email from the ASF dual-hosted git repository. jli pushed a commit to branch fix-alert-report-tab-error in repository https://gitbox.apache.org/repos/asf/superset.git
commit e6fd186336c625e6d9200115253db8071a08c22c Author: Joe Li <[email protected]> AuthorDate: Thu Feb 19 22:23:31 2026 -0800 fix(alerts): guard against undefined tabNativeFilters and improve test robustness Defend against crash when tabs API response omits native_filters by defaulting to empty object. Strengthen stale-anchor test to assert placeholder visibility instead of weak DOM selector. Scope fetchMock route resets to named routes only, fixing unnamed route leak from filter-reappears test that caused two downstream test failures. Co-Authored-By: Claude Opus 4.6 <[email protected]> --- .../src/features/alerts/AlertReportModal.test.tsx | 473 +++++++++++++++++++-- .../src/features/alerts/AlertReportModal.tsx | 19 +- 2 files changed, 458 insertions(+), 34 deletions(-) diff --git a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx index 91adaa3eeb4..3fee7eecd34 100644 --- a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx +++ b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx @@ -23,7 +23,9 @@ import { userEvent, waitFor, within, + createStore, } from 'spec/helpers/testing-library'; +import reducerIndex from 'spec/helpers/reducerIndex'; import { buildErrorTooltipMessage } from './buildErrorTooltipMessage'; import AlertReportModal, { AlertReportModalProps } from './AlertReportModal'; import { AlertObject, NotificationMethodOption } from './types'; @@ -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 }); // Related mocks const ownersEndpoint = 'glob:*/api/v1/alert/related/owners?*'; @@ -713,32 +715,36 @@ test('filter reappears in dropdown after clearing with X icon', async () => { const chartDataEndpoint = 'glob:*/api/v1/chart/data*'; fetchMock.removeRoute(tabsEndpoint); - fetchMock.get(tabsEndpoint, { - result: { - all_tabs: { tab1: 'Tab 1' }, - tab_tree: [{ title: 'Tab 1', value: 'tab1' }], - native_filters: { - all: [ - { - id: 'NATIVE_FILTER-test1', - name: 'Test Filter 1', - filterType: 'filter_select', - targets: [{ column: { name: 'test_column_1' } }], - adhoc_filters: [], - }, - ], - tab1: [ - { - id: 'NATIVE_FILTER-test2', - name: 'Test Filter 2', - filterType: 'filter_select', - targets: [{ column: { name: 'test_column_2' } }], - adhoc_filters: [], - }, - ], + fetchMock.get( + tabsEndpoint, + { + result: { + all_tabs: { tab1: 'Tab 1' }, + tab_tree: [{ title: 'Tab 1', value: 'tab1' }], + native_filters: { + all: [ + { + id: 'NATIVE_FILTER-test1', + name: 'Test Filter 1', + filterType: 'filter_select', + targets: [{ column: { name: 'test_column_1' } }], + adhoc_filters: [], + }, + ], + tab1: [ + { + id: 'NATIVE_FILTER-test2', + name: 'Test Filter 2', + filterType: 'filter_select', + targets: [{ column: { name: 'test_column_2' } }], + adhoc_filters: [], + }, + ], + }, }, }, - }); + { name: tabsEndpoint }, + ); fetchMock.post(chartDataEndpoint, { result: [{ data: [] }] }); @@ -793,3 +799,418 @@ 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(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); + }); + + const toasts = (store.getState() as Record<string, unknown>).messageToasts as { text: string }[]; + expect( + toasts.some( + (toast: { text: string }) => + toast.text === 'There was an error retrieving dashboard tabs.', + ), + ).toBe(false); + } finally { + restoreAnchorMocks(); + } +}); + +test('no error toast when anchor tab set and dashboard has zero native filters', async () => { + setupAnchorMocks({}); + + 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); + }); + + const toasts = (store.getState() as Record<string, unknown>).messageToasts as { text: string }[]; + expect( + toasts.some( + (toast: { text: string }) => + toast.text === 'There was an error retrieving dashboard tabs.', + ), + ).toBe(false); + } finally { + restoreAnchorMocks(); + } +}); + +test('stale JSON array anchor is cleared without crash or toast', async () => { + const staleAnchor = JSON.stringify(['TAB-abc', 'TAB-missing']); + setupAnchorMocks( + { + all: [ + { + id: 'NATIVE_FILTER-1', + name: 'Filter 1', + filterType: 'filter_select', + targets: [{ column: { name: 'col' } }], + adhoc_filters: [], + }, + ], + }, + staleAnchor, + { + all_tabs: { 'TAB-abc': 'Tab ABC' }, + tab_tree: [{ title: 'Tab ABC', value: 'TAB-abc' }], + }, + ); + + const store = createStore({}, reducerIndex); + + try { + render(<AlertReportModal {...generateMockedProps(true, true)} />, { + store, + }); + + userEvent.click(screen.getByTestId('contents-panel')); + await screen.findByText(/test dashboard/i); + + // Wait for the tabs useEffect to process the stale anchor + await waitFor(() => { + expect( + fetchMock.callHistory.calls().some(c => + c.url.includes('/dashboard/1/tabs'), + ), + ).toBe(true); + }); + + // No error toast dispatched (the .then() handler ran without crashing) + const toasts = (store.getState() as Record<string, unknown>).messageToasts as { text: string }[]; + expect( + toasts.some( + (toast: { text: string }) => + toast.text === 'There was an error retrieving dashboard tabs.', + ), + ).toBe(false); + + // Verify anchor was cleared: TreeSelect shows placeholder (only visible + // when value is undefined, i.e. updateAnchorState(undefined) was called) + await waitFor(() => { + const treeSelect = document.querySelector('.ant-tree-select'); + expect(treeSelect).toBeInTheDocument(); + expect( + within(treeSelect as HTMLElement).getByText('Select a tab'), + ).toBeInTheDocument(); + }); + } finally { + restoreAnchorMocks(); + } +}); + +test('tabs API failure shows danger toast via Redux store', async () => { + fetchMock.removeRoute(tabsEndpoint); + fetchMock.get(tabsEndpoint, 500, { name: tabsEndpoint }); + + const store = createStore({}, reducerIndex); + + try { + render(<AlertReportModal {...generateMockedProps(true, true)} />, { + store, + }); + + userEvent.click(screen.getByTestId('contents-panel')); + + await waitFor(() => { + const toasts = (store.getState() as Record<string, unknown>).messageToasts as { text: string }[]; + expect( + toasts.some( + (toast: { text: string }) => + toast.text === 'There was an error retrieving dashboard tabs.', + ), + ).toBe(true); + }); + } finally { + fetchMock.removeRoute(tabsEndpoint); + fetchMock.get( + tabsEndpoint, + { result: { all_tabs: {}, tab_tree: [] } }, + { name: tabsEndpoint }, + ); + } +}); + +test('null all_tabs does not crash or show error toast', async () => { + setupAnchorMocks({ + all: [ + { + id: 'NATIVE_FILTER-1', + name: 'Filter 1', + filterType: 'filter_select', + targets: [{ column: { name: 'col' } }], + adhoc_filters: [], + }, + ], + }); + + // Override tabs endpoint with null all_tabs + fetchMock.removeRoute(tabsEndpoint); + fetchMock.get( + tabsEndpoint, + { + result: { + all_tabs: null, + tab_tree: [{ title: 'Tab ABC', value: 'TAB-abc' }], + native_filters: { + all: [ + { + id: 'NATIVE_FILTER-1', + name: 'Filter 1', + filterType: 'filter_select', + targets: [{ column: { name: 'col' } }], + adhoc_filters: [], + }, + ], + }, + }, + }, + { name: tabsEndpoint }, + ); + + const store = createStore({}, reducerIndex); + + try { + render(<AlertReportModal {...generateMockedProps(true, true)} />, { + store, + }); + + userEvent.click(screen.getByTestId('contents-panel')); + await screen.findByText(/test dashboard/i); + + // Wait for tabs useEffect to complete + await waitFor(() => { + expect( + fetchMock.callHistory.calls().some(c => + c.url.includes('/dashboard/1/tabs'), + ), + ).toBe(true); + }); + + // No error toast dispatched + const toasts = (store.getState() as Record<string, unknown>).messageToasts as { text: string }[]; + expect( + toasts.some( + (toast: { text: string }) => + toast.text === 'There was an error retrieving dashboard tabs.', + ), + ).toBe(false); + + // Component remains interactive + expect( + screen.getByRole('combobox', { name: /select filter/i }), + ).toBeInTheDocument(); + } finally { + restoreAnchorMocks(); + } +}); + +test('missing native_filters in tabs response does not crash or show error toast', async () => { + setupAnchorMocks({}); + + // Override tabs endpoint with no native_filters key + fetchMock.removeRoute(tabsEndpoint); + fetchMock.get( + tabsEndpoint, + { + result: { + all_tabs: { 'TAB-abc': 'Tab ABC' }, + tab_tree: [{ title: 'Tab ABC', value: 'TAB-abc' }], + }, + }, + { name: tabsEndpoint }, + ); + + const store = createStore({}, reducerIndex); + + try { + render(<AlertReportModal {...generateMockedProps(true, true)} />, { + store, + }); + + userEvent.click(screen.getByTestId('contents-panel')); + await screen.findByText(/test dashboard/i); + + // Wait for tabs useEffect to complete + await waitFor(() => { + expect( + fetchMock.callHistory.calls().some(c => + c.url.includes('/dashboard/1/tabs'), + ), + ).toBe(true); + }); + + // No error toast dispatched + const toasts = (store.getState() as Record<string, unknown>).messageToasts as { text: string }[]; + expect( + toasts.some( + (toast: { text: string }) => + toast.text === 'There was an error retrieving dashboard tabs.', + ), + ).toBe(false); + } finally { + restoreAnchorMocks(); + } +}); + +test('anchor tab with scoped filters loads filter options correctly', async () => { + // Use JSON-parseable non-array anchor to exercise the scoped filter + // code path at line 1108 (JSON.parse('42') → 42, not an array) + setupAnchorMocks( + { + all: [ + { + id: 'NATIVE_FILTER-1', + name: 'Global Filter', + filterType: 'filter_select', + targets: [{ column: { name: 'col' } }], + adhoc_filters: [], + }, + ], + '42': [ + { + id: 'NATIVE_FILTER-2', + name: 'Tab Scoped Filter', + filterType: 'filter_select', + targets: [{ column: { name: 'col2' } }], + adhoc_filters: [], + }, + ], + }, + '42', + ); + + const store = createStore({}, reducerIndex); + + try { + render(<AlertReportModal {...generateMockedProps(true, true)} />, { + store, + }); + + userEvent.click(screen.getByTestId('contents-panel')); + await screen.findByText(/test dashboard/i); + + const filterDropdown = screen.getByRole('combobox', { + name: /select filter/i, + }); + userEvent.click(filterDropdown); + + const filterOption = await waitFor(() => { + const virtualList = document.querySelector('.rc-virtual-list'); + return within(virtualList as HTMLElement).getByText('Tab Scoped Filter'); + }); + expect(filterOption).toBeInTheDocument(); + + const toasts = (store.getState() as Record<string, unknown>).messageToasts as { text: string }[]; + expect( + toasts.some( + (toast: { text: string }) => + toast.text === 'There was an error retrieving dashboard tabs.', + ), + ).toBe(false); + } finally { + restoreAnchorMocks(); + } +}); diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx index 3974db42e33..87ae4ef43d1 100644 --- a/superset-frontend/src/features/alerts/AlertReportModal.tsx +++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx @@ -1088,14 +1088,14 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({ } setTabOptions(tabTree); - setTabNativeFilters(nativeFilters); + setTabNativeFilters(nativeFilters ?? {}); - if (isEditMode && nativeFilters.all) { + if (isEditMode && nativeFilters?.all) { // update options for all filters addNativeFilterOptions(nativeFilters.all); // Also set the available filter options for the add button setNativeFilterOptions( - nativeFilters.all.map((filter: any) => ({ + nativeFilters.all.map((filter: NativeFilterObject) => ({ value: filter.id, label: filter.name, })), @@ -1107,8 +1107,10 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({ const parsedAnchor = JSON.parse(anchor); if (!Array.isArray(parsedAnchor)) { // only show filters scoped to anchor + const anchorFilters: NativeFilterObject[] = + nativeFilters?.[anchor] ?? []; setNativeFilterOptions( - nativeFilters[anchor].map((filter: any) => ({ + anchorFilters.map((filter: NativeFilterObject) => ({ value: filter.id, label: filter.name, })), @@ -1116,7 +1118,8 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({ } if (Array.isArray(parsedAnchor)) { // Check if all elements in parsedAnchor list are in allTabs - const isValidSubset = parsedAnchor.every(tab => tab in allTabs); + const isValidSubset = + allTabs && parsedAnchor.every(tab => tab in allTabs); if (!isValidSubset) { updateAnchorState(undefined); } @@ -1124,13 +1127,13 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({ throw new Error('Parsed value is not an array'); } } catch (error) { - if (!(anchor in allTabs)) { + if (!allTabs || !(anchor in allTabs)) { updateAnchorState(undefined); } } - } else if (nativeFilters.all) { + } else if (nativeFilters?.all) { setNativeFilterOptions( - nativeFilters.all.map((filter: any) => ({ + nativeFilters.all.map((filter: NativeFilterObject) => ({ value: filter.id, label: filter.name, })),
