From cdfb168d29072d13fd20efaaddd1298e9d85f8a9 Mon Sep 17 00:00:00 2001 From: justin-park Date: Thu, 20 Apr 2023 15:42:24 -0700 Subject: [PATCH 01/15] chore(sqllab): Refactor react-query by redux-toolkit query --- superset-frontend/package-lock.json | 96 ----------- superset-frontend/package.json | 1 - .../spec/helpers/testing-library.tsx | 22 ++- superset-frontend/src/SqlLab/App.jsx | 17 +- .../components/SqlEditor/SqlEditor.test.jsx | 161 +++++++----------- .../SqlEditorLeftBar.test.jsx | 84 +++++---- .../TabbedSqlEditors.test.jsx | 9 +- .../DatabaseSelector.test.tsx | 28 +-- .../src/components/DatabaseSelector/index.tsx | 9 +- .../Datasource/DatasourceModal.test.jsx | 18 +- .../TableSelector/TableSelector.test.tsx | 25 ++- .../src/components/TableSelector/index.tsx | 5 +- .../src/hooks/apiResources/queryApi.test.ts | 104 +++++++++++ .../src/hooks/apiResources/queryApi.ts | 68 ++++++++ .../src/hooks/apiResources/schemas.test.ts | 110 +++++++----- .../src/hooks/apiResources/schemas.ts | 115 ++++++++----- .../src/hooks/apiResources/tables.test.ts | 139 ++++++++------- .../src/hooks/apiResources/tables.ts | 134 +++++++++------ superset-frontend/src/views/App.tsx | 49 +++--- superset-frontend/src/views/QueryProvider.tsx | 43 ----- superset-frontend/src/views/store.ts | 4 +- 21 files changed, 692 insertions(+), 549 deletions(-) create mode 100644 superset-frontend/src/hooks/apiResources/queryApi.test.ts create mode 100644 superset-frontend/src/hooks/apiResources/queryApi.ts delete mode 100644 superset-frontend/src/views/QueryProvider.tsx diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 506f140fca46..972b2b52a690 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -119,7 +119,6 @@ "react-jsonschema-form": "^1.8.1", "react-lines-ellipsis": "^0.15.0", "react-loadable": "^5.5.0", - "react-query": "^3.39.2", "react-redux": "^7.2.8", "react-resize-detector": "^7.1.2", "react-reverse-portal": "^2.1.1", @@ -40836,11 +40835,6 @@ "node": ">=0.10.0" } }, - "node_modules/js-sha3": { - "version": "0.8.0", - "resolved": "https://registry.npmjs.org/js-sha3/-/js-sha3-0.8.0.tgz", - "integrity": "sha512-gF1cRrHhIzNfToc802P800N8PpXS+evLLXfsVpowqmAFR9uwbi89WvXg2QspOmXL8QL86J4T1EpFu+yUkwJY3Q==" - }, "node_modules/js-string-escape": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/js-string-escape/-/js-string-escape-1.0.1.tgz", @@ -50440,55 +50434,6 @@ "react-dom": "^16.6.0 || ^17.0.0" } }, - "node_modules/react-query": { - "version": "3.39.2", - "resolved": "https://registry.npmjs.org/react-query/-/react-query-3.39.2.tgz", - "integrity": "sha512-F6hYDKyNgDQfQOuR1Rsp3VRzJnWHx6aRnnIZHMNGGgbL3SBgpZTDg8MQwmxOgpCAoqZJA+JSNCydF1xGJqKOCA==", - "dependencies": { - "@babel/runtime": "^7.5.5", - "broadcast-channel": "^3.4.1", - "match-sorter": "^6.0.2" - }, - "funding": { - "type": "github", - "url": "https://github.com/sponsors/tannerlinsley" - }, - "peerDependencies": { - "react": "^16.8.0 || ^17.0.0 || ^18.0.0" - }, - "peerDependenciesMeta": { - "react-dom": { - "optional": true - }, - "react-native": { - "optional": true - } - } - }, - "node_modules/react-query/node_modules/broadcast-channel": { - "version": "3.7.0", - "resolved": "https://registry.npmjs.org/broadcast-channel/-/broadcast-channel-3.7.0.tgz", - "integrity": "sha512-cIAKJXAxGJceNZGTZSBzMxzyOn72cVgPnKx4dc6LRjQgbaJUQqhy5rzL3zbMxkMWsGKkv2hSFkPRMEXfoMZ2Mg==", - "dependencies": { - "@babel/runtime": "^7.7.2", - "detect-node": "^2.1.0", - "js-sha3": "0.8.0", - "microseconds": "0.2.0", - "nano-time": "1.0.0", - "oblivious-set": "1.0.0", - "rimraf": "3.0.2", - "unload": "2.2.0" - } - }, - "node_modules/react-query/node_modules/unload": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/unload/-/unload-2.2.0.tgz", - "integrity": "sha512-B60uB5TNBLtN6/LsgAf3udH9saB5p7gqJwcFfbOEZ8BcBHnGwCf6G/TGiEqkRAxX7zAFIUtzdrXQSdL3Q/wqNA==", - "dependencies": { - "@babel/runtime": "^7.6.2", - "detect-node": "^2.0.4" - } - }, "node_modules/react-redux": { "version": "7.2.8", "resolved": "https://registry.npmjs.org/react-redux/-/react-redux-7.2.8.tgz", @@ -94229,11 +94174,6 @@ "resolved": "https://registry.npmjs.org/js-levenshtein/-/js-levenshtein-1.1.6.tgz", "integrity": "sha512-X2BB11YZtrRqY4EnQcLX5Rh373zbK4alC1FW7D7MBhL2gtcC17cTnr6DmfHZeS0s2rTHjUTMMHfG7gO8SSdw+g==" }, - "js-sha3": { - "version": "0.8.0", - "resolved": "https://registry.npmjs.org/js-sha3/-/js-sha3-0.8.0.tgz", - "integrity": "sha512-gF1cRrHhIzNfToc802P800N8PpXS+evLLXfsVpowqmAFR9uwbi89WvXg2QspOmXL8QL86J4T1EpFu+yUkwJY3Q==" - }, "js-string-escape": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/js-string-escape/-/js-string-escape-1.0.1.tgz", @@ -101549,42 +101489,6 @@ "react-popper": "^2.2.4" } }, - "react-query": { - "version": "3.39.2", - "resolved": "https://registry.npmjs.org/react-query/-/react-query-3.39.2.tgz", - "integrity": "sha512-F6hYDKyNgDQfQOuR1Rsp3VRzJnWHx6aRnnIZHMNGGgbL3SBgpZTDg8MQwmxOgpCAoqZJA+JSNCydF1xGJqKOCA==", - "requires": { - "@babel/runtime": "^7.5.5", - "broadcast-channel": "^3.4.1", - "match-sorter": "^6.0.2" - }, - "dependencies": { - "broadcast-channel": { - "version": "3.7.0", - "resolved": "https://registry.npmjs.org/broadcast-channel/-/broadcast-channel-3.7.0.tgz", - "integrity": "sha512-cIAKJXAxGJceNZGTZSBzMxzyOn72cVgPnKx4dc6LRjQgbaJUQqhy5rzL3zbMxkMWsGKkv2hSFkPRMEXfoMZ2Mg==", - "requires": { - "@babel/runtime": "^7.7.2", - "detect-node": "^2.1.0", - "js-sha3": "0.8.0", - "microseconds": "0.2.0", - "nano-time": "1.0.0", - "oblivious-set": "1.0.0", - "rimraf": "3.0.2", - "unload": "2.2.0" - } - }, - "unload": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/unload/-/unload-2.2.0.tgz", - "integrity": "sha512-B60uB5TNBLtN6/LsgAf3udH9saB5p7gqJwcFfbOEZ8BcBHnGwCf6G/TGiEqkRAxX7zAFIUtzdrXQSdL3Q/wqNA==", - "requires": { - "@babel/runtime": "^7.6.2", - "detect-node": "^2.0.4" - } - } - } - }, "react-redux": { "version": "7.2.8", "resolved": "https://registry.npmjs.org/react-redux/-/react-redux-7.2.8.tgz", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index c84a259a2332..098e01f6109a 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -184,7 +184,6 @@ "react-jsonschema-form": "^1.8.1", "react-lines-ellipsis": "^0.15.0", "react-loadable": "^5.5.0", - "react-query": "^3.39.2", "react-redux": "^7.2.8", "react-resize-detector": "^7.1.2", "react-reverse-portal": "^2.1.1", diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index 4790b4b718ab..202daf3f7525 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -26,8 +26,8 @@ import { DndProvider } from 'react-dnd'; import { HTML5Backend } from 'react-dnd-html5-backend'; import reducerIndex from 'spec/helpers/reducerIndex'; import { QueryParamProvider } from 'use-query-params'; -import QueryProvider from 'src/views/QueryProvider'; import { configureStore, Store } from '@reduxjs/toolkit'; +import { api } from 'src/hooks/apiResources/queryApi'; type Options = Omit & { useRedux?: boolean; @@ -40,6 +40,15 @@ type Options = Omit & { store?: Store; }; +export const storeWithQuery = configureStore({ + reducer: { + [api.reducerPath]: api.reducer, + }, + middleware: getDefaultMiddleware => + getDefaultMiddleware().concat(api.middleware), + devTools: false, +}); + export function createWrapper(options?: Options) { const { useDnd, @@ -67,7 +76,12 @@ export function createWrapper(options?: Options) { configureStore({ preloadedState: initialState || {}, devTools: false, - reducer: reducers || reducerIndex, + reducer: { + ...(reducers || reducerIndex), + [api.reducerPath]: api.reducer, + }, + middleware: getDefaultMiddleware => + getDefaultMiddleware().concat(api.middleware), }); result = {result}; @@ -81,8 +95,8 @@ export function createWrapper(options?: Options) { result = {result}; } - if (useQuery) { - result = {result}; + if (useQuery && !useRedux) { + result = {result}; } return result; diff --git a/superset-frontend/src/SqlLab/App.jsx b/superset-frontend/src/SqlLab/App.jsx index 86b981ae76df..be0a982a4951 100644 --- a/superset-frontend/src/SqlLab/App.jsx +++ b/superset-frontend/src/SqlLab/App.jsx @@ -22,7 +22,6 @@ import { Provider } from 'react-redux'; import { hot } from 'react-hot-loader/root'; import { FeatureFlag, ThemeProvider } from '@superset-ui/core'; import { GlobalStyles } from 'src/GlobalStyles'; -import QueryProvider from 'src/views/QueryProvider'; import { initFeatureFlags, isFeatureEnabled } from 'src/featureFlags'; import { setupStore } from 'src/views/store'; import setupExtensions from 'src/setup/setupExtensions'; @@ -133,15 +132,13 @@ if (sqlLabMenu) { } const Application = () => ( - - - - - - - - - + + + + + + + ); export default hot(Application); diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx index 99551b9d41e8..d8a1aa260a7d 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx @@ -17,25 +17,12 @@ * under the License. */ import React from 'react'; -import { mount } from 'enzyme'; +import { act } from 'react-dom/test-utils'; import { fireEvent, render, waitFor } from 'spec/helpers/testing-library'; -import { supersetTheme, ThemeProvider } from '@superset-ui/core'; -import { Provider } from 'react-redux'; -import thunk from 'redux-thunk'; -import configureStore from 'redux-mock-store'; import fetchMock from 'fetch-mock'; -import { - SQL_EDITOR_GUTTER_HEIGHT, - SQL_EDITOR_GUTTER_MARGIN, - SQL_TOOLBAR_HEIGHT, -} from 'src/SqlLab/constants'; -import AceEditorWrapper from 'src/SqlLab/components/AceEditorWrapper'; -import SouthPane from 'src/SqlLab/components/SouthPane'; +import { reducers } from 'src/SqlLab/reducers'; import SqlEditor from 'src/SqlLab/components/SqlEditor'; import { setupStore } from 'src/views/store'; -import { reducers } from 'src/SqlLab/reducers'; -import QueryProvider from 'src/views/QueryProvider'; -import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; import { initialState, queries, @@ -43,6 +30,7 @@ import { defaultQueryEditor, } from 'src/SqlLab/fixtures'; import SqlEditorLeftBar from 'src/SqlLab/components/SqlEditorLeftBar'; +import { api } from 'src/hooks/apiResources/queryApi'; jest.mock('src/components/AsyncAceEditor', () => ({ ...jest.requireActual('src/components/AsyncAceEditor'), @@ -51,27 +39,24 @@ jest.mock('src/components/AsyncAceEditor', () => ({ data-test="react-ace" onChange={evt => onChange(evt.target.value)} onBlur={onBlur} - > - {value} - + value={value} + /> ), })); jest.mock('src/SqlLab/components/SqlEditorLeftBar', () => jest.fn()); -const MOCKED_SQL_EDITOR_HEIGHT = 500; - fetchMock.get('glob:*/api/v1/database/*', { result: [] }); fetchMock.get('glob:*/api/v1/database/*/tables/*', { options: [] }); fetchMock.post('glob:*/sqllab/execute/*', { result: [] }); -const middlewares = [thunk]; -const mockStore = configureStore(middlewares); +let store; +let actions; const mockInitialState = { ...initialState, sqlLab: { ...initialState.sqlLab, databases: { - dbid1: { + 1991: { allow_ctas: false, allow_cvas: false, allow_dml: false, @@ -86,11 +71,10 @@ const mockInitialState = { }, unsavedQueryEditor: { id: defaultQueryEditor.id, - dbId: 'dbid1', + dbId: 1991, }, }, }; -const store = mockStore(mockInitialState); const setup = (props = {}, store) => render(, { @@ -98,6 +82,23 @@ const setup = (props = {}, store) => ...(store && { store }), }); +const logAction = () => next => action => { + if (typeof action === 'function') { + return next(action); + } + actions.push(action); + return next(action); +}; + +const createStore = initState => + setupStore({ + disableDegugger: true, + initialState: initState, + rootReducers: reducers, + middleware: getDefaultMiddleware => + getDefaultMiddleware().concat(api.middleware, logAction), + }); + describe('SqlEditor', () => { const mockedProps = { queryEditor: initialState.sqlLab.queryEditors[0], @@ -112,24 +113,20 @@ describe('SqlEditor', () => { }; beforeEach(() => { + store = createStore(mockInitialState); + actions = []; + SqlEditorLeftBar.mockClear(); SqlEditorLeftBar.mockImplementation(() => (
)); }); - const buildWrapper = (props = {}) => - mount( - - - - - , - { - wrappingComponent: ThemeProvider, - wrappingComponentProps: { theme: supersetTheme }, - }, - ); + afterEach(() => { + act(() => { + store.dispatch(api.util.resetApiState()); + }); + }); it('does not render SqlEditor if no db selected', async () => { const queryEditor = initialState.sqlLab.queryEditors[1]; @@ -152,11 +149,7 @@ describe('SqlEditor', () => { }); it('avoids rerendering EditorLeftBar while typing', async () => { - const sqlLabStore = setupStore({ - initialState: mockInitialState, - rootReducers: reducers, - }); - const { findByTestId } = setup(mockedProps, sqlLabStore); + const { findByTestId } = setup(mockedProps, store); const editor = await findByTestId('react-ace'); const sql = 'select *'; const renderCount = SqlEditorLeftBar.mock.calls.length; @@ -168,34 +161,32 @@ describe('SqlEditor', () => { it('renders sql from unsaved change', async () => { const expectedSql = 'SELECT updated_column\nFROM updated_table\nWHERE'; - const { findByTestId } = setup( - mockedProps, - mockStore({ - ...initialState, - sqlLab: { - ...initialState.sqlLab, - databases: { - dbid1: { - allow_ctas: false, - allow_cvas: false, - allow_dml: false, - allow_file_upload: false, - allow_run_async: false, - backend: 'postgresql', - database_name: 'examples', - expose_in_sqllab: true, - force_ctas_schema: null, - id: 1, - }, - }, - unsavedQueryEditor: { - id: defaultQueryEditor.id, - dbId: 'dbid1', - sql: expectedSql, + store = createStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + databases: { + 2023: { + allow_ctas: false, + allow_cvas: false, + allow_dml: false, + allow_file_upload: false, + allow_run_async: false, + backend: 'postgresql', + database_name: 'examples', + expose_in_sqllab: true, + force_ctas_schema: null, + id: 1, }, }, - }), - ); + unsavedQueryEditor: { + id: defaultQueryEditor.id, + dbId: 2023, + sql: expectedSql, + }, + }, + }); + const { findByTestId } = setup(mockedProps, store); const editor = await findByTestId('react-ace'); expect(editor).toHaveValue(expectedSql); @@ -209,7 +200,7 @@ describe('SqlEditor', () => { }); it('runs query action with ctas false', async () => { - const expectedStore = mockStore({ + store = createStore({ ...initialState, sqlLab: { ...initialState.sqlLab, @@ -234,11 +225,11 @@ describe('SqlEditor', () => { }, }, }); - const { findByTestId } = setup(mockedProps, expectedStore); + const { findByTestId } = setup(mockedProps, store); const runButton = await findByTestId('run-query-action'); fireEvent.click(runButton); await waitFor(() => - expect(expectedStore.getActions()).toContainEqual({ + expect(actions).toContainEqual({ type: 'START_QUERY', query: expect.objectContaining({ ctas: false, @@ -248,34 +239,6 @@ describe('SqlEditor', () => { ); }); - // TODO eschutho convert tests to RTL - // eslint-disable-next-line jest/no-disabled-tests - it.skip('does not overflow the editor window', async () => { - const wrapper = buildWrapper(); - await waitForComponentToPaint(wrapper); - const totalSize = - parseFloat(wrapper.find(AceEditorWrapper).props().height) + - wrapper.find(SouthPane).props().height + - SQL_TOOLBAR_HEIGHT + - SQL_EDITOR_GUTTER_MARGIN * 2 + - SQL_EDITOR_GUTTER_HEIGHT; - expect(totalSize).toEqual(MOCKED_SQL_EDITOR_HEIGHT); - }); - - // eslint-disable-next-line jest/no-disabled-tests - it.skip('does not overflow the editor window after resizing', async () => { - const wrapper = buildWrapper(); - wrapper.setState({ height: 450 }); - await waitForComponentToPaint(wrapper); - const totalSize = - parseFloat(wrapper.find(AceEditorWrapper).props().height) + - wrapper.find(SouthPane).props().height + - SQL_TOOLBAR_HEIGHT + - SQL_EDITOR_GUTTER_MARGIN * 2 + - SQL_EDITOR_GUTTER_HEIGHT; - expect(totalSize).toEqual(450); - }); - it('render a Limit Dropdown', async () => { const defaultQueryLimit = 101; const updatedProps = { ...mockedProps, defaultQueryLimit }; diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx index b3ef02743ffe..1b3f5ef3c7d0 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx @@ -17,15 +17,16 @@ * under the License. */ import React from 'react'; -import configureStore from 'redux-mock-store'; import fetchMock from 'fetch-mock'; import { render, screen, waitFor } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import { Provider } from 'react-redux'; import '@testing-library/jest-dom/extend-expect'; -import thunk from 'redux-thunk'; import SqlEditorLeftBar from 'src/SqlLab/components/SqlEditorLeftBar'; import { table, initialState, defaultQueryEditor } from 'src/SqlLab/fixtures'; +import { api } from 'src/hooks/apiResources/queryApi'; +import { setupStore } from 'src/views/store'; +import { reducers } from 'src/SqlLab/reducers'; const mockedProps = { tables: [table], @@ -37,11 +38,32 @@ const mockedProps = { }, height: 0, }; -const middlewares = [thunk]; -const mockStore = configureStore(middlewares); -const store = mockStore(initialState); + +let store; +let actions; + +const logAction = () => next => action => { + if (typeof action === 'function') { + return next(action); + } + + actions.push(action); + + return next(action); +}; + +const createStore = initState => + setupStore({ + disableDegugger: true, + initialState: initState, + rootReducers: reducers, + middleware: getDefaultMiddleware => + getDefaultMiddleware().concat(api.middleware, logAction), + }); beforeEach(() => { + store = createStore(initialState); + actions = []; fetchMock.get('glob:*/api/v1/database/?*', { result: [] }); fetchMock.get('glob:*/api/v1/database/*/schemas/?*', { count: 2, @@ -60,6 +82,8 @@ beforeEach(() => { afterEach(() => { fetchMock.restore(); + store.dispatch(api.util.resetApiState()); + jest.clearAllMocks(); }); const renderAndWait = (props, store) => @@ -112,21 +136,29 @@ test('table should be visible when expanded is true', async () => { }); test('should toggle the table when the header is clicked', async () => { - const store = mockStore(initialState); await renderAndWait(mockedProps, store); const header = (await screen.findAllByText(/ab_user/))[1]; expect(header).toBeInTheDocument(); + userEvent.click(header); await waitFor(() => { - expect(store.getActions()[store.getActions().length - 1].type).toEqual( - 'COLLAPSE_TABLE', - ); + expect(actions[actions.length - 1].type).toEqual('COLLAPSE_TABLE'); }); }); test('When changing database the table list must be updated', async () => { + store = createStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + unsavedQueryEditor: { + id: defaultQueryEditor.id, + schema: 'new_schema', + }, + }, + }); const { rerender } = await renderAndWait(mockedProps, store); expect(screen.getAllByText(/main/i)[0]).toBeInTheDocument(); @@ -145,16 +177,7 @@ test('When changing database the table list must be updated', async () => { />, { useRedux: true, - store: mockStore({ - ...initialState, - sqlLab: { - ...initialState.sqlLab, - unsavedQueryEditor: { - id: defaultQueryEditor.id, - schema: 'new_schema', - }, - }, - }), + store, }, ); expect(await screen.findByText(/new_db/i)).toBeInTheDocument(); @@ -163,22 +186,19 @@ test('When changing database the table list must be updated', async () => { test('ignore schema api when current schema is deprecated', async () => { const invalidSchemaName = 'None'; - const { rerender } = await renderAndWait( - mockedProps, - mockStore({ - ...initialState, - sqlLab: { - ...initialState.sqlLab, - unsavedQueryEditor: { - id: defaultQueryEditor.id, - schema: invalidSchemaName, - }, + store = createStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + unsavedQueryEditor: { + id: defaultQueryEditor.id, + schema: invalidSchemaName, }, - }), - ); + }, + }); + const { rerender } = await renderAndWait(mockedProps, store); expect(await screen.findByText(/Database/i)).toBeInTheDocument(); - expect(screen.queryByText(/None/i)).toBeInTheDocument(); expect(fetchMock.calls()).not.toContainEqual( expect.arrayContaining([ expect.stringContaining( diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx index 105751a4d0eb..615b703f2725 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx @@ -32,7 +32,6 @@ import TabbedSqlEditors from 'src/SqlLab/components/TabbedSqlEditors'; import SqlEditor from 'src/SqlLab/components/SqlEditor'; import { initialState } from 'src/SqlLab/fixtures'; import { newQueryTabName } from 'src/SqlLab/utils/newQueryTabName'; -import QueryProvider from 'src/views/QueryProvider'; fetchMock.get('glob:*/api/v1/database/*', {}); fetchMock.get('glob:*/savedqueryviewapi/api/get/*', {}); @@ -77,11 +76,9 @@ describe('TabbedSqlEditors', () => { const mountWithAct = async () => act(async () => { mount( - - - - - , + + + , { wrappingComponent: ThemeProvider, wrappingComponentProps: { theme: supersetTheme }, diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index a684e6d837a7..c4a7acd268f2 100644 --- a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx +++ b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx @@ -18,10 +18,16 @@ */ import React from 'react'; +import { act } from 'react-dom/test-utils'; import fetchMock from 'fetch-mock'; -import { render, screen, waitFor } from 'spec/helpers/testing-library'; -import { queryClient } from 'src/views/QueryProvider'; +import { + render, + screen, + waitFor, + storeWithQuery as store, +} from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; +import { api } from 'src/hooks/apiResources/queryApi'; import DatabaseSelector, { DatabaseSelectorProps } from '.'; import { EmptyStateSmall } from '../EmptyState'; @@ -163,24 +169,26 @@ function setupFetchMock() { } beforeEach(() => { - queryClient.clear(); setupFetchMock(); }); afterEach(() => { fetchMock.reset(); + act(() => { + store.dispatch(api.util.resetApiState()); + }); }); test('Should render', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useQuery: true }); expect(await screen.findByTestId('DatabaseSelector')).toBeInTheDocument(); }); test('Refresh should work', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useQuery: true }); expect(fetchMock.calls(schemaApiRoute).length).toBe(0); @@ -212,7 +220,7 @@ test('Refresh should work', async () => { test('Should database select display options', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useQuery: true }); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -233,7 +241,7 @@ test('should show empty state if there are no options', async () => { db={undefined} emptyState={} />, - { useRedux: true }, + { useQuery: true }, ); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', @@ -246,7 +254,7 @@ test('should show empty state if there are no options', async () => { test('Should schema select display options', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useQuery: true }); const select = screen.getByRole('combobox', { name: 'Select schema or type to search schemas', }); @@ -262,7 +270,7 @@ test('Should schema select display options', async () => { test('Sends the correct db when changing the database', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useQuery: true }); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -282,7 +290,7 @@ test('Sends the correct db when changing the database', async () => { test('Sends the correct schema when changing the schema', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useQuery: true }); const select = screen.getByRole('combobox', { name: 'Select schema or type to search schemas', }); diff --git a/superset-frontend/src/components/DatabaseSelector/index.tsx b/superset-frontend/src/components/DatabaseSelector/index.tsx index cbac98968873..8516017b20e6 100644 --- a/superset-frontend/src/components/DatabaseSelector/index.tsx +++ b/superset-frontend/src/components/DatabaseSelector/index.tsx @@ -223,14 +223,13 @@ export default function DatabaseSelector({ const { data, isFetching: loadingSchemas, - isFetched, refetch, } = useSchemas({ dbId: currentDb?.value, - onSuccess: data => { - if (data.length === 1) { - changeSchema(data[0]); - } else if (!data.find(schemaOption => schema === schemaOption.value)) { + onSuccess: (schemas, isFetched) => { + if (schemas.length === 1) { + changeSchema(schemas[0]); + } else if (!schemas.find(schemaOption => schema === schemaOption.value)) { changeSchema(undefined); } diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx index 7d378902e6e9..87a6620204e8 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx @@ -18,24 +18,21 @@ */ import React from 'react'; import { act } from 'react-dom/test-utils'; -import configureStore from 'redux-mock-store'; import { mount } from 'enzyme'; import { Provider } from 'react-redux'; import fetchMock from 'fetch-mock'; -import thunk from 'redux-thunk'; import sinon from 'sinon'; import { supersetTheme, ThemeProvider } from '@superset-ui/core'; import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; +import { storeWithQuery as store } from 'spec/helpers/testing-library'; import Modal from 'src/components/Modal'; import { DatasourceModal } from 'src/components/Datasource'; import DatasourceEditor from 'src/components/Datasource/DatasourceEditor'; import * as featureFlags from 'src/featureFlags'; import mockDatasource from 'spec/fixtures/mockDatasource'; -import QueryProvider from 'src/views/QueryProvider'; +import { api } from 'src/hooks/apiResources/queryApi'; -const mockStore = configureStore([thunk]); -const store = mockStore({}); const datasource = mockDatasource['7__table']; const SAVE_ENDPOINT = 'glob:*/api/v1/dataset/7'; @@ -55,11 +52,9 @@ const mockedProps = { async function mountAndWait(props = mockedProps) { const mounted = mount( - - - - - , + + + , { wrappingComponent: ThemeProvider, wrappingComponentProps: { theme: supersetTheme }, @@ -81,6 +76,9 @@ describe('DatasourceModal', () => { afterAll(() => { isFeatureEnabledMock.restore(); + act(() => { + store.dispatch(api.util.resetApiState()); + }); }); it('renders', () => { diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx index 1e6083e1b46f..c2bcc4c308a9 100644 --- a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx +++ b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx @@ -18,8 +18,15 @@ */ import React from 'react'; -import { render, screen, waitFor, within } from 'spec/helpers/testing-library'; -import { queryClient } from 'src/views/QueryProvider'; +import { act } from 'react-dom/test-utils'; +import { + render, + screen, + waitFor, + within, + storeWithQuery as store, +} from 'spec/helpers/testing-library'; +import { api } from 'src/hooks/apiResources/queryApi'; import fetchMock from 'fetch-mock'; import userEvent from '@testing-library/user-event'; import TableSelector, { TableSelectorMultiple } from '.'; @@ -56,11 +63,13 @@ const getSelectItemContainer = (select: HTMLElement) => ); beforeEach(() => { - queryClient.clear(); fetchMock.get(databaseApiRoute, { result: [] }); }); afterEach(() => { + act(() => { + store.dispatch(api.util.resetApiState()); + }); fetchMock.reset(); }); @@ -69,7 +78,7 @@ test('renders with default props', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useRedux: true }); + render(, { useQuery: true }); const databaseSelect = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -91,7 +100,7 @@ test('renders table options', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useRedux: true }); + render(, { useQuery: true }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', }); @@ -109,7 +118,7 @@ test('renders disabled without schema', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useRedux: true }); + render(, { useQuery: true }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', }); @@ -128,7 +137,7 @@ test('table select retain value if not in SQL Lab mode', async () => { sqlLabMode: false, }); - render(, { useRedux: true }); + render(, { useQuery: true }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', @@ -168,7 +177,7 @@ test('table multi select retain all the values selected', async () => { onTableSelectChange: callback, }); - render(, { useRedux: true }); + render(, { useQuery: true }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', diff --git a/superset-frontend/src/components/TableSelector/index.tsx b/superset-frontend/src/components/TableSelector/index.tsx index 3886a86fd20a..f2a06c56b297 100644 --- a/superset-frontend/src/components/TableSelector/index.tsx +++ b/superset-frontend/src/components/TableSelector/index.tsx @@ -175,17 +175,16 @@ const TableSelector: FunctionComponent = ({ const { data, isFetching: loadingTables, - isFetched, refetch, } = useTables({ dbId: database?.id, schema: currentSchema, - onSuccess: () => { + onSuccess: (data, isFetched) => { if (isFetched) { addSuccessToast(t('List updated')); } }, - onError: (err: Response) => { + onError: err => { getClientErrorObject(err).then(clientError => { handleError( getClientErrorMessage( diff --git a/superset-frontend/src/hooks/apiResources/queryApi.test.ts b/superset-frontend/src/hooks/apiResources/queryApi.test.ts new file mode 100644 index 000000000000..d83f464cf505 --- /dev/null +++ b/superset-frontend/src/hooks/apiResources/queryApi.test.ts @@ -0,0 +1,104 @@ +/** + * 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 configureStore, { MockStore } from 'redux-mock-store'; +import rison from 'rison'; +import { JsonResponse } from '@superset-ui/core'; +import { supersetClientQuery } from './queryApi'; + +const getBaseQueryApiMock = (store: MockStore) => ({ + ...new AbortController(), + dispatch: store.dispatch, + getState: store.getState, + extra: undefined, + endpoint: 'endpoint', + type: 'query' as const, +}); + +describe('queryApi', () => { + const mockStore = configureStore(); + const store = mockStore(); + + afterEach(() => { + fetchMock.reset(); + }); + + test('supersetClientQuery should build the endpoint with rison encoded query string and return data when successful', async () => { + const expectedData = { id: 1, name: 'Test' }; + const expectedUrl = '/api/v1/get-endpoint/'; + const expectedPostUrl = '/api/v1/post-endpoint/'; + const urlParams = { str: 'string', num: 123, bool: true }; + const getEndpoint = `glob:*${expectedUrl}?q=${rison.encode(urlParams)}`; + const postEndpoint = `glob:*${expectedPostUrl}?q=${rison.encode( + urlParams, + )}`; + fetchMock.get(getEndpoint, { result: expectedData }); + fetchMock.post(postEndpoint, { result: expectedData }); + const result = await supersetClientQuery( + { + endpoint: expectedUrl, + urlParams, + }, + getBaseQueryApiMock(store), + {}, + ); + expect(fetchMock.calls(getEndpoint)).toHaveLength(1); + expect(fetchMock.calls(postEndpoint)).toHaveLength(0); + expect((result.data as JsonResponse).json.result).toEqual(expectedData); + await supersetClientQuery( + { + method: 'post', + endpoint: expectedPostUrl, + urlParams, + }, + getBaseQueryApiMock(store), + {}, + ); + expect(fetchMock.calls(getEndpoint)).toHaveLength(1); + expect(fetchMock.calls(postEndpoint)).toHaveLength(1); + }); + + test('supersetClientQuery should return error when unsuccessful', async () => { + const expectedError = new Error('Request failed'); + const expectedUrl = '/api/v1/get-endpoint/'; + const endpoint = `glob:*${expectedUrl}`; + fetchMock.get(endpoint, { throws: expectedError }); + const result = await supersetClientQuery( + { endpoint }, + getBaseQueryApiMock(store), + {}, + ); + expect(result.error).toEqual(expectedError); + }); + + test('supersetClientQuery should return parsed response by parseMethod', async () => { + const expectedUrl = '/api/v1/get-endpoint/'; + const endpoint = `glob:*${expectedUrl}`; + const bitIntVal = '9223372036854775807'; + const expectedData = `{ "id": ${bitIntVal} }`; + fetchMock.get(endpoint, expectedData); + const result = await supersetClientQuery( + { endpoint, parseMethod: 'json-bigint' }, + getBaseQueryApiMock(store), + {}, + ); + expect(`${(result.data as JsonResponse).json.id}`).toEqual(bitIntVal); + }); +}); diff --git a/superset-frontend/src/hooks/apiResources/queryApi.ts b/superset-frontend/src/hooks/apiResources/queryApi.ts new file mode 100644 index 000000000000..9e41fa054faa --- /dev/null +++ b/superset-frontend/src/hooks/apiResources/queryApi.ts @@ -0,0 +1,68 @@ +/** + * 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 rison from 'rison'; +import { createApi, BaseQueryFn } from '@reduxjs/toolkit/query/react'; +import { + SupersetClient, + ParseMethod, + SupersetClientResponse, + JsonValue, + RequestBase, +} from '@superset-ui/core'; + +export { JsonResponse, TextResponse } from '@superset-ui/core'; + +export const supersetClientQuery: BaseQueryFn< + Pick & { + endpoint: string; + parseMethod?: ParseMethod; + transformResponse?: (response: SupersetClientResponse) => JsonValue; + urlParams?: Record; + } +> = ( + { + endpoint, + urlParams, + transformResponse, + method = 'GET', + parseMethod = 'json', + ...rest + }, + api, +) => + SupersetClient.request({ + ...rest, + endpoint: `${endpoint}${urlParams ? `?q=${rison.encode(urlParams)}` : ''}`, + method, + parseMethod, + signal: api.signal, + }) + .then(data => ({ + data: transformResponse?.(data) ?? data, + })) + .catch(error => ({ + error, + })); + +export const api = createApi({ + reducerPath: 'queryApi', + tagTypes: ['Schemas', 'Tables'], + endpoints: () => ({}), + baseQuery: supersetClientQuery, +}); diff --git a/superset-frontend/src/hooks/apiResources/schemas.test.ts b/superset-frontend/src/hooks/apiResources/schemas.test.ts index 59d00a5dc71b..b962e28ba014 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.test.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.test.ts @@ -19,28 +19,37 @@ import rison from 'rison'; import fetchMock from 'fetch-mock'; import { act, renderHook } from '@testing-library/react-hooks'; -import QueryProvider, { queryClient } from 'src/views/QueryProvider'; +import { + createWrapper, + storeWithQuery as store, +} from 'spec/helpers/testing-library'; +import { api } from 'src/hooks/apiResources/queryApi'; import { useSchemas } from './schemas'; const fakeApiResult = { result: ['test schema 1', 'test schema b'], }; +const fakeApiResult2 = { + result: ['test schema 2', 'test schema a'], +}; const expectedResult = fakeApiResult.result.map((value: string) => ({ value, label: value, title: value, })); +const expectedResult2 = fakeApiResult2.result.map((value: string) => ({ + value, + label: value, + title: value, +})); describe('useSchemas hook', () => { - beforeEach(() => { - queryClient.clear(); - jest.useFakeTimers(); - }); - afterEach(() => { fetchMock.reset(); - jest.useRealTimers(); + act(() => { + store.dispatch(api.util.resetApiState()); + }); }); test('returns api response mapping json result', async () => { @@ -48,19 +57,21 @@ describe('useSchemas hook', () => { const forceRefresh = false; const schemaApiRoute = `glob:*/api/v1/database/${expectDbId}/schemas/*`; fetchMock.get(schemaApiRoute, fakeApiResult); - const { result } = renderHook( + const onSuccess = jest.fn(); + const { result, waitFor } = renderHook( () => useSchemas({ dbId: expectDbId, + onSuccess, }), { - wrapper: QueryProvider, + wrapper: createWrapper({ + useQuery: true, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); - expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(1)); + expect(result.current.data).toEqual(expectedResult); expect( fetchMock.calls( `end:/api/v1/database/${expectDbId}/schemas/?q=${rison.encode({ @@ -68,11 +79,11 @@ describe('useSchemas hook', () => { })}`, ).length, ).toBe(1); - expect(result.current.data).toEqual(expectedResult); - await act(async () => { + expect(onSuccess).toHaveBeenCalledTimes(1); + act(() => { result.current.refetch(); }); - expect(fetchMock.calls(schemaApiRoute).length).toBe(2); + await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(2)); expect( fetchMock.calls( `end:/api/v1/database/${expectDbId}/schemas/?q=${rison.encode({ @@ -80,6 +91,7 @@ describe('useSchemas hook', () => { })}`, ).length, ).toBe(1); + expect(onSuccess).toHaveBeenCalledTimes(2); expect(result.current.data).toEqual(expectedResult); }); @@ -87,52 +99,66 @@ describe('useSchemas hook', () => { const expectDbId = 'db1'; const schemaApiRoute = `glob:*/api/v1/database/${expectDbId}/schemas/*`; fetchMock.get(schemaApiRoute, fakeApiResult); - const { result, rerender } = renderHook( + const { result, rerender, waitFor } = renderHook( () => useSchemas({ dbId: expectDbId, }), { - wrapper: QueryProvider, + wrapper: createWrapper({ + useQuery: true, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); rerender(); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); - expect(result.current.data).toEqual(expectedResult); }); it('returns refreshed data after expires', async () => { const expectDbId = 'db1'; - const schemaApiRoute = `glob:*/api/v1/database/${expectDbId}/schemas/*`; - fetchMock.get(schemaApiRoute, fakeApiResult); - const { result, rerender } = renderHook( - () => + const schemaApiRoute = `glob:*/api/v1/database/*/schemas/*`; + fetchMock.get(schemaApiRoute, url => + url.includes(expectDbId) ? fakeApiResult : fakeApiResult2, + ); + const onSuccess = jest.fn(); + const { result, rerender, waitFor } = renderHook( + ({ dbId }) => useSchemas({ - dbId: expectDbId, + dbId, + onSuccess, }), { - wrapper: QueryProvider, + initialProps: { dbId: expectDbId }, + wrapper: createWrapper({ + useQuery: true, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); - expect(fetchMock.calls(schemaApiRoute).length).toBe(1); - rerender(); - await act(async () => { - jest.runAllTimers(); - }); + + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); - queryClient.clear(); - rerender(); - await act(async () => { - jest.runAllTimers(); - }); + expect(onSuccess).toHaveBeenCalledTimes(1); + + rerender({ dbId: 'db2' }); + await waitFor(() => expect(result.current.data).toEqual(expectedResult2)); expect(fetchMock.calls(schemaApiRoute).length).toBe(2); - expect(result.current.data).toEqual(expectedResult); + expect(onSuccess).toHaveBeenCalledTimes(2); + + rerender({ dbId: expectDbId }); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + expect(fetchMock.calls(schemaApiRoute).length).toBe(2); + expect(onSuccess).toHaveBeenCalledTimes(3); + + // clean up cache + act(() => { + store.dispatch(api.util.invalidateTags(['Schemas'])); + }); + + await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(3)); + expect(fetchMock.calls(schemaApiRoute)[2][0]).toContain(expectDbId); + await waitFor(() => expect(result.current.data).toEqual(expectedResult)); }); }); diff --git a/superset-frontend/src/hooks/apiResources/schemas.ts b/superset-frontend/src/hooks/apiResources/schemas.ts index 34a50ca4e989..f9f5b5c45b5e 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.ts @@ -16,20 +16,8 @@ * specific language governing permissions and limitations * under the License. */ -import { useRef } from 'react'; -import { useQuery, UseQueryOptions } from 'react-query'; -import rison from 'rison'; -import { SupersetClient } from '@superset-ui/core'; - -export type FetchSchemasQueryParams = { - dbId?: string | number; - forceRefresh?: boolean; -}; - -type QueryData = { - json: { result: string[] }; - response: Response; -}; +import { useCallback, useEffect } from 'react'; +import { api, JsonResponse } from './queryApi'; export type SchemaOption = { value: string; @@ -37,44 +25,83 @@ export type SchemaOption = { title: string; }; -export function fetchSchemas({ dbId, forceRefresh }: FetchSchemasQueryParams) { - const queryParams = rison.encode({ force: forceRefresh }); - // TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes. - const endpoint = `/api/v1/database/${dbId}/schemas/?q=${queryParams}`; - return SupersetClient.get({ endpoint }) as Promise; -} +export type FetchSchemasQueryParams = { + dbId?: string | number; + forceRefresh: boolean; + onSuccess?: (data: SchemaOption[], isRefetched: boolean) => void; + onError?: () => void; +}; + +type Params = Omit; + +const schemaApi = api.injectEndpoints({ + endpoints: builder => ({ + schemas: builder.query({ + providesTags: [{ type: 'Schemas', id: 'LIST' }], + query: ({ dbId, forceRefresh }) => ({ + endpoint: `/api/v1/database/${dbId}/schemas/`, + // TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes. + urlParams: { + force: forceRefresh, + }, + transformResponse: ({ json }: JsonResponse) => + json.result.map((value: string) => ({ + value, + label: value, + title: value, + })), + }), + serializeQueryArgs: ({ queryArgs: { dbId } }) => ({ + dbId, + }), + }), + }), +}); -type Params = FetchSchemasQueryParams & - Pick, 'onSuccess' | 'onError'>; +const EMPTY_SCHEMAS = [] as SchemaOption[]; export function useSchemas(options: Params) { + const { useLazySchemasQuery, useSchemasQuery } = schemaApi; + const { dbId, onSuccess, onError } = options || {}; - const forceRefreshRef = useRef(false); - const params = { dbId }; - const result = useQuery( - ['schemas', { dbId }], - () => fetchSchemas({ ...params, forceRefresh: forceRefreshRef.current }), + const [trigger, refetchResult] = useLazySchemasQuery(); + const result = useSchemasQuery( + { dbId, forceRefresh: false }, { - select: ({ json }) => - json.result.map((value: string) => ({ - value, - label: value, - title: value, - })), - enabled: Boolean(dbId), - onSuccess, - onError, - onSettled: () => { - forceRefreshRef.current = false; - }, + skip: !dbId, }, ); + const refetch = useCallback(() => { + if (dbId) { + trigger({ dbId, forceRefresh: true }).then( + ({ isSuccess, isError, data }) => { + if (isSuccess) { + onSuccess?.(data, true); + } + if (isError) { + onError?.(); + } + }, + ); + } + }, [dbId]); + + useEffect(() => { + const { requestId, isSuccess, isError, isFetching, data, originalArgs } = + result; + if (!originalArgs?.forceRefresh && requestId && !isFetching) { + if (isSuccess) { + onSuccess?.(data || EMPTY_SCHEMAS, false); + } + if (isError) { + onError?.(); + } + } + }, [result]); + return { - ...result, - refetch: () => { - forceRefreshRef.current = true; - return result.refetch(); - }, + ...(refetchResult.isUninitialized ? result : refetchResult), + refetch, }; } diff --git a/superset-frontend/src/hooks/apiResources/tables.test.ts b/superset-frontend/src/hooks/apiResources/tables.test.ts index 8cc0791d5069..09995f5cfdb0 100644 --- a/superset-frontend/src/hooks/apiResources/tables.test.ts +++ b/superset-frontend/src/hooks/apiResources/tables.test.ts @@ -19,7 +19,11 @@ import rison from 'rison'; import fetchMock from 'fetch-mock'; import { act, renderHook } from '@testing-library/react-hooks'; -import QueryProvider, { queryClient } from 'src/views/QueryProvider'; +import { + createWrapper, + storeWithQuery as store, +} from 'spec/helpers/testing-library'; +import { api } from 'src/hooks/apiResources/queryApi'; import { useTables } from './tables'; const fakeApiResult = { @@ -67,14 +71,11 @@ const expectedHasMoreData = { }; describe('useTables hook', () => { - beforeEach(() => { - queryClient.clear(); - jest.useFakeTimers(); - }); - afterEach(() => { fetchMock.reset(); - jest.useRealTimers(); + act(() => { + store.dispatch(api.util.resetApiState()); + }); }); test('returns api response mapping json options', async () => { @@ -86,19 +87,19 @@ describe('useTables hook', () => { fetchMock.get(schemaApiRoute, { result: fakeSchemaApiResult, }); - const { result } = renderHook( + const { result, waitFor } = renderHook( () => useTables({ dbId: expectDbId, schema: expectedSchema, }), { - wrapper: QueryProvider, + wrapper: createWrapper({ + useQuery: true, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); + await waitFor(() => expect(result.current.data).toEqual(expectedData)); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); expect( fetchMock.calls( @@ -108,19 +109,20 @@ describe('useTables hook', () => { })}`, ).length, ).toBe(1); - expect(result.current.data).toEqual(expectedData); - await act(async () => { + act(() => { result.current.refetch(); }); + await waitFor(() => + expect( + fetchMock.calls( + `end:api/v1/database/${expectDbId}/tables/?q=${rison.encode({ + force: true, + schema_name: expectedSchema, + })}`, + ).length, + ).toBe(1), + ); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); - expect( - fetchMock.calls( - `end:api/v1/database/${expectDbId}/tables/?q=${rison.encode({ - force: true, - schema_name: expectedSchema, - })}`, - ).length, - ).toBe(1); expect(result.current.data).toEqual(expectedData); }); @@ -133,20 +135,19 @@ describe('useTables hook', () => { fetchMock.get(schemaApiRoute, { result: fakeSchemaApiResult, }); - const { result } = renderHook( + const { result, waitFor } = renderHook( () => useTables({ dbId: expectDbId, schema: unexpectedSchema, }), { - wrapper: QueryProvider, + wrapper: createWrapper({ + useQuery: true, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); - expect(fetchMock.calls(schemaApiRoute).length).toBe(1); + await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(1)); expect(result.current.data).toEqual(undefined); expect( fetchMock.calls( @@ -166,20 +167,19 @@ describe('useTables hook', () => { fetchMock.get(`glob:*/api/v1/database/${expectDbId}/schemas/*`, { result: fakeSchemaApiResult, }); - const { result } = renderHook( + const { result, waitFor } = renderHook( () => useTables({ dbId: expectDbId, schema: expectedSchema, }), { - wrapper: QueryProvider, + wrapper: createWrapper({ + useQuery: true, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); - expect(fetchMock.calls(tableApiRoute).length).toBe(1); + await waitFor(() => expect(fetchMock.calls(tableApiRoute).length).toBe(1)); expect(result.current.data).toEqual(expectedHasMoreData); }); @@ -191,58 +191,77 @@ describe('useTables hook', () => { fetchMock.get(`glob:*/api/v1/database/${expectDbId}/schemas/*`, { result: fakeSchemaApiResult, }); - const { result, rerender } = renderHook( + const { result, rerender, waitFor } = renderHook( () => useTables({ dbId: expectDbId, schema: expectedSchema, }), { - wrapper: QueryProvider, + wrapper: createWrapper({ + useQuery: true, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); - expect(fetchMock.calls(tableApiRoute).length).toBe(1); + await waitFor(() => expect(fetchMock.calls(tableApiRoute).length).toBe(1)); rerender(); + await waitFor(() => expect(result.current.data).toEqual(expectedData)); expect(fetchMock.calls(tableApiRoute).length).toBe(1); - expect(result.current.data).toEqual(expectedData); }); test('returns refreshed data after expires', async () => { const expectDbId = 'db1'; const expectedSchema = 'schema1'; const tableApiRoute = `glob:*/api/v1/database/${expectDbId}/tables/?q=*`; - fetchMock.get(tableApiRoute, fakeApiResult); + fetchMock.get(tableApiRoute, url => + url.includes(expectedSchema) ? fakeApiResult : fakeHasMoreApiResult, + ); fetchMock.get(`glob:*/api/v1/database/${expectDbId}/schemas/*`, { result: fakeSchemaApiResult, }); - const { result, rerender } = renderHook( - () => + const { result, rerender, waitFor } = renderHook( + ({ schema }) => useTables({ dbId: expectDbId, - schema: expectedSchema, + schema, }), { - wrapper: QueryProvider, + initialProps: { schema: expectedSchema }, + wrapper: createWrapper({ + useQuery: true, + }), }, ); - await act(async () => { - jest.runAllTimers(); - }); - expect(fetchMock.calls(tableApiRoute).length).toBe(1); - rerender(); - await act(async () => { - jest.runAllTimers(); - }); + + await waitFor(() => expect(result.current.data).toEqual(expectedData)); expect(fetchMock.calls(tableApiRoute).length).toBe(1); - queryClient.clear(); - rerender(); - await act(async () => { - jest.runAllTimers(); - }); + + rerender({ schema: 'schema2' }); + await waitFor(() => + expect(result.current.data).toEqual(expectedHasMoreData), + ); expect(fetchMock.calls(tableApiRoute).length).toBe(2); - expect(result.current.data).toEqual(expectedData); + + rerender({ schema: expectedSchema }); + await waitFor(() => expect(result.current.data).toEqual(expectedData)); + expect(fetchMock.calls(tableApiRoute).length).toBe(2); + + // clean up cache + act(() => { + store.dispatch(api.util.invalidateTags(['Tables'])); + }); + + await waitFor(() => expect(fetchMock.calls(tableApiRoute).length).toBe(3)); + await waitFor(() => expect(result.current.data).toEqual(expectedData)); + + rerender({ schema: 'schema2' }); + await waitFor(() => + expect(result.current.data).toEqual(expectedHasMoreData), + ); + expect(fetchMock.calls(tableApiRoute).length).toBe(4); + + rerender({ schema: expectedSchema }); + await waitFor(() => expect(result.current.data).toEqual(expectedData)); + expect(fetchMock.calls(tableApiRoute).length).toBe(4); }); }); diff --git a/superset-frontend/src/hooks/apiResources/tables.ts b/superset-frontend/src/hooks/apiResources/tables.ts index 34d286c9456a..e1a1df488245 100644 --- a/superset-frontend/src/hooks/apiResources/tables.ts +++ b/superset-frontend/src/hooks/apiResources/tables.ts @@ -16,18 +16,11 @@ * specific language governing permissions and limitations * under the License. */ -import { useRef, useMemo } from 'react'; -import { useQuery, UseQueryOptions } from 'react-query'; -import rison from 'rison'; -import { SupersetClient } from '@superset-ui/core'; +import { useCallback, useMemo, useEffect, useRef } from 'react'; +import { api } from './queryApi'; import { useSchemas } from './schemas'; -export type FetchTablesQueryParams = { - dbId?: string | number; - schema?: string; - forceRefresh?: boolean; -}; export interface Table { label: string; value: string; @@ -41,7 +34,7 @@ export interface Table { }; } -type QueryData = { +type QueryResponse = { json: { count: number; result: Table[]; @@ -54,28 +47,42 @@ export type Data = { hasMore: boolean; }; -export function fetchTables({ - dbId, - schema, - forceRefresh, -}: FetchTablesQueryParams) { - const encodedSchema = schema ? encodeURIComponent(schema) : ''; - const params = rison.encode({ - force: forceRefresh, - schema_name: encodedSchema, - }); +export type FetchTablesQueryParams = { + dbId?: string | number; + schema?: string; + forceRefresh?: boolean; + onSuccess?: (data: Data, isRefetched: boolean) => void; + onError?: (error: Response) => void; +}; - // TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes. - const endpoint = `/api/v1/database/${ - dbId ?? 'undefined' - }/tables/?q=${params}`; - return SupersetClient.get({ endpoint }) as Promise; -} +type Params = Omit; -type Params = FetchTablesQueryParams & - Pick, 'onSuccess' | 'onError'>; +const tableApi = api.injectEndpoints({ + endpoints: builder => ({ + tables: builder.query({ + providesTags: ['Tables'], + query: ({ dbId, schema, forceRefresh }) => ({ + endpoint: `/api/v1/database/${dbId ?? 'undefined'}/tables/`, + // TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes. + urlParams: { + force: forceRefresh, + schema_name: schema ? encodeURIComponent(schema) : '', + }, + transformResponse: ({ json }: QueryResponse) => ({ + options: json.result, + hasMore: json.count > json.result.length, + }), + }), + serializeQueryArgs: ({ queryArgs: { dbId, schema } }) => ({ + dbId, + schema, + }), + }), + }), +}); export function useTables(options: Params) { + const isMountedRef = useRef(false); const { data: schemaOptions, isFetching } = useSchemas({ dbId: options.dbId, }); @@ -83,33 +90,62 @@ export function useTables(options: Params) { () => new Set(schemaOptions?.map(({ value }) => value)), [schemaOptions], ); + const { useLazyTablesQuery, useTablesQuery } = tableApi; const { dbId, schema, onSuccess, onError } = options || {}; - const forceRefreshRef = useRef(false); - const params = { dbId, schema }; - const result = useQuery( - ['tables', { dbId, schema }], - () => fetchTables({ ...params, forceRefresh: forceRefreshRef.current }), + + const enabled = Boolean( + dbId && schema && !isFetching && schemaOptionsMap.has(schema), + ); + + const result = useTablesQuery( + { dbId, schema, forceRefresh: false }, { - select: ({ json }) => ({ - options: json.result, - hasMore: json.count > json.result.length, - }), - enabled: Boolean( - dbId && schema && !isFetching && schemaOptionsMap.has(schema), - ), - onSuccess, - onError, - onSettled: () => { - forceRefreshRef.current = false; - }, + skip: !enabled, }, ); + const [trigger] = useLazyTablesQuery(); + + const refetch = useCallback(() => { + if (enabled) { + trigger({ dbId, schema, forceRefresh: true }).then( + ({ isSuccess, isError, data, error }) => { + if (isSuccess) { + onSuccess?.(data, true); + } + if (isError) { + onError?.(error as Response); + } + }, + ); + } + }, [dbId, schema, enabled]); + + useEffect(() => { + if (isMountedRef.current) { + const { + requestId, + isSuccess, + isError, + isFetching, + data, + error, + originalArgs, + } = result; + if (!originalArgs?.forceRefresh && requestId && !isFetching) { + if (isSuccess) { + onSuccess?.(data, false); + } + if (isError) { + onError?.(error as Response); + } + } + } else { + isMountedRef.current = true; + } + }, [result]); return { ...result, - refetch: () => { - forceRefreshRef.current = true; - return result.refetch(); - }, + refetch, }; } diff --git a/superset-frontend/src/views/App.tsx b/superset-frontend/src/views/App.tsx index 3443e336e526..a3c0715f39ea 100644 --- a/superset-frontend/src/views/App.tsx +++ b/superset-frontend/src/views/App.tsx @@ -40,7 +40,6 @@ import { logEvent } from 'src/logger/actions'; import { store } from 'src/views/store'; import { RootContextProviders } from './RootContextProviders'; import { ScrollToTop } from './ScrollToTop'; -import QueryProvider from './QueryProvider'; setupApp(); setupPlugins(); @@ -70,31 +69,29 @@ const LocationPathnameLogger = () => { }; const App = () => ( - - - - - - - - - {routes.map(({ path, Component, props = {}, Fallback = Loading }) => ( - - }> - - - - - - ))} - - - - - + + + + + + + + {routes.map(({ path, Component, props = {}, Fallback = Loading }) => ( + + }> + + + + + + ))} + + + + ); export default hot(App); diff --git a/superset-frontend/src/views/QueryProvider.tsx b/superset-frontend/src/views/QueryProvider.tsx deleted file mode 100644 index 9fef2022d4c3..000000000000 --- a/superset-frontend/src/views/QueryProvider.tsx +++ /dev/null @@ -1,43 +0,0 @@ -/** - * 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 React from 'react'; -import { QueryClient, QueryClientProvider } from 'react-query'; - -export const queryClient = new QueryClient({ - defaultOptions: { - queries: { - staleTime: Infinity, - retry: false, - retryOnMount: false, - refetchOnMount: false, - refetchOnReconnect: false, - refetchOnWindowFocus: false, - }, - }, -}); - -type Props = { - children: React.ReactNode; -}; - -const Queryprovider: React.FC = ({ children }) => ( - {children} -); - -export default Queryprovider; diff --git a/superset-frontend/src/views/store.ts b/superset-frontend/src/views/store.ts index 9fe1443bf952..78d5a99714f0 100644 --- a/superset-frontend/src/views/store.ts +++ b/superset-frontend/src/views/store.ts @@ -17,6 +17,7 @@ * under the License. */ import { configureStore, ConfigureStoreOptions, Store } from '@reduxjs/toolkit'; +import { api } from 'src/hooks/apiResources/queryApi'; import messageToastReducer from 'src/components/MessageToasts/reducers'; import charts from 'src/components/Chart/chartReducer'; import dataMask from 'src/dataMask/reducer'; @@ -133,6 +134,7 @@ export function setupStore({ return configureStore({ preloadedState: initialState, reducer: { + [api.reducerPath]: api.reducer, ...rootReducers, }, middleware: getDefaultMiddleware => @@ -146,7 +148,7 @@ export function setupStore({ ignoredPaths: [/queryController/g], warnAfter: 200, }, - }).concat(logger), + }).concat(logger, api.middleware), devTools: process.env.WEBPACK_MODE === 'development' && !disableDebugger, ...overrides, }); From 075c1d5b15fe1891ba55e9cdbbef6187fa319b7a Mon Sep 17 00:00:00 2001 From: justin-park Date: Thu, 20 Apr 2023 19:07:02 -0700 Subject: [PATCH 02/15] undefined type exception --- superset-frontend/src/hooks/apiResources/schemas.ts | 2 +- superset-frontend/src/hooks/apiResources/tables.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/hooks/apiResources/schemas.ts b/superset-frontend/src/hooks/apiResources/schemas.ts index f9f5b5c45b5e..d5c454c71f8c 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.ts @@ -77,7 +77,7 @@ export function useSchemas(options: Params) { trigger({ dbId, forceRefresh: true }).then( ({ isSuccess, isError, data }) => { if (isSuccess) { - onSuccess?.(data, true); + onSuccess?.(data || EMPTY_SCHEMAS, true); } if (isError) { onError?.(); diff --git a/superset-frontend/src/hooks/apiResources/tables.ts b/superset-frontend/src/hooks/apiResources/tables.ts index e1a1df488245..71fc8c6c3884 100644 --- a/superset-frontend/src/hooks/apiResources/tables.ts +++ b/superset-frontend/src/hooks/apiResources/tables.ts @@ -132,7 +132,7 @@ export function useTables(options: Params) { originalArgs, } = result; if (!originalArgs?.forceRefresh && requestId && !isFetching) { - if (isSuccess) { + if (isSuccess && data) { onSuccess?.(data, false); } if (isError) { From ff3039909e95a044d22746c059513a6b4b86973b Mon Sep 17 00:00:00 2001 From: justin-park Date: Thu, 20 Apr 2023 21:23:00 -0700 Subject: [PATCH 03/15] type error --- superset-frontend/src/hooks/apiResources/tables.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/hooks/apiResources/tables.ts b/superset-frontend/src/hooks/apiResources/tables.ts index 71fc8c6c3884..387e87bafacb 100644 --- a/superset-frontend/src/hooks/apiResources/tables.ts +++ b/superset-frontend/src/hooks/apiResources/tables.ts @@ -109,7 +109,7 @@ export function useTables(options: Params) { if (enabled) { trigger({ dbId, schema, forceRefresh: true }).then( ({ isSuccess, isError, data, error }) => { - if (isSuccess) { + if (isSuccess && data) { onSuccess?.(data, true); } if (isError) { From bdc41af5d2fcc2e0e9ebe62b0f774ea26322cc9e Mon Sep 17 00:00:00 2001 From: justin-park Date: Fri, 21 Apr 2023 09:54:00 -0700 Subject: [PATCH 04/15] change useQuery by useApi for readability --- superset-frontend/spec/helpers/testing-library.tsx | 10 +++++----- .../SqlEditorLeftBar/SqlEditorLeftBar.test.jsx | 2 -- .../DatabaseSelector/DatabaseSelector.test.tsx | 14 +++++++------- .../TableSelector/TableSelector.test.tsx | 10 +++++----- .../src/hooks/apiResources/schemas.test.ts | 6 +++--- .../src/hooks/apiResources/tables.test.ts | 10 +++++----- 6 files changed, 25 insertions(+), 27 deletions(-) diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index 202daf3f7525..d6ada184b338 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -34,13 +34,13 @@ type Options = Omit & { useDnd?: boolean; useQueryParams?: boolean; useRouter?: boolean; - useQuery?: boolean; + useApi?: boolean; initialState?: {}; reducers?: {}; store?: Store; }; -export const storeWithQuery = configureStore({ +export const storeWithApi = configureStore({ reducer: { [api.reducerPath]: api.reducer, }, @@ -54,7 +54,7 @@ export function createWrapper(options?: Options) { useDnd, useRedux, useQueryParams, - useQuery = true, + useApi = true, useRouter, initialState, reducers, @@ -95,8 +95,8 @@ export function createWrapper(options?: Options) { result = {result}; } - if (useQuery && !useRedux) { - result = {result}; + if (useApi && !useRedux) { + result = {result}; } return result; diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx index 1b3f5ef3c7d0..d12938a23508 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx @@ -46,9 +46,7 @@ const logAction = () => next => action => { if (typeof action === 'function') { return next(action); } - actions.push(action); - return next(action); }; diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index c4a7acd268f2..42937ff4860d 100644 --- a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx +++ b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx @@ -181,14 +181,14 @@ afterEach(() => { test('Should render', async () => { const props = createProps(); - render(, { useQuery: true }); + render(, { useApi: true }); expect(await screen.findByTestId('DatabaseSelector')).toBeInTheDocument(); }); test('Refresh should work', async () => { const props = createProps(); - render(, { useQuery: true }); + render(, { useApi: true }); expect(fetchMock.calls(schemaApiRoute).length).toBe(0); @@ -220,7 +220,7 @@ test('Refresh should work', async () => { test('Should database select display options', async () => { const props = createProps(); - render(, { useQuery: true }); + render(, { useApi: true }); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -241,7 +241,7 @@ test('should show empty state if there are no options', async () => { db={undefined} emptyState={} />, - { useQuery: true }, + { useApi: true }, ); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', @@ -254,7 +254,7 @@ test('should show empty state if there are no options', async () => { test('Should schema select display options', async () => { const props = createProps(); - render(, { useQuery: true }); + render(, { useApi: true }); const select = screen.getByRole('combobox', { name: 'Select schema or type to search schemas', }); @@ -270,7 +270,7 @@ test('Should schema select display options', async () => { test('Sends the correct db when changing the database', async () => { const props = createProps(); - render(, { useQuery: true }); + render(, { useApi: true }); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -290,7 +290,7 @@ test('Sends the correct db when changing the database', async () => { test('Sends the correct schema when changing the schema', async () => { const props = createProps(); - render(, { useQuery: true }); + render(, { useApi: true }); const select = screen.getByRole('combobox', { name: 'Select schema or type to search schemas', }); diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx index c2bcc4c308a9..74613d395a68 100644 --- a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx +++ b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx @@ -78,7 +78,7 @@ test('renders with default props', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useQuery: true }); + render(, { useApi: true }); const databaseSelect = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -100,7 +100,7 @@ test('renders table options', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useQuery: true }); + render(, { useApi: true }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', }); @@ -118,7 +118,7 @@ test('renders disabled without schema', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useQuery: true }); + render(, { useApi: true }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', }); @@ -137,7 +137,7 @@ test('table select retain value if not in SQL Lab mode', async () => { sqlLabMode: false, }); - render(, { useQuery: true }); + render(, { useApi: true }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', @@ -177,7 +177,7 @@ test('table multi select retain all the values selected', async () => { onTableSelectChange: callback, }); - render(, { useQuery: true }); + render(, { useApi: true }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', diff --git a/superset-frontend/src/hooks/apiResources/schemas.test.ts b/superset-frontend/src/hooks/apiResources/schemas.test.ts index b962e28ba014..e5995c30471c 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.test.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.test.ts @@ -66,7 +66,7 @@ describe('useSchemas hook', () => { }), { wrapper: createWrapper({ - useQuery: true, + useApi: true, }), }, ); @@ -106,7 +106,7 @@ describe('useSchemas hook', () => { }), { wrapper: createWrapper({ - useQuery: true, + useApi: true, }), }, ); @@ -133,7 +133,7 @@ describe('useSchemas hook', () => { { initialProps: { dbId: expectDbId }, wrapper: createWrapper({ - useQuery: true, + useApi: true, }), }, ); diff --git a/superset-frontend/src/hooks/apiResources/tables.test.ts b/superset-frontend/src/hooks/apiResources/tables.test.ts index 09995f5cfdb0..339e80c7f95c 100644 --- a/superset-frontend/src/hooks/apiResources/tables.test.ts +++ b/superset-frontend/src/hooks/apiResources/tables.test.ts @@ -95,7 +95,7 @@ describe('useTables hook', () => { }), { wrapper: createWrapper({ - useQuery: true, + useApi: true, }), }, ); @@ -143,7 +143,7 @@ describe('useTables hook', () => { }), { wrapper: createWrapper({ - useQuery: true, + useApi: true, }), }, ); @@ -175,7 +175,7 @@ describe('useTables hook', () => { }), { wrapper: createWrapper({ - useQuery: true, + useApi: true, }), }, ); @@ -199,7 +199,7 @@ describe('useTables hook', () => { }), { wrapper: createWrapper({ - useQuery: true, + useApi: true, }), }, ); @@ -228,7 +228,7 @@ describe('useTables hook', () => { { initialProps: { schema: expectedSchema }, wrapper: createWrapper({ - useQuery: true, + useApi: true, }), }, ); From c128be1aa74111ce2d012a071a43fa0804af6c71 Mon Sep 17 00:00:00 2001 From: justin-park Date: Fri, 21 Apr 2023 10:27:32 -0700 Subject: [PATCH 05/15] rename storeWithQuery by storeWithApi --- .../src/components/DatabaseSelector/DatabaseSelector.test.tsx | 2 +- .../src/components/Datasource/DatasourceModal.test.jsx | 2 +- .../src/components/TableSelector/TableSelector.test.tsx | 2 +- superset-frontend/src/hooks/apiResources/schemas.test.ts | 2 +- superset-frontend/src/hooks/apiResources/tables.test.ts | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index 42937ff4860d..026eab30fcf4 100644 --- a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx +++ b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx @@ -24,7 +24,7 @@ import { render, screen, waitFor, - storeWithQuery as store, + storeWithApi as store, } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import { api } from 'src/hooks/apiResources/queryApi'; diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx index 87a6620204e8..9b5960092650 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx @@ -25,7 +25,7 @@ import sinon from 'sinon'; import { supersetTheme, ThemeProvider } from '@superset-ui/core'; import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; -import { storeWithQuery as store } from 'spec/helpers/testing-library'; +import { storeWithApi as store } from 'spec/helpers/testing-library'; import Modal from 'src/components/Modal'; import { DatasourceModal } from 'src/components/Datasource'; import DatasourceEditor from 'src/components/Datasource/DatasourceEditor'; diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx index 74613d395a68..70cc704dc542 100644 --- a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx +++ b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx @@ -24,7 +24,7 @@ import { screen, waitFor, within, - storeWithQuery as store, + storeWithApi as store, } from 'spec/helpers/testing-library'; import { api } from 'src/hooks/apiResources/queryApi'; import fetchMock from 'fetch-mock'; diff --git a/superset-frontend/src/hooks/apiResources/schemas.test.ts b/superset-frontend/src/hooks/apiResources/schemas.test.ts index e5995c30471c..1c34ffbb8a65 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.test.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.test.ts @@ -21,7 +21,7 @@ import fetchMock from 'fetch-mock'; import { act, renderHook } from '@testing-library/react-hooks'; import { createWrapper, - storeWithQuery as store, + storeWithApi as store, } from 'spec/helpers/testing-library'; import { api } from 'src/hooks/apiResources/queryApi'; import { useSchemas } from './schemas'; diff --git a/superset-frontend/src/hooks/apiResources/tables.test.ts b/superset-frontend/src/hooks/apiResources/tables.test.ts index 339e80c7f95c..3d4f97d8efb1 100644 --- a/superset-frontend/src/hooks/apiResources/tables.test.ts +++ b/superset-frontend/src/hooks/apiResources/tables.test.ts @@ -21,7 +21,7 @@ import fetchMock from 'fetch-mock'; import { act, renderHook } from '@testing-library/react-hooks'; import { createWrapper, - storeWithQuery as store, + storeWithApi as store, } from 'spec/helpers/testing-library'; import { api } from 'src/hooks/apiResources/queryApi'; import { useTables } from './tables'; From 7fc187191213c2a2478b0c0d1ba50e9532ed24d2 Mon Sep 17 00:00:00 2001 From: justin-park Date: Mon, 24 Apr 2023 15:35:43 -0700 Subject: [PATCH 06/15] make export basic query hooks --- superset-frontend/src/hooks/apiResources/schemas.ts | 4 ++-- superset-frontend/src/hooks/apiResources/tables.ts | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/hooks/apiResources/schemas.ts b/superset-frontend/src/hooks/apiResources/schemas.ts index d5c454c71f8c..176b72b04818 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.ts @@ -58,11 +58,11 @@ const schemaApi = api.injectEndpoints({ }), }); +export const { useLazySchemasQuery, useSchemasQuery } = schemaApi; + const EMPTY_SCHEMAS = [] as SchemaOption[]; export function useSchemas(options: Params) { - const { useLazySchemasQuery, useSchemasQuery } = schemaApi; - const { dbId, onSuccess, onError } = options || {}; const [trigger, refetchResult] = useLazySchemasQuery(); const result = useSchemasQuery( diff --git a/superset-frontend/src/hooks/apiResources/tables.ts b/superset-frontend/src/hooks/apiResources/tables.ts index 387e87bafacb..72e9dc8ad64b 100644 --- a/superset-frontend/src/hooks/apiResources/tables.ts +++ b/superset-frontend/src/hooks/apiResources/tables.ts @@ -81,6 +81,8 @@ const tableApi = api.injectEndpoints({ }), }); +export const { useLazyTablesQuery, useTablesQuery } = tableApi; + export function useTables(options: Params) { const isMountedRef = useRef(false); const { data: schemaOptions, isFetching } = useSchemas({ @@ -90,7 +92,6 @@ export function useTables(options: Params) { () => new Set(schemaOptions?.map(({ value }) => value)), [schemaOptions], ); - const { useLazyTablesQuery, useTablesQuery } = tableApi; const { dbId, schema, onSuccess, onError } = options || {}; const enabled = Boolean( From 5b1d12b79e7cd0880ce3b649e6f7453881b8d453 Mon Sep 17 00:00:00 2001 From: justin-park Date: Mon, 24 Apr 2023 16:21:39 -0700 Subject: [PATCH 07/15] handle error by getClientErrorObject --- .../src/hooks/apiResources/queryApi.test.ts | 6 +++--- superset-frontend/src/hooks/apiResources/queryApi.ts | 9 ++++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/hooks/apiResources/queryApi.test.ts b/superset-frontend/src/hooks/apiResources/queryApi.test.ts index d83f464cf505..7a1040334946 100644 --- a/superset-frontend/src/hooks/apiResources/queryApi.test.ts +++ b/superset-frontend/src/hooks/apiResources/queryApi.test.ts @@ -76,16 +76,16 @@ describe('queryApi', () => { }); test('supersetClientQuery should return error when unsuccessful', async () => { - const expectedError = new Error('Request failed'); + const expectedError = 'Request failed'; const expectedUrl = '/api/v1/get-endpoint/'; const endpoint = `glob:*${expectedUrl}`; - fetchMock.get(endpoint, { throws: expectedError }); + fetchMock.get(endpoint, { throws: new Error(expectedError) }); const result = await supersetClientQuery( { endpoint }, getBaseQueryApiMock(store), {}, ); - expect(result.error).toEqual(expectedError); + expect(result.error).toEqual({ error: expectedError }); }); test('supersetClientQuery should return parsed response by parseMethod', async () => { diff --git a/superset-frontend/src/hooks/apiResources/queryApi.ts b/superset-frontend/src/hooks/apiResources/queryApi.ts index 9e41fa054faa..9147173d7918 100644 --- a/superset-frontend/src/hooks/apiResources/queryApi.ts +++ b/superset-frontend/src/hooks/apiResources/queryApi.ts @@ -17,6 +17,7 @@ * under the License. */ import rison from 'rison'; +import { getClientErrorObject } from 'src/utils/getClientErrorObject'; import { createApi, BaseQueryFn } from '@reduxjs/toolkit/query/react'; import { SupersetClient, @@ -56,9 +57,11 @@ export const supersetClientQuery: BaseQueryFn< .then(data => ({ data: transformResponse?.(data) ?? data, })) - .catch(error => ({ - error, - })); + .catch(response => + getClientErrorObject(response).then(errorObj => ({ + error: errorObj, + })), + ); export const api = createApi({ reducerPath: 'queryApi', From 9ca1e855fd90a5dfbaf130849b02a354d5a45d59 Mon Sep 17 00:00:00 2001 From: justin-park Date: Tue, 25 Apr 2023 16:27:28 -0700 Subject: [PATCH 08/15] consolidate useApi option by useRedux --- .../spec/helpers/testing-library.tsx | 28 ++++++++----------- .../DatabaseSelector.test.tsx | 14 +++++----- .../TableSelector/TableSelector.test.tsx | 10 +++---- .../src/hooks/apiResources/schemas.test.ts | 6 ++-- .../src/hooks/apiResources/tables.test.ts | 10 +++---- 5 files changed, 32 insertions(+), 36 deletions(-) diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index d6ada184b338..2f225bf94568 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -34,7 +34,6 @@ type Options = Omit & { useDnd?: boolean; useQueryParams?: boolean; useRouter?: boolean; - useApi?: boolean; initialState?: {}; reducers?: {}; store?: Store; @@ -54,7 +53,6 @@ export function createWrapper(options?: Options) { useDnd, useRedux, useQueryParams, - useApi = true, useRouter, initialState, reducers, @@ -73,16 +71,18 @@ export function createWrapper(options?: Options) { if (useRedux) { const mockStore = store ?? - configureStore({ - preloadedState: initialState || {}, - devTools: false, - reducer: { - ...(reducers || reducerIndex), - [api.reducerPath]: api.reducer, - }, - middleware: getDefaultMiddleware => - getDefaultMiddleware().concat(api.middleware), - }); + (!initialState + ? storeWithApi + : configureStore({ + preloadedState: initialState, + devTools: false, + reducer: { + ...(reducers || reducerIndex), + [api.reducerPath]: api.reducer, + }, + middleware: getDefaultMiddleware => + getDefaultMiddleware().concat(api.middleware), + })); result = {result}; } @@ -95,10 +95,6 @@ export function createWrapper(options?: Options) { result = {result}; } - if (useApi && !useRedux) { - result = {result}; - } - return result; }; } diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index 026eab30fcf4..3b19c40989dd 100644 --- a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx +++ b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx @@ -181,14 +181,14 @@ afterEach(() => { test('Should render', async () => { const props = createProps(); - render(, { useApi: true }); + render(, { useRedux: true }); expect(await screen.findByTestId('DatabaseSelector')).toBeInTheDocument(); }); test('Refresh should work', async () => { const props = createProps(); - render(, { useApi: true }); + render(, { useRedux: true }); expect(fetchMock.calls(schemaApiRoute).length).toBe(0); @@ -220,7 +220,7 @@ test('Refresh should work', async () => { test('Should database select display options', async () => { const props = createProps(); - render(, { useApi: true }); + render(, { useRedux: true }); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -241,7 +241,7 @@ test('should show empty state if there are no options', async () => { db={undefined} emptyState={} />, - { useApi: true }, + { useRedux: true }, ); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', @@ -254,7 +254,7 @@ test('should show empty state if there are no options', async () => { test('Should schema select display options', async () => { const props = createProps(); - render(, { useApi: true }); + render(, { useRedux: true }); const select = screen.getByRole('combobox', { name: 'Select schema or type to search schemas', }); @@ -270,7 +270,7 @@ test('Should schema select display options', async () => { test('Sends the correct db when changing the database', async () => { const props = createProps(); - render(, { useApi: true }); + render(, { useRedux: true }); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -290,7 +290,7 @@ test('Sends the correct db when changing the database', async () => { test('Sends the correct schema when changing the schema', async () => { const props = createProps(); - render(, { useApi: true }); + render(, { useRedux: true }); const select = screen.getByRole('combobox', { name: 'Select schema or type to search schemas', }); diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx index 70cc704dc542..ed2d2f717bc9 100644 --- a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx +++ b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx @@ -78,7 +78,7 @@ test('renders with default props', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useApi: true }); + render(, { useRedux: true }); const databaseSelect = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -100,7 +100,7 @@ test('renders table options', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useApi: true }); + render(, { useRedux: true }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', }); @@ -118,7 +118,7 @@ test('renders disabled without schema', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useApi: true }); + render(, { useRedux: true }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', }); @@ -137,7 +137,7 @@ test('table select retain value if not in SQL Lab mode', async () => { sqlLabMode: false, }); - render(, { useApi: true }); + render(, { useRedux: true }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', @@ -177,7 +177,7 @@ test('table multi select retain all the values selected', async () => { onTableSelectChange: callback, }); - render(, { useApi: true }); + render(, { useRedux: true }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', diff --git a/superset-frontend/src/hooks/apiResources/schemas.test.ts b/superset-frontend/src/hooks/apiResources/schemas.test.ts index 1c34ffbb8a65..530eba05b2f9 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.test.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.test.ts @@ -66,7 +66,7 @@ describe('useSchemas hook', () => { }), { wrapper: createWrapper({ - useApi: true, + useRedux: true, }), }, ); @@ -106,7 +106,7 @@ describe('useSchemas hook', () => { }), { wrapper: createWrapper({ - useApi: true, + useRedux: true, }), }, ); @@ -133,7 +133,7 @@ describe('useSchemas hook', () => { { initialProps: { dbId: expectDbId }, wrapper: createWrapper({ - useApi: true, + useRedux: true, }), }, ); diff --git a/superset-frontend/src/hooks/apiResources/tables.test.ts b/superset-frontend/src/hooks/apiResources/tables.test.ts index 3d4f97d8efb1..1d89f199a5cf 100644 --- a/superset-frontend/src/hooks/apiResources/tables.test.ts +++ b/superset-frontend/src/hooks/apiResources/tables.test.ts @@ -95,7 +95,7 @@ describe('useTables hook', () => { }), { wrapper: createWrapper({ - useApi: true, + useRedux: true, }), }, ); @@ -143,7 +143,7 @@ describe('useTables hook', () => { }), { wrapper: createWrapper({ - useApi: true, + useRedux: true, }), }, ); @@ -175,7 +175,7 @@ describe('useTables hook', () => { }), { wrapper: createWrapper({ - useApi: true, + useRedux: true, }), }, ); @@ -199,7 +199,7 @@ describe('useTables hook', () => { }), { wrapper: createWrapper({ - useApi: true, + useRedux: true, }), }, ); @@ -228,7 +228,7 @@ describe('useTables hook', () => { { initialProps: { schema: expectedSchema }, wrapper: createWrapper({ - useApi: true, + useRedux: true, }), }, ); From 41e1994bb5ab0603976ebde8d8f2db19b729fcc5 Mon Sep 17 00:00:00 2001 From: justin-park Date: Thu, 27 Apr 2023 15:16:44 -0700 Subject: [PATCH 09/15] refactor initialState condition --- .../spec/helpers/testing-library.tsx | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index 2f225bf94568..eef9af6e119c 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -41,6 +41,7 @@ type Options = Omit & { export const storeWithApi = configureStore({ reducer: { + ...reducerIndex, [api.reducerPath]: api.reducer, }, middleware: getDefaultMiddleware => @@ -69,21 +70,20 @@ export function createWrapper(options?: Options) { } if (useRedux) { - const mockStore = - store ?? - (!initialState - ? storeWithApi - : configureStore({ - preloadedState: initialState, - devTools: false, - reducer: { - ...(reducers || reducerIndex), - [api.reducerPath]: api.reducer, - }, - middleware: getDefaultMiddleware => - getDefaultMiddleware().concat(api.middleware), - })); - + let storeWithInitialState; + if (initialState || reducers) { + storeWithInitialState = configureStore({ + preloadedState: initialState, + devTools: false, + reducer: { + ...(reducers || reducerIndex), + [api.reducerPath]: api.reducer, + }, + middleware: getDefaultMiddleware => + getDefaultMiddleware().concat(api.middleware), + }); + } + const mockStore = store ?? storeWithInitialState ?? storeWithApi; result = {result}; } From f7ddca5089c80ae9e328cb0c638c3fb32178ea76 Mon Sep 17 00:00:00 2001 From: justin-park Date: Thu, 27 Apr 2023 15:20:01 -0700 Subject: [PATCH 10/15] remove describe on test --- .../src/hooks/apiResources/queryApi.test.ts | 124 +++++++++--------- 1 file changed, 60 insertions(+), 64 deletions(-) diff --git a/superset-frontend/src/hooks/apiResources/queryApi.test.ts b/superset-frontend/src/hooks/apiResources/queryApi.test.ts index 7a1040334946..4fb71387752d 100644 --- a/superset-frontend/src/hooks/apiResources/queryApi.test.ts +++ b/superset-frontend/src/hooks/apiResources/queryApi.test.ts @@ -32,73 +32,69 @@ const getBaseQueryApiMock = (store: MockStore) => ({ type: 'query' as const, }); -describe('queryApi', () => { - const mockStore = configureStore(); - const store = mockStore(); +const mockStore = configureStore(); +const store = mockStore(); - afterEach(() => { - fetchMock.reset(); - }); +afterEach(() => { + fetchMock.reset(); +}); - test('supersetClientQuery should build the endpoint with rison encoded query string and return data when successful', async () => { - const expectedData = { id: 1, name: 'Test' }; - const expectedUrl = '/api/v1/get-endpoint/'; - const expectedPostUrl = '/api/v1/post-endpoint/'; - const urlParams = { str: 'string', num: 123, bool: true }; - const getEndpoint = `glob:*${expectedUrl}?q=${rison.encode(urlParams)}`; - const postEndpoint = `glob:*${expectedPostUrl}?q=${rison.encode( +test('supersetClientQuery should build the endpoint with rison encoded query string and return data when successful', async () => { + const expectedData = { id: 1, name: 'Test' }; + const expectedUrl = '/api/v1/get-endpoint/'; + const expectedPostUrl = '/api/v1/post-endpoint/'; + const urlParams = { str: 'string', num: 123, bool: true }; + const getEndpoint = `glob:*${expectedUrl}?q=${rison.encode(urlParams)}`; + const postEndpoint = `glob:*${expectedPostUrl}?q=${rison.encode(urlParams)}`; + fetchMock.get(getEndpoint, { result: expectedData }); + fetchMock.post(postEndpoint, { result: expectedData }); + const result = await supersetClientQuery( + { + endpoint: expectedUrl, + urlParams, + }, + getBaseQueryApiMock(store), + {}, + ); + expect(fetchMock.calls(getEndpoint)).toHaveLength(1); + expect(fetchMock.calls(postEndpoint)).toHaveLength(0); + expect((result.data as JsonResponse).json.result).toEqual(expectedData); + await supersetClientQuery( + { + method: 'post', + endpoint: expectedPostUrl, urlParams, - )}`; - fetchMock.get(getEndpoint, { result: expectedData }); - fetchMock.post(postEndpoint, { result: expectedData }); - const result = await supersetClientQuery( - { - endpoint: expectedUrl, - urlParams, - }, - getBaseQueryApiMock(store), - {}, - ); - expect(fetchMock.calls(getEndpoint)).toHaveLength(1); - expect(fetchMock.calls(postEndpoint)).toHaveLength(0); - expect((result.data as JsonResponse).json.result).toEqual(expectedData); - await supersetClientQuery( - { - method: 'post', - endpoint: expectedPostUrl, - urlParams, - }, - getBaseQueryApiMock(store), - {}, - ); - expect(fetchMock.calls(getEndpoint)).toHaveLength(1); - expect(fetchMock.calls(postEndpoint)).toHaveLength(1); - }); + }, + getBaseQueryApiMock(store), + {}, + ); + expect(fetchMock.calls(getEndpoint)).toHaveLength(1); + expect(fetchMock.calls(postEndpoint)).toHaveLength(1); +}); - test('supersetClientQuery should return error when unsuccessful', async () => { - const expectedError = 'Request failed'; - const expectedUrl = '/api/v1/get-endpoint/'; - const endpoint = `glob:*${expectedUrl}`; - fetchMock.get(endpoint, { throws: new Error(expectedError) }); - const result = await supersetClientQuery( - { endpoint }, - getBaseQueryApiMock(store), - {}, - ); - expect(result.error).toEqual({ error: expectedError }); - }); +test('supersetClientQuery should return error when unsuccessful', async () => { + const expectedError = 'Request failed'; + const expectedUrl = '/api/v1/get-endpoint/'; + const endpoint = `glob:*${expectedUrl}`; + fetchMock.get(endpoint, { throws: new Error(expectedError) }); + const result = await supersetClientQuery( + { endpoint }, + getBaseQueryApiMock(store), + {}, + ); + expect(result.error).toEqual({ error: expectedError }); +}); - test('supersetClientQuery should return parsed response by parseMethod', async () => { - const expectedUrl = '/api/v1/get-endpoint/'; - const endpoint = `glob:*${expectedUrl}`; - const bitIntVal = '9223372036854775807'; - const expectedData = `{ "id": ${bitIntVal} }`; - fetchMock.get(endpoint, expectedData); - const result = await supersetClientQuery( - { endpoint, parseMethod: 'json-bigint' }, - getBaseQueryApiMock(store), - {}, - ); - expect(`${(result.data as JsonResponse).json.id}`).toEqual(bitIntVal); - }); +test('supersetClientQuery should return parsed response by parseMethod', async () => { + const expectedUrl = '/api/v1/get-endpoint/'; + const endpoint = `glob:*${expectedUrl}`; + const bitIntVal = '9223372036854775807'; + const expectedData = `{ "id": ${bitIntVal} }`; + fetchMock.get(endpoint, expectedData); + const result = await supersetClientQuery( + { endpoint, parseMethod: 'json-bigint' }, + getBaseQueryApiMock(store), + {}, + ); + expect(`${(result.data as JsonResponse).json.id}`).toEqual(bitIntVal); }); From fd1f30db3d9564a81b975eb114dccbd3c4f54a50 Mon Sep 17 00:00:00 2001 From: justin-park Date: Thu, 27 Apr 2023 15:26:36 -0700 Subject: [PATCH 11/15] missing isMountedRef --- .../src/hooks/apiResources/schemas.ts | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/hooks/apiResources/schemas.ts b/superset-frontend/src/hooks/apiResources/schemas.ts index 176b72b04818..d278098332e4 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useCallback, useEffect } from 'react'; +import { useCallback, useEffect, useRef } from 'react'; import { api, JsonResponse } from './queryApi'; export type SchemaOption = { @@ -63,6 +63,7 @@ export const { useLazySchemasQuery, useSchemasQuery } = schemaApi; const EMPTY_SCHEMAS = [] as SchemaOption[]; export function useSchemas(options: Params) { + const isMountedRef = useRef(false); const { dbId, onSuccess, onError } = options || {}; const [trigger, refetchResult] = useLazySchemasQuery(); const result = useSchemasQuery( @@ -88,15 +89,19 @@ export function useSchemas(options: Params) { }, [dbId]); useEffect(() => { - const { requestId, isSuccess, isError, isFetching, data, originalArgs } = - result; - if (!originalArgs?.forceRefresh && requestId && !isFetching) { - if (isSuccess) { - onSuccess?.(data || EMPTY_SCHEMAS, false); - } - if (isError) { - onError?.(); + if (isMountedRef.current) { + const { requestId, isSuccess, isError, isFetching, data, originalArgs } = + result; + if (!originalArgs?.forceRefresh && requestId && !isFetching) { + if (isSuccess) { + onSuccess?.(data || EMPTY_SCHEMAS, false); + } + if (isError) { + onError?.(); + } } + } else { + isMountedRef.current = true; } }, [result]); From a090a3f5b61d447abb955f4fa0292a9d514b1e92 Mon Sep 17 00:00:00 2001 From: justin-park Date: Fri, 28 Apr 2023 09:07:04 -0700 Subject: [PATCH 12/15] clean up storeWithApi usage --- superset-frontend/spec/helpers/testing-library.tsx | 9 +++------ .../DatabaseSelector/DatabaseSelector.test.tsx | 14 +++++++------- .../TableSelector/TableSelector.test.tsx | 13 ++++++++----- .../src/hooks/apiResources/schemas.test.ts | 3 +++ .../src/hooks/apiResources/tables.test.ts | 5 +++++ 5 files changed, 26 insertions(+), 18 deletions(-) diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index eef9af6e119c..6e36b37bdd98 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -41,7 +41,6 @@ type Options = Omit & { export const storeWithApi = configureStore({ reducer: { - ...reducerIndex, [api.reducerPath]: api.reducer, }, middleware: getDefaultMiddleware => @@ -70,9 +69,9 @@ export function createWrapper(options?: Options) { } if (useRedux) { - let storeWithInitialState; - if (initialState || reducers) { - storeWithInitialState = configureStore({ + const mockStore = + store ?? + configureStore({ preloadedState: initialState, devTools: false, reducer: { @@ -82,8 +81,6 @@ export function createWrapper(options?: Options) { middleware: getDefaultMiddleware => getDefaultMiddleware().concat(api.middleware), }); - } - const mockStore = store ?? storeWithInitialState ?? storeWithApi; result = {result}; } diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index 3b19c40989dd..8af12f3495bb 100644 --- a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx +++ b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx @@ -181,14 +181,14 @@ afterEach(() => { test('Should render', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); expect(await screen.findByTestId('DatabaseSelector')).toBeInTheDocument(); }); test('Refresh should work', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); expect(fetchMock.calls(schemaApiRoute).length).toBe(0); @@ -220,7 +220,7 @@ test('Refresh should work', async () => { test('Should database select display options', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -241,7 +241,7 @@ test('should show empty state if there are no options', async () => { db={undefined} emptyState={} />, - { useRedux: true }, + { useRedux: true, store }, ); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', @@ -254,7 +254,7 @@ test('should show empty state if there are no options', async () => { test('Should schema select display options', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const select = screen.getByRole('combobox', { name: 'Select schema or type to search schemas', }); @@ -270,7 +270,7 @@ test('Should schema select display options', async () => { test('Sends the correct db when changing the database', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const select = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -290,7 +290,7 @@ test('Sends the correct db when changing the database', async () => { test('Sends the correct schema when changing the schema', async () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const select = screen.getByRole('combobox', { name: 'Select schema or type to search schemas', }); diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx index ed2d2f717bc9..81d713aa8a65 100644 --- a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx +++ b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx @@ -78,7 +78,7 @@ test('renders with default props', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const databaseSelect = screen.getByRole('combobox', { name: 'Select database or type to search databases', }); @@ -100,7 +100,7 @@ test('renders table options', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', }); @@ -118,7 +118,10 @@ test('renders disabled without schema', async () => { fetchMock.get(tablesApiRoute, getTableMockFunction()); const props = createProps(); - render(, { useRedux: true }); + render(, { + useRedux: true, + store, + }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', }); @@ -137,7 +140,7 @@ test('table select retain value if not in SQL Lab mode', async () => { sqlLabMode: false, }); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', @@ -177,7 +180,7 @@ test('table multi select retain all the values selected', async () => { onTableSelectChange: callback, }); - render(, { useRedux: true }); + render(, { useRedux: true, store }); const tableSelect = screen.getByRole('combobox', { name: 'Select table or type to search tables', diff --git a/superset-frontend/src/hooks/apiResources/schemas.test.ts b/superset-frontend/src/hooks/apiResources/schemas.test.ts index 530eba05b2f9..657a46261c77 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.test.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.test.ts @@ -67,6 +67,7 @@ describe('useSchemas hook', () => { { wrapper: createWrapper({ useRedux: true, + store, }), }, ); @@ -107,6 +108,7 @@ describe('useSchemas hook', () => { { wrapper: createWrapper({ useRedux: true, + store, }), }, ); @@ -134,6 +136,7 @@ describe('useSchemas hook', () => { initialProps: { dbId: expectDbId }, wrapper: createWrapper({ useRedux: true, + store, }), }, ); diff --git a/superset-frontend/src/hooks/apiResources/tables.test.ts b/superset-frontend/src/hooks/apiResources/tables.test.ts index 1d89f199a5cf..387ecbd7b727 100644 --- a/superset-frontend/src/hooks/apiResources/tables.test.ts +++ b/superset-frontend/src/hooks/apiResources/tables.test.ts @@ -96,6 +96,7 @@ describe('useTables hook', () => { { wrapper: createWrapper({ useRedux: true, + store, }), }, ); @@ -144,6 +145,7 @@ describe('useTables hook', () => { { wrapper: createWrapper({ useRedux: true, + store, }), }, ); @@ -176,6 +178,7 @@ describe('useTables hook', () => { { wrapper: createWrapper({ useRedux: true, + store, }), }, ); @@ -200,6 +203,7 @@ describe('useTables hook', () => { { wrapper: createWrapper({ useRedux: true, + store, }), }, ); @@ -229,6 +233,7 @@ describe('useTables hook', () => { initialProps: { schema: expectedSchema }, wrapper: createWrapper({ useRedux: true, + store, }), }, ); From fc847878600aaab9406935c816e3a2542c8d7e1a Mon Sep 17 00:00:00 2001 From: justin-park Date: Wed, 3 May 2023 13:08:59 -0700 Subject: [PATCH 13/15] fix exhaustive-deps issues --- .../src/hooks/apiResources/schemas.ts | 23 ++++++++++++++----- .../src/hooks/apiResources/tables.ts | 21 ++++++++++++----- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/superset-frontend/src/hooks/apiResources/schemas.ts b/superset-frontend/src/hooks/apiResources/schemas.ts index d278098332e4..55c944d435b8 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.ts @@ -17,6 +17,7 @@ * under the License. */ import { useCallback, useEffect, useRef } from 'react'; +import useEffectEvent from 'src/hooks/useEffectEvent'; import { api, JsonResponse } from './queryApi'; export type SchemaOption = { @@ -73,20 +74,30 @@ export function useSchemas(options: Params) { }, ); + const handleOnSuccess = useEffectEvent( + (data: SchemaOption[], isRefetched: boolean) => { + onSuccess?.(data, isRefetched); + }, + ); + + const handleOnError = useEffectEvent(() => { + onError?.(); + }); + const refetch = useCallback(() => { if (dbId) { trigger({ dbId, forceRefresh: true }).then( ({ isSuccess, isError, data }) => { if (isSuccess) { - onSuccess?.(data || EMPTY_SCHEMAS, true); + handleOnSuccess(data || EMPTY_SCHEMAS, true); } if (isError) { - onError?.(); + handleOnError(); } }, ); } - }, [dbId]); + }, [dbId, handleOnError, handleOnSuccess, trigger]); useEffect(() => { if (isMountedRef.current) { @@ -94,16 +105,16 @@ export function useSchemas(options: Params) { result; if (!originalArgs?.forceRefresh && requestId && !isFetching) { if (isSuccess) { - onSuccess?.(data || EMPTY_SCHEMAS, false); + handleOnSuccess(data || EMPTY_SCHEMAS, false); } if (isError) { - onError?.(); + handleOnError(); } } } else { isMountedRef.current = true; } - }, [result]); + }, [result, handleOnSuccess, handleOnError]); return { ...(refetchResult.isUninitialized ? result : refetchResult), diff --git a/superset-frontend/src/hooks/apiResources/tables.ts b/superset-frontend/src/hooks/apiResources/tables.ts index 72e9dc8ad64b..5d28cba4880f 100644 --- a/superset-frontend/src/hooks/apiResources/tables.ts +++ b/superset-frontend/src/hooks/apiResources/tables.ts @@ -17,6 +17,7 @@ * under the License. */ import { useCallback, useMemo, useEffect, useRef } from 'react'; +import useEffectEvent from 'src/hooks/useEffectEvent'; import { api } from './queryApi'; import { useSchemas } from './schemas'; @@ -106,20 +107,28 @@ export function useTables(options: Params) { ); const [trigger] = useLazyTablesQuery(); + const handleOnSuccess = useEffectEvent((data: Data, isRefetched: boolean) => { + onSuccess?.(data, isRefetched); + }); + + const handleOnError = useEffectEvent((error: Response) => { + onError?.(error); + }); + const refetch = useCallback(() => { if (enabled) { trigger({ dbId, schema, forceRefresh: true }).then( ({ isSuccess, isError, data, error }) => { if (isSuccess && data) { - onSuccess?.(data, true); + handleOnSuccess(data, true); } if (isError) { - onError?.(error as Response); + handleOnError(error as Response); } }, ); } - }, [dbId, schema, enabled]); + }, [dbId, schema, enabled, handleOnSuccess, handleOnError, trigger]); useEffect(() => { if (isMountedRef.current) { @@ -134,16 +143,16 @@ export function useTables(options: Params) { } = result; if (!originalArgs?.forceRefresh && requestId && !isFetching) { if (isSuccess && data) { - onSuccess?.(data, false); + handleOnSuccess(data, false); } if (isError) { - onError?.(error as Response); + handleOnError(error as Response); } } } else { isMountedRef.current = true; } - }, [result]); + }, [result, handleOnSuccess, handleOnError]); return { ...result, From cc5324f9e33566a82d28e7b083147dbed333fceb Mon Sep 17 00:00:00 2001 From: justin-park Date: Thu, 4 May 2023 16:09:08 -0700 Subject: [PATCH 14/15] rename storeWithApi by defaultStore --- .../spec/helpers/testing-library.tsx | 33 ++++++++----------- .../DatabaseSelector.test.tsx | 2 +- .../Datasource/DatasourceModal.test.jsx | 2 +- .../TableSelector/TableSelector.test.tsx | 2 +- .../src/hooks/apiResources/schemas.test.ts | 2 +- .../src/hooks/apiResources/tables.test.ts | 2 +- 6 files changed, 19 insertions(+), 24 deletions(-) diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index 6e36b37bdd98..0c7b6a0a66a5 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -39,14 +39,19 @@ type Options = Omit & { store?: Store; }; -export const storeWithApi = configureStore({ - reducer: { - [api.reducerPath]: api.reducer, - }, - middleware: getDefaultMiddleware => - getDefaultMiddleware().concat(api.middleware), - devTools: false, -}); +const createStore = (initialState: object = {}, reducers: object = {}) => + configureStore({ + preloadedState: initialState, + reducer: { + ...reducers, + [api.reducerPath]: api.reducer, + }, + middleware: getDefaultMiddleware => + getDefaultMiddleware().concat(api.middleware), + devTools: false, + }); + +export const defaultStore = createStore(); export function createWrapper(options?: Options) { const { @@ -70,17 +75,7 @@ export function createWrapper(options?: Options) { if (useRedux) { const mockStore = - store ?? - configureStore({ - preloadedState: initialState, - devTools: false, - reducer: { - ...(reducers || reducerIndex), - [api.reducerPath]: api.reducer, - }, - middleware: getDefaultMiddleware => - getDefaultMiddleware().concat(api.middleware), - }); + store ?? createStore(initialState, reducers || reducerIndex); result = {result}; } diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index 8af12f3495bb..7635361d8933 100644 --- a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx +++ b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx @@ -24,7 +24,7 @@ import { render, screen, waitFor, - storeWithApi as store, + defaultStore as store, } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import { api } from 'src/hooks/apiResources/queryApi'; diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx index 9b5960092650..d5619c552792 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx @@ -25,7 +25,7 @@ import sinon from 'sinon'; import { supersetTheme, ThemeProvider } from '@superset-ui/core'; import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; -import { storeWithApi as store } from 'spec/helpers/testing-library'; +import { defaultStore as store } from 'spec/helpers/testing-library'; import Modal from 'src/components/Modal'; import { DatasourceModal } from 'src/components/Datasource'; import DatasourceEditor from 'src/components/Datasource/DatasourceEditor'; diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx index 81d713aa8a65..f6107d4cacb2 100644 --- a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx +++ b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx @@ -24,7 +24,7 @@ import { screen, waitFor, within, - storeWithApi as store, + defaultStore as store, } from 'spec/helpers/testing-library'; import { api } from 'src/hooks/apiResources/queryApi'; import fetchMock from 'fetch-mock'; diff --git a/superset-frontend/src/hooks/apiResources/schemas.test.ts b/superset-frontend/src/hooks/apiResources/schemas.test.ts index 657a46261c77..70c1289d576a 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.test.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.test.ts @@ -21,7 +21,7 @@ import fetchMock from 'fetch-mock'; import { act, renderHook } from '@testing-library/react-hooks'; import { createWrapper, - storeWithApi as store, + defaultStore as store, } from 'spec/helpers/testing-library'; import { api } from 'src/hooks/apiResources/queryApi'; import { useSchemas } from './schemas'; diff --git a/superset-frontend/src/hooks/apiResources/tables.test.ts b/superset-frontend/src/hooks/apiResources/tables.test.ts index 387ecbd7b727..e461b4f86b2b 100644 --- a/superset-frontend/src/hooks/apiResources/tables.test.ts +++ b/superset-frontend/src/hooks/apiResources/tables.test.ts @@ -21,7 +21,7 @@ import fetchMock from 'fetch-mock'; import { act, renderHook } from '@testing-library/react-hooks'; import { createWrapper, - storeWithApi as store, + defaultStore as store, } from 'spec/helpers/testing-library'; import { api } from 'src/hooks/apiResources/queryApi'; import { useTables } from './tables'; From 7fc1210a68c4248096865425b82eabe5c1ada84f Mon Sep 17 00:00:00 2001 From: justin-park Date: Mon, 8 May 2023 11:20:57 -0700 Subject: [PATCH 15/15] remove refetchResult since result are same --- superset-frontend/src/hooks/apiResources/schemas.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/hooks/apiResources/schemas.ts b/superset-frontend/src/hooks/apiResources/schemas.ts index 55c944d435b8..22fd2cf38b09 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.ts @@ -66,7 +66,7 @@ const EMPTY_SCHEMAS = [] as SchemaOption[]; export function useSchemas(options: Params) { const isMountedRef = useRef(false); const { dbId, onSuccess, onError } = options || {}; - const [trigger, refetchResult] = useLazySchemasQuery(); + const [trigger] = useLazySchemasQuery(); const result = useSchemasQuery( { dbId, forceRefresh: false }, { @@ -117,7 +117,7 @@ export function useSchemas(options: Params) { }, [result, handleOnSuccess, handleOnError]); return { - ...(refetchResult.isUninitialized ? result : refetchResult), + ...result, refetch, }; }