fix: make sure that gsheets db connection form loads properly (apache…

…#22361)
preset-io · Dec 10, 2022 · 368e7e6 · 368e7e6
1 parent 8610012
commit 368e7e6
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 86 deletions.
diff --git a/...rset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm/index.tsx b/...rset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm/index.tsx
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { FormEvent, useEffect } from 'react';
+import React, { FormEvent } from 'react';
 import { SupersetTheme, JsonObject } from '@superset-ui/core';
 import { InputProps } from 'antd/lib/input';
 import { Form } from 'src/components/Form';
@@ -103,24 +103,7 @@ const FORM_FIELD_MAP = {
   account: validatedInputField,
 };
 
-const DatabaseConnectionForm = ({
-  dbModel: { parameters, default_driver },
-  db,
-  editNewDb,
-  getPlaceholder,
-  getValidation,
-  isEditMode = false,
-  onAddTableCatalog,
-  onChange,
-  onExtraInputChange,
-  onParametersChange,
-  onParametersUploadFileChange,
-  onQueryChange,
-  onRemoveTableCatalog,
-  setDatabaseDriver,
-  sslForced,
-  validationErrors,
-}: {
+interface DatabaseConnectionFormProps {
   isEditMode?: boolean;
   sslForced: boolean;
   editNewDb?: boolean;
@@ -146,52 +129,64 @@ const DatabaseConnectionForm = ({
   validationErrors: JsonObject | null;
   getValidation: () => void;
   getPlaceholder?: (field: string) => string | undefined;
-  setDatabaseDriver: (driver: string) => void;
-}) => {
-  useEffect(() => {
-    setDatabaseDriver(default_driver);
-  }, [default_driver]);
-  return (
-    <Form>
-      <div
-        // @ts-ignore
-        css={(theme: SupersetTheme) => [
-          formScrollableStyles,
-          validatedFormStyles(theme),
-        ]}
-      >
-        {parameters &&
-          FormFieldOrder.filter(
-            (key: string) =>
-              Object.keys(parameters.properties).includes(key) ||
-              key === 'database_name',
-          ).map(field =>
-            FORM_FIELD_MAP[field]({
-              required: parameters.required?.includes(field),
-              changeMethods: {
-                onParametersChange,
-                onChange,
-                onQueryChange,
-                onParametersUploadFileChange,
-                onAddTableCatalog,
-                onRemoveTableCatalog,
-                onExtraInputChange,
-              },
-              validationErrors,
-              getValidation,
-              db,
-              key: field,
-              field,
-              isEditMode,
-              sslForced,
-              editNewDb,
-              placeholder: getPlaceholder ? getPlaceholder(field) : undefined,
-            }),
-          )}
-      </div>
-    </Form>
-  );
-};
+}
+
+const DatabaseConnectionForm = ({
+  dbModel: { parameters },
+  db,
+  editNewDb,
+  getPlaceholder,
+  getValidation,
+  isEditMode = false,
+  onAddTableCatalog,
+  onChange,
+  onExtraInputChange,
+  onParametersChange,
+  onParametersUploadFileChange,
+  onQueryChange,
+  onRemoveTableCatalog,
+  sslForced,
+  validationErrors,
+}: DatabaseConnectionFormProps) => (
+  <Form>
+    <div
+      // @ts-ignore
+      css={(theme: SupersetTheme) => [
+        formScrollableStyles,
+        validatedFormStyles(theme),
+      ]}
+    >
+      {parameters &&
+        FormFieldOrder.filter(
+          (key: string) =>
+            Object.keys(parameters.properties).includes(key) ||
+            key === 'database_name',
+        ).map(field =>
+          FORM_FIELD_MAP[field]({
+            required: parameters.required?.includes(field),
+            changeMethods: {
+              onParametersChange,
+              onChange,
+              onQueryChange,
+              onParametersUploadFileChange,
+              onAddTableCatalog,
+              onRemoveTableCatalog,
+              onExtraInputChange,
+            },
+            validationErrors,
+            getValidation,
+            db,
+            key: field,
+            field,
+            isEditMode,
+            sslForced,
+            editNewDb,
+            placeholder: getPlaceholder ? getPlaceholder(field) : undefined,
+          }),
+        )}
+    </div>
+  </Form>
+);
 export const FormFieldMap = FORM_FIELD_MAP;
 
 export default DatabaseConnectionForm;
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
@@ -1786,4 +1786,34 @@ describe('dbReducer', () => {
       catalog: [],
     });
   });
+
+  test('it will add db information when one is selected', () => {
+    const { backend, ...db } = databaseFixture;
+    const action: DBReducerActionType = {
+      type: ActionType.dbSelected,
+      payload: {
+        engine_information: {
+          supports_file_upload: true,
+        },
+        ...db,
+        driver: db.driver,
+        engine: backend,
+      },
+    };
+    const currentState = dbReducer({}, action);
+
+    expect(currentState).toEqual({
+      database_name: db.database_name,
+      engine: backend,
+      configuration_method: db.configuration_method,
+      engine_information: {
+        supports_file_upload: true,
+      },
+      driver: db.driver,
+      expose_in_sqllab: true,
+      extra: '{"allows_virtual_table_explore":true}',
+      is_managed_externally: false,
+      name: 'PostgresDB',
+    });
+  });
 });
diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
@@ -135,7 +135,6 @@ export enum ActionType {
   addTableCatalogSheet,
   configMethodChange,
   dbSelected,
-  driverChange,
   editorChange,
   extraEditorChange,
   extraInputChange,
@@ -180,6 +179,7 @@ export type DBReducerActionType =
         engine?: string;
         configuration_method: CONFIGURATION_METHOD;
         engine_information?: {};
+        driver?: string;
       };
     }
   | {
@@ -198,10 +198,6 @@ export type DBReducerActionType =
         engine?: string;
         configuration_method: CONFIGURATION_METHOD;
       };
-    }
-  | {
-      type: ActionType.driverChange;
-      payload: string;
     };
 
 const StyledBtns = styled.div`
@@ -426,12 +422,6 @@ export function dbReducer(
         ...action.payload,
       };
 
-    case ActionType.driverChange:
-      return {
-        ...trimmedState,
-        driver: action.payload,
-      };
-
     case ActionType.reset:
     default:
       return null;
@@ -753,7 +743,8 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
       const selectedDbModel = availableDbs?.databases.filter(
         (db: DatabaseObject) => db.name === database_name,
       )[0];
-      const { engine, parameters, engine_information } = selectedDbModel;
+      const { engine, parameters, engine_information, default_driver } =
+        selectedDbModel;
       const isDynamic = parameters !== undefined;
       setDB({
         type: ActionType.dbSelected,
@@ -764,6 +755,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
             ? CONFIGURATION_METHOD.DYNAMIC_FORM
             : CONFIGURATION_METHOD.SQLALCHEMY_URI,
           engine_information,
+          driver: default_driver,
         },
       });
 
@@ -1292,9 +1284,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
         sslForced={sslForced}
         dbModel={dbModel}
         db={db as DatabaseObject}
-        setDatabaseDriver={(driver: string) => {
-          onChange(ActionType.driverChange, driver);
-        }}
         onParametersChange={({ target }: { target: HTMLInputElement }) =>
           onChange(ActionType.parametersChange, {
             type: target.type,
@@ -1466,9 +1455,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
               sslForced={sslForced}
               dbModel={dbModel}
               db={db as DatabaseObject}
-              setDatabaseDriver={(driver: string) => {
-                onChange(ActionType.driverChange, driver);
-              }}
               onParametersChange={({ target }: { target: HTMLInputElement }) =>
                 onChange(ActionType.parametersChange, {
                   type: target.type,
@@ -1660,9 +1646,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
                   db={db}
                   sslForced={sslForced}
                   dbModel={dbModel}
-                  setDatabaseDriver={(driver: string) => {
-                    onChange(ActionType.driverChange, driver);
-                  }}
                   onAddTableCatalog={() => {
                     setDB({ type: ActionType.addTableCatalogSheet });
                   }}