fix(ui): handle test suite tab loading race (#29561)

Author: shah-harshit

openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/TestSuite.spec.ts

@@ -10,9 +10,10 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License.
  */
-import { expect } from '@playwright/test';
+import { expect, Route } from '@playwright/test';
 import { PLAYWRIGHT_INGESTION_TAG_OBJ } from '../../constant/config';
 import { Domain } from '../../support/domain/Domain';
+import { BundleTestSuiteClass } from '../../support/entity/BundleTestSuiteClass';
 import { EntityTypeEndpoint } from '../../support/entity/Entity.interface';
 import { TableClass } from '../../support/entity/TableClass';
 import { UserClass } from '../../support/user/UserClass';
@@ -50,6 +51,16 @@ const user1 = new UserClass();
 const user2 = new UserClass();
 const domain1 = new Domain();
 const domain2 = new Domain();
+const bundleTestSuite = new BundleTestSuiteClass();
+
+const createDeferred = () => {
+  let resolveDeferred: () => void = () => undefined;
+  const promise = new Promise<void>((resolve) => {
+    resolveDeferred = resolve;
+  });
+
+  return { promise, resolve: resolveDeferred };
+};
 
 test.beforeAll(async ({ browser }) => {
   const { apiContext, afterAction } = await performAdminLogin(browser);
@@ -60,13 +71,83 @@ test.beforeAll(async ({ browser }) => {
   await table.createTestCase(apiContext);
   await domain1.create(apiContext);
   await domain2.create(apiContext);
+  await bundleTestSuite.createBundleTestSuite(apiContext);
   await afterAction();
 });
 
+test.afterAll(async ({ browser }) => {
+  const bundleSuiteName = bundleTestSuite.bundleTestSuiteResponseData?.name;
+
+  if (bundleSuiteName) {
+    const { apiContext, afterAction } = await performAdminLogin(browser);
+    await apiContext.delete(
+      `/api/v1/dataQuality/testSuites/name/${encodeURIComponent(
+        bundleSuiteName
+      )}?hardDelete=true&recursive=true`
+    );
+    await afterAction();
+  }
+});
+
 test.beforeEach(async ({ page }) => {
   await redirectToHomePage(page);
 });
 
+test('Test suite tab switching keeps active bundle suite data after stale table suite response', async ({
+  page,
+}) => {
+  const bundleSuiteName =
+    bundleTestSuite.bundleTestSuiteResponseData?.name ?? '';
+  const tableFqn = table.entityResponseData?.fullyQualifiedName ?? '';
+  const tableSuiteRequestReceived = createDeferred();
+  const tableSuiteResponseRelease = createDeferred();
+  const tableSuiteResponseFulfilled = createDeferred();
+
+  expect(bundleSuiteName).not.toBe('');
+  expect(tableFqn).not.toBe('');
+
+  await page.route(
+    '**/api/v1/dataQuality/testSuites/search/list**',
+    async (route: Route) => {
+      const requestUrl = new URL(route.request().url());
+      const testSuiteType = requestUrl.searchParams.get('testSuiteType');
+
+      if (testSuiteType === 'basic') {
+        tableSuiteRequestReceived.resolve();
+        const tableSuiteResponse = await route.fetch();
+        await tableSuiteResponseRelease.promise;
+
+        await route.fulfill({
+          response: tableSuiteResponse,
+        });
+        tableSuiteResponseFulfilled.resolve();
+
+        return;
+      }
+
+      await route.continue();
+    }
+  );
+
+  await page.goto('/data-quality/test-suites/table-suites');
+  await tableSuiteRequestReceived.promise;
+
+  await expect(page.getByTestId('test-suite-table')).toBeVisible();
+  await expect(page.getByTestId('loader')).toBeVisible();
+
+  await page.getByTestId('bundle-suite-radio-btn').click();
+
+  await expect(page.getByTestId(bundleSuiteName)).toBeVisible();
+
+  tableSuiteResponseRelease.resolve();
+  await tableSuiteResponseFulfilled.promise;
+
+  await expect(page.getByTestId(bundleSuiteName)).toBeVisible();
+  await expect(
+    page.getByTestId('test-suite-table').getByText(tableFqn)
+  ).not.toBeVisible();
+});
+
 test(
   'Logical TestSuite',
   PLAYWRIGHT_INGESTION_TAG_OBJ,

openmetadata-ui/src/main/resources/ui/src/components/DataQuality/TestSuite/TestSuiteList/TestSuites.component.tsx

@@ -19,7 +19,7 @@ import { Col, Form, Row, Select, Space, Typography } from 'antd';
 import { AxiosError } from 'axios';
 import { isEmpty } from 'lodash';
 import QueryString from 'qs';
-import { useCallback, useEffect, useMemo, useState } from 'react';
+import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import type { SortDescriptor } from 'react-aria-components';
 import { useTranslation } from 'react-i18next';
 import { Link, useNavigate, useParams } from 'react-router-dom';
@@ -59,6 +59,7 @@ import { getEntityDetailsPath } from '../../../../utils/RouterUtils';
 import { showErrorToast } from '../../../../utils/ToastUtils';
 import ErrorPlaceHolder from '../../../common/ErrorWithPlaceholder/ErrorPlaceHolder';
 import FilterTablePlaceHolder from '../../../common/ErrorWithPlaceholder/FilterTablePlaceHolder';
+import Loader from '../../../common/Loader/Loader';
 import NextPrevious from '../../../common/NextPrevious/NextPrevious';
 import { PagingHandlerParams } from '../../../common/NextPrevious/NextPrevious.interface';
 import { OwnerLabel } from '../../../common/OwnerLabel/OwnerLabel.component';
@@ -116,6 +117,7 @@ export const TestSuites = () => {
   } = usePaging();
 
   const [isLoading, setIsLoading] = useState<boolean>(true);
+  const latestRequestId = useRef(0);
 
   const ownerFilterValue = useMemo(() => {
     return selectedOwner
@@ -155,41 +157,55 @@ export const TestSuites = () => {
     });
   }, [testSuites, sortDescriptor]);
 
-  const fetchTestSuites = async (
-    currentPage = INITIAL_PAGING_VALUE,
-    params?: ListTestSuitePramsBySearch
-  ) => {
-    setIsLoading(true);
-    try {
-      const result = await getListTestSuitesBySearch({
-        ...params,
-        fields: [TabSpecificField.OWNERS, TabSpecificField.SUMMARY],
-        q: searchValue ? `*${searchValue}*` : undefined,
-        owner: ownerFilterValue?.key,
-        offset: (currentPage - 1) * pageSize,
-        includeEmptyTestSuites: subTab !== DataQualitySubTabs.TABLE_SUITES,
-        testSuiteType:
-          subTab === DataQualitySubTabs.TABLE_SUITES
-            ? TestSuiteType.basic
-            : TestSuiteType.logical,
-        sortField: 'lastResultTimestamp',
-        sortType: SORT_ORDER.DESC,
-      });
-      setTestSuites(result.data);
-      handlePagingChange(result.paging);
-    } catch (error) {
-      showErrorToast(error as AxiosError);
-    } finally {
-      setIsLoading(false);
-    }
-  };
+  const fetchTestSuites = useCallback(
+    async (
+      currentPage = INITIAL_PAGING_VALUE,
+      params?: ListTestSuitePramsBySearch
+    ) => {
+      const requestId = latestRequestId.current + 1;
+      latestRequestId.current = requestId;
+
+      setIsLoading(true);
+      try {
+        const result = await getListTestSuitesBySearch({
+          ...params,
+          fields: [TabSpecificField.OWNERS, TabSpecificField.SUMMARY],
+          q: searchValue ? `*${searchValue}*` : undefined,
+          owner: ownerFilterValue?.key,
+          offset: (currentPage - 1) * pageSize,
+          includeEmptyTestSuites: subTab !== DataQualitySubTabs.TABLE_SUITES,
+          testSuiteType:
+            subTab === DataQualitySubTabs.TABLE_SUITES
+              ? TestSuiteType.basic
+              : TestSuiteType.logical,
+          sortField: 'lastResultTimestamp',
+          sortType: SORT_ORDER.DESC,
+        });
+        if (requestId !== latestRequestId.current) {
+          return;
+        }
+
+        setTestSuites(result.data);
+        handlePagingChange(result.paging);
+      } catch (error) {
+        if (requestId === latestRequestId.current) {
+          showErrorToast(error as AxiosError);
+        }
+      } finally {
+        if (requestId === latestRequestId.current) {
+          setIsLoading(false);
+        }
+      }
+    },
+    [searchValue, ownerFilterValue?.key, pageSize, subTab, handlePagingChange]
+  );
 
   const handleTestSuitesPageChange = useCallback(
     ({ currentPage }: PagingHandlerParams) => {
       fetchTestSuites(currentPage, { limit: pageSize });
       handlePageChange(currentPage);
     },
-    [pageSize, handlePageChange]
+    [fetchTestSuites, pageSize, handlePageChange]
   );
 
   const handleSearchParam = (
@@ -315,7 +331,15 @@ export const TestSuites = () => {
     } else {
       setIsLoading(false);
     }
-  }, [testSuitePermission, pageSize, searchValue, owner, subTab, currentPage]);
+  }, [
+    testSuitePermission,
+    pageSize,
+    searchValue,
+    owner,
+    subTab,
+    currentPage,
+    fetchTestSuites,
+  ]);
 
   if (!testSuitePermission?.ViewAll && !testSuitePermission?.ViewBasic) {
     return (
@@ -417,7 +441,15 @@ export const TestSuites = () => {
             </Table.Header>
             <Table.Body
               items={isLoading ? [] : sortedData}
-              renderEmptyState={() => (isLoading ? <></> : noDataPlaceholder)}>
+              renderEmptyState={() =>
+                isLoading ? (
+                  <div className="tw:flex tw:justify-center tw:p-8">
+                    <Loader />
+                  </div>
+                ) : (
+                  noDataPlaceholder
+                )
+              }>
               {(record) => renderRow(record as TestSuite)}
             </Table.Body>
           </Table>

openmetadata-ui/src/main/resources/ui/src/components/DataQuality/TestSuite/TestSuiteList/TestSuites.test.tsx

@@ -11,8 +11,11 @@
  *  limitations under the License.
  */
 import { act, fireEvent, render, screen } from '@testing-library/react';
-import { MemoryRouter, useNavigate } from 'react-router-dom';
-import { DataQualityPageTabs } from '../../../../pages/DataQuality/DataQualityPage.interface';
+import { MemoryRouter, useNavigate, useParams } from 'react-router-dom';
+import {
+  DataQualityPageTabs,
+  DataQualitySubTabs,
+} from '../../../../pages/DataQuality/DataQualityPage.interface';
 import { getListTestSuitesBySearch } from '../../../../rest/testAPI';
 import observabilityRouterClassBase from '../../../../utils/ObservabilityRouterClassBase';
 import { TestSuites } from './TestSuites.component';
@@ -140,9 +143,13 @@ jest.mock('@openmetadata/ui-core-components', () => {
     dependencies?: unknown[];
   }) => (
     <tbody>
-      {items && items.length > 0
-        ? items.map((item) => children(item))
-        : renderEmptyState?.()}
+      {items && items.length > 0 ? (
+        items.map((item) => children(item))
+      ) : (
+        <tr>
+          <td colSpan={4}>{renderEmptyState?.()}</td>
+        </tr>
+      )}
     </tbody>
   );
 
@@ -257,6 +264,10 @@ jest.mock('../../../common/NextPrevious/NextPrevious', () => {
   return jest.fn().mockImplementation(() => <div>NextPrevious.component</div>);
 });
 
+jest.mock('../../../common/Loader/Loader', () =>
+  jest.fn().mockImplementation(() => <div data-testid="loader">Loader</div>)
+);
+
 jest.mock('../../../../utils/ObservabilityRouterClassBase', () => ({
   __esModule: true,
   default: {
@@ -351,6 +362,10 @@ describe('TestSuites component', () => {
     jest.clearAllMocks();
     testSuitePermission.ViewAll = true;
     mockLocation.search = '';
+    (useParams as jest.Mock).mockReturnValue({
+      tab: 'test-cases',
+      subTab: 'table-suites',
+    });
   });
 
   it('component should render', async () => {
@@ -528,6 +543,77 @@ describe('TestSuites component', () => {
     ).toBeInTheDocument();
   });
 
+  it('should render loader while test suites are loading', async () => {
+    (getListTestSuitesBySearch as jest.Mock).mockImplementationOnce(
+      () => new Promise(() => undefined)
+    );
+
+    render(<TestSuites />);
+
+    expect(await screen.findByTestId('loader')).toBeInTheDocument();
+    expect(
+      screen.queryByTestId('filter-table-placeholder')
+    ).not.toBeInTheDocument();
+  });
+
+  it('should ignore stale table suite response after switching to bundle suites', async () => {
+    let resolveTableSuites: (value: typeof mockList) => void;
+    let resolveBundleSuites: (value: typeof mockList) => void;
+    const tableSuitesResponse = new Promise<typeof mockList>((resolve) => {
+      resolveTableSuites = resolve;
+    });
+    const bundleSuitesResponse = new Promise<typeof mockList>((resolve) => {
+      resolveBundleSuites = resolve;
+    });
+    const bundleSuiteName = 'bundle.suite';
+
+    (getListTestSuitesBySearch as jest.Mock)
+      .mockImplementationOnce(() => tableSuitesResponse)
+      .mockImplementationOnce(() => bundleSuitesResponse);
+
+    const { rerender } = render(<TestSuites />);
+
+    expect(await screen.findByTestId('loader')).toBeInTheDocument();
+
+    (useParams as jest.Mock).mockReturnValue({
+      tab: DataQualityPageTabs.TEST_CASES,
+      subTab: DataQualitySubTabs.BUNDLE_SUITES,
+    });
+
+    rerender(<TestSuites />);
+
+    await act(async () => {
+      resolveBundleSuites({
+        data: [
+          {
+            id: 'bundle-id',
+            name: bundleSuiteName,
+            fullyQualifiedName: bundleSuiteName,
+            basic: false,
+          },
+        ],
+        paging: {
+          offset: 0,
+          limit: 15,
+          total: 1,
+        },
+      });
+    });
+
+    expect(await screen.findByTestId(bundleSuiteName)).toBeInTheDocument();
+
+    await act(async () => {
+      resolveTableSuites(mockList);
+    });
+
+    expect(screen.getByTestId(bundleSuiteName)).toBeInTheDocument();
+    expect(
+      screen.queryByTestId(
+        'sample_data.ecommerce_db.shopify.dim_address.testSuite'
+      )
+    ).not.toBeInTheDocument();
+  });
+
   it('should not render pagination when showPagination is false', async () => {
     (getListTestSuitesBySearch as jest.Mock).mockImplementationOnce(() =>
       Promise.resolve({ data: [], paging: { total: 5 } })