From 5539f87912d6c99f81ea91f8001f36f43ce7c139 Mon Sep 17 00:00:00 2001 From: "JUST.in DO IT" Date: Fri, 19 Jul 2024 10:21:16 -0700 Subject: [PATCH] fix(sqllab): prev shema/table options remained on fail (#29638) --- .../AceEditorWrapper/useKeywords.ts | 6 +-- .../SqlLab/components/QueryHistory/index.tsx | 6 ++- .../SqlEditorLeftBar.test.tsx | 37 ++++++++++++++++++- .../SqlLab/components/TableElement/index.tsx | 4 +- .../DatabaseSelector.test.tsx | 5 +-- .../src/components/DatabaseSelector/index.tsx | 2 +- .../src/components/TableSelector/index.tsx | 2 +- .../src/hooks/apiResources/sqlLab.ts | 4 +- .../src/hooks/apiResources/tables.ts | 8 ++-- 9 files changed, 55 insertions(+), 19 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts index df45290f6..e02145f96 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts @@ -77,7 +77,7 @@ export function useKeywords( // skipFetch is used to prevent re-evaluating memoized keywords // due to updated api results by skip flag const skipFetch = hasFetchedKeywords && skip; - const { data: schemaOptions } = useSchemasQueryState( + const { currentData: schemaOptions } = useSchemasQueryState( { dbId, catalog: catalog || undefined, @@ -85,7 +85,7 @@ export function useKeywords( }, { skip: skipFetch || !dbId }, ); - const { data: tableData } = useTablesQueryState( + const { currentData: tableData } = useTablesQueryState( { dbId, catalog, @@ -95,7 +95,7 @@ export function useKeywords( { skip: skipFetch || !dbId || !schema }, ); - const { data: functionNames, isError } = useDatabaseFunctionsQuery( + const { currentData: functionNames, isError } = useDatabaseFunctionsQuery( { dbId }, { skip: skipFetch || !dbId }, ); diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx index 90b905c5a..4093a8feb 100644 --- a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx @@ -70,7 +70,11 @@ const QueryHistory = ({ ({ sqlLab: { queries } }: SqlLabRootState) => queries, shallowEqual, ); - const { data, isLoading, isFetching } = useEditorQueriesQuery( + const { + currentData: data, + isLoading, + isFetching, + } = useEditorQueriesQuery( { editorId: `${queryEditorId}`, pageIndex }, { skip: !isFeatureEnabled(FeatureFlag.SqllabBackendPersistence), diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx index d322b8cf3..3abb5b400 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx @@ -47,7 +47,14 @@ beforeEach(() => { count: 0, result: [], }); - fetchMock.get('glob:*/api/v1/database/*/schemas/?*', { + fetchMock.get('glob:*/api/v1/database/3/schemas/?*', { + error: 'Unauthorized', + }); + fetchMock.get('glob:*/api/v1/database/1/schemas/?*', { + count: 2, + result: ['main', 'db1_schema', 'db1_schema2'], + }); + fetchMock.get('glob:*/api/v1/database/2/schemas/?*', { count: 2, result: ['main', 'new_schema'], }); @@ -198,7 +205,7 @@ test('should toggle the table when the header is clicked', async () => { ); }); -test('When changing database the table list must be updated', async () => { +test('When changing database the schema and table list must be updated', async () => { const { rerender } = await renderAndWait(mockedProps, undefined, { ...initialState, sqlLab: { @@ -245,6 +252,32 @@ test('When changing database the table list must be updated', async () => { expect(updatedDbSelector[0]).toBeInTheDocument(); const updatedTableSelector = await screen.findAllByText(/new_table/i); expect(updatedTableSelector[0]).toBeInTheDocument(); + + const select = screen.getByRole('combobox', { + name: 'Select schema or type to search schemas', + }); + userEvent.click(select); + expect( + await screen.findByRole('option', { name: 'main' }), + ).toBeInTheDocument(); + expect( + await screen.findByRole('option', { name: 'new_schema' }), + ).toBeInTheDocument(); + rerender( + , + ); + userEvent.click(select); + expect( + await screen.findByText('No compatible schema found'), + ).toBeInTheDocument(); }); test('ignore schema api when current schema is deprecated', async () => { diff --git a/superset-frontend/src/SqlLab/components/TableElement/index.tsx b/superset-frontend/src/SqlLab/components/TableElement/index.tsx index 87c2821f6..faa56db6b 100644 --- a/superset-frontend/src/SqlLab/components/TableElement/index.tsx +++ b/superset-frontend/src/SqlLab/components/TableElement/index.tsx @@ -105,7 +105,7 @@ const TableElement = ({ table, ...props }: TableElementProps) => { const theme = useTheme(); const dispatch = useDispatch(); const { - data: tableMetadata, + currentData: tableMetadata, isSuccess: isMetadataSuccess, isLoading: isMetadataLoading, isError: hasMetadataError, @@ -119,7 +119,7 @@ const TableElement = ({ table, ...props }: TableElementProps) => { { skip: !expanded }, ); const { - data: tableExtendedMetadata, + currentData: tableExtendedMetadata, isSuccess: isExtraMetadataSuccess, isLoading: isExtraMetadataLoading, isError: hasExtendedMetadataError, diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index bebdf0193..6b8d45dd0 100644 --- a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx +++ b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx @@ -272,7 +272,6 @@ test('should display options in order of the api response', async () => { }); test('Should fetch the search keyword when total count exceeds initial options', async () => { - fetchMock.reset(); fetchMock.get( databaseApiRoute, { @@ -365,7 +364,7 @@ test('Sends the correct schema when changing the schema', async () => { }); await waitFor(() => expect(fetchMock.calls(databaseApiRoute).length).toBe(1)); rerender(); - expect(props.onSchemaChange).toBeCalledTimes(0); + expect(props.onSchemaChange).toHaveBeenCalledTimes(0); const select = screen.getByRole('combobox', { name: 'Select schema or type to search schemas', }); @@ -376,5 +375,5 @@ test('Sends the correct schema when changing the schema', async () => { await waitFor(() => expect(props.onSchemaChange).toHaveBeenCalledWith('information_schema'), ); - expect(props.onSchemaChange).toBeCalledTimes(1); + expect(props.onSchemaChange).toHaveBeenCalledTimes(1); }); diff --git a/superset-frontend/src/components/DatabaseSelector/index.tsx b/superset-frontend/src/components/DatabaseSelector/index.tsx index 4cf841156..321d753bc 100644 --- a/superset-frontend/src/components/DatabaseSelector/index.tsx +++ b/superset-frontend/src/components/DatabaseSelector/index.tsx @@ -260,7 +260,7 @@ export default function DatabaseSelector({ } const { - data: schemaData, + currentData: schemaData, isFetching: loadingSchemas, refetch: refetchSchemas, } = useSchemas({ diff --git a/superset-frontend/src/components/TableSelector/index.tsx b/superset-frontend/src/components/TableSelector/index.tsx index a80c9418a..940f42cb3 100644 --- a/superset-frontend/src/components/TableSelector/index.tsx +++ b/superset-frontend/src/components/TableSelector/index.tsx @@ -188,7 +188,7 @@ const TableSelector: FunctionComponent = ({ SelectValue | undefined >(undefined); const { - data, + currentData: data, isFetching: loadingTables, refetch, } = useTables({ diff --git a/superset-frontend/src/hooks/apiResources/sqlLab.ts b/superset-frontend/src/hooks/apiResources/sqlLab.ts index 45f7c83a3..54e24ae13 100644 --- a/superset-frontend/src/hooks/apiResources/sqlLab.ts +++ b/superset-frontend/src/hooks/apiResources/sqlLab.ts @@ -69,7 +69,7 @@ export type InitialState = { }[]; }; -const queryValidationApi = api.injectEndpoints({ +const initialStateApi = api.injectEndpoints({ endpoints: builder => ({ sqlLabInitialState: builder.query({ providesTags: ['SqlLabInitialState'], @@ -83,4 +83,4 @@ const queryValidationApi = api.injectEndpoints({ }); export const { useSqlLabInitialStateQuery: useSqlLabInitialState } = - queryValidationApi; + initialStateApi; diff --git a/superset-frontend/src/hooks/apiResources/tables.ts b/superset-frontend/src/hooks/apiResources/tables.ts index d90c528b4..f2accf499 100644 --- a/superset-frontend/src/hooks/apiResources/tables.ts +++ b/superset-frontend/src/hooks/apiResources/tables.ts @@ -152,7 +152,7 @@ export const { export function useTables(options: Params) { const { dbId, catalog, schema, onSuccess, onError } = options || {}; const isMountedRef = useRef(false); - const { data: schemaOptions, isFetching } = useSchemas({ + const { currentData: schemaOptions, isFetching } = useSchemas({ dbId, catalog: catalog || undefined, }); @@ -203,13 +203,13 @@ export function useTables(options: Params) { isSuccess, isError, isFetching, - data, + currentData, error, originalArgs, } = result; if (!originalArgs?.forceRefresh && requestId && !isFetching) { - if (isSuccess && data) { - handleOnSuccess(data, false); + if (isSuccess && currentData) { + handleOnSuccess(currentData, false); } if (isError) { handleOnError(error as Response);