fix: clear modal state after adding dataset (apache#17044)

* fix: clear modal state after adding dataset * Fix test * Small fixes
QAlexBall · Oct 10, 2021 · c115d61 · c115d61
1 parent b921ef7
commit c115d61
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 64 deletions.
diff --git 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
@@ -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<object>) => 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: (
-              <SelectLabel
-                backend={row.backend}
-                databaseName={row.database_name}
-              />
-            ),
-            value: row.id,
-            id: row.id,
-            database_name: row.database_name,
-            backend: row.backend,
-          }),
-        );
+        const options = result.map((row: DatabaseObject) => ({
+          label: (
+            <SelectLabel
+              backend={row.backend}
+              databaseName={row.database_name}
+            />
+          ),
+          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
@@ -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
@@ -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<TableSelectorProps> = ({
   database,
-  dbId,
   formMode = false,
   getDbList,
   handleError,
@@ -165,7 +156,9 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
   sqlLabMode = true,
   tableName,
 }) => {
-  const [currentDbId, setCurrentDbId] = useState<number | undefined>(dbId);
+  const [currentDatabase, setCurrentDatabase] = useState<
+    DatabaseObject | undefined
+  >(database);
   const [currentSchema, setCurrentSchema] = useState<string | undefined>(
     schema,
   );
@@ -176,13 +169,22 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
   const [tableOptions, setTableOptions] = useState<TableOption[]>([]);
 
   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<TableSelectorProps> = ({
     // 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<TableSelectorProps> = ({
     }
   };
 
-  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<TableSelectorProps> = ({
   function renderDatabaseSelector() {
     return (
       <DatabaseSelector
-        db={database}
+        key={currentDatabase?.id}
+        db={currentDatabase}
         formMode={formMode}
         getDbList={getDbList}
         handleError={handleError}

diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDatasetModal.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDatasetModal.tsx
@@ -19,10 +19,10 @@
 import React, { FunctionComponent, useState, useEffect } from 'react';
 import { styled, t } from '@superset-ui/core';
 import { useSingleViewResource } from 'src/views/CRUD/hooks';
-import { isEmpty, isNil } from 'lodash';
 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';
 
 type DatasetAddObject = {
   id: number;
@@ -50,9 +50,11 @@ const DatasetModal: FunctionComponent<DatasetModalProps> = ({
   onHide,
   show,
 }) => {
+  const [currentDatabase, setCurrentDatabase] = useState<
+    DatabaseObject | undefined
+  >();
   const [currentSchema, setSchema] = useState<string | undefined>('');
   const [currentTableName, setTableName] = useState('');
-  const [datasourceId, setDatasourceId] = useState<number>(0);
   const [disableSave, setDisableSave] = useState(true);
   const { createResource } = useSingleViewResource<Partial<DatasetAddObject>>(
     'dataset',
@@ -61,15 +63,11 @@ const DatasetModal: FunctionComponent<DatasetModalProps> = ({
   );
 
   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<DatasetModalProps> = ({
     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,30 +107,30 @@ const DatasetModal: FunctionComponent<DatasetModalProps> = ({
         onDatasetAdd({ id: response.id, ...response });
       }
       addSuccessToast(t('The dataset has been saved'));
-      onHide();
+      hide();
     });
   };
 
   return (
     <Modal
       disablePrimaryButton={disableSave}
       onHandledPrimaryAction={onSave}
-      onHide={onHide}
+      onHide={hide}
       primaryButtonName={t('Add')}
       show={show}
       title={t('Add dataset')}
     >
       <TableSelectorContainer>
         <TableSelector
           clearable={false}
-          dbId={datasourceId}
           formMode
-          handleError={addDangerToast}
+          database={currentDatabase}
+          schema={currentSchema}
+          tableName={currentTableName}
           onDbChange={onDbChange}
           onSchemaChange={onSchemaChange}
           onTableChange={onTableChange}
-          schema={currentSchema}
-          tableName={currentTableName}
+          handleError={addDangerToast}
         />
       </TableSelectorContainer>
     </Modal>