From 78166cde5a6ca51e4b71db05b06d818f2a9d096d Mon Sep 17 00:00:00 2001 From: "JUST.in DO IT" Date: Fri, 10 May 2024 10:18:14 -0700 Subject: [PATCH] fix(SIP-95): missing catalog cache key (#28415) --- .../SqlEditorLeftBar.test.tsx | 21 ++++---- .../src/components/DatabaseSelector/index.tsx | 2 +- .../src/hooks/apiResources/schemas.test.ts | 49 ++++++++++++++++++- .../src/hooks/apiResources/schemas.ts | 3 +- 4 files changed, 61 insertions(+), 14 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx index 27d6c44d016b3..962b81062a49a 100644 --- 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,14 +107,11 @@ 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', @@ -122,14 +119,16 @@ test('table should be visible when expanded is true', async () => { 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 index 23767ba9f7bff..14753c6dfe9dd 100644 --- 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 index 62e154acbe09d..78df4f850e1e0 100644 --- 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 index d5cec1daea298..51e8528ce5d33 100644 --- 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, }), }), }),