sadpandajoe commented on code in PR #45142:
URL: https://github.com/apache/superset/pull/45142#discussion_r4235324952
##########
superset-frontend/src/features/alerts/AlertReportModal.test.tsx:
##########
@@ -2221,6 +2221,378 @@ test('filter reappears in dropdown after clearing with
X icon', async () => {
);
}, 45000);
+test.each(['older', 'latest'] as const)(
+ 'overlapping filter requests preserve the latest selection when %s finishes
first',
+ async firstToFinish => {
+ fetchMock.removeRoute(tabsEndpoint);
+ fetchMock.get(tabsEndpoint, tabsWithFilters, { name: tabsEndpoint });
+
+ type Response = {
+ json: { result: { data: { country: string }[] }[] };
+ };
+ let resolveOlder!: (response: Response) => void;
+ let resolveLatest!: (response: Response) => void;
+ const olderRequest = new Promise<Response>(resolve => {
+ resolveOlder = resolve;
+ });
+ const latestRequest = new Promise<Response>(resolve => {
+ resolveLatest = resolve;
+ });
+ mockGetChartDataRequest
+ .mockReturnValueOnce(olderRequest)
+ .mockReturnValueOnce(latestRequest);
+
+ render(<AlertReportModal {...generateMockedProps(true, true)} />, {
+ useRedux: true,
+ });
+ await userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+
+ const filterSelect = screen.getByRole('combobox', {
+ name: /select filter/i,
+ });
+ const valueSelect = screen.getByRole('combobox', {
+ name: /select value/i,
+ });
+ const selectCountry = async () => {
+ fireEvent.mouseDown(filterSelect);
+ await userEvent.click(await screen.findByText('Country Filter'));
+ };
+ await waitFor(() => expect(filterSelect).toBeEnabled());
+ await selectCountry();
+ await waitFor(() =>
+ expect(mockGetChartDataRequest).toHaveBeenCalledTimes(1),
+ );
+ expect(valueSelect).toBeDisabled();
+
+ const clearIcon = filterSelect
+ .closest('.ant-select')!
+ .querySelector('.ant-select-clear [aria-label="close-circle"]')!;
+ await userEvent.click(clearIcon);
+ await selectCountry();
+ await waitFor(() =>
+ expect(mockGetChartDataRequest).toHaveBeenCalledTimes(2),
+ );
+
+ const olderResponse: Response = {
+ json: { result: [{ data: [{ country: 'Stale country' }] }] },
+ };
+ const latestResponse: Response = {
+ json: { result: [{ data: [{ country: 'Latest country' }] }] },
+ };
+ if (firstToFinish === 'older') {
+ await act(async () => {
+ resolveOlder(olderResponse);
+ await olderRequest;
+ });
+ expect(valueSelect).toBeDisabled();
+ expect(
+
valueSelect.closest('.ant-select')!.querySelector('[aria-busy="true"]'),
+ ).toBeInTheDocument();
+ }
+
+ await act(async () => {
+ resolveLatest(latestResponse);
+ await latestRequest;
+ });
+ await waitFor(() => expect(valueSelect).toBeEnabled());
+
+ if (firstToFinish === 'latest') {
+ await act(async () => {
+ resolveOlder(olderResponse);
+ await olderRequest;
+ });
+ }
+
+ fireEvent.mouseDown(valueSelect);
+ expect(await screen.findByText('Latest country')).toBeInTheDocument();
+ expect(screen.queryByText('Stale country')).not.toBeInTheDocument();
+ expect(
+ valueSelect.closest('.ant-select')!.querySelector('[aria-busy="true"]'),
+ ).not.toBeInTheDocument();
+ },
+);
+
+test('an older filter request rejection does not replace the latest options
with an error', async () => {
+ fetchMock.removeRoute(tabsEndpoint);
+ fetchMock.get(tabsEndpoint, tabsWithFilters, { name: tabsEndpoint });
+
+ type Response = {
+ json: { result: { data: { country: string }[] }[] };
+ };
+ let rejectOlder!: (error: Error) => void;
+ let resolveLatest!: (response: Response) => void;
+ const olderRequest = new Promise<Response>((_, reject) => {
+ rejectOlder = reject;
+ });
+ const latestRequest = new Promise<Response>(resolve => {
+ resolveLatest = resolve;
+ });
+ mockGetChartDataRequest
+ .mockReturnValueOnce(olderRequest)
+ .mockReturnValueOnce(latestRequest);
+
+ render(<AlertReportModal {...generateMockedProps(true, true)} />, {
+ useRedux: true,
+ });
+ await userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+
+ const filterSelect = screen.getByRole('combobox', {
+ name: /select filter/i,
+ });
+ const valueSelect = screen.getByRole('combobox', {
+ name: /select value/i,
+ });
+ const selectCountry = async () => {
+ fireEvent.mouseDown(filterSelect);
+ await userEvent.click(await screen.findByText('Country Filter'));
+ };
+ await waitFor(() => expect(filterSelect).toBeEnabled());
+ await selectCountry();
+ await waitFor(() =>
expect(mockGetChartDataRequest).toHaveBeenCalledTimes(1));
+
+ const clearIcon = filterSelect
+ .closest('.ant-select')!
+ .querySelector('.ant-select-clear [aria-label="close-circle"]')!;
+ await userEvent.click(clearIcon);
+ await selectCountry();
+ await waitFor(() =>
expect(mockGetChartDataRequest).toHaveBeenCalledTimes(2));
+
+ await act(async () => {
+ resolveLatest({
+ json: { result: [{ data: [{ country: 'Latest country' }] }] },
+ });
+ await latestRequest;
+ });
+ await waitFor(() => expect(valueSelect).toBeEnabled());
+
+ await act(async () => {
+ rejectOlder(new Error('Older request failed'));
+ await olderRequest.catch(() => {});
+ });
+
+ fireEvent.mouseDown(valueSelect);
+ const latestCountryOption = await screen.findByText('Latest country');
+ expect(latestCountryOption).toBeInTheDocument();
+ await userEvent.click(latestCountryOption);
+ expect(screen.queryByText('Cannot load filter')).not.toBeInTheDocument();
+ expect(screen.queryByText('Network error')).not.toBeInTheDocument();
+});
+
+test('pending filter options follow the filter when an earlier row is
deleted', async () => {
+ fetchMock.removeRoute(tabsEndpoint);
+ fetchMock.get(tabsEndpoint, tabsWithFilters, { name: tabsEndpoint });
+ let resolveCity!: (response: {
+ json: { result: { data: { city: string }[] }[] };
+ }) => void;
+ const cityRequest = new Promise<{
+ json: { result: { data: { city: string }[] }[] };
+ }>(resolve => {
+ resolveCity = resolve;
+ });
+ mockGetChartDataRequest
+ .mockResolvedValueOnce({
+ json: { result: [{ data: [{ country: 'US' }] }] },
+ })
+ .mockReturnValueOnce(cityRequest);
+
+ render(<AlertReportModal {...generateMockedProps(true, true)} />, {
+ useRedux: true,
+ });
+ await userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+ const countrySelect = screen.getByRole('combobox', {
+ name: /select filter/i,
+ });
+ await waitFor(() => expect(countrySelect).toBeEnabled());
+ fireEvent.mouseDown(countrySelect);
+ await userEvent.click(await screen.findByText('Country Filter'));
+ await waitFor(() =>
+ expect(
+ screen.getByRole('combobox', { name: /select value/i }),
+ ).toBeEnabled(),
+ );
+ await userEvent.click(screen.getByText(/apply another dashboard filter/i));
+ fireEvent.mouseDown(
+ screen.getAllByRole('combobox', { name: /select filter/i })[1],
+ );
+ const dropdowns = document.querySelectorAll('.ant-select-dropdown');
+ await userEvent.click(
+ await within(dropdowns[dropdowns.length - 1] as HTMLElement).findByText(
+ 'City Filter',
+ ),
+ );
+ await waitFor(() =>
expect(mockGetChartDataRequest).toHaveBeenCalledTimes(2));
+ fireEvent.click(document.querySelectorAll('.filters-trashcan')[0]);
+ expect(
+ screen.getAllByRole('combobox', { name: /select filter/i }),
+ ).toHaveLength(1);
+ const valueSelect = screen.getByRole('combobox', { name: /select value/i });
+ expect(valueSelect).toBeDisabled();
+ await act(async () => {
+ resolveCity({ json: { result: [{ data: [{ city: 'London' }] }] } });
+ await cityRequest;
+ });
+ await waitFor(() => expect(valueSelect).toBeEnabled());
+ fireEvent.mouseDown(valueSelect);
+ expect(await screen.findByText('London')).toBeInTheDocument();
+});
+
+test.each(
+ ['filter_select', 'filter_timegrain'].flatMap(filterType => [
Review Comment:
The earlier bot thread on this modal caught that a failed `filter_range`
options request must keep the manual min/max inputs, and the render order here
(range branch before the error branch) is what guarantees it. This failure
matrix only covers `filter_select` and `filter_timegrain`, and nothing else in
the file uses `filter_range`, so moving the error check above the range branch
would leave a failed range filter with no way to enter bounds and every test
would still pass. Could you add a `filter_range` case that rejects the options
request and asserts both number inputs remain editable and no `Cannot load
filter` panel appears?
--
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]
