You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2022/08/24 17:29:36 UTC

[superset] branch master updated: fix(database): make to display validation error msg when all cases (#20095)

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new d568999592 fix(database): make to display validation error msg when all cases (#20095)
d568999592 is described below

commit d568999592bb687d862dcfbf6f76c7ff7ee5610d
Author: smileydev <47...@users.noreply.github.com>
AuthorDate: Wed Aug 24 13:29:22 2022 -0400

    fix(database): make to display validation error msg when all cases (#20095)
    
    * fix(database): make to display validation error msg when all cases
    
    * fix(db): make to update the alert error condition
    
    * fix(db): make to add error detail display
    
    * fix(db): make to update error alert display by superset error style guide.
    
    * fix(db): make to style modal header title with h4
    
    * fix(db): make to place see more on bottom instead of top
    
    * fix(db): make to fix shortly
    
    * fix(db): make to fix lint issue
    
    Co-authored-by: Evan Rusackas <ev...@preset.io>
---
 .../components/ErrorMessage/ErrorAlert.test.tsx    |  6 ++++
 .../src/components/ErrorMessage/ErrorAlert.tsx     | 19 +++++++++++-
 .../ErrorMessage/ErrorMessageWithStackTrace.tsx    |  3 ++
 .../CRUD/data/database/DatabaseModal/index.tsx     | 34 +++++++++++++++-------
 4 files changed, 50 insertions(+), 12 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 eff95313da..681d0a1f79 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,
@@ -64,7 +65,6 @@ import ExtraOptions from './ExtraOptions';
 import SqlAlchemyForm from './SqlAlchemyForm';
 import DatabaseConnectionForm from './DatabaseConnectionForm';
 import {
-  antDErrorAlertStyles,
   antDAlertStyles,
   antdWarningAlertStyles,
   StyledAlertMargin,
@@ -116,6 +116,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;
@@ -475,7 +481,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(
@@ -1126,22 +1132,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')}
+        <ErrorMessageWithStackTrace
+          title={t('Database Creation Error')}
           description={t(alertErrors[0])}
+          subtitle={t(validationErrors?.description)}
+          copyText={t(validationErrors?.description)}
         />
       );
     }
@@ -1480,7 +1488,9 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
               onChange(ActionType.extraEditorChange, payload);
             }}
           />
-          {showDBError && errorAlert()}
+          {showDBError && (
+            <ErrorAlertContainer>{errorAlert()}</ErrorAlertContainer>
+          )}
         </Tabs.TabPane>
       </TabsStyled>
     </Modal>
@@ -1641,7 +1651,9 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
                   )}
                 </div>
                 {/* Step 2 */}
-                {showDBError && errorAlert()}
+                {showDBError && (
+                  <ErrorAlertContainer>{errorAlert()}</ErrorAlertContainer>
+                )}
               </>
             ))}
         </>