feat: updated Error Alert

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx

@@ -74,6 +74,24 @@ import {
 } from './styles';
 import ModalHeader, { DOCUMENTATION_LINK } from './ModalHeader';
 
+const errorAlertMapping = {
+  CONNECTION_MISSING_PARAMETERS_ERROR: {
+    message: 'Missing Required Fields',
+    description: 'Please complete all required fields.',
+  },
+  CONNECTION_INVALID_HOSTNAME_ERROR: {
+    message: 'Could not verify the host',
+    description:
+      'The host is invalid. Please verify that this field is entered correctly.',
+  },
+  CONNECTION_PORT_CLOSED_ERROR: {
+    message: 'Port is closed',
+    description: 'Please verify that port is open to connect.',
+  },
+  CONNECTION_INVALID_PORT_ERROR: {
+    message: 'The port must be a whole number less than or equal to 65535.',
+  },
+};
 interface DatabaseModalProps {
   addDangerToast: (msg: string) => void;
   addSuccessToast: (msg: string) => void;
@@ -316,7 +334,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
 
   // Database fetch logic
   const {
-    state: { loading: dbLoading, resource: dbFetched, error: dbError },
+    state: { loading: dbLoading, resource: dbFetched, error: dbErrors },
     fetchResource,
     createResource,
     updateResource,
@@ -326,12 +344,15 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
     addDangerToast,
   );
 
+  const showDBError = validationErrors || dbErrors;
+  const isEmpty = (data?: Object | null) =>
+    data && Object.keys(data).length === 0;
+
   const dbModel: DatabaseForm =
     availableDbs?.databases?.find(
       (available: { engine: string | undefined }) =>
         // TODO: we need a centralized engine in one place
-        available.engine ===
-        (isEditMode || editNewDb ? db?.backend || db?.engine : db?.engine),
+        available.engine === (isEditMode ? db?.backend : db?.engine),
     ) || {};
 
   // Test Connection logic
@@ -370,7 +391,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
 
     // Validate DB before saving
     await getValidation(dbToUpdate, true);
-    if (validationErrors) {
+    if (validationErrors && !isEmpty(validationErrors)) {
       return;
     }
 
@@ -416,6 +437,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
       const result = await updateResource(
         db.id as number,
         dbToUpdate as DatabaseObject,
+        true,
       );
       if (result) {
         if (onDatabaseAdd) {
@@ -443,7 +465,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
         });
       }
       setLoading(true);
-      const dbId = await createResource(dbToUpdate as DatabaseObject);
+      const dbId = await createResource(dbToUpdate as DatabaseObject, true);
       if (dbId) {
         setHasConnectedDb(true);
         if (onDatabaseAdd) {
@@ -651,15 +673,44 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
     setTabKey(key);
   };
 
-  const errorAlert = () => (
-    <Alert
-      type="error"
-      css={(theme: SupersetTheme) => antDErrorAlertStyles(theme)}
-      message="Missing Required Fields"
-      description="Please complete all required fields."
-      showIcon
-    />
-  );
+  const errorAlert = () => {
+    if (
+      isEmpty(dbErrors) ||
+      (isEmpty(validationErrors) &&
+        !(validationErrors?.error_type in errorAlertMapping))
+    ) {
+      return <></>;
+    }
+
+    if (validationErrors) {
+      return (
+        <Alert
+          type="error"
+          css={(theme: SupersetTheme) => antDErrorAlertStyles(theme)}
+          message={
+            errorAlertMapping[validationErrors?.error_type]?.message ||
+            validationErrors?.error_type
+          }
+          description={
+            errorAlertMapping[validationErrors?.error_type]?.description ||
+            JSON.stringify(validationErrors)
+          }
+          showIcon
+          closable={false}
+        />
+      );
+    }
+
+    const message: Array<string> = Object.values(dbErrors);
+    return (
+      <Alert
+        type="error"
+        css={(theme: SupersetTheme) => antDErrorAlertStyles(theme)}
+        message="Database Creation Error"
+        description={message[0]}
+      />
+    );
+  };
 
   const renderFinishState = () => {
     if (!editNewDb) {
@@ -719,7 +770,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
         }
         getValidation={() => getValidation(db)}
         validationErrors={validationErrors}
-        editNewDb={editNewDb}
       />
     );
   };
@@ -897,7 +947,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
               onChange(ActionType.extraEditorChange, payload);
             }}
           />
-          {dbError && errorAlert()}
+          {showDBError && errorAlert()}
         </Tabs.TabPane>
       </Tabs>
     </Modal>
@@ -1023,7 +1073,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
                   />
                 </div>
                 {/* Step 2 */}
-                {dbError && errorAlert()}
+                {showDBError && errorAlert()}
               </>
             ))}
         </>

superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts

@@ -181,7 +181,7 @@ export const antDAlertStyles = (theme: SupersetTheme) => css`
 `;
 
 export const antDErrorAlertStyles = (theme: SupersetTheme) => css`
-  border: ${theme.colors.error.base} 2px solid;
+  border: ${theme.colors.error.base} 1px solid;
   padding: ${theme.gridUnit * 4}px;
   margin: ${theme.gridUnit * 8}px ${theme.gridUnit * 4}px;
   color: ${theme.colors.error.dark2};

superset-frontend/src/views/CRUD/hooks.ts

@@ -211,7 +211,7 @@ export function useListViewResource<D extends object = any>(
 interface SingleViewResourceState<D extends object = any> {
   loading: boolean;
   resource: D | null;
-  error: string | Record<string, string[] | string> | null;
+  error: any | null;
 }
 
 export function useSingleViewResource<D extends object = any>(
@@ -269,7 +269,7 @@ export function useSingleViewResource<D extends object = any>(
   );
 
   const createResource = useCallback(
-    (resource: D) => {
+    (resource: D, hideToast = false) => {
       // Set loading state
       updateState({
         loading: true,
@@ -289,13 +289,16 @@ export function useSingleViewResource<D extends object = any>(
             return json.id;
           },
           createErrorHandler((errMsg: Record<string, string[] | string>) => {
-            handleErrorMsg(
-              t(
-                'An error occurred while creating %ss: %s',
-                resourceLabel,
-                parsedErrorMessage(errMsg),
-              ),
-            );
+            // we did not want toasts for db-connection-ui but did not want to disable it everywhere
+            if (!hideToast) {
+              handleErrorMsg(
+                t(
+                  'An error occurred while creating %ss: %s',
+                  resourceLabel,
+                  parsedErrorMessage(errMsg),
+                ),
+              );
+            }
 
             updateState({
               error: errMsg,
@@ -310,7 +313,7 @@ export function useSingleViewResource<D extends object = any>(
   );
 
   const updateResource = useCallback(
-    (resourceID: number, resource: D) => {
+    (resourceID: number, resource: D, hideToast = false) => {
       // Set loading state
       updateState({
         loading: true,
@@ -330,13 +333,15 @@ export function useSingleViewResource<D extends object = any>(
             return json.result;
           },
           createErrorHandler(errMsg => {
-            handleErrorMsg(
-              t(
-                'An error occurred while fetching %ss: %s',
-                resourceLabel,
-                JSON.stringify(errMsg),
-              ),
-            );
+            if (!hideToast) {
+              handleErrorMsg(
+                t(
+                  'An error occurred while fetching %ss: %s',
+                  resourceLabel,
+                  JSON.stringify(errMsg),
+                ),
+              );
+            }
 
             updateState({
               error: errMsg,
@@ -651,20 +656,20 @@ export function useDatabaseValidation() {
           if (typeof e.json === 'function') {
             e.json().then(({ errors = [] }: JsonObject) => {
               const parsedErrors = errors
-                .filter((error: { error_type: string }) => {
-                  const skipValidationError = ![
-                    'CONNECTION_MISSING_PARAMETERS_ERROR',
-                    'CONNECTION_ACCESS_DENIED_ERROR',
-                  ].includes(error.error_type);
-                  return skipValidationError || onCreate;
-                })
+                .filter(
+                  (error: { error_type: string }) =>
+                    error.error_type !==
+                      'CONNECTION_MISSING_PARAMETERS_ERROR' || onCreate,
+                )
                 .reduce(
                   (
                     obj: {},
                     {
+                      error_type,
                       extra,
                       message,
                     }: {
+                      error_type: string;
                       extra: { invalid?: string[]; missing?: string[] };
                       message: string;
                     },
@@ -673,11 +678,16 @@ export function useDatabaseValidation() {
                     // error can't be mapped to a parameter
                     // so leave it alone
                     if (extra.invalid) {
-                      return { ...obj, [extra.invalid[0]]: message };
+                      return {
+                        ...obj,
+                        [extra.invalid[0]]: message,
+                        error_type,
+                      };
                     }
                     if (extra.missing) {
                       return {
                         ...obj,
+                        error_type,
                         ...Object.assign(
                           {},
                           ...extra.missing.map(field => ({

superset/databases/commands/exceptions.py

@@ -39,7 +39,7 @@ class DatabaseExistsValidationError(ValidationError):
 
     def __init__(self) -> None:
         super().__init__(
-            _("A database with the same name already exists"),
+            _("A database with the same name already exists."),
             field_name="database_name",
         )
 

tests/databases/api_tests.py

@@ -420,7 +420,9 @@ def test_create_database_unique_validate(self):
         rv = self.client.post(uri, json=database_data)
         response = json.loads(rv.data.decode("utf-8"))
         expected_response = {
-            "message": {"database_name": "A database with the same name already exists"}
+            "message": {
+                "database_name": "A database with the same name already exists."
+            }
         }
         self.assertEqual(rv.status_code, 422)
         self.assertEqual(response, expected_response)
@@ -566,7 +568,9 @@ def test_update_database_uniqueness(self):
         rv = self.client.put(uri, json=database_data)
         response = json.loads(rv.data.decode("utf-8"))
         expected_response = {
-            "message": {"database_name": "A database with the same name already exists"}
+            "message": {
+                "database_name": "A database with the same name already exists."
+            }
         }
         self.assertEqual(rv.status_code, 422)
         self.assertEqual(response, expected_response)