From 51a34d7d58a8c110d403e22db30a799a68a81ed5 Mon Sep 17 00:00:00 2001 From: "JUST.in DO IT" Date: Thu, 22 Jun 2023 15:37:03 -0700 Subject: [PATCH] chore(sqllab): Remove table metadata from state (#24371) --- superset-frontend/src/SqlLab/App.jsx | 25 +++ .../src/SqlLab/actions/sqlLab.js | 183 +++++++----------- .../src/SqlLab/actions/sqlLab.test.js | 118 +++++------ .../AceEditorWrapper.test.tsx | 1 - .../components/AceEditorWrapper/index.tsx | 6 +- .../src/SqlLab/components/SqlEditor/index.jsx | 1 - .../components/SqlEditorLeftBar/index.tsx | 6 +- .../TableElement/TableElement.test.jsx | 161 --------------- .../TableElement/TableElement.test.tsx | 177 +++++++++++++++++ .../SqlLab/components/TableElement/index.tsx | 125 ++++++++---- .../src/SqlLab/reducers/getInitialState.js | 37 ++-- .../utils/reduxStateToLocalStorageHelper.js | 16 ++ .../src/hooks/apiResources/queryApi.ts | 8 +- .../src/hooks/apiResources/tables.ts | 61 +++++- 14 files changed, 515 insertions(+), 410 deletions(-) delete mode 100644 superset-frontend/src/SqlLab/components/TableElement/TableElement.test.jsx create mode 100644 superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx diff --git a/superset-frontend/src/SqlLab/App.jsx b/superset-frontend/src/SqlLab/App.jsx index be0a982a4951d..dbbab0d184bad 100644 --- a/superset-frontend/src/SqlLab/App.jsx +++ b/superset-frontend/src/SqlLab/App.jsx @@ -26,10 +26,12 @@ import { initFeatureFlags, isFeatureEnabled } from 'src/featureFlags'; import { setupStore } from 'src/views/store'; import setupExtensions from 'src/setup/setupExtensions'; import getBootstrapData from 'src/utils/getBootstrapData'; +import { api } from 'src/hooks/apiResources/queryApi'; import getInitialState from './reducers/getInitialState'; import { reducers } from './reducers/index'; import App from './components/App'; import { + emptyTablePersistData, emptyQueryResults, clearQueryEditors, } from './utils/reduxStateToLocalStorageHelper'; @@ -62,6 +64,7 @@ const sqlLabPersistStateConfig = { if (path === 'sqlLab') { subset[path] = { ...state[path], + tables: emptyTablePersistData(state[path].tables), queries: emptyQueryResults(state[path].queries), queryEditors: clearQueryEditors(state[path].queryEditors), unsavedQueryEditor: clearQueryEditors([ @@ -119,6 +122,28 @@ export const store = setupStore({ }), }); +// Rehydrate server side persisted table metadata +initialState.sqlLab.tables.forEach( + ({ name: table, schema, dbId, persistData }) => { + if (dbId && schema && table && persistData?.columns) { + store.dispatch( + api.util.upsertQueryData( + 'tableMetadata', + { dbId, schema, table }, + persistData, + ), + ); + store.dispatch( + api.util.upsertQueryData( + 'tableExtendedMetadata', + { dbId, schema, table }, + {}, + ), + ); + } + }, +); + // Highlight the navbar menu const menus = document.querySelectorAll('.nav.navbar-nav li.dropdown'); const sqlLabMenu = Array.prototype.slice diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 8b908d2666311..addad284e7024 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -1100,65 +1100,7 @@ export function mergeTable(table, query, prepend) { return { type: MERGE_TABLE, table, query, prepend }; } -function getTableMetadata(table, query, dispatch) { - return SupersetClient.get({ - endpoint: encodeURI( - `/api/v1/database/${query.dbId}/table/${encodeURIComponent( - table.name, - )}/${encodeURIComponent(table.schema)}/`, - ), - }) - .then(({ json }) => { - const newTable = { - ...table, - ...json, - expanded: true, - isMetadataLoading: false, - }; - dispatch(mergeTable(newTable)); // Merge table to tables in state - return newTable; - }) - .catch(() => - Promise.all([ - dispatch( - mergeTable({ - ...table, - isMetadataLoading: false, - }), - ), - dispatch( - addDangerToast(t('An error occurred while fetching table metadata')), - ), - ]), - ); -} - -function getTableExtendedMetadata(table, query, dispatch) { - return SupersetClient.get({ - endpoint: encodeURI( - `/api/v1/database/${query.dbId}/table_extra/` + - `${encodeURIComponent(table.name)}/${encodeURIComponent( - table.schema, - )}/`, - ), - }) - .then(({ json }) => { - dispatch( - mergeTable({ ...table, ...json, isExtraMetadataLoading: false }), - ); - return json; - }) - .catch(() => - Promise.all([ - dispatch(mergeTable({ ...table, isExtraMetadataLoading: false })), - dispatch( - addDangerToast(t('An error occurred while fetching table metadata')), - ), - ]), - ); -} - -export function addTable(queryEditor, database, tableName, schemaName) { +export function addTable(queryEditor, tableName, schemaName) { return function (dispatch, getState) { const query = getUpToDateQuery(getState(), queryEditor, queryEditor.id); const table = { @@ -1171,67 +1113,90 @@ export function addTable(queryEditor, database, tableName, schemaName) { mergeTable( { ...table, - isMetadataLoading: true, - isExtraMetadataLoading: true, + id: shortid.generate(), expanded: true, }, null, true, ), ); + }; +} - return Promise.all([ - getTableMetadata(table, query, dispatch), - getTableExtendedMetadata(table, query, dispatch), - ]).then(([newTable, json]) => { - const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.post({ - endpoint: encodeURI('/tableschemaview/'), - postPayload: { table: { ...newTable, ...json } }, - }) - : Promise.resolve({ json: { id: shortid.generate() } }); - - if (!database.disable_data_preview && database.id === query.dbId) { - const dataPreviewQuery = { - id: shortid.generate(), - dbId: query.dbId, - sql: newTable.selectStar, - tableName: table.name, - sqlEditorId: null, - tab: '', - runAsync: database.allow_run_async, - ctas: false, - isDataPreview: true, - }; - Promise.all([ - dispatch( - mergeTable( - { - ...newTable, - dataPreviewQueryId: dataPreviewQuery.id, - }, - dataPreviewQuery, - ), +export function runTablePreviewQuery(newTable) { + return function (dispatch, getState) { + const { + sqlLab: { databases }, + } = getState(); + const database = databases[newTable.dbId]; + const { dbId } = newTable; + + if (database && !database.disable_data_preview) { + const dataPreviewQuery = { + id: shortid.generate(), + dbId, + sql: newTable.selectStar, + tableName: newTable.name, + sqlEditorId: null, + tab: '', + runAsync: database.allow_run_async, + ctas: false, + isDataPreview: true, + }; + return Promise.all([ + dispatch( + mergeTable( + { + id: newTable.id, + dbId: newTable.dbId, + schema: newTable.schema, + name: newTable.name, + queryEditorId: newTable.queryEditorId, + dataPreviewQueryId: dataPreviewQuery.id, + }, + dataPreviewQuery, ), - dispatch(runQuery(dataPreviewQuery)), - ]); - } + ), + dispatch(runQuery(dataPreviewQuery)), + ]); + } + return Promise.resolve(); + }; +} - return sync - .then(({ json: resultJson }) => - dispatch(mergeTable({ ...table, id: resultJson.id })), - ) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while fetching table metadata. ' + - 'Please contact your administrator.', - ), +export function syncTable(table, tableMetadata) { + return function (dispatch) { + const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) + ? SupersetClient.post({ + endpoint: encodeURI('/tableschemaview/'), + postPayload: { table: { ...tableMetadata, ...table } }, + }) + : Promise.resolve({ json: { id: table.id } }); + + return sync + .then(({ json: resultJson }) => { + const newTable = { ...table, id: resultJson.id }; + dispatch( + mergeTable({ + ...newTable, + expanded: true, + initialized: true, + }), + ); + if (!table.dataPreviewQueryId) { + dispatch(runTablePreviewQuery({ ...tableMetadata, ...newTable })); + } + }) + .catch(() => + dispatch( + addDangerToast( + t( + 'An error occurred while fetching table metadata. ' + + 'Please contact your administrator.', ), ), - ); - }); + ), + ); }; } diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index 01886d6f77d08..2f26ec16d520a 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -821,43 +821,7 @@ describe('async actions', () => { }); describe('addTable', () => { - it('updates the table schema state in the backend', () => { - expect.assertions(6); - - const database = { disable_data_preview: true }; - const tableName = 'table'; - const schemaName = 'schema'; - const store = mockStore(initialState); - const expectedActionTypes = [ - actions.MERGE_TABLE, // addTable - actions.MERGE_TABLE, // getTableMetadata - actions.MERGE_TABLE, // getTableExtendedMetadata - actions.MERGE_TABLE, // addTable - ]; - const request = actions.addTable( - query, - database, - tableName, - schemaName, - ); - return request(store.dispatch, store.getState).then(() => { - expect(store.getActions().map(a => a.type)).toEqual( - expectedActionTypes, - ); - expect(store.getActions()[0].prepend).toBeTruthy(); - expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(1); - expect(fetchMock.calls(getTableMetadataEndpoint)).toHaveLength(1); - expect(fetchMock.calls(getExtraTableMetadataEndpoint)).toHaveLength( - 1, - ); - - // tab state is not updated, since no query was run - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(0); - }); - }); - - it('fetches table schema state from unsaved change', () => { - const database = { disable_data_preview: true }; + it('dispatches table state from unsaved change', () => { const tableName = 'table'; const schemaName = 'schema'; const expectedDbId = 473892; @@ -871,31 +835,47 @@ describe('async actions', () => { }, }, }); - const request = actions.addTable( - query, - database, - tableName, - schemaName, + const request = actions.addTable(query, tableName, schemaName); + request(store.dispatch, store.getState); + expect(store.getActions()[0]).toEqual( + expect.objectContaining({ + table: expect.objectContaining({ + name: tableName, + schema: schemaName, + dbId: expectedDbId, + }), + }), ); + }); + }); + + describe('syncTable', () => { + it('updates the table schema state in the backend', () => { + expect.assertions(4); + + const tableName = 'table'; + const schemaName = 'schema'; + const store = mockStore(initialState); + const expectedActionTypes = [ + actions.MERGE_TABLE, // syncTable + ]; + const request = actions.syncTable(query, tableName, schemaName); return request(store.dispatch, store.getState).then(() => { - expect( - fetchMock.calls( - `glob:**/api/v1/database/${expectedDbId}/table/*/*/`, - ), - ).toHaveLength(1); - expect( - fetchMock.calls( - `glob:**/api/v1/database/${expectedDbId}/table_extra/*/*/`, - ), - ).toHaveLength(1); + expect(store.getActions().map(a => a.type)).toEqual( + expectedActionTypes, + ); + expect(store.getActions()[0].prepend).toBeFalsy(); + expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(1); // tab state is not updated, since no query was run expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(0); }); }); + }); + describe('runTablePreviewQuery', () => { it('updates and runs data preview query when configured', () => { - expect.assertions(5); + expect.assertions(3); const results = { data: mockBigNumber, @@ -906,34 +886,32 @@ describe('async actions', () => { overwriteRoutes: true, }); - const database = { disable_data_preview: false, id: 1 }; const tableName = 'table'; const schemaName = 'schema'; - const store = mockStore(initialState); + const store = mockStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + databases: { + 1: { disable_data_preview: false }, + }, + }, + }); const expectedActionTypes = [ - actions.MERGE_TABLE, // addTable - actions.MERGE_TABLE, // getTableMetadata - actions.MERGE_TABLE, // getTableExtendedMetadata actions.MERGE_TABLE, // addTable (data preview) actions.START_QUERY, // runQuery (data preview) - actions.MERGE_TABLE, // addTable actions.QUERY_SUCCESS, // querySuccess ]; - const request = actions.addTable( - query, - database, - tableName, - schemaName, - ); + const request = actions.runTablePreviewQuery({ + dbId: 1, + name: tableName, + schema: schemaName, + }); return request(store.dispatch, store.getState).then(() => { expect(store.getActions().map(a => a.type)).toEqual( expectedActionTypes, ); - expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(1); - expect(fetchMock.calls(getTableMetadataEndpoint)).toHaveLength(1); - expect(fetchMock.calls(getExtraTableMetadataEndpoint)).toHaveLength( - 1, - ); + expect(fetchMock.calls(runQueryEndpoint)).toHaveLength(1); // tab state is not updated, since the query is a data preview expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(0); }); diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx index 7638003e9025c..c69b92346db8a 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx @@ -51,7 +51,6 @@ const setup = (queryEditor: QueryEditor, store?: Store) => queryEditorId={queryEditor.id} height="100px" hotkeys={[]} - database={{}} onChange={jest.fn()} onBlur={jest.fn()} autocomplete diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx index 5f7d92e943a80..1db1cc5d9b492 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx @@ -55,7 +55,6 @@ type AceEditorWrapperProps = { onBlur: (sql: string) => void; onChange: (sql: string) => void; queryEditorId: string; - database: any; extendedTables?: Array<{ name: string; columns: any[] }>; height: string; hotkeys: HotKey[]; @@ -86,7 +85,6 @@ const AceEditorWrapper = ({ onBlur = () => {}, onChange = () => {}, queryEditorId, - database, extendedTables = [], height, hotkeys, @@ -258,9 +256,7 @@ const AceEditorWrapper = ({ const completer = { insertMatch: (editor: Editor, data: any) => { if (data.meta === 'table') { - dispatch( - addTable(queryEditor, database, data.value, queryEditor.schema), - ); + dispatch(addTable(queryEditor, data.value, queryEditor.schema)); } let { caption } = data; diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx index 6399baa1cdc63..b07c31fd75fde 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx @@ -666,7 +666,6 @@ const SqlEditor = ({ onBlur={setQueryEditorAndSaveSql} onChange={onSqlChanged} queryEditorId={queryEditor.id} - database={database} extendedTables={tables} height={`${aceEditorHeight}px`} hotkeys={hotkeys} diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx index 453d80f112f4d..e89bdc57b7fb3 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx @@ -164,9 +164,9 @@ const SqlEditorLeftBar = ({ return true; }); - tablesToAdd.forEach(tableName => - dispatch(addTable(queryEditor, database, tableName, schemaName)), - ); + tablesToAdd.forEach(tableName => { + dispatch(addTable(queryEditor, tableName, schemaName)); + }); dispatch(removeTables(currentTables)); }; diff --git a/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.jsx b/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.jsx deleted file mode 100644 index 60efa3a59677f..0000000000000 --- a/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.jsx +++ /dev/null @@ -1,161 +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 { mount } from 'enzyme'; -import { Provider } from 'react-redux'; -import configureStore from 'redux-mock-store'; -import thunk from 'redux-thunk'; -import { supersetTheme, ThemeProvider } from '@superset-ui/core'; -import Collapse from 'src/components/Collapse'; -import { IconTooltip } from 'src/components/IconTooltip'; -import TableElement from 'src/SqlLab/components/TableElement'; -import ColumnElement from 'src/SqlLab/components/ColumnElement'; -import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; -import { initialState, table } from 'src/SqlLab/fixtures'; - -const middlewares = [thunk]; -const mockStore = configureStore(middlewares); - -describe('TableElement', () => { - const store = mockStore(initialState); - const mockedProps = { - table, - timeout: 0, - }; - it('renders', () => { - expect(React.isValidElement()).toBe(true); - }); - it('renders with props', () => { - expect(React.isValidElement()).toBe(true); - }); - it('has 5 IconTooltip elements', () => { - const wrapper = mount( - - - , - { - wrappingComponent: ThemeProvider, - wrappingComponentProps: { - theme: supersetTheme, - }, - }, - ); - expect(wrapper.find(IconTooltip)).toHaveLength(4); - }); - it('has 14 columns', () => { - const wrapper = mount( - - - , - { - wrappingComponent: ThemeProvider, - wrappingComponentProps: { - theme: supersetTheme, - }, - }, - ); - expect(wrapper.find(ColumnElement)).toHaveLength(14); - }); - it('mounts', () => { - const wrapper = mount( - - - , - { - wrappingComponent: ThemeProvider, - wrappingComponentProps: { - theme: supersetTheme, - }, - }, - ); - - expect(wrapper.find(TableElement)).toHaveLength(1); - }); - it('fades table', async () => { - const wrapper = mount( - - - , - { - wrappingComponent: ThemeProvider, - wrappingComponentProps: { - theme: supersetTheme, - }, - }, - ); - expect(wrapper.find('[data-test="fade"]').first().props().hovered).toBe( - false, - ); - wrapper.find('.header-container').hostNodes().simulate('mouseEnter'); - await waitForComponentToPaint(wrapper, 300); - expect(wrapper.find('[data-test="fade"]').first().props().hovered).toBe( - true, - ); - }); - it('sorts columns', () => { - const wrapper = mount( - - - - - , - { - wrappingComponent: ThemeProvider, - wrappingComponentProps: { - theme: supersetTheme, - }, - }, - ); - expect( - wrapper.find(IconTooltip).at(1).hasClass('fa-sort-alpha-asc'), - ).toEqual(true); - expect( - wrapper.find(IconTooltip).at(1).hasClass('fa-sort-numeric-asc'), - ).toEqual(false); - wrapper.find('.header-container').hostNodes().simulate('click'); - expect(wrapper.find(ColumnElement).first().props().column.name).toBe('id'); - wrapper.find('.header-container').simulate('mouseEnter'); - wrapper.find('.sort-cols').hostNodes().simulate('click'); - expect( - wrapper.find(IconTooltip).at(1).hasClass('fa-sort-numeric-asc'), - ).toEqual(true); - expect( - wrapper.find(IconTooltip).at(1).hasClass('fa-sort-alpha-asc'), - ).toEqual(false); - expect(wrapper.find(ColumnElement).first().props().column.name).toBe( - 'active', - ); - }); - it('removes the table', () => { - const wrapper = mount( - - - , - { - wrappingComponent: ThemeProvider, - wrappingComponentProps: { - theme: supersetTheme, - }, - }, - ); - wrapper.find('.table-remove').hostNodes().simulate('click'); - expect(store.getActions()).toHaveLength(1); - expect(store.getActions()[0].type).toEqual('REMOVE_DATA_PREVIEW'); - }); -}); diff --git a/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx b/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx new file mode 100644 index 0000000000000..89c98cfd335e7 --- /dev/null +++ b/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx @@ -0,0 +1,177 @@ +/** + * 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 fetchMock from 'fetch-mock'; +import * as featureFlags from 'src/featureFlags'; +import { FeatureFlag } from '@superset-ui/core'; +import TableElement, { Column } from 'src/SqlLab/components/TableElement'; +import { table, initialState } from 'src/SqlLab/fixtures'; +import { render, waitFor, fireEvent } from 'spec/helpers/testing-library'; + +jest.mock('src/components/Loading', () => () => ( +
+)); +jest.mock('src/components/IconTooltip', () => ({ + IconTooltip: ({ + onClick, + tooltip, + }: { + onClick: () => void; + tooltip: string; + }) => ( + + ), +})); +jest.mock( + 'src/SqlLab/components/ColumnElement', + () => + ({ column }: { column: Column }) => +
{column.name}
, +); +const getTableMetadataEndpoint = 'glob:**/api/v1/database/*/table/*/*/'; +const getExtraTableMetadataEndpoint = + 'glob:**/api/v1/database/*/table_extra/*/*/'; +const updateTableSchemaEndpoint = 'glob:*/tableschemaview/*/expanded'; + +beforeEach(() => { + fetchMock.get(getTableMetadataEndpoint, table); + fetchMock.get(getExtraTableMetadataEndpoint, {}); + fetchMock.post(updateTableSchemaEndpoint, {}); +}); + +afterEach(() => { + fetchMock.reset(); +}); + +const mockedProps = { + table: { + ...table, + initialized: true, + }, +}; + +test('renders', () => { + expect(React.isValidElement()).toBe(true); +}); + +test('renders with props', () => { + expect(React.isValidElement()).toBe(true); +}); + +test('has 4 IconTooltip elements', async () => { + const { getAllByTestId } = render(, { + useRedux: true, + initialState, + }); + await waitFor(() => + expect(getAllByTestId('mock-icon-tooltip')).toHaveLength(4), + ); +}); + +test('has 14 columns', async () => { + const { getAllByTestId } = render(, { + useRedux: true, + initialState, + }); + await waitFor(() => + expect(getAllByTestId('mock-column-element')).toHaveLength(14), + ); +}); + +test('fades table', async () => { + const { getAllByTestId } = render(, { + useRedux: true, + initialState, + }); + await waitFor(() => + expect(getAllByTestId('mock-icon-tooltip')).toHaveLength(4), + ); + const style = window.getComputedStyle(getAllByTestId('fade')[0]); + expect(style.opacity).toBe('0'); + fireEvent.mouseEnter(getAllByTestId('table-element-header-container')[0]); + await waitFor(() => + expect(window.getComputedStyle(getAllByTestId('fade')[0]).opacity).toBe( + '1', + ), + ); +}); + +test('sorts columns', async () => { + const { getAllByTestId, getByText } = render( + , + { + useRedux: true, + initialState, + }, + ); + await waitFor(() => + expect(getAllByTestId('mock-icon-tooltip')).toHaveLength(4), + ); + expect( + getAllByTestId('mock-column-element').map(el => el.textContent), + ).toEqual(table.columns.map(col => col.name)); + fireEvent.click(getByText('Sort columns alphabetically')); + const sorted = [...table.columns.map(col => col.name)].sort(); + expect( + getAllByTestId('mock-column-element').map(el => el.textContent), + ).toEqual(sorted); + expect(getAllByTestId('mock-column-element')[0]).toHaveTextContent('active'); +}); + +test('removes the table', async () => { + const updateTableSchemaEndpoint = 'glob:*/tableschemaview/*'; + fetchMock.delete(updateTableSchemaEndpoint, {}); + const isFeatureEnabledMock = jest + .spyOn(featureFlags, 'isFeatureEnabled') + .mockImplementation( + featureFlag => featureFlag === FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, + ); + const { getAllByTestId, getByText } = render( + , + { + useRedux: true, + initialState, + }, + ); + await waitFor(() => + expect(getAllByTestId('mock-icon-tooltip')).toHaveLength(4), + ); + expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(0); + fireEvent.click(getByText('Remove table preview')); + await waitFor(() => + expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(1), + ); + isFeatureEnabledMock.mockClear(); +}); + +test('fetches table metadata when expanded', async () => { + render(, { + useRedux: true, + initialState, + }); + expect(fetchMock.calls(getTableMetadataEndpoint)).toHaveLength(0); + expect(fetchMock.calls(getExtraTableMetadataEndpoint)).toHaveLength(0); + await waitFor(() => + expect(fetchMock.calls(getTableMetadataEndpoint)).toHaveLength(1), + ); + expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(0); + expect(fetchMock.calls(getExtraTableMetadataEndpoint)).toHaveLength(1); +}); diff --git a/superset-frontend/src/SqlLab/components/TableElement/index.tsx b/superset-frontend/src/SqlLab/components/TableElement/index.tsx index 0f1140f9e87cd..147422b3c017b 100644 --- a/superset-frontend/src/SqlLab/components/TableElement/index.tsx +++ b/superset-frontend/src/SqlLab/components/TableElement/index.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useState, useRef } from 'react'; +import React, { useState, useRef, useEffect } from 'react'; import { useDispatch } from 'react-redux'; import Collapse from 'src/components/Collapse'; import Card from 'src/components/Card'; @@ -24,16 +24,26 @@ import ButtonGroup from 'src/components/ButtonGroup'; import { css, t, styled } from '@superset-ui/core'; import { debounce } from 'lodash'; -import { removeDataPreview, removeTables } from 'src/SqlLab/actions/sqlLab'; +import { + removeDataPreview, + removeTables, + addDangerToast, + syncTable, +} from 'src/SqlLab/actions/sqlLab'; +import { + useTableExtendedMetadataQuery, + useTableMetadataQuery, +} from 'src/hooks/apiResources'; import { Tooltip } from 'src/components/Tooltip'; import CopyToClipboard from 'src/components/CopyToClipboard'; import { IconTooltip } from 'src/components/IconTooltip'; import ModalTrigger from 'src/components/ModalTrigger'; import Loading from 'src/components/Loading'; +import useEffectEvent from 'src/hooks/useEffectEvent'; import ColumnElement, { ColumnKeyTypeType } from '../ColumnElement'; import ShowSQL from '../ShowSQL'; -interface Column { +export interface Column { name: string; keys?: { type: ColumnKeyTypeType }[]; type: string; @@ -41,18 +51,12 @@ interface Column { export interface Table { id: string; + dbId: number; + schema: string; name: string; - partitions?: { - partitionQuery: string; - latest: object[]; - }; - metadata?: Record; - indexes?: object[]; - selectStar?: string; - view?: string; - isMetadataLoading: boolean; - isExtraMetadataLoading: boolean; - columns: Column[]; + dataPreviewQueryId?: string | null; + expanded?: boolean; + initialized?: boolean; } export interface TableElementProps { @@ -106,7 +110,61 @@ const StyledCollapsePanel = styled(Collapse.Panel)` `; const TableElement = ({ table, ...props }: TableElementProps) => { + const { dbId, schema, name, expanded } = table; const dispatch = useDispatch(); + const { + data: tableMetadata, + isSuccess: isMetadataSuccess, + isLoading: isMetadataLoading, + isError: hasMetadataError, + } = useTableMetadataQuery( + { + dbId, + schema, + table: name, + }, + { skip: !expanded }, + ); + const { + data: tableExtendedMetadata, + isSuccess: isExtraMetadataSuccess, + isLoading: isExtraMetadataLoading, + isError: hasExtendedMetadataError, + } = useTableExtendedMetadataQuery( + { + dbId, + schema, + table: name, + }, + { skip: !expanded }, + ); + + useEffect(() => { + if (hasMetadataError || hasExtendedMetadataError) { + dispatch( + addDangerToast(t('An error occurred while fetching table metadata')), + ); + } + }, [hasMetadataError, hasExtendedMetadataError, dispatch]); + + const tableData = { + ...tableMetadata, + ...tableExtendedMetadata, + }; + + // TODO: migrate syncTable logic by SIP-93 + const syncTableMetadata = useEffectEvent(() => { + const { initialized } = table; + if (!initialized) { + dispatch(syncTable(table, tableData)); + } + }); + + useEffect(() => { + if (isMetadataSuccess && isExtraMetadataSuccess) { + syncTableMetadata(); + } + }, [isMetadataSuccess, isExtraMetadataSuccess, syncTableMetadata]); const [sortColumns, setSortColumns] = useState(false); const [hovered, setHovered] = useState(false); @@ -128,11 +186,11 @@ const TableElement = ({ table, ...props }: TableElementProps) => { const renderWell = () => { let partitions; let metadata; - if (table.partitions) { + if (tableData.partitions) { let partitionQuery; let partitionClipBoard; - if (table.partitions.partitionQuery) { - ({ partitionQuery } = table.partitions); + if (tableData.partitions.partitionQuery) { + ({ partitionQuery } = tableData.partitions); const tt = t('Copy partition query to clipboard'); partitionClipBoard = ( { /> ); } - const latest = Object.entries(table.partitions?.latest || []) + const latest = Object.entries(tableData.partitions?.latest || []) .map(([key, value]) => `${key}=${value}`) .join('/'); @@ -157,8 +215,8 @@ const TableElement = ({ table, ...props }: TableElementProps) => { ); } - if (table.metadata) { - metadata = Object.entries(table.metadata).map(([key, value]) => ( + if (tableData.metadata) { + metadata = Object.entries(tableData.metadata).map(([key, value]) => (
{key}: {value} @@ -183,17 +241,17 @@ const TableElement = ({ table, ...props }: TableElementProps) => { const renderControls = () => { let keyLink; const KEYS_FOR_TABLE_TEXT = t('Keys for table'); - if (table?.indexes?.length) { + if (tableData?.indexes?.length) { keyLink = ( ( + modalTitle={`${KEYS_FOR_TABLE_TEXT} ${name}`} + modalBody={tableData.indexes.map((ix, i) => ( 
{JSON.stringify(ix, null, '  ')}
))} triggerNode={ } /> @@ -214,7 +272,7 @@ const TableElement = ({ table, ...props }: TableElementProps) => { : t('Sort columns alphabetically') } /> - {table.selectStar && ( + {tableData.selectStar && ( { } - text={table.selectStar} + text={tableData.selectStar} shouldShowText={false} /> )} - {table.view && ( + {tableData.view && ( @@ -253,6 +311,7 @@ const TableElement = ({ table, ...props }: TableElementProps) => { return (
setHover(true)} onMouseLeave={() => setHover(false)} @@ -260,7 +319,7 @@ const TableElement = ({ table, ...props }: TableElementProps) => { { ref={tableNameRef} className="table-name" > - {table.name} + {name}
- {table.isMetadataLoading || table.isExtraMetadataLoading ? ( + {isMetadataLoading || isExtraMetadataLoading ? ( ) : ( { const renderBody = () => { let cols; - if (table.columns) { - cols = table.columns.slice(); + if (tableData.columns) { + cols = tableData.columns.slice(); if (sortColumns) { cols.sort((a: Column, b: Column) => { const colA = a.name.toUpperCase(); diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.js b/superset-frontend/src/SqlLab/reducers/getInitialState.js index 03d6ec55245cb..edb778fcfa9d6 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.js +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.js @@ -116,16 +116,7 @@ export default function getInitialState({ activeTab.table_schemas .filter(tableSchema => tableSchema.description !== null) .forEach(tableSchema => { - const { - columns, - selectStar, - primaryKey, - foreignKeys, - indexes, - dataPreviewQueryId, - partitions, - metadata, - } = tableSchema.description; + const { dataPreviewQueryId, ...persistData } = tableSchema.description; const table = { dbId: tableSchema.database_id, queryEditorId: tableSchema.tab_state_id.toString(), @@ -133,16 +124,9 @@ export default function getInitialState({ name: tableSchema.table, expanded: tableSchema.expanded, id: tableSchema.id, - isMetadataLoading: false, - isExtraMetadataLoading: false, dataPreviewQueryId, - columns, - selectStar, - primaryKey, - foreignKeys, - indexes, - partitions, - metadata, + persistData, + initialized: true, }; tables = { ...tables, @@ -184,16 +168,21 @@ export default function getInitialState({ }, }; }); - tables = sqlLab.tables.reduce( - (merged, table) => ({ + const expandedTables = new Set(); + tables = sqlLab.tables.reduce((merged, table) => { + const expanded = !expandedTables.has(table.queryEditorId); + if (expanded) { + expandedTables.add(table.queryEditorId); + } + return { ...merged, [table.id]: { ...tables[table.id], ...table, + expanded, }, - }), - tables, - ); + }; + }, tables); Object.values(sqlLab.queries).forEach(query => { queries[query.id] = { ...query, inLocalStorage: true }; }); diff --git a/superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js b/superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js index d03354f1666a0..2fb1da1783cad 100644 --- a/superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js +++ b/superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import pick from 'lodash/pick'; import { BYTES_PER_CHAR, KB_STORAGE, @@ -50,6 +51,21 @@ function shouldEmptyQueryResults(query) { ); } +export function emptyTablePersistData(tables) { + return tables + .map(table => + pick(table, [ + 'id', + 'name', + 'dbId', + 'schema', + 'dataPreviewQueryId', + 'queryEditorId', + ]), + ) + .filter(({ queryEditorId }) => Boolean(queryEditorId)); +} + export function emptyQueryResults(queries) { return Object.keys(queries).reduce((accu, key) => { const { results } = queries[key]; diff --git a/superset-frontend/src/hooks/apiResources/queryApi.ts b/superset-frontend/src/hooks/apiResources/queryApi.ts index 159e6095f1b9a..2881fc2252cb5 100644 --- a/superset-frontend/src/hooks/apiResources/queryApi.ts +++ b/superset-frontend/src/hooks/apiResources/queryApi.ts @@ -65,7 +65,13 @@ export const supersetClientQuery: BaseQueryFn< export const api = createApi({ reducerPath: 'queryApi', - tagTypes: ['Schemas', 'Tables', 'DatabaseFunctions', 'QueryValidations'], + tagTypes: [ + 'Schemas', + 'Tables', + 'DatabaseFunctions', + 'QueryValidations', + 'TableMetadatas', + ], endpoints: () => ({}), baseQuery: supersetClientQuery, }); diff --git a/superset-frontend/src/hooks/apiResources/tables.ts b/superset-frontend/src/hooks/apiResources/tables.ts index 5d28cba4880f8..2ac1fe566ad41 100644 --- a/superset-frontend/src/hooks/apiResources/tables.ts +++ b/superset-frontend/src/hooks/apiResources/tables.ts @@ -18,7 +18,7 @@ */ import { useCallback, useMemo, useEffect, useRef } from 'react'; import useEffectEvent from 'src/hooks/useEffectEvent'; -import { api } from './queryApi'; +import { api, JsonResponse } from './queryApi'; import { useSchemas } from './schemas'; @@ -56,6 +56,39 @@ export type FetchTablesQueryParams = { onError?: (error: Response) => void; }; +export type FetchTableMetadataQueryParams = { + dbId: string | number; + schema: string; + table: string; +}; + +type ColumnKeyTypeType = 'pk' | 'fk' | 'index'; +interface Column { + name: string; + keys?: { type: ColumnKeyTypeType }[]; + type: string; +} + +export type TableMetaData = { + name: string; + partitions?: { + partitionQuery: string; + latest: object[]; + }; + metadata?: Record; + indexes?: object[]; + selectStar?: string; + view?: string; + columns: Column[]; +}; + +type TableMetadataReponse = { + json: TableMetaData; + response: Response; +}; + +export type TableExtendedMetadata = Record; + type Params = Omit; const tableApi = api.injectEndpoints({ @@ -79,10 +112,34 @@ const tableApi = api.injectEndpoints({ schema, }), }), + tableMetadata: builder.query({ + query: ({ dbId, schema, table }) => ({ + endpoint: `/api/v1/database/${dbId}/table/${encodeURIComponent( + table, + )}/${encodeURIComponent(schema)}/`, + transformResponse: ({ json }: TableMetadataReponse) => json, + }), + }), + tableExtendedMetadata: builder.query< + TableExtendedMetadata, + FetchTableMetadataQueryParams + >({ + query: ({ dbId, schema, table }) => ({ + endpoint: `/api/v1/database/${dbId}/table_extra/${encodeURIComponent( + table, + )}/${encodeURIComponent(schema)}/`, + transformResponse: ({ json }: JsonResponse) => json, + }), + }), }), }); -export const { useLazyTablesQuery, useTablesQuery } = tableApi; +export const { + useLazyTablesQuery, + useTablesQuery, + useTableMetadataQuery, + useTableExtendedMetadataQuery, +} = tableApi; export function useTables(options: Params) { const isMountedRef = useRef(false);