From 16a1df75fcb5b2d0916abe648b717add36c43b3e Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Sun, 10 Oct 2021 16:21:14 -0700 Subject: [PATCH] fix: clear modal state after adding dataset (#17044) * fix: clear modal state after adding dataset * Fix test * Small fixes --- .../DatabaseSelector.test.tsx | 8 ++- .../src/components/DatabaseSelector/index.tsx | 44 +++++++++-------- .../TableSelector/TableSelector.test.tsx | 7 ++- .../src/components/TableSelector/index.tsx | 49 +++++++++---------- .../CRUD/data/dataset/AddDatasetModal.tsx | 47 +++++++++++------- 5 files changed, 91 insertions(+), 64 deletions(-) diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index b5ebd044ee0a..af3fa96109f6 100644 --- a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx +++ b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx @@ -26,7 +26,12 @@ import DatabaseSelector from '.'; const SupersetClientGet = jest.spyOn(SupersetClient, 'get'); const createProps = () => ({ - db: { id: 1, database_name: 'test', backend: 'test-postgresql' }, + db: { + id: 1, + database_name: 'test', + backend: 'test-postgresql', + allow_multi_schema_metadata_fetch: false, + }, formMode: false, isDatabaseSelectEnabled: true, readOnly: false, @@ -246,6 +251,7 @@ test('Sends the correct db when changing the database', async () => { id: 2, database_name: 'test-mysql', backend: 'mysql', + allow_multi_schema_metadata_fetch: false, }), ), ); diff --git a/superset-frontend/src/components/DatabaseSelector/index.tsx b/superset-frontend/src/components/DatabaseSelector/index.tsx index 020087286f72..dd6b824c89aa 100644 --- a/superset-frontend/src/components/DatabaseSelector/index.tsx +++ b/superset-frontend/src/components/DatabaseSelector/index.tsx @@ -63,21 +63,25 @@ type DatabaseValue = { id: number; database_name: string; backend: string; + allow_multi_schema_metadata_fetch: boolean; +}; + +export type DatabaseObject = { + id: number; + database_name: string; + backend: string; + allow_multi_schema_metadata_fetch: boolean; }; type SchemaValue = { label: string; value: string }; interface DatabaseSelectorProps { - db?: { id: number; database_name: string; backend: string }; + db?: DatabaseObject; formMode?: boolean; getDbList?: (arg0: any) => {}; handleError: (msg: string) => void; isDatabaseSelectEnabled?: boolean; - onDbChange?: (db: { - id: number; - database_name: string; - backend: string; - }) => void; + onDbChange?: (db: DatabaseObject) => void; onSchemaChange?: (schema?: string) => void; onSchemasLoad?: (schemas: Array) => void; readOnly?: boolean; @@ -165,20 +169,20 @@ export default function DatabaseSelector({ if (result.length === 0) { handleError(t("It seems you don't have access to any database")); } - const options = result.map( - (row: { id: number; database_name: string; backend: string }) => ({ - label: ( - - ), - value: row.id, - id: row.id, - database_name: row.database_name, - backend: row.backend, - }), - ); + const options = result.map((row: DatabaseObject) => ({ + label: ( + + ), + value: row.id, + id: row.id, + database_name: row.database_name, + backend: row.backend, + allow_multi_schema_metadata_fetch: + row.allow_multi_schema_metadata_fetch, + })); return { data: options, totalCount: options.length, diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx index 0c5dbcdcca0a..51e40b511d33 100644 --- a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx +++ b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx @@ -26,7 +26,12 @@ import TableSelector from '.'; const SupersetClientGet = jest.spyOn(SupersetClient, 'get'); const createProps = () => ({ - dbId: 1, + database: { + id: 1, + database_name: 'main', + backend: 'sqlite', + allow_multi_schema_metadata_fetch: false, + }, schema: 'test_schema', handleError: jest.fn(), }); diff --git a/superset-frontend/src/components/TableSelector/index.tsx b/superset-frontend/src/components/TableSelector/index.tsx index 8e5abe90970b..e7a99cc71c3f 100644 --- a/superset-frontend/src/components/TableSelector/index.tsx +++ b/superset-frontend/src/components/TableSelector/index.tsx @@ -27,7 +27,9 @@ import { styled, SupersetClient, t } from '@superset-ui/core'; import { Select } from 'src/components'; import { FormLabel } from 'src/components/Form'; import Icons from 'src/components/Icons'; -import DatabaseSelector from 'src/components/DatabaseSelector'; +import DatabaseSelector, { + DatabaseObject, +} from 'src/components/DatabaseSelector'; import RefreshLabel from 'src/components/RefreshLabel'; import CertifiedIcon from 'src/components/CertifiedIcon'; import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip'; @@ -76,22 +78,12 @@ const TableLabel = styled.span` interface TableSelectorProps { clearable?: boolean; - database?: { - id: number; - database_name: string; - backend: string; - allow_multi_schema_metadata_fetch: boolean; - }; - dbId: number; + database?: DatabaseObject; formMode?: boolean; getDbList?: (arg0: any) => {}; handleError: (msg: string) => void; isDatabaseSelectEnabled?: boolean; - onDbChange?: (db: { - id: number; - database_name: string; - backend: string; - }) => void; + onDbChange?: (db: DatabaseObject) => void; onSchemaChange?: (schema?: string) => void; onSchemasLoad?: () => void; onTableChange?: (tableName?: string, schema?: string) => void; @@ -150,7 +142,6 @@ const TableOption = ({ table }: { table: Table }) => { const TableSelector: FunctionComponent = ({ database, - dbId, formMode = false, getDbList, handleError, @@ -165,7 +156,9 @@ const TableSelector: FunctionComponent = ({ sqlLabMode = true, tableName, }) => { - const [currentDbId, setCurrentDbId] = useState(dbId); + const [currentDatabase, setCurrentDatabase] = useState< + DatabaseObject | undefined + >(database); const [currentSchema, setCurrentSchema] = useState( schema, ); @@ -176,13 +169,22 @@ const TableSelector: FunctionComponent = ({ const [tableOptions, setTableOptions] = useState([]); useEffect(() => { - if (currentDbId && currentSchema) { + // reset selections + if (database === undefined) { + setCurrentDatabase(undefined); + setCurrentSchema(undefined); + setCurrentTable(undefined); + } + }, [database]); + + useEffect(() => { + if (currentDatabase && currentSchema) { setLoadingTables(true); const encodedSchema = encodeURIComponent(currentSchema); const forceRefresh = refresh !== previousRefresh; // TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes. const endpoint = encodeURI( - `/superset/tables/${currentDbId}/${encodedSchema}/undefined/${forceRefresh}/`, + `/superset/tables/${currentDatabase.id}/${encodedSchema}/undefined/${forceRefresh}/`, ); if (previousRefresh !== refresh) { @@ -223,7 +225,7 @@ const TableSelector: FunctionComponent = ({ // We are using the refresh state to re-trigger the query // previousRefresh should be out of dependencies array // eslint-disable-next-line react-hooks/exhaustive-deps - }, [currentDbId, currentSchema, onTablesLoad, refresh]); + }, [currentDatabase, currentSchema, onTablesLoad, refresh]); function renderSelectRow(select: ReactNode, refreshBtn: ReactNode) { return ( @@ -241,12 +243,8 @@ const TableSelector: FunctionComponent = ({ } }; - const internalDbChange = (db: { - id: number; - database_name: string; - backend: string; - }) => { - setCurrentDbId(db?.id); + const internalDbChange = (db: DatabaseObject) => { + setCurrentDatabase(db); if (onDbChange) { onDbChange(db); } @@ -263,7 +261,8 @@ const TableSelector: FunctionComponent = ({ function renderDatabaseSelector() { return ( = ({ onHide, show, }) => { + const [currentDatabase, setCurrentDatabase] = useState< + DatabaseObject | undefined + >(); const [currentSchema, setSchema] = useState(''); const [currentTableName, setTableName] = useState(''); - const [datasourceId, setDatasourceId] = useState(0); const [disableSave, setDisableSave] = useState(true); const { createResource } = useSingleViewResource>( 'dataset', @@ -61,15 +63,11 @@ const DatasetModal: FunctionComponent = ({ ); useEffect(() => { - setDisableSave(isNil(datasourceId) || isEmpty(currentTableName)); - }, [currentTableName, datasourceId]); + setDisableSave(currentDatabase === undefined || currentTableName === ''); + }, [currentTableName, currentDatabase]); - const onDbChange = (db: { - id: number; - database_name: string; - backend: string; - }) => { - setDatasourceId(db.id); + const onDbChange = (db: DatabaseObject) => { + setCurrentDatabase(db); }; const onSchemaChange = (schema?: string) => { @@ -80,9 +78,24 @@ const DatasetModal: FunctionComponent = ({ setTableName(tableName); }; + const clearModal = () => { + setSchema(''); + setTableName(''); + setCurrentDatabase(undefined); + setDisableSave(true); + }; + + const hide = () => { + clearModal(); + onHide(); + }; + const onSave = () => { + if (currentDatabase === undefined) { + return; + } const data = { - database: datasourceId, + database: currentDatabase.id, ...(currentSchema ? { schema: currentSchema } : {}), table_name: currentTableName, }; @@ -94,7 +107,7 @@ const DatasetModal: FunctionComponent = ({ onDatasetAdd({ id: response.id, ...response }); } addSuccessToast(t('The dataset has been saved')); - onHide(); + hide(); }); }; @@ -102,7 +115,7 @@ const DatasetModal: FunctionComponent = ({ = ({