This is an automated email from the ASF dual-hosted git repository. beto pushed a commit to branch default-db-schema-dropdown in repository https://gitbox.apache.org/repos/asf/superset.git
commit db201285e58c5f614bba44a586804a58773c8bd1 Author: Beto Dealmeida <[email protected]> AuthorDate: Thu Dec 18 18:51:57 2025 -0500 Testing --- .../DatabaseSelector/DatabaseSelector.test.tsx | 81 +++++++++++++ .../src/components/DatabaseSelector/index.tsx | 46 +++++++- .../{schemas.test.ts => catalogs.test.ts} | 131 +++++++++------------ .../src/hooks/apiResources/catalogs.ts | 12 +- .../src/hooks/apiResources/schemas.test.ts | 48 ++++++-- .../src/hooks/apiResources/schemas.ts | 12 +- superset-frontend/src/hooks/apiResources/tables.ts | 2 +- 7 files changed, 236 insertions(+), 96 deletions(-) diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index f8d0425c5e..8d3d8bd8b0 100644 --- a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx +++ b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx @@ -163,11 +163,13 @@ const fakeDatabaseApiResultInReverseOrder = { const fakeSchemaApiResult = { count: 2, result: ['information_schema', 'public'], + default: 'public', }; const fakeCatalogApiResult = { count: 0, result: [], + default: null, }; const fakeFunctionNamesApiResult = { @@ -382,3 +384,82 @@ test('Sends the correct schema when changing the schema', async () => { ); expect(props.onSchemaChange).toHaveBeenCalledTimes(1); }); + +test('Auto-selects default schema on first load when no schema is provided', async () => { + fetchMock.get( + schemaApiRoute, + { + result: ['information_schema', 'public', 'other_schema'], + default: 'public', + }, + { overwriteRoutes: true }, + ); + + const props = { + ...createProps(), + schema: undefined, + }; + + render(<DatabaseSelector {...props} />, { useRedux: true, store }); + + // Wait for schemas to load and default to be applied + await waitFor(() => { + expect(props.onSchemaChange).toHaveBeenCalledWith('public'); + }); +}); + +test('Does not auto-select default schema when schema is already provided', async () => { + fetchMock.get( + schemaApiRoute, + { + result: ['information_schema', 'public', 'other_schema'], + default: 'public', + }, + { overwriteRoutes: true }, + ); + + const props = { + ...createProps(), + schema: 'information_schema', + }; + + render(<DatabaseSelector {...props} />, { useRedux: true, store }); + + // Wait for schemas to load + await waitFor(() => { + expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + }); + + // Should not call onSchemaChange since schema is already set + expect(props.onSchemaChange).not.toHaveBeenCalled(); +}); + +test('Auto-selects default catalog on first load for multi-catalog database', async () => { + fetchMock.get( + catalogApiRoute, + { + result: ['catalog_a', 'catalog_b', 'catalog_c'], + default: 'catalog_b', + }, + { overwriteRoutes: true }, + ); + + const props = { + ...createProps(), + db: { + id: 1, + database_name: 'test-multicatalog', + backend: 'test-postgresql', + allow_multi_catalog: true, + }, + catalog: undefined, + onCatalogChange: jest.fn(), + }; + + render(<DatabaseSelector {...props} />, { useRedux: true, store }); + + // Wait for catalogs to load and default to be applied + await waitFor(() => { + expect(props.onCatalogChange).toHaveBeenCalledWith('catalog_b'); + }); +}); diff --git a/superset-frontend/src/components/DatabaseSelector/index.tsx b/superset-frontend/src/components/DatabaseSelector/index.tsx index 6fc50b3e55..7d110b4215 100644 --- a/superset-frontend/src/components/DatabaseSelector/index.tsx +++ b/superset-frontend/src/components/DatabaseSelector/index.tsx @@ -144,6 +144,9 @@ export function DatabaseSelector({ ); const schemaRef = useRef(schema); schemaRef.current = schema; + // Track if we've applied defaults to avoid re-applying after user clears selection + const appliedCatalogDefaultRef = useRef<string | null>(null); + const appliedSchemaDefaultRef = useRef<string | null>(null); const { addSuccessToast } = useToasts(); const sortComparator = useCallback( (itemA: AntdLabeledValueWithOrder, itemB: AntdLabeledValueWithOrder) => @@ -240,20 +243,34 @@ export function DatabaseSelector({ } const { - currentData: schemaData, + data: schemaData, isFetching: loadingSchemas, refetch: refetchSchemas, } = useSchemas({ dbId: currentDb?.value, catalog: currentCatalog?.value, - onSuccess: (schemas, isFetched) => { + onSuccess: (schemas, isFetched, defaultSchema) => { setErrorPayload(null); if (schemas.length === 1) { changeSchema(schemas[0]); } else if ( !schemas.find(schemaOption => schemaRef.current === schemaOption.value) ) { - changeSchema(undefined); + // Current selection not in list - try to apply default on first load + if ( + defaultSchema && + appliedSchemaDefaultRef.current !== defaultSchema + ) { + const defaultOption = schemas.find(s => s.value === defaultSchema); + if (defaultOption) { + appliedSchemaDefaultRef.current = defaultSchema; + changeSchema(defaultOption); + } else { + changeSchema(undefined); + } + } else { + changeSchema(undefined); + } } if (isFetched) { @@ -274,6 +291,8 @@ export function DatabaseSelector({ function changeCatalog(catalog: CatalogOption | null | undefined) { setCurrentCatalog(catalog); setCurrentSchema(undefined); + // Reset schema default ref so default can be applied for the new catalog + appliedSchemaDefaultRef.current = null; if (onCatalogChange && catalog?.value !== catalogRef.current) { onCatalogChange(catalog?.value); } @@ -285,7 +304,7 @@ export function DatabaseSelector({ refetch: refetchCatalogs, } = useCatalogs({ dbId: showCatalogSelector ? currentDb?.value : undefined, - onSuccess: (catalogs, isFetched) => { + onSuccess: (catalogs, isFetched, defaultCatalog) => { setErrorPayload(null); if (!showCatalogSelector) { changeCatalog(null); @@ -296,7 +315,21 @@ export function DatabaseSelector({ catalogOption => catalogRef.current === catalogOption.value, ) ) { - changeCatalog(undefined); + // Current selection not in list - try to apply default on first load + if ( + defaultCatalog && + appliedCatalogDefaultRef.current !== defaultCatalog + ) { + const defaultOption = catalogs.find(c => c.value === defaultCatalog); + if (defaultOption) { + appliedCatalogDefaultRef.current = defaultCatalog; + changeCatalog(defaultOption); + } else { + changeCatalog(undefined); + } + } else { + changeCatalog(undefined); + } } if (showCatalogSelector && isFetched) { @@ -326,6 +359,9 @@ export function DatabaseSelector({ setCurrentDb(databaseWithId); setCurrentCatalog(undefined); setCurrentSchema(undefined); + // Reset default refs so defaults can be applied for the new database + appliedCatalogDefaultRef.current = null; + appliedSchemaDefaultRef.current = null; if (onDbChange) { onDbChange(databaseWithId); } diff --git a/superset-frontend/src/hooks/apiResources/schemas.test.ts b/superset-frontend/src/hooks/apiResources/catalogs.test.ts similarity index 54% copy from superset-frontend/src/hooks/apiResources/schemas.test.ts copy to superset-frontend/src/hooks/apiResources/catalogs.test.ts index b47ef894d0..ea5bd1a077 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.test.ts +++ b/superset-frontend/src/hooks/apiResources/catalogs.test.ts @@ -24,16 +24,15 @@ import { defaultStore as store, } from 'spec/helpers/testing-library'; import { api } from 'src/hooks/apiResources/queryApi'; -import { useSchemas } from './schemas'; +import { useCatalogs } from './catalogs'; const fakeApiResult = { - result: ['test schema 1', 'test schema b'], + result: ['catalog_a', 'catalog_b'], + default: 'catalog_a', }; const fakeApiResult2 = { - result: ['test schema 2', 'test schema a'], -}; -const fakeApiResult3 = { - result: ['test schema 3', 'test schema c'], + result: ['catalog_c', 'catalog_d'], + default: null, }; const expectedResult = fakeApiResult.result.map((value: string) => ({ @@ -46,28 +45,23 @@ const expectedResult2 = fakeApiResult2.result.map((value: string) => ({ label: value, title: value, })); -const expectedResult3 = fakeApiResult3.result.map((value: string) => ({ - value, - label: value, - title: value, -})); // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks -describe('useSchemas hook', () => { +describe('useCatalogs hook', () => { beforeEach(() => { fetchMock.reset(); store.dispatch(api.util.resetApiState()); }); - test('returns api response mapping json result', async () => { + test('returns api response mapping json result with default catalog', async () => { const expectDbId = 'db1'; const forceRefresh = false; - const schemaApiRoute = `glob:*/api/v1/database/${expectDbId}/schemas/*`; - fetchMock.get(schemaApiRoute, fakeApiResult); + const catalogApiRoute = `glob:*/api/v1/database/${expectDbId}/catalogs/*`; + fetchMock.get(catalogApiRoute, fakeApiResult); const onSuccess = jest.fn(); const { result, waitFor } = renderHook( () => - useSchemas({ + useCatalogs({ dbId: expectDbId, onSuccess, }), @@ -78,11 +72,14 @@ describe('useSchemas hook', () => { }), }, ); - await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(1)); + await waitFor(() => + expect(fetchMock.calls(catalogApiRoute).length).toBe(1), + ); expect(result.current.data).toEqual(expectedResult); + expect(result.current.defaultCatalog).toBe('catalog_a'); expect( fetchMock.calls( - `end:/api/v1/database/${expectDbId}/schemas/?q=${rison.encode({ + `end:/api/v1/database/${expectDbId}/catalogs/?q=${rison.encode({ force: forceRefresh, })}`, ).length, @@ -91,25 +88,28 @@ describe('useSchemas hook', () => { act(() => { result.current.refetch(); }); - await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(2)); + await waitFor(() => + expect(fetchMock.calls(catalogApiRoute).length).toBe(2), + ); expect( fetchMock.calls( - `end:/api/v1/database/${expectDbId}/schemas/?q=${rison.encode({ + `end:/api/v1/database/${expectDbId}/catalogs/?q=${rison.encode({ force: true, })}`, ).length, ).toBe(1); expect(onSuccess).toHaveBeenCalledTimes(2); expect(result.current.data).toEqual(expectedResult); + expect(result.current.defaultCatalog).toBe('catalog_a'); }); test('returns cached data without api request', async () => { const expectDbId = 'db1'; - const schemaApiRoute = `glob:*/api/v1/database/${expectDbId}/schemas/*`; - fetchMock.get(schemaApiRoute, fakeApiResult); + const catalogApiRoute = `glob:*/api/v1/database/${expectDbId}/catalogs/*`; + fetchMock.get(catalogApiRoute, fakeApiResult); const { result, rerender, waitFor } = renderHook( () => - useSchemas({ + useCatalogs({ dbId: expectDbId, }), { @@ -120,22 +120,24 @@ describe('useSchemas hook', () => { }, ); await waitFor(() => expect(result.current.data).toEqual(expectedResult)); - expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + expect(result.current.defaultCatalog).toBe('catalog_a'); + expect(fetchMock.calls(catalogApiRoute).length).toBe(1); rerender(); await waitFor(() => expect(result.current.data).toEqual(expectedResult)); - expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + expect(result.current.defaultCatalog).toBe('catalog_a'); + expect(fetchMock.calls(catalogApiRoute).length).toBe(1); }); - test('returns refreshed data after expires', async () => { + test('returns refreshed data after switching databases', async () => { const expectDbId = 'db1'; - const schemaApiRoute = `glob:*/api/v1/database/*/schemas/*`; - fetchMock.get(schemaApiRoute, url => + const catalogApiRoute = `glob:*/api/v1/database/*/catalogs/*`; + fetchMock.get(catalogApiRoute, url => url.includes(expectDbId) ? fakeApiResult : fakeApiResult2, ); const onSuccess = jest.fn(); const { result, rerender, waitFor } = renderHook( ({ dbId }) => - useSchemas({ + useCatalogs({ dbId, onSuccess, }), @@ -148,74 +150,55 @@ describe('useSchemas hook', () => { }, ); - await waitFor(() => - expect(result.current.currentData).toEqual(expectedResult), - ); - expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + expect(result.current.defaultCatalog).toBe('catalog_a'); + expect(fetchMock.calls(catalogApiRoute).length).toBe(1); expect(onSuccess).toHaveBeenCalledTimes(1); rerender({ dbId: 'db2' }); - await waitFor(() => - expect(result.current.currentData).toEqual(expectedResult2), - ); - expect(fetchMock.calls(schemaApiRoute).length).toBe(2); + await waitFor(() => expect(result.current.data).toEqual(expectedResult2)); + expect(result.current.defaultCatalog).toBeNull(); + expect(fetchMock.calls(catalogApiRoute).length).toBe(2); expect(onSuccess).toHaveBeenCalledTimes(2); rerender({ dbId: expectDbId }); - await waitFor(() => - expect(result.current.currentData).toEqual(expectedResult), - ); - expect(fetchMock.calls(schemaApiRoute).length).toBe(2); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + expect(result.current.defaultCatalog).toBe('catalog_a'); + expect(fetchMock.calls(catalogApiRoute).length).toBe(2); expect(onSuccess).toHaveBeenCalledTimes(2); // clean up cache act(() => { - store.dispatch(api.util.invalidateTags(['Schemas'])); + store.dispatch(api.util.invalidateTags(['Catalogs'])); }); - await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(4)); - expect(fetchMock.calls(schemaApiRoute)[2][0]).toContain(expectDbId); await waitFor(() => - expect(result.current.currentData).toEqual(expectedResult), + expect(fetchMock.calls(catalogApiRoute).length).toBe(4), ); + expect(fetchMock.calls(catalogApiRoute)[2][0]).toContain(expectDbId); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + expect(result.current.defaultCatalog).toBe('catalog_a'); }); - 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, + test('returns null defaultCatalog when API response has no default', async () => { + const expectDbId = 'db-no-default'; + const catalogApiRoute = `glob:*/api/v1/database/${expectDbId}/catalogs/*`; + fetchMock.get(catalogApiRoute, { result: ['catalog1', 'catalog2'] }); + const { result, waitFor } = renderHook( + () => + useCatalogs({ + dbId: expectDbId, }), { - 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(1); - - 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); + await waitFor(() => + expect(fetchMock.calls(catalogApiRoute).length).toBe(1), + ); + expect(result.current.defaultCatalog).toBeNull(); }); }); diff --git a/superset-frontend/src/hooks/apiResources/catalogs.ts b/superset-frontend/src/hooks/apiResources/catalogs.ts index 636b5d92ec..69aaff9a55 100644 --- a/superset-frontend/src/hooks/apiResources/catalogs.ts +++ b/superset-frontend/src/hooks/apiResources/catalogs.ts @@ -30,7 +30,11 @@ export type CatalogOption = { export type FetchCatalogsQueryParams = { dbId?: string | number; forceRefresh: boolean; - onSuccess?: (data: CatalogOption[], isRefetched: boolean) => void; + onSuccess?: ( + data: CatalogOption[], + isRefetched: boolean, + defaultCatalog: string | null, + ) => void; onError?: (error: ClientErrorObject) => void; }; @@ -97,7 +101,11 @@ export function useCatalogs(options: Params) { if (dbId && (!result.currentData || forceRefresh)) { trigger({ dbId, forceRefresh }).then(({ isSuccess, isError, data }) => { if (isSuccess) { - onSuccess?.(data?.catalogs || EMPTY_CATALOGS, forceRefresh); + onSuccess?.( + data?.catalogs || EMPTY_CATALOGS, + forceRefresh, + data?.defaultCatalog ?? null, + ); } if (isError) { onError?.(result.error as ClientErrorObject); diff --git a/superset-frontend/src/hooks/apiResources/schemas.test.ts b/superset-frontend/src/hooks/apiResources/schemas.test.ts index b47ef894d0..d0c7801581 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.test.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.test.ts @@ -28,12 +28,15 @@ import { useSchemas } from './schemas'; const fakeApiResult = { result: ['test schema 1', 'test schema b'], + default: 'test schema 1', }; const fakeApiResult2 = { result: ['test schema 2', 'test schema a'], + default: null, }; const fakeApiResult3 = { result: ['test schema 3', 'test schema c'], + default: 'test schema c', }; const expectedResult = fakeApiResult.result.map((value: string) => ({ @@ -80,6 +83,7 @@ describe('useSchemas hook', () => { ); await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(1)); expect(result.current.data).toEqual(expectedResult); + expect(result.current.defaultSchema).toBe('test schema 1'); expect( fetchMock.calls( `end:/api/v1/database/${expectDbId}/schemas/?q=${rison.encode({ @@ -120,9 +124,11 @@ describe('useSchemas hook', () => { }, ); await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + expect(result.current.defaultSchema).toBe('test schema 1'); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); rerender(); await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + expect(result.current.defaultSchema).toBe('test schema 1'); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); }); @@ -148,23 +154,20 @@ describe('useSchemas hook', () => { }, ); - await waitFor(() => - expect(result.current.currentData).toEqual(expectedResult), - ); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + expect(result.current.defaultSchema).toBe('test schema 1'); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); expect(onSuccess).toHaveBeenCalledTimes(1); rerender({ dbId: 'db2' }); - await waitFor(() => - expect(result.current.currentData).toEqual(expectedResult2), - ); + await waitFor(() => expect(result.current.data).toEqual(expectedResult2)); + expect(result.current.defaultSchema).toBeNull(); expect(fetchMock.calls(schemaApiRoute).length).toBe(2); expect(onSuccess).toHaveBeenCalledTimes(2); rerender({ dbId: expectDbId }); - await waitFor(() => - expect(result.current.currentData).toEqual(expectedResult), - ); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + expect(result.current.defaultSchema).toBe('test schema 1'); expect(fetchMock.calls(schemaApiRoute).length).toBe(2); expect(onSuccess).toHaveBeenCalledTimes(2); @@ -175,9 +178,7 @@ describe('useSchemas hook', () => { await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(4)); expect(fetchMock.calls(schemaApiRoute)[2][0]).toContain(expectDbId); - await waitFor(() => - expect(result.current.currentData).toEqual(expectedResult), - ); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); }); test('returns correct schema list by a catalog', async () => { @@ -208,14 +209,37 @@ describe('useSchemas hook', () => { await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(1)); expect(result.current.data).toEqual(expectedResult3); + expect(result.current.defaultSchema).toBe('test schema c'); expect(onSuccess).toHaveBeenCalledTimes(1); rerender({ dbId, catalog: 'catalog2' }); await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(2)); expect(result.current.data).toEqual(expectedResult2); + expect(result.current.defaultSchema).toBeNull(); rerender({ dbId, catalog: expectCatalog }); expect(result.current.data).toEqual(expectedResult3); + expect(result.current.defaultSchema).toBe('test schema c'); expect(fetchMock.calls(schemaApiRoute).length).toBe(2); }); + + test('returns null defaultSchema when API response has no default', async () => { + const expectDbId = 'db-no-default'; + const schemaApiRoute = `glob:*/api/v1/database/${expectDbId}/schemas/*`; + fetchMock.get(schemaApiRoute, { result: ['schema1', 'schema2'] }); + const { result, waitFor } = renderHook( + () => + useSchemas({ + dbId: expectDbId, + }), + { + wrapper: createWrapper({ + useRedux: true, + store, + }), + }, + ); + await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(1)); + expect(result.current.defaultSchema).toBeNull(); + }); }); diff --git a/superset-frontend/src/hooks/apiResources/schemas.ts b/superset-frontend/src/hooks/apiResources/schemas.ts index 41a729c118..20261b4f3e 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.ts @@ -31,7 +31,11 @@ export type FetchSchemasQueryParams = { dbId?: string | number; catalog?: string; forceRefresh: boolean; - onSuccess?: (data: SchemaOption[], isRefetched: boolean) => void; + onSuccess?: ( + data: SchemaOption[], + isRefetched: boolean, + defaultSchema: string | null, + ) => void; onError?: (error: ClientErrorObject) => void; }; @@ -106,7 +110,11 @@ export function useSchemas(options: Params) { trigger({ dbId, catalog, forceRefresh }).then( ({ isSuccess, isError, data }) => { if (isSuccess) { - onSuccess?.(data?.schemas || EMPTY_SCHEMAS, forceRefresh); + onSuccess?.( + data?.schemas || EMPTY_SCHEMAS, + forceRefresh, + data?.defaultSchema ?? null, + ); } if (isError) { onError?.(result.error as ClientErrorObject); diff --git a/superset-frontend/src/hooks/apiResources/tables.ts b/superset-frontend/src/hooks/apiResources/tables.ts index 81792b4a52..621ca7cc54 100644 --- a/superset-frontend/src/hooks/apiResources/tables.ts +++ b/superset-frontend/src/hooks/apiResources/tables.ts @@ -167,7 +167,7 @@ export const { export function useTables(options: Params) { const { dbId, catalog, schema, onSuccess, onError } = options || {}; const isMountedRef = useRef(false); - const { currentData: schemaOptions, isFetching } = useSchemas({ + const { data: schemaOptions, isFetching } = useSchemas({ dbId, catalog: catalog || undefined, });
