Skip to content

Commit

Permalink
feat: updated Error Alert (#15377)
Browse files Browse the repository at this point in the history
  • Loading branch information
@AAfghahi
AAfghahi authored Jun 25, 2021
1 parent 913b29c commit 56480ea
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 46 deletions.
84 changes: 67 additions & 17 deletions superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,24 @@ import {
} from './styles';
import ModalHeader, { DOCUMENTATION_LINK } from './ModalHeader';

const errorAlertMapping = {
CONNECTION_MISSING_PARAMETERS_ERROR: {
message: 'Missing Required Fields',
description: 'Please complete all required fields.',
},
CONNECTION_INVALID_HOSTNAME_ERROR: {
message: 'Could not verify the host',
description:
'The host is invalid. Please verify that this field is entered correctly.',
},
CONNECTION_PORT_CLOSED_ERROR: {
message: 'Port is closed',
description: 'Please verify that port is open to connect.',
},
CONNECTION_INVALID_PORT_ERROR: {
message: 'The port must be a whole number less than or equal to 65535.',
},
};
interface DatabaseModalProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
Expand Down Expand Up @@ -316,7 +334,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({

// Database fetch logic
const {
state: { loading: dbLoading, resource: dbFetched, error: dbError },
state: { loading: dbLoading, resource: dbFetched, error: dbErrors },
fetchResource,
createResource,
updateResource,
Expand All @@ -326,12 +344,15 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
addDangerToast,
);

const showDBError = validationErrors || dbErrors;
const isEmpty = (data?: Object | null) =>
data && Object.keys(data).length === 0;

const dbModel: DatabaseForm =
availableDbs?.databases?.find(
(available: { engine: string | undefined }) =>
// TODO: we need a centralized engine in one place
available.engine ===
(isEditMode || editNewDb ? db?.backend || db?.engine : db?.engine),
available.engine === (isEditMode ? db?.backend : db?.engine),
) || {};

// Test Connection logic
Expand Down Expand Up @@ -370,7 +391,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({

// Validate DB before saving
await getValidation(dbToUpdate, true);
if (validationErrors) {
if (validationErrors && !isEmpty(validationErrors)) {
return;
}

Expand Down Expand Up @@ -416,6 +437,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
const result = await updateResource(
db.id as number,
dbToUpdate as DatabaseObject,
true,
);
if (result) {
if (onDatabaseAdd) {
Expand Down Expand Up @@ -443,7 +465,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
});
}
setLoading(true);
const dbId = await createResource(dbToUpdate as DatabaseObject);
const dbId = await createResource(dbToUpdate as DatabaseObject, true);
if (dbId) {
setHasConnectedDb(true);
if (onDatabaseAdd) {
Expand Down Expand Up @@ -651,15 +673,44 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
setTabKey(key);
};

const errorAlert = () => (
<Alert
type="error"
css={(theme: SupersetTheme) => antDErrorAlertStyles(theme)}
message="Missing Required Fields"
description="Please complete all required fields."
showIcon
/>
);
const errorAlert = () => {
if (
isEmpty(dbErrors) ||
(isEmpty(validationErrors) &&
!(validationErrors?.error_type in errorAlertMapping))
) {
return <></>;
}

if (validationErrors) {
return (
<Alert
type="error"
css={(theme: SupersetTheme) => antDErrorAlertStyles(theme)}
message={
errorAlertMapping[validationErrors?.error_type]?.message ||
validationErrors?.error_type
}
description={
errorAlertMapping[validationErrors?.error_type]?.description ||
JSON.stringify(validationErrors)
}
showIcon
closable={false}
/>
);
}

const message: Array<string> = Object.values(dbErrors);
return (
<Alert
type="error"
css={(theme: SupersetTheme) => antDErrorAlertStyles(theme)}
message="Database Creation Error"
description={message[0]}
/>
);
};

const renderFinishState = () => {
if (!editNewDb) {
Expand Down Expand Up @@ -719,7 +770,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
}
getValidation={() => getValidation(db)}
validationErrors={validationErrors}
editNewDb={editNewDb}
/>
);
};
Expand Down Expand Up @@ -897,7 +947,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
onChange(ActionType.extraEditorChange, payload);
}}
/>
{dbError && errorAlert()}
{showDBError && errorAlert()}
</Tabs.TabPane>
</Tabs>
</Modal>
Expand Down Expand Up @@ -1023,7 +1073,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
/>
</div>
{/* Step 2 */}
{dbError && errorAlert()}
{showDBError && errorAlert()}
</>
))}
</>
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ export const antDAlertStyles = (theme: SupersetTheme) => css`
`;

export const antDErrorAlertStyles = (theme: SupersetTheme) => css`
border: ${theme.colors.error.base} 2px solid;
border: ${theme.colors.error.base} 1px solid;
padding: ${theme.gridUnit * 4}px;
margin: ${theme.gridUnit * 8}px ${theme.gridUnit * 4}px;
color: ${theme.colors.error.dark2};
Expand Down
60 changes: 35 additions & 25 deletions superset-frontend/src/views/CRUD/hooks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ export function useListViewResource<D extends object = any>(
interface SingleViewResourceState<D extends object = any> {
loading: boolean;
resource: D | null;
error: string | Record<string, string[] | string> | null;
error: any | null;
}

export function useSingleViewResource<D extends object = any>(
Expand Down Expand Up @@ -269,7 +269,7 @@ export function useSingleViewResource<D extends object = any>(
);

const createResource = useCallback(
(resource: D) => {
(resource: D, hideToast = false) => {
// Set loading state
updateState({
loading: true,
Expand All @@ -289,13 +289,16 @@ export function useSingleViewResource<D extends object = any>(
return json.id;
},
createErrorHandler((errMsg: Record<string, string[] | string>) => {
handleErrorMsg(
t(
'An error occurred while creating %ss: %s',
resourceLabel,
parsedErrorMessage(errMsg),
),
);
// we did not want toasts for db-connection-ui but did not want to disable it everywhere
if (!hideToast) {
handleErrorMsg(
t(
'An error occurred while creating %ss: %s',
resourceLabel,
parsedErrorMessage(errMsg),
),
);
}

updateState({
error: errMsg,
Expand All @@ -310,7 +313,7 @@ export function useSingleViewResource<D extends object = any>(
);

const updateResource = useCallback(
(resourceID: number, resource: D) => {
(resourceID: number, resource: D, hideToast = false) => {
// Set loading state
updateState({
loading: true,
Expand All @@ -330,13 +333,15 @@ export function useSingleViewResource<D extends object = any>(
return json.result;
},
createErrorHandler(errMsg => {
handleErrorMsg(
t(
'An error occurred while fetching %ss: %s',
resourceLabel,
JSON.stringify(errMsg),
),
);
if (!hideToast) {
handleErrorMsg(
t(
'An error occurred while fetching %ss: %s',
resourceLabel,
JSON.stringify(errMsg),
),
);
}

updateState({
error: errMsg,
Expand Down Expand Up @@ -651,20 +656,20 @@ export function useDatabaseValidation() {
if (typeof e.json === 'function') {
e.json().then(({ errors = [] }: JsonObject) => {
const parsedErrors = errors
.filter((error: { error_type: string }) => {
const skipValidationError = ![
'CONNECTION_MISSING_PARAMETERS_ERROR',
'CONNECTION_ACCESS_DENIED_ERROR',
].includes(error.error_type);
return skipValidationError || onCreate;
})
.filter(
(error: { error_type: string }) =>
error.error_type !==
'CONNECTION_MISSING_PARAMETERS_ERROR' || onCreate,
)
.reduce(
(
obj: {},
{
error_type,
extra,
message,
}: {
error_type: string;
extra: { invalid?: string[]; missing?: string[] };
message: string;
},
Expand All @@ -673,11 +678,16 @@ export function useDatabaseValidation() {
// error can't be mapped to a parameter
// so leave it alone
if (extra.invalid) {
return { ...obj, [extra.invalid[0]]: message };
return {
...obj,
[extra.invalid[0]]: message,
error_type,
};
}
if (extra.missing) {
return {
...obj,
error_type,
...Object.assign(
{},
...extra.missing.map(field => ({
Expand Down
2 changes: 1 addition & 1 deletion superset/databases/commands/exceptions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class DatabaseExistsValidationError(ValidationError):

def __init__(self) -> None:
super().__init__(
_("A database with the same name already exists"),
_("A database with the same name already exists."),
field_name="database_name",
)

Expand Down
8 changes: 6 additions & 2 deletions tests/databases/api_tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,9 @@ def test_create_database_unique_validate(self):
rv = self.client.post(uri, json=database_data)
response = json.loads(rv.data.decode("utf-8"))
expected_response = {
"message": {"database_name": "A database with the same name already exists"}
"message": {
"database_name": "A database with the same name already exists."
}
}
self.assertEqual(rv.status_code, 422)
self.assertEqual(response, expected_response)
Expand Down Expand Up @@ -566,7 +568,9 @@ def test_update_database_uniqueness(self):
rv = self.client.put(uri, json=database_data)
response = json.loads(rv.data.decode("utf-8"))
expected_response = {
"message": {"database_name": "A database with the same name already exists"}
"message": {
"database_name": "A database with the same name already exists."
}
}
self.assertEqual(rv.status_code, 422)
self.assertEqual(response, expected_response)
Expand Down

0 comments on commit 56480ea

Please sign in to comment.