Skip to content

Commit

Permalink
chore: simplify error messaging in database modal (#19165)
Browse files Browse the repository at this point in the history
* testing for dbconn modal error message

* remove consoles and error alert mapping

* lint fix

* update url message

* add modal fix

* update comments

* fix err msg bugs

* fix pylint

* fix line

* fix tests

* fix assertions
  • Loading branch information
pkdotson authored Apr 21, 2022
1 parent fa68036 commit ad71542
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 82 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export const TableCatalog = ({
}: FieldPropTypes) => {
const tableCatalog = db?.catalog || [];
const catalogError = validationErrors || {};

return (
<StyledCatalogTable>
<h4 className="gsheet-title">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,49 +96,6 @@ const engineSpecificAlertMapping = {
},
};

const errorAlertMapping = {
GENERIC_DB_ENGINE_ERROR: {
message: t('Generic database engine error'),
},
CONNECTION_MISSING_PARAMETERS_ERROR: {
message: t('Missing Required Fields'),
description: t('Please complete all required fields.'),
},
CONNECTION_INVALID_HOSTNAME_ERROR: {
message: t('Could not verify the host'),
description: t(
'The host is invalid. Please verify that this field is entered correctly.',
),
},
CONNECTION_PORT_CLOSED_ERROR: {
message: t('Port is closed'),
description: t('Please verify that port is open to connect.'),
},
CONNECTION_INVALID_PORT_ERROR: {
message: t('Invalid Port Number'),
description: t(
'The port must be a whole number less than or equal to 65535.',
),
},
CONNECTION_ACCESS_DENIED_ERROR: {
message: t('Invalid account information'),
description: t('Either the username or password is incorrect.'),
},
CONNECTION_INVALID_PASSWORD_ERROR: {
message: t('Invalid account information'),
description: t('Either the username or password is incorrect.'),
},
INVALID_PAYLOAD_SCHEMA_ERROR: {
message: t('Incorrect Fields'),
description: t('Please make sure all fields are filled out correctly'),
},
TABLE_DOES_NOT_EXIST_ERROR: {
message: t('URL could not be identified'),
description: t(
'The URL could not be identified. Please check for typos and make sure that "Type of google sheet allowed" selection matches the input',
),
},
};
const googleSheetConnectionEngine = 'gsheets';

interface DatabaseModalProps {
Expand Down Expand Up @@ -227,7 +184,7 @@ function dbReducer(
};
let query = {};
let query_input = '';
let deserializeExtraJSON = { allows_virtual_table_explore: true };
let deserializeExtraJSON = {};
let extra_json: DatabaseObject['extra_json'];

switch (action.type) {
Expand Down Expand Up @@ -576,8 +533,8 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({

if (dbToUpdate.configuration_method === CONFIGURATION_METHOD.DYNAMIC_FORM) {
// Validate DB before saving
await getValidation(dbToUpdate, true);
if (validationErrors && !isEmpty(validationErrors)) {
const errors = await getValidation(dbToUpdate, true);
if ((validationErrors && !isEmpty(validationErrors)) || errors) {
return;
}
const parameters_schema = isEditMode
Expand Down Expand Up @@ -679,7 +636,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
passwords,
confirmedOverwrite,
);

if (dbId) {
onClose();
addSuccessToast(t('Database connected'));
Expand Down Expand Up @@ -1112,44 +1068,21 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
);
};

// eslint-disable-next-line consistent-return
const errorAlert = () => {
if (
isEmpty(dbErrors) ||
(isEmpty(validationErrors) &&
!(validationErrors?.error_type in errorAlertMapping))
) {
return <></>;
}

if (validationErrors) {
if (isEmpty(dbErrors) === false) {
const message: Array<string> =
typeof dbErrors === 'object' ? Object.values(dbErrors) : [];
return (
<Alert
type="error"
css={(theme: SupersetTheme) => antDErrorAlertStyles(theme)}
message={
errorAlertMapping[validationErrors?.error_type]?.message ||
validationErrors?.error_type
}
description={
errorAlertMapping[validationErrors?.error_type]?.description ||
validationErrors?.description ||
JSON.stringify(validationErrors)
}
showIcon
closable={false}
message={t('Database Creation Error')}
description={message?.[0] || dbErrors}
/>
);
}
const message: Array<string> =
typeof dbErrors === 'object' ? Object.values(dbErrors) : [];
return (
<Alert
type="error"
css={(theme: SupersetTheme) => antDErrorAlertStyles(theme)}
message={t('Database Creation Error')}
description={message?.[0] || dbErrors}
/>
);
return <></>;
};

const renderFinishState = () => {
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/views/CRUD/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,7 @@ export function useDatabaseValidation() {
{},
);
setValidationErrors(parsedErrors);
return parsedErrors;
});
}
// eslint-disable-next-line no-console
Expand Down
6 changes: 5 additions & 1 deletion superset/db_engine_specs/gsheets.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,11 @@ def validate_parameters(
except Exception: # pylint: disable=broad-except
errors.append(
SupersetError(
message="URL could not be identified",
message=(
"The URL could not be identified. Please check for typos "
"and make sure that ‘Type of Google Sheets allowed’ "
"selection matches the input."
),
error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR,
level=ErrorLevel.WARNING,
extra={"catalog": {"idx": idx, "url": True}},
Expand Down
18 changes: 15 additions & 3 deletions tests/unit_tests/db_engine_specs/test_gsheets.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,11 @@ def test_validate_parameters_catalog(

assert errors == [
SupersetError(
message="URL could not be identified",
message=(
"The URL could not be identified. Please check for typos "
"and make sure that ‘Type of Google Sheets allowed’ "
"selection matches the input."
),
error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR,
level=ErrorLevel.WARNING,
extra={
Expand All @@ -97,7 +101,11 @@ def test_validate_parameters_catalog(
},
),
SupersetError(
message="URL could not be identified",
message=(
"The URL could not be identified. Please check for typos "
"and make sure that ‘Type of Google Sheets allowed’ "
"selection matches the input."
),
error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR,
level=ErrorLevel.WARNING,
extra={
Expand Down Expand Up @@ -158,7 +166,11 @@ def test_validate_parameters_catalog_and_credentials(
errors = GSheetsEngineSpec.validate_parameters(parameters) # ignore: type
assert errors == [
SupersetError(
message="URL could not be identified",
message=(
"The URL could not be identified. Please check for typos "
"and make sure that ‘Type of Google Sheets allowed’ "
"selection matches the input."
),
error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR,
level=ErrorLevel.WARNING,
extra={
Expand Down

0 comments on commit ad71542

Please sign in to comment.