fix(SIP-95): missing catalog cache key (#28415)

apache · May 10, 2024 · 3a62eab · 3a62eab
1 parent 26df7b4
commit 3a62eab
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 14 deletions.
diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx
@@ -18,7 +18,7 @@
  */
 import React from 'react';
 import fetchMock from 'fetch-mock';
-import { render, screen, waitFor } from 'spec/helpers/testing-library';
+import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
 import userEvent from '@testing-library/user-event';
 import SqlEditorLeftBar, {
   SqlEditorLeftBarProps,
@@ -107,29 +107,28 @@ test('renders a TableElement', async () => {
 });
 
 test('table should be visible when expanded is true', async () => {
-  const { container, getByText, getByRole } = await renderAndWait(
-    mockedProps,
-    undefined,
-    {
+  const { container, getByText, getByRole, getAllByLabelText } =
+    await renderAndWait(mockedProps, undefined, {
       ...initialState,
       sqlLab: { ...initialState.sqlLab, tables: [table] },
-    },
-  );
+    });
 
   const dbSelect = getByRole('combobox', {
     name: 'Select database or type to search databases',
   });
   const schemaSelect = getByRole('combobox', {
     name: 'Select schema or type to search schemas',
   });
-  const dropdown = getByText(/Select table/i);
-  const abUser = getByText(/ab_user/i);
+  const tableSelect = getAllByLabelText(
+    /Select table or type to search tables/i,
+  )[0];
+  const tableOption = within(tableSelect).getByText(/ab_user/i);
 
   expect(getByText(/Database/i)).toBeInTheDocument();
   expect(dbSelect).toBeInTheDocument();
   expect(schemaSelect).toBeInTheDocument();
-  expect(dropdown).toBeInTheDocument();
-  expect(abUser).toBeInTheDocument();
+  expect(tableSelect).toBeInTheDocument();
+  expect(tableOption).toBeInTheDocument();
   expect(
     container.querySelector('.ant-collapse-content-active'),
   ).toBeInTheDocument();

diff --git a/superset-frontend/src/components/DatabaseSelector/index.tsx b/superset-frontend/src/components/DatabaseSelector/index.tsx
@@ -278,7 +278,7 @@ export default function DatabaseSelector({
     isFetching: loadingCatalogs,
     refetch: refetchCatalogs,
   } = useCatalogs({
-    dbId: currentDb?.value,
+    dbId: showCatalogSelector ? currentDb?.value : undefined,
     onSuccess: (catalogs, isFetched) => {
       if (!showCatalogSelector) {
         changeCatalog(null);

diff --git a/superset-frontend/src/hooks/apiResources/schemas.test.ts b/superset-frontend/src/hooks/apiResources/schemas.test.ts
@@ -32,6 +32,9 @@ const fakeApiResult = {
 const fakeApiResult2 = {
   result: ['test schema 2', 'test schema a'],
 };
+const fakeApiResult3 = {
+  result: ['test schema 3', 'test schema c'],
+};
 
 const expectedResult = fakeApiResult.result.map((value: string) => ({
   value,
@@ -43,6 +46,11 @@ const expectedResult2 = fakeApiResult2.result.map((value: string) => ({
   label: value,
   title: value,
 }));
+const expectedResult3 = fakeApiResult3.result.map((value: string) => ({
+  value,
+  label: value,
+  title: value,
+}));
 
 describe('useSchemas hook', () => {
   afterEach(() => {
@@ -119,7 +127,7 @@ describe('useSchemas hook', () => {
     expect(fetchMock.calls(schemaApiRoute).length).toBe(1);
   });
 
-  it('returns refreshed data after expires', async () => {
+  test('returns refreshed data after expires', async () => {
     const expectDbId = 'db1';
     const schemaApiRoute = `glob:*/api/v1/database/*/schemas/*`;
     fetchMock.get(schemaApiRoute, url =>
@@ -164,4 +172,43 @@ describe('useSchemas hook', () => {
     expect(fetchMock.calls(schemaApiRoute)[2][0]).toContain(expectDbId);
     await waitFor(() => expect(result.current.data).toEqual(expectedResult));
   });
+
+  test('returns correct schema list by a catalog', async () => {
+    const dbId = '1';
+    const expectCatalog = 'catalog3';
+    const schemaApiRoute = `glob:*/api/v1/database/*/schemas/*`;
+    fetchMock.get(schemaApiRoute, url =>
+      url.includes(`catalog:${expectCatalog}`)
+        ? fakeApiResult3
+        : fakeApiResult2,
+    );
+    const onSuccess = jest.fn();
+    const { result, rerender, waitFor } = renderHook(
+      ({ dbId, catalog }) =>
+        useSchemas({
+          dbId,
+          catalog,
+          onSuccess,
+        }),
+      {
+        initialProps: { dbId, catalog: expectCatalog },
+        wrapper: createWrapper({
+          useRedux: true,
+          store,
+        }),
+      },
+    );
+
+    await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(1));
+    expect(result.current.data).toEqual(expectedResult3);
+    expect(onSuccess).toHaveBeenCalledTimes(2);
+
+    rerender({ dbId, catalog: 'catalog2' });
+    await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(2));
+    expect(result.current.data).toEqual(expectedResult2);
+
+    rerender({ dbId, catalog: expectCatalog });
+    expect(result.current.data).toEqual(expectedResult3);
+    expect(fetchMock.calls(schemaApiRoute).length).toBe(2);
+  });
 });
diff --git a/superset-frontend/src/hooks/apiResources/schemas.ts b/superset-frontend/src/hooks/apiResources/schemas.ts
@@ -54,8 +54,9 @@ const schemaApi = api.injectEndpoints({
             title: value,
           })),
       }),
-      serializeQueryArgs: ({ queryArgs: { dbId } }) => ({
+      serializeQueryArgs: ({ queryArgs: { dbId, catalog } }) => ({
         dbId,
+        catalog,
       }),
     }),
   }),