sadpandajoe commented on code in PR #38591:
URL: https://github.com/apache/superset/pull/38591#discussion_r2997661955
##########
superset-frontend/src/features/alerts/AlertReportModal.test.tsx:
##########
@@ -1488,3 +2251,310 @@ test('tabs metadata overwrites seeded filter options',
async () => {
within(selectContainer).queryByTitle('Country'),
).not.toBeInTheDocument();
});
+
+test('selecting filter triggers chart data request with correct params', async
() => {
+ mockGetChartDataRequest.mockReset();
+ fetchMock.removeRoute(tabsEndpoint);
+ fetchMock.get(tabsEndpoint, tabsWithFilters, { name: tabsEndpoint });
+
+ mockGetChartDataRequest.mockResolvedValue({
+ json: { result: [{ data: [{ country: 'US' }, { country: 'UK' }] }] },
+ });
+
+ render(<AlertReportModal {...generateMockedProps(true, true)} />, {
+ useRedux: true,
+ });
+
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+
+ // Wait for filter dropdown to be available
+ const filterDropdown = await waitFor(() =>
+ screen.getByRole('combobox', { name: /select filter/i }),
+ );
+
+ // Select the Country Filter using comboboxSelect pattern
+ await comboboxSelect(filterDropdown, 'Country Filter', () =>
+ document.querySelector(
+ '.ant-select-selection-item[title="Country Filter"]',
+ ),
+ );
+
+ // getChartDataRequest should have been called for filter values
+ await waitFor(() => {
+ expect(mockGetChartDataRequest).toHaveBeenCalledTimes(1);
+ });
+
+ // Verify it was called with correct datasource and groupby
+ const callArgs = mockGetChartDataRequest.mock.calls[0][0];
+ expect(callArgs.formData.groupby).toEqual(['country']);
+ expect(callArgs.formData.datasource).toBe('1__table');
+
+ mockGetChartDataRequest.mockReset();
+});
+
+test('selected filter excluded from other row dropdowns', async () => {
+ mockGetChartDataRequest.mockReset();
+ fetchMock.removeRoute(tabsEndpoint);
+ fetchMock.get(tabsEndpoint, tabsWithFilters, { name: tabsEndpoint });
+
+ mockGetChartDataRequest.mockResolvedValue({
+ json: { result: [{ data: [{ country: 'US' }] }] },
+ });
+
+ render(<AlertReportModal {...generateMockedProps(true, true)} />, {
+ useRedux: true,
+ });
+
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+
+ // Wait for filter dropdown
+ const filterDropdown = await waitFor(() =>
+ screen.getByRole('combobox', { name: /select filter/i }),
+ );
+
+ // Select Country Filter in row 1
+ await comboboxSelect(filterDropdown, 'Country Filter', () =>
+ document.querySelector(
+ '.ant-select-selection-item[title="Country Filter"]',
+ ),
+ );
+
+ // Wait for getChartDataRequest to complete AND state update to propagate.
+ // The value dropdown becomes non-disabled once optionFilterValues is
populated.
+ await waitFor(() => {
+ expect(mockGetChartDataRequest).toHaveBeenCalled();
+ });
+ await waitFor(
+ () => {
+ const valueSelects = screen.queryAllByRole('combobox', {
+ name: /select value/i,
+ });
+ expect(valueSelects.length).toBeGreaterThan(0);
+ },
+ { timeout: 5000 },
+ );
+
+ // Add second filter row
+ const addFilterButton = screen.getByText(/apply another dashboard filter/i);
+ userEvent.click(addFilterButton);
+
+ // Wait for second row
+ await waitFor(() => {
+ expect(
+ screen.getAllByRole('combobox', { name: /select filter/i }),
+ ).toHaveLength(2);
+ });
+
+ // Open filter dropdown in row 2
+ const filterDropdowns = screen.getAllByRole('combobox', {
+ name: /select filter/i,
+ });
+ userEvent.click(filterDropdowns[1]);
+
+ // Country Filter should be excluded (dedup), City Filter should remain.
+ // Use the last virtual list since the first may belong to the closed row 1
dropdown.
+ await waitFor(() => {
+ const virtualLists = document.querySelectorAll('.rc-virtual-list');
+ const lastVirtualList = virtualLists[virtualLists.length - 1];
+ expect(
+ within(lastVirtualList as HTMLElement).queryByText('Country Filter'),
+ ).not.toBeInTheDocument();
+ expect(
+ within(lastVirtualList as HTMLElement).getByText('City Filter'),
+ ).toBeInTheDocument();
+ });
+
+ mockGetChartDataRequest.mockReset();
+}, 30000);
+
+test('invalid CC email blocks submit', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+
+ // Wait for validation to fully propagate (fetch → state → validate →
enforce)
+ await screen.findByText('Edit alert');
+ await waitFor(
+ () => {
+ expect(screen.getByRole('button', { name: /save/i })).toBeEnabled();
+ },
+ { timeout: 5000 },
+ );
+
+ // Open notification panel and show CC field
+ userEvent.click(screen.getByTestId('notification-method-panel'));
+ const addCcButton = await screen.findByText(/Add CC Recipients/i);
+ userEvent.click(addCcButton);
+
+ // Type invalid email in CC field
+ const ccInput = await screen.findByTestId('cc');
+ userEvent.type(ccInput, 'not-an-email');
+ fireEvent.blur(ccInput);
+
+ // Save should now be disabled due to invalid email format
+ await waitFor(() => {
+ expect(screen.getByRole('button', { name: /save/i })).toBeDisabled();
+ });
+});
+
+test('invalid BCC email blocks submit', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+
+ // Wait for validation to fully propagate (fetch → state → validate →
enforce)
+ await screen.findByText('Edit alert');
+ await waitFor(
+ () => {
+ expect(screen.getByRole('button', { name: /save/i })).toBeEnabled();
+ },
+ { timeout: 5000 },
+ );
+
+ // Open notification panel and show BCC field
+ userEvent.click(screen.getByTestId('notification-method-panel'));
+ const addBccButton = await screen.findByText(/Add BCC Recipients/i);
+ userEvent.click(addBccButton);
+
+ // Type invalid email in BCC field
+ const bccInput = await screen.findByTestId('bcc');
+ userEvent.type(bccInput, 'not-an-email');
+ fireEvent.blur(bccInput);
+
+ // Save should now be disabled due to invalid email format
+ await waitFor(() => {
+ expect(screen.getByRole('button', { name: /save/i })).toBeDisabled();
+ });
+});
+
+test('invalid saved anchor is reset on dashboard load', async () => {
+ fetchMock.removeRoute(tabsEndpoint);
+ fetchMock.get(tabsEndpoint, tabsWithFilters, { name: tabsEndpoint });
+
+ const props = generateMockedProps(true, true);
+ const editProps = {
+ ...props,
+ alert: { ...validAlert, id: 7 },
+ };
+
+ render(<AlertReportModal {...editProps} />, {
+ useRedux: true,
+ });
+
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+
+ // Wait for dashboard tabs to load
+ await waitFor(() => {
+
expect(fetchMock.callHistory.calls(tabsEndpoint).length).toBeGreaterThan(0);
+ });
+
+ // The saved anchor 'TAB_999' is NOT in the dashboard's tabs (TAB_1, TAB_2),
+ // so it should be reset to undefined. Tab selector shows placeholder.
+ await waitFor(() => {
+ expect(screen.getByText(/select a tab/i)).toBeInTheDocument();
+ });
+
+ // TAB_999 should NOT appear as a selected value anywhere
+ expect(
+ document.querySelector('.ant-select-selection-item[title="TAB_999"]'),
+ ).not.toBeInTheDocument();
+});
+
+test('clearing notification recipients disables submit and prevents API call',
async () => {
+ fetchMock.put(
+ 'glob:*/api/v1/report/2',
+ { id: 2, result: {} },
+ { name: 'put-no-recipients' },
+ );
+
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+
+ // Wait for all validation to pass (5 checkmarks = fully valid alert)
+ await waitFor(() => {
+ expect(
+ screen.queryAllByRole('img', { name: /check-circle/i }),
+ ).toHaveLength(5);
+ });
+
+ // Save should be enabled initially
+ expect(screen.getByRole('button', { name: /save/i })).toBeEnabled();
+
+ // Open notification panel and clear the recipients field
+ userEvent.click(screen.getByTestId('notification-method-panel'));
+ const recipientInput = await screen.findByTestId('recipients');
+ userEvent.clear(recipientInput);
+ fireEvent.blur(recipientInput);
+
+ // Save should be disabled — empty recipients block submission
+ await waitFor(() => {
+ expect(screen.getByRole('button', { name: /save/i })).toBeDisabled();
+ });
+
+ // No PUT was emitted — the disabled button prevented any payload from being
sent.
+ // This confirms that a payload with empty recipients is never transmitted.
+ expect(fetchMock.callHistory.calls('put-no-recipients')).toHaveLength(0);
+
+ fetchMock.removeRoute('put-no-recipients');
+});
+
+test('empty recipients array is omitted from payload', () => {
+ // Direct test of the payload cleanup logic from AlertReportModal (lines
941-943).
+ // UI validation prevents submitting with empty recipients, so this exercises
+ // the defensive `delete data.recipients` branch in isolation.
+ const data: Record<string, unknown> = {
+ name: 'Test Alert',
+ recipients: [],
+ type: 'Alert',
+ };
+
+ // Mirrors AlertReportModal.tsx lines 941-943
+ if (data.recipients && !(data.recipients as unknown[]).length) {
+ delete data.recipients;
+ }
+
+ expect(data).not.toHaveProperty('recipients');
+ expect(data).toHaveProperty('name', 'Test Alert');
+});
+
+test('non-empty recipients array is preserved in payload', () => {
+ // Complementary to the empty-recipients test: verifies that populated
+ // recipients survive the cleanup logic unchanged.
+ const data: Record<string, unknown> = {
+ name: 'Test Alert',
+ recipients: [{ type: 'Email', recipient_config_json: { target:
'[email protected]' } }],
+ type: 'Alert',
+ };
+
+ if (data.recipients && !(data.recipients as unknown[]).length) {
+ delete data.recipients;
+ }
+
+ expect(data).toHaveProperty('recipients');
+ expect((data.recipients as unknown[]).length).toBe(1);
+});
+
+test('modal reopen resets local state', async () => {
+ const props = generateMockedProps(true, false, true);
+
+ render(<AlertReportModal {...props} />, { useRedux: true });
+
+ // Type a name to dirty the form
+ const nameInput = screen.getByPlaceholderText(/enter report name/i);
+ userEvent.type(nameInput, 'Temporary Report');
+ expect(nameInput).toHaveValue('Temporary Report');
Review Comment:
Fixed in 7a83cb5 — added `await` before `userEvent.type`.
--
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]
