bito-code-review[bot] commented on code in PR #38591:
URL: https://github.com/apache/superset/pull/38591#discussion_r2997893816
##########
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();
+});
Review Comment:
<!-- Bito Reply -->
Yes, the test 'invalid saved anchor is reset on dashboard load' uses `alert:
{ ...validAlert, id: 7 }`, which fetches from the
`FETCH_REPORT_INVALID_ANCHOR_ENDPOINT` mock. This mock returns `extra: {
dashboard: { anchor: 'TAB_999' } }`, confirming the anchor is set.
--
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]
