From d130b4a24f67a6440101f267d2d3640e34f4a154 Mon Sep 17 00:00:00 2001
From: Yongjie Zhao <yongjie.zhao@gmail.com>
Date: Tue, 6 Sep 2022 22:16:33 +0800
Subject: [PATCH] fix: database modal crashed when use SQLAlchemy URI string
 (#21342)

Co-authored-by: smileydev <47900232+prosdev0107@users.noreply.github.com>
Co-authored-by: Evan Rusackas <evan@preset.io>
---
 .../ErrorMessage/ErrorAlert.test.tsx          |  6 ++++
 .../components/ErrorMessage/ErrorAlert.tsx    | 19 +++++++++-
 .../ErrorMessageWithStackTrace.tsx            |  3 ++
 .../data/database/DatabaseModal/index.tsx     | 36 ++++++++++++-------
 4 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/superset-frontend/src/components/ErrorMessage/ErrorAlert.test.tsx b/superset-frontend/src/components/ErrorMessage/ErrorAlert.test.tsx
index 7133f25c9e..d9fdf5efc8 100644
--- a/superset-frontend/src/components/ErrorMessage/ErrorAlert.test.tsx
+++ b/superset-frontend/src/components/ErrorMessage/ErrorAlert.test.tsx
@@ -31,6 +31,7 @@ const mockedProps = {
   subtitle: 'Error subtitle',
   title: 'Error title',
   source: 'dashboard' as ErrorSource,
+  description: 'we are unable to connect db.',
 };
 
 test('should render', () => {
@@ -63,6 +64,11 @@ test('should render the error title', () => {
   expect(screen.getByText('Error title')).toBeInTheDocument();
 });
 
+test('should render the error description', () => {
+  render(<ErrorAlert {...mockedProps} />, { useRedux: true });
+  expect(screen.getByText('we are unable to connect db.')).toBeInTheDocument();
+});
+
 test('should render the error subtitle', () => {
   render(<ErrorAlert {...mockedProps} />, { useRedux: true });
   const button = screen.getByText('See more');
diff --git a/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx b/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx
index 3340c8ad6d..7b8488a5b9 100644
--- a/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx
+++ b/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx
@@ -87,6 +87,7 @@ interface ErrorAlertProps {
   source?: ErrorSource;
   subtitle: ReactNode;
   title: ReactNode;
+  description?: string;
 }
 
 export default function ErrorAlert({
@@ -96,6 +97,7 @@ export default function ErrorAlert({
   source = 'dashboard',
   subtitle,
   title,
+  description,
 }: ErrorAlertProps) {
   const theme = useTheme();
 
@@ -116,7 +118,7 @@ export default function ErrorAlert({
           )}
           <strong>{title}</strong>
         </LeftSideContent>
-        {!isExpandable && (
+        {!isExpandable && !description && (
           <span
             role="button"
             tabIndex={0}
@@ -127,6 +129,21 @@ export default function ErrorAlert({
           </span>
         )}
       </div>
+      {description && (
+        <div className="error-body">
+          <p>{description}</p>
+          {!isExpandable && (
+            <span
+              role="button"
+              tabIndex={0}
+              className="link"
+              onClick={() => setIsModalOpen(true)}
+            >
+              {t('See more')}
+            </span>
+          )}
+        </div>
+      )}
       {isExpandable ? (
         <div className="error-body">
           <p>{subtitle}</p>
diff --git a/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx b/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx
index 44bd560f39..b54277b4ac 100644
--- a/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx
+++ b/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx
@@ -32,6 +32,7 @@ type Props = {
   copyText?: string;
   stackTrace?: string;
   source?: ErrorSource;
+  description?: string;
   errorMitigationFunction?: () => void;
 };
 
@@ -43,6 +44,7 @@ export default function ErrorMessageWithStackTrace({
   link,
   stackTrace,
   source,
+  description,
 }: Props) {
   // Check if a custom error message component was registered for this message
   if (error) {
@@ -66,6 +68,7 @@ export default function ErrorMessageWithStackTrace({
       title={title}
       subtitle={subtitle}
       copyText={copyText}
+      description={description}
       source={source}
       body={
         link || stackTrace ? (
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 6f55598ec3..d33a5cb521 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
@@ -42,6 +42,7 @@ import IconButton from 'src/components/IconButton';
 import InfoTooltip from 'src/components/InfoTooltip';
 import withToasts from 'src/components/MessageToasts/withToasts';
 import ValidatedInput from 'src/components/Form/LabeledErrorBoundInput';
+import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace';
 import ErrorAlert from 'src/components/ImportModal/ErrorAlert';
 import {
   testDatabaseConnection,
@@ -65,7 +66,6 @@ import ExtraOptions from './ExtraOptions';
 import SqlAlchemyForm from './SqlAlchemyForm';
 import DatabaseConnectionForm from './DatabaseConnectionForm';
 import {
-  antDErrorAlertStyles,
   antDAlertStyles,
   antdWarningAlertStyles,
   StyledAlertMargin,
@@ -112,6 +112,12 @@ const TabsStyled = styled(Tabs)`
   }
 `;
 
+const ErrorAlertContainer = styled.div`
+  ${({ theme }) => `
+    margin: ${theme.gridUnit * 8}px ${theme.gridUnit * 4}px;
+  `};
+`;
+
 interface DatabaseModalProps {
   addDangerToast: (msg: string) => void;
   addSuccessToast: (msg: string) => void;
@@ -471,7 +477,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
     )?.parameters !== undefined;
   const showDBError = validationErrors || dbErrors;
   const isEmpty = (data?: Object | null) =>
-    data && Object.keys(data).length === 0;
+    !data || (data && Object.keys(data).length === 0);
 
   const dbModel: DatabaseForm =
     availableDbs?.databases?.find(
@@ -1135,22 +1141,24 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
   // eslint-disable-next-line consistent-return
   const errorAlert = () => {
     let alertErrors: string[] = [];
-    if (isEmpty(dbErrors) === false) {
+    if (!isEmpty(dbErrors)) {
       alertErrors = typeof dbErrors === 'object' ? Object.values(dbErrors) : [];
-    } else if (db?.engine === Engines.Snowflake) {
+    } else if (!isEmpty(validationErrors)) {
       alertErrors =
         validationErrors?.error_type === 'GENERIC_DB_ENGINE_ERROR'
-          ? [validationErrors?.description]
+          ? [
+              'We are unable to connect to your database. Click "See more" for database-provided information that may help troubleshoot the issue.',
+            ]
           : [];
     }
 
     if (alertErrors.length) {
       return (
-        <Alert
-          type="error"
-          css={(theme: SupersetTheme) => antDErrorAlertStyles(theme)}
-          message={t('Database Creation Error')}
-          description={t(alertErrors[0])}
+        <ErrorMessageWithStackTrace
+          title={t('Database Creation Error')}
+          description={alertErrors[0]}
+          subtitle={validationErrors?.description}
+          copyText={validationErrors?.description}
         />
       );
     }
@@ -1491,7 +1499,9 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
               onChange(ActionType.extraEditorChange, payload);
             }}
           />
-          {showDBError && errorAlert()}
+          {showDBError && (
+            <ErrorAlertContainer>{errorAlert()}</ErrorAlertContainer>
+          )}
         </Tabs.TabPane>
       </TabsStyled>
     </Modal>
@@ -1653,7 +1663,9 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
                   )}
                 </div>
                 {/* Step 2 */}
-                {showDBError && errorAlert()}
+                {showDBError && (
+                  <ErrorAlertContainer>{errorAlert()}</ErrorAlertContainer>
+                )}
               </>
             ))}
         </>