From dbbf58c10d20ffccd1d2e035db79e9fafe02a9d7 Mon Sep 17 00:00:00 2001 From: AAfghahi <48933336+AAfghahi@users.noreply.github.com> Date: Fri, 11 Jun 2021 11:55:17 -0400 Subject: [PATCH 1/6] fix: add icons (#15122) * added alerts * revisions * added icon --- .../src/views/CRUD/data/database/DatabaseModal/index.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 40721000a9ed0..e35020065974d 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -361,6 +361,7 @@ const DatabaseModal: FunctionComponent = ({ ))} antDAlertStyles(theme)} type="info" message={t('Want to add a new database?')} @@ -532,6 +533,7 @@ const DatabaseModal: FunctionComponent = ({ antDAlertStyles(theme)} message="Additional fields may be required" + showIcon description={ <> Select databases require additional fields to be completed in @@ -548,7 +550,6 @@ const DatabaseModal: FunctionComponent = ({ } type="info" - showIcon /> )} @@ -660,6 +661,7 @@ const DatabaseModal: FunctionComponent = ({ antDAlertStyles(theme)} type="info" + showIcon message={t('Whitelisting IPs')} description={connectionAlert.WHITELISTED_IPS} /> From 8a4ff629cced16551f140f9d85bd00c104f62246 Mon Sep 17 00:00:00 2001 From: Arash Date: Thu, 17 Jun 2021 11:24:32 -0400 Subject: [PATCH 2/6] spinner --- .../src/views/CRUD/data/database/DatabaseModal/index.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 a2f3145a78886..83e5fc06c5f14 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -49,6 +49,7 @@ import { DatabaseForm, CONFIGURATION_METHOD, } from 'src/views/CRUD/data/database/types'; +import Loading from 'src/components/Loading'; import ExtraOptions from './ExtraOptions'; import SqlAlchemyForm from './SqlAlchemyForm'; import DatabaseConnectionForm from './DatabaseConnectionForm'; @@ -376,7 +377,7 @@ const DatabaseModal: FunctionComponent = ({ ), }); } - + setLoading(true); const result = await updateResource( db.id as number, update as DatabaseObject, @@ -411,7 +412,7 @@ const DatabaseModal: FunctionComponent = ({ ), }); } - + setLoading(true); const dbId = await createResource(update as DatabaseObject); if (dbId) { setHasConnectedDb(true); @@ -425,6 +426,7 @@ const DatabaseModal: FunctionComponent = ({ } } } + setLoading(false); }; const onChange = (type: any, payload: any) => { @@ -593,7 +595,7 @@ const DatabaseModal: FunctionComponent = ({ if (isLoading) { setLoading(false); } - }, [availableDbs, isLoading]); + }, [availableDbs]); const tabChange = (key: string) => { setTabKey(key); @@ -910,6 +912,7 @@ const DatabaseModal: FunctionComponent = ({ ))} )} + {isLoading && } ); }; From 4a9f6418077b2153dfbd1b3e210aebc2404e4da9 Mon Sep 17 00:00:00 2001 From: Arash Date: Thu, 17 Jun 2021 18:48:16 -0400 Subject: [PATCH 3/6] added errorAlert --- .../data/database/DatabaseModal/index.tsx | 26 +++++++++++++++++++ .../data/database/DatabaseModal/styles.ts | 21 +++++++++++++++ 2 files changed, 47 insertions(+) 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 2c9c49d58473d..6cacfcf3c8d6a 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -54,6 +54,7 @@ import ExtraOptions from './ExtraOptions'; import SqlAlchemyForm from './SqlAlchemyForm'; import DatabaseConnectionForm from './DatabaseConnectionForm'; import { + antDErrorAlertStyles, antDAlertStyles, antDModalNoPaddingStyles, antDModalStyles, @@ -285,6 +286,7 @@ const DatabaseModal: FunctionComponent = ({ const [tabKey, setTabKey] = useState(DEFAULT_TAB_KEY); const [availableDbs, getAvailableDbs] = useAvailableDatabases(); const [validationErrors, getValidation] = useDatabaseValidation(); + const [hasErrors, setHasErrors] = useState(false); const [hasConnectedDb, setHasConnectedDb] = useState(false); const [dbName, setDbName] = useState(''); const [isLoading, setLoading] = useState(false); @@ -596,10 +598,32 @@ const DatabaseModal: FunctionComponent = ({ } }, [availableDbs]); + useEffect(() => { + if (validationErrors) { + setHasErrors(true); + } else { + setHasErrors(false); + } + }, [validationErrors]); + // Used as componentDidMount + const tabChange = (key: string) => { setTabKey(key); }; + const errorAlert = () => { + const errors = validationErrors; + return ( + antDErrorAlertStyles(theme)} + message="Missing Required Fields" + description={errors ? errors.port : ''} + showIcon + /> + ); + }; + const isDynamic = (engine: string | undefined) => availableDbs?.databases.filter( (DB: DatabaseObject) => DB.engine === engine, @@ -763,6 +787,7 @@ const DatabaseModal: FunctionComponent = ({ onChange(ActionType.extraEditorChange, payload); }} /> + {hasErrors && errorAlert} @@ -910,6 +935,7 @@ const DatabaseModal: FunctionComponent = ({ Connect this database with a SQLAlchemy URI string instead {/* Step 2 */} + {hasErrors && errorAlert()} ))} diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts index 6bcb86f94c2c3..e4f6ae7aad312 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts @@ -161,6 +161,27 @@ export const antDAlertStyles = (theme: SupersetTheme) => css` } `; +export const antDErrorAlertStyles = (theme: SupersetTheme) => css` + padding: ${theme.gridUnit * 4}px; + margin: ${theme.gridUnit * 8}px ${theme.gridUnit * 4}px; + .ant-alert-message { + color: ${theme.colors.info.dark2}; + font-size: ${theme.typography.sizes.s + 1}px; + font-weight: bold; + } + .ant-alert-description { + color: ${theme.colors.info.dark2}; + font-size: ${theme.typography.sizes.s + 1}px; + line-height: ${theme.gridUnit * 4}px; + .ant-alert-icon { + margin-right: ${theme.gridUnit * 2.5}px; + font-size: ${theme.typography.sizes.l + 1}px; + position: relative; + top: ${theme.gridUnit / 4}px; + } + } +`; + export const formHelperStyles = (theme: SupersetTheme) => css` .required { margin-left: ${theme.gridUnit / 2}px; From 6a88a41d139d44440561780209f1d86e6befbcfb Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 18 Jun 2021 15:59:17 -0400 Subject: [PATCH 4/6] added revisions --- .../DatabaseModal/DatabaseConnectionForm.tsx | 28 ++++++++++--------- .../data/database/DatabaseModal/index.tsx | 9 ++++-- superset/db_engine_specs/base.py | 12 +++++++- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx index e37fcd711bdf9..6bc74275bdcc0 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx @@ -189,19 +189,21 @@ const portField = ({ validationErrors, db, }: FieldPropTypes) => ( - + <> + + ); const databaseField = ({ required, 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 92a8e986c61da..620180c3332c0 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -418,6 +418,9 @@ const DatabaseModal: FunctionComponent = ({ }); } setLoading(true); + await getValidation(db); + await validationErrors; + console.log(validationErrors); const dbId = await createResource(update as DatabaseObject); if (dbId) { setHasConnectedDb(true); @@ -613,7 +616,7 @@ const DatabaseModal: FunctionComponent = ({ type="error" css={(theme: SupersetTheme) => antDErrorAlertStyles(theme)} message="Missing Required Fields" - description={errors?.port || ''} + description="" showIcon /> ); @@ -882,8 +885,8 @@ const DatabaseModal: FunctionComponent = ({ css={(theme: SupersetTheme) => antDAlertStyles(theme)} type="info" showIcon - message={t('Whitelisting IPs')} - description={connectionAlert.WHITELISTED_IPS} + message={t('Allowlist')} + description={connectionAlert.ALLOWED_IPS} /> )} Date: Tue, 22 Jun 2021 16:27:58 -0400 Subject: [PATCH 5/6] current work --- .../data/database/DatabaseModal/index.tsx | 27 ++++++++----------- superset-frontend/src/views/CRUD/hooks.ts | 13 +++++---- 2 files changed, 19 insertions(+), 21 deletions(-) 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 620180c3332c0..b7305a31a33c7 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -418,9 +418,7 @@ const DatabaseModal: FunctionComponent = ({ }); } setLoading(true); - await getValidation(db); - await validationErrors; - console.log(validationErrors); + await getValidation(db, true); const dbId = await createResource(update as DatabaseObject); if (dbId) { setHasConnectedDb(true); @@ -609,18 +607,15 @@ const DatabaseModal: FunctionComponent = ({ setTabKey(key); }; - const errorAlert = () => { - const errors = dbError; - return ( - antDErrorAlertStyles(theme)} - message="Missing Required Fields" - description="" - showIcon - /> - ); - }; + const errorAlert = () => ( + antDErrorAlertStyles(theme)} + message="Missing Required Fields" + description="Please complete all required fields." + showIcon + /> + ); const isDynamic = (engine: string | undefined) => availableDbs?.databases.filter( @@ -785,7 +780,7 @@ const DatabaseModal: FunctionComponent = ({ onChange(ActionType.extraEditorChange, payload); }} /> - {dbError && errorAlert} + {dbError && errorAlert()} diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index 3132422d841c9..25180ee7c21c0 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -638,7 +638,7 @@ export function useDatabaseValidation() { null, ); const getValidation = useCallback( - (database: Partial | null) => { + (database: Partial | null, checkParams?: boolean) => { SupersetClient.post({ endpoint: '/api/v1/database/validate_parameters', body: JSON.stringify(database), @@ -648,13 +648,14 @@ export function useDatabaseValidation() { setValidationErrors(null); }) .catch(e => { + console.log('check Params', checkParams); if (typeof e.json === 'function') { e.json().then(({ errors = [] }: JsonObject) => { const parsedErrors = errors - .filter( - (error: { error_type: string }) => - error.error_type !== 'CONNECTION_MISSING_PARAMETERS_ERROR', - ) + // .filter( + // (error: { error_type: string }) => + // error.error_type !== 'CONNECTION_MISSING_PARAMETERS_ERROR', + // ) .reduce( ( obj: {}, @@ -676,7 +677,9 @@ export function useDatabaseValidation() { }, {}, ); + console.log(parsedErrors); setValidationErrors(parsedErrors); + console.log(validationErrors); }); } else { // eslint-disable-next-line no-console From a8bc0f502f92043df72ff9d19dcdcfd9c6bd655e Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 22 Jun 2021 17:33:52 -0400 Subject: [PATCH 6/6] revisions --- .../CRUD/data/database/DatabaseModal/index.tsx | 2 +- superset-frontend/src/views/CRUD/hooks.ts | 13 +++++-------- superset/db_engine_specs/base.py | 3 ++- 3 files changed, 8 insertions(+), 10 deletions(-) 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 b7305a31a33c7..86699fe2b2759 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -418,7 +418,7 @@ const DatabaseModal: FunctionComponent = ({ }); } setLoading(true); - await getValidation(db, true); + await getValidation(db); const dbId = await createResource(update as DatabaseObject); if (dbId) { setHasConnectedDb(true); diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index 25180ee7c21c0..3132422d841c9 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -638,7 +638,7 @@ export function useDatabaseValidation() { null, ); const getValidation = useCallback( - (database: Partial | null, checkParams?: boolean) => { + (database: Partial | null) => { SupersetClient.post({ endpoint: '/api/v1/database/validate_parameters', body: JSON.stringify(database), @@ -648,14 +648,13 @@ export function useDatabaseValidation() { setValidationErrors(null); }) .catch(e => { - console.log('check Params', checkParams); if (typeof e.json === 'function') { e.json().then(({ errors = [] }: JsonObject) => { const parsedErrors = errors - // .filter( - // (error: { error_type: string }) => - // error.error_type !== 'CONNECTION_MISSING_PARAMETERS_ERROR', - // ) + .filter( + (error: { error_type: string }) => + error.error_type !== 'CONNECTION_MISSING_PARAMETERS_ERROR', + ) .reduce( ( obj: {}, @@ -677,9 +676,7 @@ export function useDatabaseValidation() { }, {}, ); - console.log(parsedErrors); setValidationErrors(parsedErrors); - console.log(validationErrors); }); } else { // eslint-disable-next-line no-console diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index 314f1070aeb65..943ebc42b73db 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -1441,6 +1441,7 @@ def validate_parameters( ), ) return errors + port = parameters.get("port", None) if not port: return errors @@ -1449,7 +1450,7 @@ def validate_parameters( except (ValueError, TypeError): errors.append( SupersetError( - message="Port must be a valid integer", + message="Port must be a valid integer.", error_type=SupersetErrorType.CONNECTION_INVALID_PORT_ERROR, level=ErrorLevel.ERROR, extra={"invalid": ["port"]},