From 779b372d8992386b6c8769af4bfa22a3e65c7b4d Mon Sep 17 00:00:00 2001 From: "JUST.in DO IT" Date: Tue, 23 May 2023 10:42:00 -0700 Subject: [PATCH] chore(sqllab): Remove functionNames from sqlLab state (#24026) --- .../spec/helpers/testing-library.tsx | 2 +- .../src/SqlLab/actions/sqlLab.js | 31 ------- .../components/AceEditorWrapper/index.tsx | 25 +++-- .../components/SqlEditorLeftBar/index.tsx | 2 - superset-frontend/src/SqlLab/fixtures.ts | 1 - .../src/SqlLab/reducers/getInitialState.js | 2 - .../src/SqlLab/reducers/sqlLab.js | 12 --- .../src/SqlLab/reducers/sqlLab.test.js | 9 +- superset-frontend/src/SqlLab/types.ts | 1 - .../apiResources/databaseFunctions.test.ts | 92 +++++++++++++++++++ .../hooks/apiResources/databaseFunctions.ts | 45 +++++++++ .../src/hooks/apiResources/queryApi.ts | 2 +- 12 files changed, 161 insertions(+), 63 deletions(-) create mode 100644 superset-frontend/src/hooks/apiResources/databaseFunctions.test.ts create mode 100644 superset-frontend/src/hooks/apiResources/databaseFunctions.ts diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index 0c7b6a0a6..eb3e1401e 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -73,7 +73,7 @@ export function createWrapper(options?: Options) { result = {result}; } - if (useRedux) { + if (useRedux || store) { const mockStore = store ?? createStore(initialState, reducers || reducerIndex); result = {result}; diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 3b9e68b9c..cb7a7e49b 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -1555,34 +1555,3 @@ export function createCtasDatasource(vizOptions) { }); }; } - -export function queryEditorSetFunctionNames(queryEditor, dbId) { - return function (dispatch) { - return SupersetClient.get({ - endpoint: encodeURI(`/api/v1/database/${dbId}/function_names/`), - }) - .then(({ json }) => - dispatch({ - type: QUERY_EDITOR_SET_FUNCTION_NAMES, - queryEditor, - functionNames: json.function_names, - }), - ) - .catch(err => { - if (err.status === 404) { - // for databases that have been deleted, just reset the function names - dispatch({ - type: QUERY_EDITOR_SET_FUNCTION_NAMES, - queryEditor, - functionNames: [], - }); - } else { - dispatch( - addDangerToast( - t('An error occurred while fetching function names.'), - ), - ); - } - }); - }; -} diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx index b60faacbe..a89c46506 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx @@ -18,14 +18,14 @@ */ import React, { useState, useEffect, useRef, useMemo } from 'react'; import { useDispatch } from 'react-redux'; -import { css, styled, usePrevious } from '@superset-ui/core'; +import { css, styled, usePrevious, t } from '@superset-ui/core'; import { areArraysShallowEqual } from 'src/reduxUtils'; import sqlKeywords from 'src/SqlLab/utils/sqlKeywords'; import { queryEditorSetSelectedText, - queryEditorSetFunctionNames, addTable, + addDangerToast, } from 'src/SqlLab/actions/sqlLab'; import { SCHEMA_AUTOCOMPLETE_SCORE, @@ -40,6 +40,7 @@ import { } from 'src/components/AsyncAceEditor'; import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor'; import { useSchemas, useTables } from 'src/hooks/apiResources'; +import { useDatabaseFunctionsQuery } from 'src/hooks/apiResources/databaseFunctions'; type HotKey = { key: string; @@ -95,7 +96,6 @@ const AceEditorWrapper = ({ 'id', 'dbId', 'sql', - 'functionNames', 'validationResult', 'schema', ]); @@ -109,8 +109,20 @@ const AceEditorWrapper = ({ }), }); + const { data: functionNames, isError } = useDatabaseFunctionsQuery( + { dbId: queryEditor.dbId }, + { skip: !autocomplete || !queryEditor.dbId }, + ); + + useEffect(() => { + if (isError) { + dispatch( + addDangerToast(t('An error occurred while fetching function names.')), + ); + } + }, [dispatch, isError]); + const currentSql = queryEditor.sql ?? ''; - const functionNames = queryEditor.functionNames ?? []; // Loading schema, table and column names as auto-completable words const { schemas, schemaWords } = useMemo( @@ -139,9 +151,6 @@ const AceEditorWrapper = ({ useEffect(() => { // Making sure no text is selected from previous mount dispatch(queryEditorSetSelectedText(queryEditor, null)); - if (queryEditor.dbId) { - dispatch(queryEditorSetFunctionNames(queryEditor, queryEditor.dbId)); - } setAutoCompleter(); }, []); @@ -238,7 +247,7 @@ const AceEditorWrapper = ({ meta: 'column', })); - const functionWords = functionNames.map(func => ({ + const functionWords = (functionNames ?? []).map(func => ({ name: func, value: func, score: SQL_FUNCTIONS_AUTOCOMPLETE_SCORE, diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx index 1298722d5..453d80f11 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx @@ -29,7 +29,6 @@ import querystring from 'query-string'; import { queryEditorSetDb, - queryEditorSetFunctionNames, addTable, removeTables, collapseTable, @@ -142,7 +141,6 @@ const SqlEditorLeftBar = ({ const onDbChange = ({ id: dbId }: { id: number }) => { setEmptyState(false); dispatch(queryEditorSetDb(queryEditor, dbId)); - dispatch(queryEditorSetFunctionNames(queryEditor, dbId)); }; const selectedTableNames = useMemo( diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index cfd15bedb..e295b0236 100644 --- a/superset-frontend/src/SqlLab/fixtures.ts +++ b/superset-frontend/src/SqlLab/fixtures.ts @@ -185,7 +185,6 @@ export const defaultQueryEditor = { name: 'Untitled Query 1', schema: 'main', remoteId: null, - functionNames: [], hideLeftBar: false, templateParams: '{}', }; diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.js b/superset-frontend/src/SqlLab/reducers/getInitialState.js index 5cd678657..03d6ec552 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.js +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.js @@ -56,7 +56,6 @@ export default function getInitialState({ autorun: false, templateParams: null, dbId: defaultDbId, - functionNames: [], queryLimit: common.conf.DEFAULT_SQLLAB_LIMIT, validationResult: { id: null, @@ -87,7 +86,6 @@ export default function getInitialState({ autorun: activeTab.autorun, templateParams: activeTab.template_params || undefined, dbId: activeTab.database_id, - functionNames: [], schema: activeTab.schema, queryLimit: activeTab.query_limit, validationResult: { diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js index eafc326aa..6ff81f03d 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.js @@ -563,18 +563,6 @@ export default function sqlLabReducer(state = {}, action) { ), }; }, - [actions.QUERY_EDITOR_SET_FUNCTION_NAMES]() { - return { - ...state, - ...alterUnsavedQueryEditorState( - state, - { - functionNames: action.functionNames, - }, - action.queryEditor.id, - ), - }; - }, [actions.QUERY_EDITOR_SET_SCHEMA]() { return { ...state, diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js index dd4c0be4b..f1c482e67 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js @@ -209,14 +209,15 @@ describe('sqlLabReducer', () => { newState = sqlLabReducer(newState, action); expect(newState.unsavedQueryEditor.sql).toBe(expectedSql); const interceptedAction = { - type: actions.QUERY_EDITOR_SET_FUNCTION_NAMES, + type: actions.QUERY_EDITOR_PERSIST_HEIGHT, queryEditor: newState.queryEditors[0], - functionNames: ['func1', 'func2'], + northPercent: 46, + southPercent: 54, }; newState = sqlLabReducer(newState, interceptedAction); expect(newState.unsavedQueryEditor.sql).toBe(expectedSql); - expect(newState.queryEditors[0].functionNames).toBe( - interceptedAction.functionNames, + expect(newState.queryEditors[0].northPercent).toBe( + interceptedAction.northPercent, ); }); }); diff --git a/superset-frontend/src/SqlLab/types.ts b/superset-frontend/src/SqlLab/types.ts index e209be04b..f7ab930b4 100644 --- a/superset-frontend/src/SqlLab/types.ts +++ b/superset-frontend/src/SqlLab/types.ts @@ -39,7 +39,6 @@ export interface QueryEditor { autorun: boolean; sql: string; remoteId: number | null; - functionNames: string[]; validationResult?: { completed: boolean; errors: SupersetError[]; diff --git a/superset-frontend/src/hooks/apiResources/databaseFunctions.test.ts b/superset-frontend/src/hooks/apiResources/databaseFunctions.test.ts new file mode 100644 index 000000000..8eca5112a --- /dev/null +++ b/superset-frontend/src/hooks/apiResources/databaseFunctions.test.ts @@ -0,0 +1,92 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import fetchMock from 'fetch-mock'; +import { act, renderHook } from '@testing-library/react-hooks'; +import { + createWrapper, + defaultStore as store, +} from 'spec/helpers/testing-library'; +import { api } from 'src/hooks/apiResources/queryApi'; +import { useDatabaseFunctionsQuery } from './databaseFunctions'; + +const fakeApiResult = { + function_names: ['abs', 'avg', 'sum'], +}; + +const expectedResult = fakeApiResult.function_names; +const expectDbId = 'db1'; +const dbFunctionNamesApiRoute = `glob:*/api/v1/database/${expectDbId}/function_names/`; + +afterEach(() => { + fetchMock.reset(); + act(() => { + store.dispatch(api.util.resetApiState()); + }); +}); + +beforeEach(() => { + fetchMock.get(dbFunctionNamesApiRoute, fakeApiResult); +}); + +test('returns api response mapping json result', async () => { + const { result, waitFor } = renderHook( + () => + useDatabaseFunctionsQuery({ + dbId: expectDbId, + }), + { + wrapper: createWrapper({ + useRedux: true, + store, + }), + }, + ); + await waitFor(() => + expect(fetchMock.calls(dbFunctionNamesApiRoute).length).toBe(1), + ); + expect(result.current.data).toEqual(expectedResult); + expect(fetchMock.calls(dbFunctionNamesApiRoute).length).toBe(1); + act(() => { + result.current.refetch(); + }); + await waitFor(() => + expect(fetchMock.calls(dbFunctionNamesApiRoute).length).toBe(2), + ); + expect(result.current.data).toEqual(expectedResult); +}); + +test('returns cached data without api request', async () => { + const { result, waitFor, rerender } = renderHook( + () => + useDatabaseFunctionsQuery({ + dbId: expectDbId, + }), + { + wrapper: createWrapper({ + store, + useRedux: true, + }), + }, + ); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + expect(fetchMock.calls(dbFunctionNamesApiRoute).length).toBe(1); + rerender(); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + expect(fetchMock.calls(dbFunctionNamesApiRoute).length).toBe(1); +}); diff --git a/superset-frontend/src/hooks/apiResources/databaseFunctions.ts b/superset-frontend/src/hooks/apiResources/databaseFunctions.ts new file mode 100644 index 000000000..ed7fbb2c4 --- /dev/null +++ b/superset-frontend/src/hooks/apiResources/databaseFunctions.ts @@ -0,0 +1,45 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { api } from './queryApi'; + +export type FetchDataFunctionsQueryParams = { + dbId?: string | number; +}; + +type FunctionNamesResponse = { + json: { + function_names: string[]; + }; + response: Response; +}; + +const databaseFunctionApi = api.injectEndpoints({ + endpoints: builder => ({ + databaseFunctions: builder.query({ + providesTags: ['DatabaseFunctions'], + query: ({ dbId }) => ({ + endpoint: `/api/v1/database/${dbId}/function_names/`, + transformResponse: ({ json }: FunctionNamesResponse) => + json.function_names, + }), + }), + }), +}); + +export const { useDatabaseFunctionsQuery } = databaseFunctionApi; diff --git a/superset-frontend/src/hooks/apiResources/queryApi.ts b/superset-frontend/src/hooks/apiResources/queryApi.ts index 9147173d7..5ccc878b0 100644 --- a/superset-frontend/src/hooks/apiResources/queryApi.ts +++ b/superset-frontend/src/hooks/apiResources/queryApi.ts @@ -65,7 +65,7 @@ export const supersetClientQuery: BaseQueryFn< export const api = createApi({ reducerPath: 'queryApi', - tagTypes: ['Schemas', 'Tables'], + tagTypes: ['Schemas', 'Tables', 'DatabaseFunctions'], endpoints: () => ({}), baseQuery: supersetClientQuery, });