From 21cc49509faafa39484ff2978f36557b9d96fc5c Mon Sep 17 00:00:00 2001 From: Lily Kuang Date: Tue, 16 Mar 2021 09:33:36 -0700 Subject: [PATCH] chore: improve modal error handling (#13342) * improve modal error handling * update hook to handle string error message --- .../CRUD/data/database/DatabaseModal.tsx | 32 +++--------- .../CRUD/data/dataset/AddDatasetModal.tsx | 50 ++++++++----------- superset-frontend/src/views/CRUD/hooks.ts | 46 ++++++++++++++--- superset-frontend/src/views/CRUD/types.ts | 10 ++++ superset-frontend/src/views/CRUD/utils.tsx | 4 +- 5 files changed, 79 insertions(+), 63 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal.tsx index c48afd5bf..15483bc57 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal.tsx @@ -17,11 +17,13 @@ * under the License. */ import React, { FunctionComponent, useState, useEffect } from 'react'; -import { styled, t, SupersetClient } from '@superset-ui/core'; +import { styled, t } from '@superset-ui/core'; import InfoTooltip from 'src/common/components/InfoTooltip'; -import { useSingleViewResource } from 'src/views/CRUD/hooks'; +import { + useSingleViewResource, + testDatabaseConnection, +} from 'src/views/CRUD/hooks'; import withToasts from 'src/messageToasts/enhancers/withToasts'; -import { getClientErrorObject } from 'src/utils/getClientErrorObject'; import Icon from 'src/components/Icon'; import Modal from 'src/common/components/Modal'; import Tabs from 'src/common/components/Tabs'; @@ -168,29 +170,7 @@ const DatabaseModal: FunctionComponent = ({ server_cert: db ? db.server_cert || undefined : undefined, }; - SupersetClient.post({ - endpoint: 'api/v1/database/test_connection', - body: JSON.stringify(connection), - headers: { 'Content-Type': 'application/json' }, - }) - .then(() => { - addSuccessToast(t('Connection looks good!')); - }) - .catch(response => - getClientErrorObject(response).then(error => { - addDangerToast( - error?.message - ? `${t('ERROR: ')}${ - typeof error.message === 'string' - ? error.message - : Object.entries(error.message as Record) - .map(([key, value]) => `(${key}) ${value.join(', ')}`) - .join('\n') - }` - : t('ERROR: Connection failed. '), - ); - }), - ); + testDatabaseConnection(connection, addDangerToast, addSuccessToast); }; // Functions diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDatasetModal.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDatasetModal.tsx index 363833588..88781707c 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/AddDatasetModal.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/AddDatasetModal.tsx @@ -17,13 +17,13 @@ * under the License. */ import React, { FunctionComponent, useState } from 'react'; -import { styled, SupersetClient, t } from '@superset-ui/core'; +import { styled, t } from '@superset-ui/core'; +import { useSingleViewResource } from 'src/views/CRUD/hooks'; import { isEmpty, isNil } from 'lodash'; import Icon from 'src/components/Icon'; import Modal from 'src/common/components/Modal'; import TableSelector from 'src/components/TableSelector'; import withToasts from 'src/messageToasts/enhancers/withToasts'; -import { createErrorHandler } from 'src/views/CRUD/utils'; type DatasetAddObject = { id: number; @@ -59,6 +59,11 @@ const DatasetModal: FunctionComponent = ({ const [currentTableName, setTableName] = useState(''); const [datasourceId, setDatasourceId] = useState(0); const [disableSave, setDisableSave] = useState(true); + const { createResource } = useSingleViewResource>( + 'dataset', + t('dataset'), + addDangerToast, + ); const onChange = ({ dbId, @@ -76,32 +81,21 @@ const DatasetModal: FunctionComponent = ({ }; const onSave = () => { - SupersetClient.post({ - endpoint: '/api/v1/dataset/', - body: JSON.stringify({ - database: datasourceId, - ...(currentSchema ? { schema: currentSchema } : {}), - table_name: currentTableName, - }), - headers: { 'Content-Type': 'application/json' }, - }) - .then(({ json = {} }) => { - if (onDatasetAdd) { - onDatasetAdd({ id: json.id, ...json.result }); - } - addSuccessToast(t('The dataset has been saved')); - onHide(); - }) - .catch( - createErrorHandler((errMsg: unknown) => - addDangerToast( - t( - 'Error while saving dataset: %s', - (errMsg as { table_name?: string }).table_name, - ), - ), - ), - ); + const data = { + database: datasourceId, + ...(currentSchema ? { schema: currentSchema } : {}), + table_name: currentTableName, + }; + createResource(data).then(response => { + if (!response) { + return; + } + if (onDatasetAdd) { + onDatasetAdd({ id: response.id, ...response }); + } + addSuccessToast(t('The dataset has been saved')); + onHide(); + }); }; return ( diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index 67b0a7201..1545c8240 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -26,7 +26,7 @@ import { FilterValue } from 'src/components/ListView/types'; import Chart, { Slice } from 'src/types/Chart'; import copyTextToClipboard from 'src/utils/copy'; import { getClientErrorObject } from 'src/utils/getClientErrorObject'; -import { FavoriteStatus, ImportResourceName } from './types'; +import { FavoriteStatus, ImportResourceName, DatabaseObject } from './types'; interface ListViewResourceState { loading: boolean; @@ -38,6 +38,17 @@ interface ListViewResourceState { lastFetched?: string; } +const parsedErrorMessage = ( + errorMessage: Record | string, +) => { + if (typeof errorMessage === 'string') { + return errorMessage; + } + return Object.entries(errorMessage) + .map(([key, value]) => `(${key}) ${value.join(', ')}`) + .join('\n'); +}; + export function useListViewResource( resource: string, resourceLabel: string, // resourceLabel for translations @@ -188,7 +199,7 @@ export function useListViewResource( interface SingleViewResourceState { loading: boolean; resource: D | null; - error: string | null; + error: string | Record | null; } export function useSingleViewResource( @@ -224,12 +235,12 @@ export function useSingleViewResource( }); return json.result; }, - createErrorHandler(errMsg => { + createErrorHandler((errMsg: Record) => { handleErrorMsg( t( 'An error occurred while fetching %ss: %s', resourceLabel, - JSON.stringify(errMsg), + parsedErrorMessage(errMsg), ), ); @@ -265,12 +276,12 @@ export function useSingleViewResource( }); return json.id; }, - createErrorHandler(errMsg => { + createErrorHandler((errMsg: Record) => { handleErrorMsg( t( 'An error occurred while creating %ss: %s', resourceLabel, - JSON.stringify(errMsg), + parsedErrorMessage(errMsg), ), ); @@ -439,7 +450,7 @@ export function useImportResource( t( 'An error occurred while importing %s: %s', resourceLabel, - errMsg, + parsedErrorMessage(errMsg), ), ); return false; @@ -449,7 +460,7 @@ export function useImportResource( t( 'An error occurred while importing %s: %s', resourceLabel, - JSON.stringify(errMsg), + parsedErrorMessage(errMsg), ), ); } else { @@ -605,3 +616,22 @@ export const copyQueryLink = ( addDangerToast(t('Sorry, your browser does not support copying.')); }); }; + +export const testDatabaseConnection = ( + connection: DatabaseObject, + handleErrorMsg: (errorMsg: string) => void, + addSuccessToast: (arg0: string) => void, +) => { + SupersetClient.post({ + endpoint: 'api/v1/database/test_connection', + body: JSON.stringify(connection), + headers: { 'Content-Type': 'application/json' }, + }).then( + () => { + addSuccessToast(t('Connection looks good!')); + }, + createErrorHandler((errMsg: Record | string) => { + handleErrorMsg(t(`${t('ERROR: ')}${parsedErrorMessage(errMsg)}`)); + }), + ); +}; diff --git a/superset-frontend/src/views/CRUD/types.ts b/superset-frontend/src/views/CRUD/types.ts index 8e54bbd04..b9908c756 100644 --- a/superset-frontend/src/views/CRUD/types.ts +++ b/superset-frontend/src/views/CRUD/types.ts @@ -116,3 +116,13 @@ export enum QueryObjectColumns { } export type ImportResourceName = 'chart' | 'dashboard' | 'database' | 'dataset'; + +export type DatabaseObject = { + allow_run_async?: boolean; + database_name?: string; + encrypted_extra?: string; + extra?: string; + impersonate_user?: boolean; + server_cert?: string; + sqlalchemy_uri: string; +}; diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx index 638735dfc..c60c1413f 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -160,7 +160,9 @@ export const getRecentAcitivtyObjs = ( export const createFetchRelated = createFetchResourceMethod('related'); export const createFetchDistinct = createFetchResourceMethod('distinct'); -export function createErrorHandler(handleErrorFunc: (errMsg?: string) => void) { +export function createErrorHandler( + handleErrorFunc: (errMsg?: string | Record) => void, +) { return async (e: SupersetClientResponse | string) => { const parsedError = await getClientErrorObject(e); logging.error(e);