sadpandajoe commented on code in PR #38591:
URL: https://github.com/apache/superset/pull/38591#discussion_r3012541819
##########
superset-frontend/src/features/alerts/AlertReportModal.test.tsx:
##########
@@ -867,177 +949,962 @@ test('filter reappears in dropdown after clearing with
X icon', async () => {
userEvent.click(screen.getByTestId('contents-panel'));
await screen.findByText(/test dashboard/i);
- const filterDropdown = screen.getByRole('combobox', {
- name: /select filter/i,
- });
- expect(filterDropdown).toBeInTheDocument();
+ const tabSelector = document.querySelector('.ant-select-disabled');
+ expect(tabSelector).toBeInTheDocument();
+});
- userEvent.click(filterDropdown);
+test('dashboard with no tabs and no filters hides filter add link', async ()
=> {
+ fetchMock.removeRoute(tabsEndpoint);
+ fetchMock.get(tabsEndpoint, noTabsResponse, { name: tabsEndpoint });
- const filterOption = await waitFor(() => {
- const virtualList = document.querySelector('.rc-virtual-list');
- return within(virtualList as HTMLElement).getByText('Test Filter 1');
+ render(<AlertReportModal {...generateMockedProps(true, true)} />, {
+ useRedux: true,
});
- userEvent.click(filterOption);
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+ // Wait for tabs fetch to complete
await waitFor(() => {
- const selectionItem = document.querySelector(
- '.ant-select-selection-item[title="Test Filter 1"]',
- );
- expect(selectionItem).toBeInTheDocument();
+
expect(fetchMock.callHistory.calls(tabsEndpoint).length).toBeGreaterThan(0);
});
- const selectContainer = filterDropdown.closest('.ant-select');
-
- const clearIcon = selectContainer?.querySelector(
- '.ant-select-clear [aria-label="close-circle"]',
- );
- expect(clearIcon).toBeInTheDocument();
- userEvent.click(clearIcon as Element);
+ // Tab selector should be disabled (no tabs)
+ const disabledSelects = document.querySelectorAll('.ant-select-disabled');
+ expect(disabledSelects.length).toBeGreaterThanOrEqual(1);
- await waitFor(() => {
- const selectionItem = document.querySelector(
- '.ant-select-selection-item[title="Test Filter 1"]',
- );
- expect(selectionItem).not.toBeInTheDocument();
- });
+ // Filter Select should also be disabled (no filter options available)
+ expect(disabledSelects.length).toBeGreaterThanOrEqual(2);
- userEvent.click(filterDropdown);
- await waitFor(() => {
- const virtualList = document.querySelector('.rc-virtual-list');
- expect(
- within(virtualList as HTMLElement).getByText('Test Filter 1'),
- ).toBeInTheDocument();
- });
+ // "Apply another dashboard filter" link should NOT appear
+ // because noTabsResponse has empty native_filters ({})
+ expect(
+ screen.queryByText(/apply another dashboard filter/i),
+ ).not.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 } },
+test('dashboard switching resets tab and filter selections', async () => {
+ // Return dashboard options so user can switch
+ const dashboardOptions = {
+ result: [
+ { text: 'Test Dashboard', value: 1 },
+ { text: 'Other Dashboard', value: 99 },
+ ],
+ count: 2,
};
+ fetchMock.removeRoute(dashboardEndpoint);
+ fetchMock.get(dashboardEndpoint, dashboardOptions);
+ fetchMock.removeRoute(reportDashboardEndpoint);
+ fetchMock.get(reportDashboardEndpoint, dashboardOptions);
- const defaultTabs = {
- all_tabs: { [anchor]: `Tab ${anchor}` },
- tab_tree: [{ title: `Tab ${anchor}`, value: anchor }],
- };
- const tabs = tabsOverride ?? defaultTabs;
+ // Dashboard 1 has tabs and filters
+ fetchMock.removeRoute(tabsEndpoint);
+ fetchMock.get(tabsEndpoint, tabsWithFilters, { name: tabsEndpoint });
- // Clear call history so waitFor assertions don't match calls from prior
tests.
- fetchMock.callHistory.clear();
+ // Dashboard 99 has no tabs
+ const tabs99 = 'glob:*/api/v1/dashboard/99/tabs';
+ fetchMock.get(tabs99, noTabsResponse, { name: tabs99 });
- // 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);
+ render(<AlertReportModal {...generateMockedProps(true, true)} />, {
+ useRedux: true,
+ });
- 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,
- },
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+
+ // Wait for tabs endpoint call to complete (proves filter data is loaded)
+ await waitFor(
+ () => {
+ expect(fetchMock.callHistory.calls(tabsEndpoint).length).toBeGreaterThan(
+ 0,
+ );
},
- { name: tabsEndpoint },
+ { timeout: 5000 },
);
-};
-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 },
+ // Wait for tabs to load from dashboard 1
+ await waitFor(() => {
+ expect(screen.getAllByText(/select tab/i)).toHaveLength(1);
+ });
+
+ // Verify filters are available — this proves dashboard 1 has filter options
+ const filterCombobox = await waitFor(() =>
+ screen.getByRole('combobox', { name: /select filter/i }),
);
-};
-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: [],
- },
- ],
+ // Confirm the filter dropdown has options by opening it
+ fireEvent.mouseDown(filterCombobox);
+ await screen.findByText('Country Filter', {}, { timeout: 5000 });
+ // Close the dropdown by pressing Escape
+ fireEvent.keyDown(filterCombobox, { key: 'Escape' });
+
+ // Switch to "Other Dashboard"
+ const dashboardSelect = screen.getByRole('combobox', {
+ name: /dashboard/i,
});
+ userEvent.clear(dashboardSelect);
+ userEvent.type(dashboardSelect, 'Other Dashboard{enter}');
- const store = createStore({}, reducerIndex);
+ // Tab selector should reset: "Other Dashboard" has no tabs, so disabled
with placeholder
+ await waitFor(
+ () => {
+ expect(screen.getByText(/select a tab/i)).toBeInTheDocument();
+ },
+ { timeout: 10000 },
+ );
- try {
- render(<AlertReportModal {...generateMockedProps(true, true)} />, {
- store,
+ // Filter row should reset to empty (no filter selected)
+ await waitFor(() => {
+ const filterSelects = screen.getAllByRole('combobox', {
+ name: /select filter/i,
});
-
- userEvent.click(screen.getByTestId('contents-panel'));
- await screen.findByText(/test dashboard/i);
-
- await waitFor(() => {
+ // Filter select should have no selected value (placeholder state)
+ filterSelects.forEach(select => {
+ const container = select.closest('.ant-select');
expect(
- fetchMock.callHistory
- .calls()
- .some(c => c.url.includes('/dashboard/1/tabs')),
- ).toBe(true);
+ container?.querySelector('.ant-select-selection-item'),
+ ).not.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();
- }
-});
-
-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,
- });
+ // Restore dashboard endpoints
+ fetchMock.removeRoute(dashboardEndpoint);
+ fetchMock.get(dashboardEndpoint, { result: [] });
+ fetchMock.removeRoute(reportDashboardEndpoint);
+ fetchMock.get(reportDashboardEndpoint, { result: [] });
+ fetchMock.removeRoute(tabs99);
+}, 45000);
+
+test('different dashboard populates its own tabs and filters', async () => {
+ // Set up a report (id:99) that uses dashboard 99 instead of dashboard 1.
+ // This tests that the component correctly loads tabs and filters for a
+ // different dashboard (the "dashboard B has its own data" case).
+ const FETCH_REPORT_DASH99_ENDPOINT = 'glob:*/api/v1/report/99';
+ fetchMock.get(FETCH_REPORT_DASH99_ENDPOINT, {
+ result: {
+ ...generateMockPayload(true),
+ id: 99,
+ type: 'Report',
+ dashboard: { id: 99, dashboard_title: 'Other Dashboard' },
+ },
+ });
- userEvent.click(screen.getByTestId('contents-panel'));
+ // Dashboard 99 has its own tabs (Tab Alpha, Tab Beta) and a Region Filter
+ const tabs99Endpoint = 'glob:*/api/v1/dashboard/99/tabs';
+ const dash99Tabs = {
+ result: {
+ all_tabs: { TAB_A: 'Tab Alpha', TAB_B: 'Tab Beta' },
+ tab_tree: [
+ { title: 'Tab Alpha', value: 'TAB_A' },
+ { title: 'Tab Beta', value: 'TAB_B' },
+ ],
+ native_filters: {
+ all: [
+ {
+ id: 'NATIVE_FILTER-R1',
+ name: 'Region Filter',
+ filterType: 'filter_select',
+ targets: [{ column: { name: 'region' }, datasetId: 3 }],
+ adhoc_filters: [],
+ },
+ ],
+ TAB_A: [
+ {
+ id: 'NATIVE_FILTER-R1',
+ name: 'Region Filter',
+ filterType: 'filter_select',
+ targets: [{ column: { name: 'region' }, datasetId: 3 }],
+ adhoc_filters: [],
+ },
+ ],
+ TAB_B: [],
+ },
+ },
+ };
+ fetchMock.get(tabs99Endpoint, dash99Tabs, { name: 'tabs-99' });
+
+ const props = generateMockedProps(true, true);
+ const dash99Props = { ...props, alert: { ...validAlert, id: 99 } };
+
+ render(<AlertReportModal {...dash99Props} />, { useRedux: true });
+
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/other dashboard/i);
+
+ // Wait for dashboard 99 tabs to load — increase timeout because the
+ // component must first fetch /api/v1/report/99, then extract dashboard_id,
+ // then fetch /api/v1/dashboard/99/tabs. This three-step async chain needs
+ // more time in CI.
+ await waitFor(
+ () => {
+ expect(fetchMock.callHistory.calls('tabs-99').length).toBeGreaterThan(0);
+ },
+ { timeout: 10000 },
+ );
+
+ // Tab selector should be enabled (dashboard 99 has tabs)
+ await waitFor(() => {
+ const treeSelect = document.querySelector('.ant-tree-select');
+ expect(treeSelect).toBeInTheDocument();
+ expect(treeSelect).not.toHaveClass('ant-select-disabled');
+ });
+
+ // Filter dropdown should show dashboard 99's "Region Filter"
+ const filterSelect = await waitFor(() =>
+ screen.getByRole('combobox', { name: /select filter/i }),
+ );
+ fireEvent.mouseDown(filterSelect);
+
+ await screen.findByText('Region Filter', {}, { timeout: 5000 });
+
+ // Dashboard 1's filters (Country/City) should NOT appear
+ expect(screen.queryByText('Country Filter')).not.toBeInTheDocument();
+
+ // Cleanup
+ fetchMock.removeRoute(FETCH_REPORT_DASH99_ENDPOINT);
+ fetchMock.removeRoute('tabs-99');
+}, 45000);
+
+test('dashboard tabs fetch failure shows error toast', async () => {
+ fetchMock.removeRoute(tabsEndpoint);
+ fetchMock.get(tabsEndpoint, 500, { name: tabsEndpoint });
+
+ const store = createStore({}, reducerIndex);
+
+ render(<AlertReportModal {...generateMockedProps(true, true)} />, {
+ useRedux: true,
+ store,
+ });
+
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+
+ // Tab selector should remain disabled (no tabs loaded)
+ const tabSelector = document.querySelector('.ant-select-disabled');
+ expect(tabSelector).toBeInTheDocument();
+
+ // Verify the tabs request was attempted
+ const tabsCalls = fetchMock.callHistory.calls(tabsEndpoint);
+ expect(tabsCalls.length).toBeGreaterThan(0);
+
+ // Verify danger toast was dispatched for the fetch failure
+ await waitFor(() => {
+ const toasts = (store.getState() as any).messageToasts;
+ expect(toasts.length).toBeGreaterThan(0);
+ expect(
+ toasts.some((t: { text: string }) =>
+ t.text.includes('error retrieving dashboard tabs'),
+ ),
+ ).toBe(true);
+ });
+});
+
+test('switching content type to chart hides tab and filter sections', async ()
=> {
+ fetchMock.removeRoute(tabsEndpoint);
+ fetchMock.get(tabsEndpoint, tabsWithFilters, { name: tabsEndpoint });
+
+ render(<AlertReportModal {...generateMockedProps(true, true)} />, {
+ useRedux: true,
+ });
+
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+
+ // Tab selector and filter dropdowns should be visible for dashboard
+ expect(screen.getAllByText(/select tab/i)).toHaveLength(1);
+ expect(
+ screen.getByRole('combobox', { name: /select filter/i }),
+ ).toBeInTheDocument();
+
+ // Switch to chart
+ const contentTypeSelector = screen.getByRole('combobox', {
+ name: /select content type/i,
+ });
+ await comboboxSelect(contentTypeSelector, 'Chart', () =>
+ screen.getByRole('combobox', { name: /chart/i }),
+ );
+
+ // Tab and filter sections should be hidden
+ expect(screen.queryByText(/select tab/i)).not.toBeInTheDocument();
+ expect(
+ screen.queryByRole('combobox', { name: /select filter/i }),
+ ).not.toBeInTheDocument();
+});
+
+test('adding and removing dashboard filter rows', async () => {
+ fetchMock.removeRoute(tabsEndpoint);
+ fetchMock.get(tabsEndpoint, tabsWithFilters, { name: tabsEndpoint });
+
+ render(<AlertReportModal {...generateMockedProps(true, true)} />, {
+ useRedux: true,
+ });
+
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+
+ // Wait for filter options to load
+ await waitFor(() => {
+ expect(
+ screen.getByRole('combobox', { name: /select filter/i }),
+ ).toBeInTheDocument();
+ });
+
+ // Should start with 1 filter row
+ const initialFilterSelects = screen.getAllByRole('combobox', {
+ name: /select filter/i,
+ });
+ expect(initialFilterSelects).toHaveLength(1);
+
+ // Click "Apply another dashboard filter"
+ const addFilterButton = screen.getByText(/apply another dashboard filter/i);
+ userEvent.click(addFilterButton);
+
+ // Should now have 2 filter rows
+ await waitFor(() => {
+ expect(
+ screen.getAllByRole('combobox', { name: /select filter/i }),
+ ).toHaveLength(2);
+ });
+
+ // Remove the second filter row by clicking its delete icon
+ const deleteIcons = document.querySelectorAll('.filters-trashcan');
+ expect(deleteIcons.length).toBeGreaterThanOrEqual(2);
+ fireEvent.click(deleteIcons[deleteIcons.length - 1]);
+
+ // Should be back to 1 filter row
+ await waitFor(() => {
+ expect(
+ screen.getAllByRole('combobox', { name: /select filter/i }),
+ ).toHaveLength(1);
+ });
+});
+
+test('alert shows condition section, report does not', () => {
+ // Alert has 5 sections
+ const { unmount } = render(
+ <AlertReportModal {...generateMockedProps(false)} />,
+ { useRedux: true },
+ );
+ expect(screen.getAllByRole('tab')).toHaveLength(5);
+ expect(screen.getByTestId('alert-condition-panel')).toBeInTheDocument();
+ unmount();
+
+ // Report has 4 sections, no condition panel
+ render(<AlertReportModal {...generateMockedProps(true)} />, {
+ useRedux: true,
+ });
+ expect(screen.getAllByRole('tab')).toHaveLength(4);
+
expect(screen.queryByTestId('alert-condition-panel')).not.toBeInTheDocument();
+});
+
+test('submit includes conditionNotNull without threshold in alert payload',
async () => {
+ // Mock payload returns id:1, so updateResource PUTs to /api/v1/report/1
+ fetchMock.put(
+ 'glob:*/api/v1/report/1',
+ { id: 1, result: {} },
+ { name: 'put-condition' },
+ );
+
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+
+ // Wait for resource to load and all validation to pass
+ await waitFor(
+ () => {
+ expect(
+ screen.queryAllByRole('img', { name: /check-circle/i }),
+ ).toHaveLength(5);
+ },
+ { timeout: 10000 },
+ );
+
+ // Open condition panel and select "not null"
+ userEvent.click(screen.getByTestId('alert-condition-panel'));
+ await screen.findByText(/smaller than/i);
+ const condition = screen.getByRole('combobox', { name: /condition/i });
+ await comboboxSelect(
+ condition,
+ 'not null',
+ () => screen.getAllByText(/not null/i)[0],
+ );
+
+ // Wait for the threshold input to become disabled after "not null"
selection —
+ // in CI the state update from comboboxSelect can lag behind the DOM
assertion.
+ await waitFor(() => {
+ expect(screen.getByRole('spinbutton')).toBeDisabled();
+ });
+
+ // Wait for Save to be enabled and click
+ await waitFor(() => {
+ expect(screen.getByRole('button', { name: /save/i })).toBeEnabled();
+ });
+ userEvent.click(screen.getByRole('button', { name: /save/i }));
+
+ // Verify the PUT payload
+ await waitFor(() => {
+ const calls = fetchMock.callHistory.calls('put-condition');
+ expect(calls.length).toBeGreaterThan(0);
+ });
+
+ const calls = fetchMock.callHistory.calls('put-condition');
+ const body = JSON.parse(calls[calls.length - 1].options.body as string);
+ expect(body.validator_type).toBe('not null');
+ expect(body.validator_config_json).toEqual({});
+
+ fetchMock.removeRoute('put-condition');
+}, 45000);
+
+test('edit mode submit uses PUT and excludes read-only fields', async () => {
+ // Mock payload returns id:1, so updateResource PUTs to /api/v1/report/1
+ fetchMock.put(
+ 'glob:*/api/v1/report/1',
+ { id: 1, result: {} },
+ { name: 'put-edit' },
+ );
+
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+
+ // Wait for resource to load and all validation to pass
+ await waitFor(() => {
+ expect(
+ screen.queryAllByRole('img', { name: /check-circle/i }),
+ ).toHaveLength(5);
+ });
+
+ await waitFor(() => {
+ expect(screen.getByRole('button', { name: /save/i })).toBeEnabled();
+ });
+ userEvent.click(screen.getByRole('button', { name: /save/i }));
+
+ await waitFor(() => {
+ const calls = fetchMock.callHistory.calls('put-edit');
+ expect(calls.length).toBeGreaterThan(0);
+ });
+
+ const calls = fetchMock.callHistory.calls('put-edit');
+ const body = JSON.parse(calls[calls.length - 1].options.body as string);
+
+ // Edit mode strips these read-only fields
+ expect(body).not.toHaveProperty('id');
+ expect(body).not.toHaveProperty('created_by');
+ expect(body).not.toHaveProperty('last_eval_dttm');
+ expect(body).not.toHaveProperty('last_state');
+ expect(body).not.toHaveProperty('last_value');
+ expect(body).not.toHaveProperty('last_value_row_json');
+
+ // Core fields remain
+ expect(body.type).toBe('Alert');
+ expect(body.name).toBe('Test Alert');
+
+ // Recipients from the loaded resource should be in payload
+ expect(body.recipients).toBeDefined();
+ expect(body.recipients.length).toBeGreaterThan(0);
+ expect(body.recipients[0].type).toBe('Email');
+
+ fetchMock.removeRoute('put-edit');
+});
+
+test('edit mode preserves extra.dashboard tab/filter state in payload', async
() => {
+ // Report 3 has extra.dashboard.nativeFilters + anchor:'TAB_1'
+ fetchMock.put(
+ 'glob:*/api/v1/report/3',
+ { id: 3, result: {} },
+ { name: 'put-extra-dashboard' },
+ );
+
+ // Provide tabs so TAB_1 is in allTabs and anchor is preserved
+ fetchMock.removeRoute(tabsEndpoint);
+ fetchMock.get(tabsEndpoint, tabsWithFilters, { name: tabsEndpoint });
+
+ const props = {
+ ...generateMockedProps(true, true, true),
+ alert: { ...validAlert, id: 3 },
+ };
+
+ render(<AlertReportModal {...props} />, {
+ useRedux: true,
+ });
+
+ // Wait for Save to be enabled (report type has fewer validation sections)
+ await waitFor(
+ () => {
+ expect(screen.getByRole('button', { name: /save/i })).toBeEnabled();
+ },
+ { timeout: 10000 },
+ );
+ userEvent.click(screen.getByRole('button', { name: /save/i }));
+
+ await waitFor(() => {
+ const calls = fetchMock.callHistory.calls('put-extra-dashboard');
+ expect(calls.length).toBeGreaterThan(0);
+ });
+
+ const calls = fetchMock.callHistory.calls('put-extra-dashboard');
+ const body = JSON.parse(calls[calls.length - 1].options.body as string);
+
+ // extra.dashboard structure must be preserved in the payload
+ expect(body.extra).toBeDefined();
+ expect(body.extra.dashboard).toBeDefined();
+ expect(body.extra.dashboard.nativeFilters).toEqual(
+ expect.arrayContaining([
+ expect.objectContaining({ nativeFilterId: 'NATIVE_FILTER-abc123' }),
+ ]),
+ );
+ expect(body.extra.dashboard.anchor).toBe('TAB_1');
+
+ fetchMock.removeRoute('put-extra-dashboard');
+ fetchMock.removeRoute(tabsEndpoint);
+ fetchMock.get(
+ tabsEndpoint,
+ { result: { all_tabs: {}, tab_tree: [] } },
+ { name: tabsEndpoint },
+ );
+});
+
+test('create mode submits POST and calls onAdd with response', async () => {
+ fetchMock.post(
+ 'glob:*/api/v1/report/',
+ { id: 100, result: {} },
+ { name: 'create-post' },
+ );
+
+ const props = generateMockedProps(true); // isReport, create mode (no alert)
+ const onAdd = jest.fn();
+ const createProps = { ...props, onAdd };
+
+ render(<AlertReportModal {...createProps} />, { useRedux: true });
+
+ expect(screen.getByText('Add report')).toBeInTheDocument();
+
+ // Fill name — fireEvent.change is instant vs userEvent.type firing 13
+ // individual keystroke events, keeping this test well under the 20s timeout.
+ const nameInput = screen.getByPlaceholderText(/enter report name/i);
+ fireEvent.change(nameInput, { target: { value: 'My New Report' } });
+
+ // Open contents panel — content type defaults to Dashboard
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByRole('combobox', { name: /select content type/i });
+
+ // Switch content type to Chart (default is Dashboard)
+ const contentTypeSelect = screen.getByRole('combobox', {
+ name: /select content type/i,
+ });
+ userEvent.click(contentTypeSelect);
+ const chartOption = await screen.findByText('Chart');
+ userEvent.click(chartOption);
+
+ // Select a chart from the chart combobox
+ const chartSelect = await screen.findByRole('combobox', {
+ name: /chart/i,
+ });
+ userEvent.type(chartSelect, 'table');
+ const tableChart = await screen.findByText('table chart');
+ userEvent.click(tableChart);
+
+ // Open notification panel and set recipient email
+ userEvent.click(screen.getByTestId('notification-method-panel'));
+ const recipientInput = await screen.findByTestId('recipients');
+ fireEvent.change(recipientInput, { target: { value: '[email protected]' } });
+
+ // Wait for Add button to be enabled (use exact name to avoid matching
+ // "Add CC Recipients" and "Add BCC Recipients" buttons)
+ await waitFor(
+ () => {
+ expect(screen.getByRole('button', { name: 'Add' })).toBeEnabled();
+ },
+ { timeout: 5000 },
+ );
+
+ // Click Add
+ userEvent.click(screen.getByRole('button', { name: 'Add' }));
+
+ // Verify POST was called (not PUT)
+ await waitFor(() => {
+ const calls = fetchMock.callHistory.calls('create-post');
+ expect(calls.length).toBeGreaterThan(0);
+ });
+
+ const calls = fetchMock.callHistory.calls('create-post');
+ const body = JSON.parse(calls[0].options.body as string);
+
+ expect(body.type).toBe('Report');
+ expect(body.name).toBe('My New Report');
+ expect(body.chart).toBe(1);
+ // Chart content type means dashboard is null (mutually exclusive)
+ expect(body.dashboard).toBeNull();
+ expect(body.recipients).toBeDefined();
+ expect(body.recipients[0].type).toBe('Email');
+ expect(body.recipients[0].recipient_config_json.target).toBe(
+ '[email protected]',
+ );
+
+ // Verify onAdd was called with the response id
+ await waitFor(() => {
+ expect(onAdd).toHaveBeenCalledWith(100);
+ });
+
+ fetchMock.removeRoute('create-post');
+}, 45000);
+
+test('create mode defaults to dashboard content type with chart null', async
() => {
+ // Coverage strategy: the create-mode POST pathway is tested via the chart
+ // POST test above. The dashboard content payload (dashboard=value,
chart=null)
+ // is tested via the edit-mode PUT test below. This test verifies that create
+ // mode reports default to Dashboard content type, completing the coverage:
+ // create POST method ← chart POST test
+ // dashboard payload shape ← edit-mode dashboard PUT test
+ // default content type = Dashboard ← THIS test
+
+ const props = generateMockedProps(true); // isReport, create mode
+ render(<AlertReportModal {...props} />, { useRedux: true });
+
+ // Open contents panel
+ userEvent.click(screen.getByTestId('contents-panel'));
+ const contentTypeSelect = await screen.findByRole('combobox', {
+ name: /select content type/i,
+ });
+
+ // Default content type should be "Dashboard" (not "Chart")
+ const selectedItem = contentTypeSelect
+ .closest('.ant-select')
+ ?.querySelector('.ant-select-selection-item');
+ expect(selectedItem).toBeInTheDocument();
+ expect(selectedItem?.textContent).toBe('Dashboard');
+
+ // Dashboard selector should be rendered (not chart selector)
+ expect(
+ screen.getByRole('combobox', { name: /dashboard/i }),
+ ).toBeInTheDocument();
+ expect(
+ screen.queryByRole('combobox', { name: /chart/i }),
+ ).not.toBeInTheDocument();
+});
+
+test('dashboard content type submits dashboard id and null chart', async () =>
{
+ // Use a custom alert prop with dashboard content from the start,
+ // matching the fetched resource shape (FETCH_DASHBOARD_ENDPOINT).
+ const dashboardAlert: AlertObject = {
+ ...validAlert,
+ id: 1,
+ dashboard_id: 1,
+ chart_id: 0,
+ dashboard: {
+ id: 1,
+ value: 1,
+ label: 'Test Dashboard',
+ dashboard_title: 'Test Dashboard',
+ } as any,
+ chart: undefined as any,
+ };
+
+ fetchMock.put(
+ 'glob:*/api/v1/report/1',
+ { id: 1, result: {} },
+ { name: 'put-dashboard-payload' },
+ );
+
+ const props = generateMockedProps(false, false);
+ const editProps = { ...props, alert: dashboardAlert };
+
+ render(<AlertReportModal {...editProps} />, {
+ useRedux: true,
+ });
+
+ // Wait for resource to load and all validation to pass
+ await waitFor(
+ () => {
+ expect(
+ screen.queryAllByRole('img', { name: /check-circle/i }),
+ ).toHaveLength(5);
+ },
+ { timeout: 5000 },
+ );
+
+ await waitFor(() => {
+ expect(screen.getByRole('button', { name: /save/i })).toBeEnabled();
+ });
+ userEvent.click(screen.getByRole('button', { name: /save/i }));
+
+ await waitFor(() => {
+ const calls = fetchMock.callHistory.calls('put-dashboard-payload');
+ expect(calls.length).toBeGreaterThan(0);
+ });
+
+ const calls = fetchMock.callHistory.calls('put-dashboard-payload');
+ const body = JSON.parse(calls[calls.length - 1].options.body as string);
+
+ // Dashboard payload: dashboard field is the numeric ID, chart is null
+ expect(body.dashboard).toBe(1);
+ expect(body.chart).toBeNull();
+ expect(body.name).toBe('Test Alert');
+ expect(body.recipients).toBeDefined();
+
+ fetchMock.removeRoute('put-dashboard-payload');
+});
+
+// --------------- Existing filter reappear test ------------------
+
+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: [],
+ },
+ ],
+ },
+ },
+ },
+ { name: tabsEndpoint },
+ );
+
+ fetchMock.post(
+ chartDataEndpoint,
+ { result: [{ data: [] }] },
+ {
+ name: 'clear-icon-chart-data',
+ },
+ );
+
+ render(<AlertReportModal {...generateMockedProps(true, true)} />, {
+ useRedux: true,
+ });
+
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+
+ // Wait for tabs endpoint to be called so filter options are populated before
+ // opening the dropdown — in CI the async chain (fetch report → set
dashboard →
+ // fetch tabs → set filter options) can lag behind the UI interactions.
+ await waitFor(
+ () => {
+ const tabsCalls = fetchMock.callHistory.calls(tabsEndpoint);
+ expect(tabsCalls.length).toBeGreaterThan(0);
+ },
+ { timeout: 5000 },
+ );
+
+ const filterDropdown = screen.getByRole('combobox', {
+ name: /select filter/i,
+ });
+ expect(filterDropdown).toBeInTheDocument();
+
+ // Value selector should be disabled before any filter is selected
+ const valueSelect = screen.getByRole('combobox', { name: /select value/i });
+
expect(valueSelect.closest('.ant-select')).toHaveClass('ant-select-disabled');
+
+ // Open dropdown and find filter option. Use waitFor to retry mouseDown
+ // because Ant Design Select may not open on the first attempt in CI.
+ let filterOption: HTMLElement;
+ await waitFor(
+ () => {
+ fireEvent.mouseDown(filterDropdown);
+ filterOption = screen.getByText('Test Filter 1');
+ expect(filterOption).toBeInTheDocument();
+ },
+ { timeout: 10000 },
+ );
+
+ userEvent.click(filterOption!);
+
+ await waitFor(() => {
+ const selectionItem = document.querySelector(
+ '.ant-select-selection-item[title="Test Filter 1"]',
+ );
+ expect(selectionItem).toBeInTheDocument();
+ });
+
+ // After selecting a filter, getChartDataRequest resolves and value selector
becomes enabled
+ await waitFor(() => {
+ expect(valueSelect.closest('.ant-select')).not.toHaveClass(
+ 'ant-select-disabled',
+ );
+ });
+
+ const selectContainer = filterDropdown.closest('.ant-select');
+
+ const clearIcon = selectContainer?.querySelector(
+ '.ant-select-clear [aria-label="close-circle"]',
+ );
+ expect(clearIcon).toBeInTheDocument();
+ userEvent.click(clearIcon as Element);
+
+ await waitFor(() => {
+ const selectionItem = document.querySelector(
+ '.ant-select-selection-item[title="Test Filter 1"]',
+ );
+ expect(selectionItem).not.toBeInTheDocument();
+ });
+
+ // After clearing, value selector should be disabled again
(optionFilterValues reset)
+ await waitFor(() => {
+ expect(valueSelect.closest('.ant-select')).toHaveClass(
+ 'ant-select-disabled',
+ );
+ });
+
+ // Re-open the dropdown — the filter should reappear as an option.
+ // Use waitFor to retry the mouseDown + text check because Ant Design
+ // may still be processing the clear event internally.
+ await waitFor(
+ () => {
+ fireEvent.mouseDown(filterDropdown);
+ expect(screen.getByText('Test Filter 1')).toBeInTheDocument();
+ },
+ { timeout: 10000 },
+ );
+}, 45000);
+
+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;
+
+ // Clear call history so waitFor assertions don't match calls from prior
tests.
+ fetchMock.callHistory.clear();
Review Comment:
Fixed in `32cdc22` — replaced with `fetchMock.clearHistory()`.
##########
superset-frontend/src/features/alerts/components/NumberInput.test.tsx:
##########
@@ -0,0 +1,67 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { render, screen, fireEvent } from 'spec/helpers/testing-library';
+import NumberInput from './NumberInput';
+
+const defaultProps = {
+ timeUnit: 'seconds',
+ min: 0,
+ name: 'timeout',
+ value: '30',
+ placeholder: 'Enter value',
+ onChange: jest.fn(),
+};
+
+test('renders value with timeUnit suffix when not focused', () => {
+ render(<NumberInput {...defaultProps} />);
+ const input = screen.getByPlaceholderText('Enter value');
+ expect(input).toHaveValue('30 seconds');
+});
+
+test('strips suffix on focus and restores on blur', () => {
+ render(<NumberInput {...defaultProps} />);
+ const input = screen.getByPlaceholderText('Enter value');
+
+ fireEvent.focus(input);
+ expect(input).toHaveValue('30');
+
+ fireEvent.blur(input);
+ expect(input).toHaveValue('30 seconds');
+});
+
+test('renders empty string when value is falsy', () => {
+ render(<NumberInput {...defaultProps} value="" />);
+ const input = screen.getByPlaceholderText('Enter value');
+ expect(input).toHaveValue('');
+});
+
+test('renders empty string when value is zero', () => {
+ render(<NumberInput {...defaultProps} value={0} />);
+ const input = screen.getByPlaceholderText('Enter value');
+ expect(input).toHaveValue('');
Review Comment:
Good catch — real truthiness bug in `NumberInput.tsx:47` (`value ? ... : ''`
treats 0 as empty). The test documents current component behavior; fixing the
component is a production change out of scope for this test PR (tracked as
BUG-5).
##########
superset-frontend/src/pages/AlertReportList/AlertReportList.test.tsx:
##########
@@ -17,270 +17,467 @@
* under the License.
*/
import fetchMock from 'fetch-mock';
-import configureStore from 'redux-mock-store';
-import thunk from 'redux-thunk';
import {
render,
screen,
fireEvent,
waitFor,
+ createStore,
} from 'spec/helpers/testing-library';
+import { Provider } from 'react-redux';
import { MemoryRouter } from 'react-router-dom';
import { QueryParamProvider } from 'use-query-params';
import { ReactRouter5Adapter } from 'use-query-params/adapters/react-router-5';
-import React from 'react';
import AlertListComponent from 'src/pages/AlertReportList';
-// Cast to accept partial mock props in tests
+jest.setTimeout(30000);
+
const AlertList = AlertListComponent as unknown as React.FC<
Record<string, any>
>;
Review Comment:
Already fixed in `eede7ccfb9` — `import type React from 'react'` is on line
19. This comment referenced a stale snapshot.
--
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]
