From 577ac81686a9bf2074320e2eeb84bfa4d5562822 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Date: Thu, 19 Jan 2023 14:53:48 +0200 Subject: [PATCH] chore(ci): fix numpy type errors and revert #22610 (#22782) --- setup.cfg | 4 +- .../DatasourcePanel/DatasourcePanel.test.tsx | 21 +++ .../ColumnSelectPopover.tsx | 4 +- .../src/pages/ChartCreation/index.tsx | 5 +- .../views/CRUD/data/database/DatabaseList.tsx | 4 - .../database/DatabaseModal/index.test.tsx | 8 - .../data/database/DatabaseModal/index.tsx | 6 +- .../dataset/AddDataset/AddDataset.test.tsx | 8 - .../AddDataset/DatasetPanel/fixtures.ts | 1 - .../dataset/AddDataset/DatasetPanel/index.tsx | 17 +- .../dataset/AddDataset/Footer/Footer.test.tsx | 8 - .../data/dataset/AddDataset/Footer/index.tsx | 14 +- .../AddDataset/LeftPanel/LeftPanel.test.tsx | 22 ++- .../dataset/AddDataset/LeftPanel/index.tsx | 106 ++++------- .../CRUD/data/dataset/AddDataset/index.tsx | 30 ++- .../CRUD/data/dataset/AddDataset/types.tsx | 8 +- .../CRUD/data/dataset/AddDatasetModal.tsx | 172 ++++++++++++++++++ .../DatasetLayout/DatasetLayout.test.tsx | 8 - .../views/CRUD/data/dataset/DatasetList.tsx | 45 ++++- .../src/views/CRUD/data/hooks.ts | 10 +- .../src/views/components/RightMenu.test.tsx | 3 + .../src/views/components/RightMenu.tsx | 17 +- superset/db_engine_specs/hive.py | 4 +- superset/reports/commands/alert.py | 10 +- superset/result_set.py | 15 +- .../utils/pandas_postprocessing/boxplot.py | 10 +- .../utils/pandas_postprocessing/flatten.py | 2 +- superset/utils/pandas_postprocessing/utils.py | 2 +- superset/views/datasource/views.py | 5 +- 29 files changed, 373 insertions(+), 196 deletions(-) create mode 100644 superset-frontend/src/views/CRUD/data/dataset/AddDatasetModal.tsx diff --git a/setup.cfg b/setup.cfg index c4cd568c51..a9470d51bd 100644 --- a/setup.cfg +++ b/setup.cfg @@ -17,9 +17,9 @@ [metadata] name = Superset summary = a data exploration platform -description-file = README.md +description_file = README.md author = Apache Superset Dev -author-email = dev@superset.apache.org +author_email = dev@superset.apache.org license = Apache License, Version 2.0 [files] diff --git a/superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx b/superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx index 95258f443e..eedc2fb101 100644 --- a/superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx +++ b/superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx @@ -186,6 +186,27 @@ test('should render a create dataset infobox', async () => { expect(infoboxText).toBeVisible(); }); +test('should render a save dataset modal when "Create a dataset" is clicked', async () => { + const newProps = { + ...props, + datasource: { + ...datasource, + type: DatasourceType.Query, + }, + }; + render(, { useRedux: true, useDnd: true }); + + const createButton = await screen.findByRole('button', { + name: /create a dataset/i, + }); + + userEvent.click(createButton); + + const saveDatasetModalTitle = screen.getByText(/save or overwrite dataset/i); + + expect(saveDatasetModalTitle).toBeVisible(); +}); + test('should not render a save dataset modal when datasource is not query or dataset', async () => { const newProps = { ...props, diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx index dbbc8fe948..fee2eb941f 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx @@ -231,9 +231,7 @@ const ColumnSelectPopover = ({ }, []); const setDatasetAndClose = () => { - if (setDatasetModal) { - setDatasetModal(true); - } + if (setDatasetModal) setDatasetModal(true); onClose(); }; diff --git a/superset-frontend/src/pages/ChartCreation/index.tsx b/superset-frontend/src/pages/ChartCreation/index.tsx index bea530fd3a..4aeb7aeed4 100644 --- a/superset-frontend/src/pages/ChartCreation/index.tsx +++ b/superset-frontend/src/pages/ChartCreation/index.tsx @@ -337,7 +337,10 @@ export class ChartCreation extends React.PureComponent< const isButtonDisabled = this.isBtnDisabled(); const datasetHelpText = this.state.canCreateDataset ? ( - + {t('Add a dataset')} {` ${t('or')} `} diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx index 0e3642493a..744edb51b1 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx @@ -21,7 +21,6 @@ import React, { useState, useMemo, useEffect } from 'react'; import rison from 'rison'; import { useSelector } from 'react-redux'; import { useQueryParams, BooleanParam } from 'use-query-params'; -import { LocalStorageKeys, setItem } from 'src/utils/localStorageHelpers'; import Loading from 'src/components/Loading'; import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; @@ -158,9 +157,6 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) { refreshData(); addSuccessToast(t('Deleted: %s', dbName)); - // Delete user-selected db from local storage - setItem(LocalStorageKeys.db, null); - // Close delete modal setDatabaseCurrentlyDeleting(null); }, diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.tsx index 9542450f44..ebc20eb388 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.tsx @@ -43,14 +43,6 @@ jest.mock('@superset-ui/core', () => ({ isFeatureEnabled: () => true, })); -const mockHistoryPush = jest.fn(); -jest.mock('react-router-dom', () => ({ - ...jest.requireActual('react-router-dom'), - useHistory: () => ({ - push: mockHistoryPush, - }), -})); - const dbProps = { show: true, database_name: 'my database', diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index 5f85ae0985..603f449fec 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -31,7 +31,6 @@ import React, { useReducer, Reducer, } from 'react'; -import { useHistory } from 'react-router-dom'; import { setItem, LocalStorageKeys } from 'src/utils/localStorageHelpers'; import { UploadChangeParam, UploadFile } from 'antd/lib/upload/interface'; import Tabs from 'src/components/Tabs'; @@ -519,7 +518,6 @@ const DatabaseModal: FunctionComponent = ({ t('database'), addDangerToast, ); - const history = useHistory(); const [tabKey, setTabKey] = useState(DEFAULT_TAB_KEY); const [availableDbs, getAvailableDbs] = useAvailableDatabases(); @@ -1297,7 +1295,7 @@ const DatabaseModal: FunctionComponent = ({ onClick={() => { setLoading(true); fetchAndSetDB(); - history.push('/dataset/add/'); + window.location.href = '/tablemodelview/list#create'; }} > {t('CREATE DATASET')} @@ -1308,7 +1306,7 @@ const DatabaseModal: FunctionComponent = ({ onClick={() => { setLoading(true); fetchAndSetDB(); - history.push(`/superset/sqllab/?db=true`); + window.location.href = `/superset/sqllab/?db=true`; }} > {t('QUERY DATA IN SQL LAB')} diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/AddDataset.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/AddDataset.test.tsx index 7e5a7de12a..39fb2b295b 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/AddDataset.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/AddDataset.test.tsx @@ -20,14 +20,6 @@ import React from 'react'; import { render, screen } from 'spec/helpers/testing-library'; import AddDataset from 'src/views/CRUD/data/dataset/AddDataset'; -const mockHistoryPush = jest.fn(); -jest.mock('react-router-dom', () => ({ - ...jest.requireActual('react-router-dom'), - useHistory: () => ({ - push: mockHistoryPush, - }), -})); - describe('AddDataset', () => { it('renders a blank state AddDataset', async () => { render(, { useRedux: true }); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/fixtures.ts b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/fixtures.ts index 5c09188c61..d3ee58da14 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/fixtures.ts +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/fixtures.ts @@ -40,7 +40,6 @@ export const exampleDataset: DatasetObject[] = [ id: 1, database_name: 'test_database', owners: [1], - backend: 'test_backend', }, schema: 'test_schema', dataset_name: 'example_dataset', diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/index.tsx index 73bea70b41..15e6225e56 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/index.tsx @@ -17,9 +17,8 @@ * under the License. */ import React, { useEffect, useState, useRef } from 'react'; -import { SupersetClient, logging, t } from '@superset-ui/core'; +import { SupersetClient } from '@superset-ui/core'; import { DatasetObject } from 'src/views/CRUD/data/dataset/AddDataset/types'; -import { addDangerToast } from 'src/components/MessageToasts/actions'; import DatasetPanel from './DatasetPanel'; import { ITableColumn, IDatabaseTable, isIDatabaseTable } from './types'; @@ -95,17 +94,9 @@ const DatasetPanelWrapper = ({ setColumnList([]); setHasColumns?.(false); setHasError(true); - addDangerToast( - t( - 'The API response from %s does not match the IDatabaseTable interface.', - path, - ), - ); - logging.error( - t( - 'The API response from %s does not match the IDatabaseTable interface.', - path, - ), + // eslint-disable-next-line no-console + console.error( + `The API response from ${path} does not match the IDatabaseTable interface.`, ); } } catch (error) { diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/Footer.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/Footer.test.tsx index a1818cdf22..8fcbd83b15 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/Footer.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/Footer.test.tsx @@ -20,14 +20,6 @@ import React from 'react'; import { render, screen } from 'spec/helpers/testing-library'; import Footer from 'src/views/CRUD/data/dataset/AddDataset/Footer'; -const mockHistoryPush = jest.fn(); -jest.mock('react-router-dom', () => ({ - ...jest.requireActual('react-router-dom'), - useHistory: () => ({ - push: mockHistoryPush, - }), -})); - const mockedProps = { url: 'realwebsite.com', }; diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx index 7f6717857f..5aecec0bb8 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx @@ -17,7 +17,6 @@ * under the License. */ import React from 'react'; -import { useHistory } from 'react-router-dom'; import Button from 'src/components/Button'; import { t } from '@superset-ui/core'; import { useSingleViewResource } from 'src/views/CRUD/hooks'; @@ -50,12 +49,12 @@ const LOG_ACTIONS = [ ]; function Footer({ + url, datasetObject, addDangerToast, hasColumns = false, datasets, }: FooterProps) { - const history = useHistory(); const { createResource } = useSingleViewResource>( 'dataset', t('dataset'), @@ -73,6 +72,11 @@ function Footer({ return LOG_ACTIONS[value]; }; + const goToPreviousUrl = () => { + // this is a placeholder url until the final feature gets implemented + // at that point we will be passing in the url of the previous location. + window.location.href = url; + }; const cancelButtonOnClick = () => { if (!datasetObject) { @@ -81,7 +85,7 @@ function Footer({ const logAction = createLogAction(datasetObject); logEvent(logAction, datasetObject); } - history.goBack(); + goToPreviousUrl(); }; const tooltipText = t('Select a database table.'); @@ -100,13 +104,13 @@ function Footer({ if (typeof response === 'number') { logEvent(LOG_ACTIONS_DATASET_CREATION_SUCCESS, datasetObject); // When a dataset is created the response we get is its ID number - history.push(`/chart/add/?dataset=${datasetObject.table_name}`); + goToPreviousUrl(); } }); } }; - const CREATE_DATASET_TEXT = t('Create Dataset and Create Chart'); + const CREATE_DATASET_TEXT = t('Create Dataset'); const disabledCheck = !datasetObject?.table_name || !hasColumns || diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/LeftPanel.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/LeftPanel.test.tsx index bc8c1d03de..7457f0c250 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/LeftPanel.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/LeftPanel.test.tsx @@ -21,7 +21,6 @@ import fetchMock from 'fetch-mock'; import userEvent from '@testing-library/user-event'; import { render, screen, waitFor } from 'spec/helpers/testing-library'; import LeftPanel from 'src/views/CRUD/data/dataset/AddDataset/LeftPanel'; -import { exampleDataset } from 'src/views/CRUD/data/dataset/AddDataset/DatasetPanel/fixtures'; const databasesEndpoint = 'glob:*/api/v1/database/?q*'; const schemasEndpoint = 'glob:*/api/v1/database/*/schemas*'; @@ -182,7 +181,7 @@ test('does not render blank state if there is nothing selected', async () => { }); test('renders list of options when user clicks on schema', async () => { - render(, { + render(, { useRedux: true, }); @@ -190,21 +189,23 @@ test('renders list of options when user clicks on schema', async () => { const databaseSelect = screen.getByRole('combobox', { name: 'Select database or type database name', }); - userEvent.click(databaseSelect); - expect(await screen.findByText('test-postgres')).toBeInTheDocument(); - userEvent.click(screen.getByText('test-postgres')); - - // Schema select will be automatically populated if there is only one schema + // Schema select should be disabled until database is selected const schemaSelect = screen.getByRole('combobox', { name: /select schema or type schema name/i, }); + userEvent.click(databaseSelect); + expect(await screen.findByText('test-postgres')).toBeInTheDocument(); + expect(schemaSelect).toBeDisabled(); + userEvent.click(screen.getByText('test-postgres')); + + // Wait for schema field to be enabled await waitFor(() => { expect(schemaSelect).toBeEnabled(); }); }); test('searches for a table name', async () => { - render(, { + render(, { useRedux: true, }); @@ -244,8 +245,9 @@ test('renders a warning icon when a table name has a pre-existing dataset', asyn render( , { useRedux: true, diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx index 14dd7dca4b..4f7dfca196 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx @@ -16,13 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React, { - useEffect, - useState, - SetStateAction, - Dispatch, - useCallback, -} from 'react'; +import React, { useEffect, useState, SetStateAction, Dispatch } from 'react'; import { SupersetClient, t, @@ -46,16 +40,13 @@ import { emptyStateComponent, } from 'src/components/EmptyState'; import { useToasts } from 'src/components/MessageToasts/withToasts'; -import { LocalStorageKeys, getItem } from 'src/utils/localStorageHelpers'; -import { - DatasetActionType, - DatasetObject, -} from 'src/views/CRUD/data/dataset/AddDataset/types'; +import { DatasetActionType } from '../types'; interface LeftPanelProps { setDataset: Dispatch>; - dataset?: Partial | null; - datasetNames?: (string | null | undefined)[] | undefined; + schema?: string | null | undefined; + dbId?: number; + datasets?: (string | null | undefined)[] | undefined; } const SearchIcon = styled(Icons.Search)` @@ -154,8 +145,9 @@ const LeftPanelStyle = styled.div` export default function LeftPanel({ setDataset, - dataset, - datasetNames, + schema, + dbId, + datasets, }: LeftPanelProps) { const theme = useTheme(); @@ -168,14 +160,11 @@ export default function LeftPanel({ const { addDangerToast } = useToasts(); - const setDatabase = useCallback( - (db: Partial) => { - setDataset({ type: DatasetActionType.selectDatabase, payload: { db } }); - setSelectedTable(null); - setResetTables(true); - }, - [setDataset], - ); + const setDatabase = (db: Partial) => { + setDataset({ type: DatasetActionType.selectDatabase, payload: { db } }); + setSelectedTable(null); + setResetTables(true); + }; const setTable = (tableName: string, index: number) => { setSelectedTable(index); @@ -185,32 +174,28 @@ export default function LeftPanel({ }); }; - const getTablesList = useCallback( - (url: string) => { - SupersetClient.get({ url }) - .then(({ json }) => { - const options: TableOption[] = json.options.map((table: Table) => { - const option: TableOption = { - value: table.value, - label: , - text: table.label, - }; + const getTablesList = (url: string) => { + SupersetClient.get({ url }) + .then(({ json }) => { + const options: TableOption[] = json.options.map((table: Table) => { + const option: TableOption = { + value: table.value, + label: , + text: table.label, + }; - return option; - }); - - setTableOptions(options); - setLoadTables(false); - setResetTables(false); - setRefresh(false); - }) - .catch(error => { - addDangerToast(t('There was an error fetching tables')); - logging.error(t('There was an error fetching tables'), error); + return option; }); - }, - [addDangerToast], - ); + + setTableOptions(options); + setLoadTables(false); + setResetTables(false); + setRefresh(false); + }) + .catch(error => + logging.error('There was an error fetching tables', error), + ); + }; const setSchema = (schema: string) => { if (schema) { @@ -224,28 +209,16 @@ export default function LeftPanel({ setResetTables(true); }; - const encodedSchema = dataset?.schema - ? encodeURIComponent(dataset?.schema) - : undefined; - - useEffect(() => { - const currentUserSelectedDb = getItem( - LocalStorageKeys.db, - null, - ) as DatabaseObject; - if (currentUserSelectedDb) { - setDatabase(currentUserSelectedDb); - } - }, [setDatabase]); + const encodedSchema = schema ? encodeURIComponent(schema) : undefined; useEffect(() => { if (loadTables) { const endpoint = encodeURI( - `/superset/tables/${dataset?.db?.id}/${encodedSchema}/${refresh}/`, + `/superset/tables/${dbId}/${encodedSchema}/${refresh}/`, ); getTablesList(endpoint); } - }, [loadTables, dataset?.db?.id, encodedSchema, getTablesList, refresh]); + }, [loadTables]); useEffect(() => { if (resetTables) { @@ -289,7 +262,6 @@ export default function LeftPanel({ {SELECT_DATABASE_AND_SCHEMA_TEXT}

{loadTables && !refresh && Loader(TABLE_LOADING_TEXT)} - {dataset?.schema && !loadTables && !tableOptions.length && !searchVal && ( + {schema && !loadTables && !tableOptions.length && !searchVal && (
)} - {dataset?.schema && (tableOptions.length > 0 || searchVal.length > 0) && ( + {schema && (tableOptions.length > 0 || searchVal.length > 0) && ( <>

{SELECT_DATABASE_TABLE_TEXT}

@@ -351,7 +323,7 @@ export default function LeftPanel({ onClick={() => setTable(option.value, i)} > {option.label} - {datasetNames?.includes(option.value) && ( + {datasets?.includes(option.value) && ( { + const getDatasetsList = async () => { await UseGetDatasetsList(queryParams) .then(json => { setDatasets(json?.result); }) - .catch(error => { - addDangerToast(t('There was an error fetching dataset')); - logging.error(t('There was an error fetching dataset'), error); - }); - }, [queryParams]); + .catch(error => + logging.error('There was an error fetching dataset', error), + ); + }; useEffect(() => { if (dataset?.schema) { getDatasetsList(); } - }, [dataset?.schema, getDatasetsList]); + }, [dataset?.schema]); const HeaderComponent = () => (
@@ -123,8 +114,9 @@ export default function AddDataset() { const LeftPanelComponent = () => ( ); diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/types.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/types.tsx index 89473e5426..dbeef93ead 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/types.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/types.tsx @@ -16,8 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -import { DatabaseObject } from 'src/components/DatabaseSelector'; - export enum DatasetActionType { selectDatabase, selectSchema, @@ -26,7 +24,11 @@ export enum DatasetActionType { } export interface DatasetObject { - db: DatabaseObject & { owners: [number] }; + db: { + id: number; + database_name?: string; + owners?: number[]; + }; schema?: string | null; dataset_name: string; table_name?: string | null; diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDatasetModal.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDatasetModal.tsx new file mode 100644 index 0000000000..d4b1470f38 --- /dev/null +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDatasetModal.tsx @@ -0,0 +1,172 @@ +/** + * 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, { FunctionComponent, useState, useEffect } from 'react'; +import { styled, t } from '@superset-ui/core'; +import { useSingleViewResource } from 'src/views/CRUD/hooks'; +import Modal from 'src/components/Modal'; +import TableSelector from 'src/components/TableSelector'; +import withToasts from 'src/components/MessageToasts/withToasts'; +import { DatabaseObject } from 'src/components/DatabaseSelector'; +import { + getItem, + LocalStorageKeys, + setItem, +} from 'src/utils/localStorageHelpers'; +import { isEmpty } from 'lodash'; + +type DatasetAddObject = { + id: number; + database: number; + schema: string; + table_name: string; +}; +interface DatasetModalProps { + addDangerToast: (msg: string) => void; + addSuccessToast: (msg: string) => void; + onDatasetAdd?: (dataset: DatasetAddObject) => void; + onHide: () => void; + show: boolean; + history?: any; // So we can render the modal when not using SPA +} + +const TableSelectorContainer = styled.div` + padding-bottom: 340px; + width: 65%; +`; + +const DatasetModal: FunctionComponent = ({ + addDangerToast, + onDatasetAdd, + onHide, + show, + history, +}) => { + const [currentDatabase, setCurrentDatabase] = useState< + DatabaseObject | undefined + >(); + const [currentSchema, setSchema] = useState(''); + const [currentTableName, setTableName] = useState(''); + const [disableSave, setDisableSave] = useState(true); + const { + createResource, + state: { loading }, + } = useSingleViewResource>( + 'dataset', + t('dataset'), + addDangerToast, + ); + + useEffect(() => { + setDisableSave(currentDatabase === undefined || currentTableName === ''); + }, [currentTableName, currentDatabase]); + + useEffect(() => { + const currentUserSelectedDb = getItem( + LocalStorageKeys.db, + null, + ) as DatabaseObject; + if (currentUserSelectedDb) setCurrentDatabase(currentUserSelectedDb); + }, []); + + const onDbChange = (db: DatabaseObject) => { + setCurrentDatabase(db); + }; + + const onSchemaChange = (schema?: string) => { + setSchema(schema); + }; + + const onTableChange = (tableName: string) => { + setTableName(tableName); + }; + + const clearModal = () => { + setSchema(''); + setTableName(''); + setCurrentDatabase(undefined); + setDisableSave(true); + }; + + const cleanup = () => { + clearModal(); + setItem(LocalStorageKeys.db, null); + }; + + const hide = () => { + cleanup(); + onHide(); + }; + + const onSave = () => { + if (currentDatabase === undefined) { + return; + } + const data = { + database: currentDatabase.id, + ...(currentSchema ? { schema: currentSchema } : {}), + table_name: currentTableName, + }; + createResource(data).then(response => { + if (!response) { + return; + } + if (onDatasetAdd) { + onDatasetAdd({ id: response.id, ...response }); + } + // We need to be able to work with no SPA routes opening the modal + // So useHistory wont be available always thus we check for it + if (!isEmpty(history)) { + history?.push(`/chart/add?dataset=${currentTableName}`); + cleanup(); + } else { + window.location.href = `/chart/add?dataset=${currentTableName}`; + cleanup(); + onHide(); + } + }); + }; + + return ( + + + + + + ); +}; + +export default withToasts(DatasetModal); diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/DatasetLayout.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/DatasetLayout.test.tsx index a1939761aa..35375a52b4 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/DatasetLayout.test.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/DatasetLayout.test.tsx @@ -25,14 +25,6 @@ import DatasetPanel from 'src/views/CRUD/data/dataset/AddDataset/DatasetPanel'; import RightPanel from 'src/views/CRUD/data/dataset/AddDataset/RightPanel'; import Footer from 'src/views/CRUD/data/dataset/AddDataset/Footer'; -const mockHistoryPush = jest.fn(); -jest.mock('react-router-dom', () => ({ - ...jest.requireActual('react-router-dom'), - useHistory: () => ({ - push: mockHistoryPush, - }), -})); - describe('DatasetLayout', () => { it('renders nothing when no components are passed in', () => { render(); diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx index 0906776506..68c65a7ad1 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -22,14 +22,16 @@ import React, { useState, useMemo, useCallback, + useEffect, } from 'react'; -import { useHistory } from 'react-router-dom'; import rison from 'rison'; +import { useHistory, useLocation } from 'react-router-dom'; import { createFetchRelated, createFetchDistinct, createErrorHandler, } from 'src/views/CRUD/utils'; +import { getItem, LocalStorageKeys } from 'src/utils/localStorageHelpers'; import { ColumnObject } from 'src/views/CRUD/data/dataset/types'; import { useListViewResource } from 'src/views/CRUD/hooks'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; @@ -58,6 +60,7 @@ import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip'; import { isUserAdmin } from 'src/dashboard/util/permissionUtils'; import { GenericLink } from 'src/components/GenericLink/GenericLink'; +import AddDatasetModal from './AddDatasetModal'; import { PAGE_SIZE, @@ -136,7 +139,6 @@ const DatasetList: FunctionComponent = ({ addSuccessToast, user, }) => { - const history = useHistory(); const { state: { loading, @@ -150,6 +152,9 @@ const DatasetList: FunctionComponent = ({ refreshData, } = useListViewResource('dataset', t('dataset'), addDangerToast); + const [datasetAddModalOpen, setDatasetAddModalOpen] = + useState(false); + const [datasetCurrentlyDeleting, setDatasetCurrentlyDeleting] = useState< (Dataset & { chart_count: number; dashboard_count: number }) | null >(null); @@ -186,6 +191,12 @@ const DatasetList: FunctionComponent = ({ hasPerm('can_export') && isFeatureEnabled(FeatureFlag.VERSIONED_EXPORT); const initialSort = SORT_BY; + useEffect(() => { + const db = getItem(LocalStorageKeys.db, null); + if (!loading && db) { + setDatasetAddModalOpen(true); + } + }, [loading]); const openDatasetEditModal = useCallback( ({ id }: Dataset) => { @@ -592,6 +603,26 @@ const DatasetList: FunctionComponent = ({ }); } + const CREATE_HASH = '#create'; + const location = useLocation(); + const history = useHistory(); + + // Sync Dataset Add modal with #create hash + useEffect(() => { + const modalOpen = location.hash === CREATE_HASH && canCreate; + setDatasetAddModalOpen(modalOpen); + }, [canCreate, location.hash]); + + // Add #create hash + const openDatasetAddModal = useCallback(() => { + history.replace(`${location.pathname}${location.search}${CREATE_HASH}`); + }, [history, location.pathname, location.search]); + + // Remove #create hash + const closeDatasetAddModal = useCallback(() => { + history.replace(`${location.pathname}${location.search}`); + }, [history, location.pathname, location.search]); + if (canCreate) { buttonArr.push({ name: ( @@ -599,9 +630,7 @@ const DatasetList: FunctionComponent = ({ {t('Dataset')}{' '} ), - onClick: () => { - history.push('/dataset/add/'); - }, + onClick: openDatasetAddModal, buttonStyle: 'primary', }); @@ -698,6 +727,12 @@ const DatasetList: FunctionComponent = ({ return ( <> + {datasetCurrentlyDeleting && ( endpoint: `/api/v1/dataset/?q=${queryParams}`, }) .then(({ json }) => json) - .catch(error => { - addDangerToast(t('There was an error fetching dataset')); - logging.error(t('There was an error fetching dataset'), error); - }); + .catch(error => + logging.error('There was an error fetching dataset', error), + ); diff --git a/superset-frontend/src/views/components/RightMenu.test.tsx b/superset-frontend/src/views/components/RightMenu.test.tsx index 907c305ff6..ec12e644db 100644 --- a/superset-frontend/src/views/components/RightMenu.test.tsx +++ b/superset-frontend/src/views/components/RightMenu.test.tsx @@ -30,6 +30,9 @@ jest.mock('react-redux', () => ({ })); jest.mock('src/views/CRUD/data/database/DatabaseModal', () => () => ); +jest.mock('src/views/CRUD/data/dataset/AddDatasetModal.tsx', () => () => ( + +)); const dropdownItems = [ { diff --git a/superset-frontend/src/views/components/RightMenu.tsx b/superset-frontend/src/views/components/RightMenu.tsx index 1957060502..59e7f7f448 100644 --- a/superset-frontend/src/views/components/RightMenu.tsx +++ b/superset-frontend/src/views/components/RightMenu.tsx @@ -51,6 +51,7 @@ import { GlobalMenuDataOptions, RightMenuProps, } from './types'; +import AddDatasetModal from '../CRUD/data/dataset/AddDatasetModal'; const extensionsRegistry = getExtensionsRegistry(); @@ -142,6 +143,7 @@ const RightMenu = ({ HAS_GSHEETS_INSTALLED, } = useSelector(state => state.common.conf); const [showDatabaseModal, setShowDatabaseModal] = useState(false); + const [showDatasetModal, setShowDatasetModal] = useState(false); const [engine, setEngine] = useState(''); const canSql = findPermission('can_sqllab', 'Superset', roles); const canDashboard = findPermission('can_write', 'Dashboard', roles); @@ -177,7 +179,6 @@ const RightMenu = ({ { label: t('Create dataset'), name: GlobalMenuDataOptions.DATASET_CREATION, - url: '/dataset/add/', perm: canDataset && nonExamplesDBConnected, }, { @@ -285,6 +286,8 @@ const RightMenu = ({ } else if (itemChose.key === GlobalMenuDataOptions.GOOGLE_SHEETS) { setShowDatabaseModal(true); setEngine('Google Sheets'); + } else if (itemChose.key === GlobalMenuDataOptions.DATASET_CREATION) { + setShowDatasetModal(true); } }; @@ -293,6 +296,10 @@ const RightMenu = ({ setShowDatabaseModal(false); }; + const handleOnHideDatasetModalModal = () => { + setShowDatasetModal(false); + }; + const isDisabled = isAdmin && !allowUploads; const tooltipText = t( @@ -337,6 +344,7 @@ const RightMenu = ({ ); const handleDatabaseAdd = () => setQuery({ databaseAdded: true }); + const handleDatasetAdd = () => setQuery({ datasetAdded: true }); const theme = useTheme(); @@ -350,6 +358,13 @@ const RightMenu = ({ onDatabaseAdd={handleDatabaseAdd} /> )} + {canDataset && ( + + )} {environmentTag?.text && (