diff --git a/superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx b/superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx index eedc2fb10117e..95258f443ec45 100644 --- a/superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx +++ b/superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx @@ -186,27 +186,6 @@ 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 fee2eb941ff60..dbbc8fe948432 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx @@ -231,7 +231,9 @@ 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 4aeb7aeed4df8..bea530fd3ad5e 100644 --- a/superset-frontend/src/pages/ChartCreation/index.tsx +++ b/superset-frontend/src/pages/ChartCreation/index.tsx @@ -337,10 +337,7 @@ 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 744edb51b18e6..0e3642493ad07 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx @@ -21,6 +21,7 @@ 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'; @@ -157,6 +158,9 @@ 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 96bcace64761d..8457d8d1747d4 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,6 +43,14 @@ 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 fae25219cf87c..39f6c158748be 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -31,6 +31,7 @@ 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'; @@ -518,6 +519,7 @@ const DatabaseModal: FunctionComponent = ({ t('database'), addDangerToast, ); + const history = useHistory(); const [tabKey, setTabKey] = useState(DEFAULT_TAB_KEY); const [availableDbs, getAvailableDbs] = useAvailableDatabases(); @@ -1295,7 +1297,7 @@ const DatabaseModal: FunctionComponent = ({ onClick={() => { setLoading(true); fetchAndSetDB(); - window.location.href = '/tablemodelview/list#create'; + history.push('/dataset/add/'); }} > {t('CREATE DATASET')} @@ -1306,7 +1308,7 @@ const DatabaseModal: FunctionComponent = ({ onClick={() => { setLoading(true); fetchAndSetDB(); - window.location.href = `/superset/sqllab/?db=true`; + history.push(`/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 39fb2b295b302..7e5a7de12ab3a 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,6 +20,14 @@ 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 d3ee58da14b9d..5c09188c61a87 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,6 +40,7 @@ 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 15e6225e5612a..73bea70b41f2f 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,8 +17,9 @@ * under the License. */ import React, { useEffect, useState, useRef } from 'react'; -import { SupersetClient } from '@superset-ui/core'; +import { SupersetClient, logging, t } 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'; @@ -94,9 +95,17 @@ const DatasetPanelWrapper = ({ setColumnList([]); setHasColumns?.(false); setHasError(true); - // eslint-disable-next-line no-console - console.error( - `The API response from ${path} does not match the IDatabaseTable interface.`, + 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, + ), ); } } 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 8fcbd83b15cc4..a1818cdf2291f 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,6 +20,14 @@ 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 5aecec0bb8920..7f6717857f569 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,6 +17,7 @@ * 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'; @@ -49,12 +50,12 @@ const LOG_ACTIONS = [ ]; function Footer({ - url, datasetObject, addDangerToast, hasColumns = false, datasets, }: FooterProps) { + const history = useHistory(); const { createResource } = useSingleViewResource>( 'dataset', t('dataset'), @@ -72,11 +73,6 @@ 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) { @@ -85,7 +81,7 @@ function Footer({ const logAction = createLogAction(datasetObject); logEvent(logAction, datasetObject); } - goToPreviousUrl(); + history.goBack(); }; const tooltipText = t('Select a database table.'); @@ -104,13 +100,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 - goToPreviousUrl(); + history.push(`/chart/add/?dataset=${datasetObject.table_name}`); } }); } }; - const CREATE_DATASET_TEXT = t('Create Dataset'); + const CREATE_DATASET_TEXT = t('Create Dataset and Create Chart'); 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 7457f0c25095c..bc8c1d03debef 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,6 +21,7 @@ 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*'; @@ -181,7 +182,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, }); @@ -189,23 +190,21 @@ test('renders list of options when user clicks on schema', async () => { const databaseSelect = screen.getByRole('combobox', { name: 'Select database or type database name', }); - // 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 + // Schema select will be automatically populated if there is only one schema + const schemaSelect = screen.getByRole('combobox', { + name: /select schema or type schema name/i, + }); await waitFor(() => { expect(schemaSelect).toBeEnabled(); }); }); test('searches for a table name', async () => { - render(, { + render(, { useRedux: true, }); @@ -245,9 +244,8 @@ 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 4f7dfca196f4f..14dd7dca4b842 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,7 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useEffect, useState, SetStateAction, Dispatch } from 'react'; +import React, { + useEffect, + useState, + SetStateAction, + Dispatch, + useCallback, +} from 'react'; import { SupersetClient, t, @@ -40,13 +46,16 @@ import { emptyStateComponent, } from 'src/components/EmptyState'; import { useToasts } from 'src/components/MessageToasts/withToasts'; -import { DatasetActionType } from '../types'; +import { LocalStorageKeys, getItem } from 'src/utils/localStorageHelpers'; +import { + DatasetActionType, + DatasetObject, +} from 'src/views/CRUD/data/dataset/AddDataset/types'; interface LeftPanelProps { setDataset: Dispatch>; - schema?: string | null | undefined; - dbId?: number; - datasets?: (string | null | undefined)[] | undefined; + dataset?: Partial | null; + datasetNames?: (string | null | undefined)[] | undefined; } const SearchIcon = styled(Icons.Search)` @@ -145,9 +154,8 @@ const LeftPanelStyle = styled.div` export default function LeftPanel({ setDataset, - schema, - dbId, - datasets, + dataset, + datasetNames, }: LeftPanelProps) { const theme = useTheme(); @@ -160,11 +168,14 @@ export default function LeftPanel({ const { addDangerToast } = useToasts(); - const setDatabase = (db: Partial) => { - setDataset({ type: DatasetActionType.selectDatabase, payload: { db } }); - setSelectedTable(null); - setResetTables(true); - }; + const setDatabase = useCallback( + (db: Partial) => { + setDataset({ type: DatasetActionType.selectDatabase, payload: { db } }); + setSelectedTable(null); + setResetTables(true); + }, + [setDataset], + ); const setTable = (tableName: string, index: number) => { setSelectedTable(index); @@ -174,28 +185,32 @@ export default function LeftPanel({ }); }; - 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, - }; + 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, + }; - return option; - }); + return option; + }); - setTableOptions(options); - setLoadTables(false); - setResetTables(false); - setRefresh(false); - }) - .catch(error => - logging.error('There was an error fetching tables', error), - ); - }; + 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); + }); + }, + [addDangerToast], + ); const setSchema = (schema: string) => { if (schema) { @@ -209,16 +224,28 @@ export default function LeftPanel({ setResetTables(true); }; - const encodedSchema = schema ? encodeURIComponent(schema) : undefined; + const encodedSchema = dataset?.schema + ? encodeURIComponent(dataset?.schema) + : undefined; + + useEffect(() => { + const currentUserSelectedDb = getItem( + LocalStorageKeys.db, + null, + ) as DatabaseObject; + if (currentUserSelectedDb) { + setDatabase(currentUserSelectedDb); + } + }, [setDatabase]); useEffect(() => { if (loadTables) { const endpoint = encodeURI( - `/superset/tables/${dbId}/${encodedSchema}/${refresh}/`, + `/superset/tables/${dataset?.db?.id}/${encodedSchema}/${refresh}/`, ); getTablesList(endpoint); } - }, [loadTables]); + }, [loadTables, dataset?.db?.id, encodedSchema, getTablesList, refresh]); useEffect(() => { if (resetTables) { @@ -262,6 +289,7 @@ export default function LeftPanel({ {SELECT_DATABASE_AND_SCHEMA_TEXT}

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

{SELECT_DATABASE_TABLE_TEXT}

@@ -323,7 +351,7 @@ export default function LeftPanel({ onClick={() => setTable(option.value, i)} > {option.label} - {datasets?.includes(option.value) && ( + {datasetNames?.includes(option.value) && ( { + const getDatasetsList = useCallback(async () => { await UseGetDatasetsList(queryParams) .then(json => { setDatasets(json?.result); }) - .catch(error => - logging.error('There was an error fetching dataset', error), - ); - }; + .catch(error => { + addDangerToast(t('There was an error fetching dataset')); + logging.error(t('There was an error fetching dataset'), error); + }); + }, [queryParams]); useEffect(() => { if (dataset?.schema) { getDatasetsList(); } - }, [dataset?.schema]); + }, [dataset?.schema, getDatasetsList]); const HeaderComponent = () => (
@@ -114,9 +123,8 @@ 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 dbeef93ead7b0..89473e5426cbb 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/types.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/types.tsx @@ -16,6 +16,8 @@ * specific language governing permissions and limitations * under the License. */ +import { DatabaseObject } from 'src/components/DatabaseSelector'; + export enum DatasetActionType { selectDatabase, selectSchema, @@ -24,11 +26,7 @@ export enum DatasetActionType { } export interface DatasetObject { - db: { - id: number; - database_name?: string; - owners?: number[]; - }; + db: DatabaseObject & { 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 deleted file mode 100644 index d4b1470f38f1c..0000000000000 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDatasetModal.tsx +++ /dev/null @@ -1,172 +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, { 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 35375a52b4fb7..a1939761aa7c5 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,6 +25,14 @@ 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 68c65a7ad167e..09067765060e1 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -22,16 +22,14 @@ 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'; @@ -60,7 +58,6 @@ 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, @@ -139,6 +136,7 @@ const DatasetList: FunctionComponent = ({ addSuccessToast, user, }) => { + const history = useHistory(); const { state: { loading, @@ -152,9 +150,6 @@ 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); @@ -191,12 +186,6 @@ 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) => { @@ -603,26 +592,6 @@ 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: ( @@ -630,7 +599,9 @@ const DatasetList: FunctionComponent = ({ {t('Dataset')}{' '} ), - onClick: openDatasetAddModal, + onClick: () => { + history.push('/dataset/add/'); + }, buttonStyle: 'primary', }); @@ -727,12 +698,6 @@ const DatasetList: FunctionComponent = ({ return ( <> - {datasetCurrentlyDeleting && ( endpoint: `/api/v1/dataset/?q=${queryParams}`, }) .then(({ json }) => json) - .catch(error => - logging.error('There was an error fetching dataset', error), - ); + .catch(error => { + addDangerToast(t('There was an error fetching dataset')); + logging.error(t('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 ec12e644dbde3..907c305ff64c8 100644 --- a/superset-frontend/src/views/components/RightMenu.test.tsx +++ b/superset-frontend/src/views/components/RightMenu.test.tsx @@ -30,9 +30,6 @@ 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 59e7f7f448664..1957060502761 100644 --- a/superset-frontend/src/views/components/RightMenu.tsx +++ b/superset-frontend/src/views/components/RightMenu.tsx @@ -51,7 +51,6 @@ import { GlobalMenuDataOptions, RightMenuProps, } from './types'; -import AddDatasetModal from '../CRUD/data/dataset/AddDatasetModal'; const extensionsRegistry = getExtensionsRegistry(); @@ -143,7 +142,6 @@ 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); @@ -179,6 +177,7 @@ const RightMenu = ({ { label: t('Create dataset'), name: GlobalMenuDataOptions.DATASET_CREATION, + url: '/dataset/add/', perm: canDataset && nonExamplesDBConnected, }, { @@ -286,8 +285,6 @@ const RightMenu = ({ } else if (itemChose.key === GlobalMenuDataOptions.GOOGLE_SHEETS) { setShowDatabaseModal(true); setEngine('Google Sheets'); - } else if (itemChose.key === GlobalMenuDataOptions.DATASET_CREATION) { - setShowDatasetModal(true); } }; @@ -296,10 +293,6 @@ const RightMenu = ({ setShowDatabaseModal(false); }; - const handleOnHideDatasetModalModal = () => { - setShowDatasetModal(false); - }; - const isDisabled = isAdmin && !allowUploads; const tooltipText = t( @@ -344,7 +337,6 @@ const RightMenu = ({ ); const handleDatabaseAdd = () => setQuery({ databaseAdded: true }); - const handleDatasetAdd = () => setQuery({ datasetAdded: true }); const theme = useTheme(); @@ -358,13 +350,6 @@ const RightMenu = ({ onDatabaseAdd={handleDatabaseAdd} /> )} - {canDataset && ( - - )} {environmentTag?.text && (