You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by hu...@apache.org on 2021/05/25 20:30:55 UTC
[superset] branch hugh/bg-validation-db-modal created (now e764072)
This is an automated email from the ASF dual-hosted git repository.
hugh pushed a change to branch hugh/bg-validation-db-modal
in repository https://gitbox.apache.org/repos/asf/superset.git.
at e764072 use new validation component
This branch includes the following new commits:
new e8cd4f4 split db modal file
new 95917e4 hook up available databases
new e764072 use new validation component
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[superset] 03/03: use new validation component
Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
hugh pushed a commit to branch hugh/bg-validation-db-modal
in repository https://gitbox.apache.org/repos/asf/superset.git
commit e7640724877caf356c114659f1d09e5e2691cf9f
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Fri May 21 10:57:08 2021 -0700
use new validation component
---
.../cypress/integration/database/modal.test.ts | 24 +++
superset-frontend/src/components/Form/FormItem.tsx | 4 +
.../src/components/Form/LabeledErrorBoundInput.tsx | 18 +-
.../DatabaseModal/DatabaseConnectionForm.tsx | 240 +++++++++++++--------
.../data/database/DatabaseModal/index.test.jsx | 2 +-
.../CRUD/data/database/DatabaseModal/index.tsx | 33 +--
.../CRUD/data/database/DatabaseModal/styles.ts | 33 ++-
.../src/views/CRUD/data/database/types.ts | 3 +-
superset-frontend/src/views/CRUD/hooks.ts | 49 +++++
superset/databases/commands/validate.py | 6 +-
superset/databases/schemas.py | 6 +
superset/db_engine_specs/base.py | 6 +-
12 files changed, 284 insertions(+), 140 deletions(-)
diff --git a/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts b/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts
index e29ec48..cf27d21 100644
--- a/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts
+++ b/superset-frontend/cypress-base/cypress/integration/database/modal.test.ts
@@ -88,4 +88,28 @@ describe('Add database', () => {
// cy.wait(1000); // wait for potential (incorrect) closing annimation
// cy.get('[data-test="database-modal"]').should('be.visible');
});
+
+ it('should close modal after save', () => {
+ cy.get('[data-test="btn-create-database"]').click();
+
+ // type values
+ cy.get('[data-test="database-modal"] input[name="database_name"]')
+ .focus()
+ .type('cypress');
+ cy.get('[data-test="database-modal"] input[name="sqlalchemy_uri"]')
+ .focus()
+ .type('gsheets://');
+
+ // click save
+ cy.get('[data-test="modal-confirm-button"]:not(:disabled)').click();
+
+ // should show error alerts and keep modal open
+ cy.get('.toast').contains('error');
+ cy.wait(1000); // wait for potential (incorrect) closing annimation
+ cy.get('[data-test="database-modal"]').should('be.visible');
+
+ // should be able to close modal
+ cy.get('[data-test="modal-cancel-button"]').click();
+ cy.get('[data-test="database-modal"]').should('not.be.visible');
+ });
});
diff --git a/superset-frontend/src/components/Form/FormItem.tsx b/superset-frontend/src/components/Form/FormItem.tsx
index ab301a8..2731a21 100644
--- a/superset-frontend/src/components/Form/FormItem.tsx
+++ b/superset-frontend/src/components/Form/FormItem.tsx
@@ -41,6 +41,10 @@ const StyledItem = styled(Form.Item)`
}
}
}
+ .ant-form-item-explain {
+ color: ${theme.colors.grayscale.light1};
+ font-size: ${theme.typography.sizes.s - 1}px;
+ }
`}
`;
diff --git a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx
index 8569b55..75df2bb 100644
--- a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx
+++ b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx
@@ -25,17 +25,18 @@ import FormLabel from './FormLabel';
export interface LabeledErrorBoundInputProps {
label?: string;
validationMethods:
- | { onBlur: (value: any) => string }
- | { onChange: (value: any) => string };
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
errorMessage: string | null;
helpText?: string;
required?: boolean;
id?: string;
+ classname?: string;
[x: string]: any;
}
const StyledInput = styled(Input)`
- margin: 8px 0;
+ margin: ${({ theme }) => `${theme.gridUnit}px 0 ${theme.gridUnit * 2}px`};
`;
const alertIconStyles = (theme: SupersetTheme, hasError: boolean) => css`
@@ -60,6 +61,12 @@ const alertIconStyles = (theme: SupersetTheme, hasError: boolean) => css`
}
}`}
`;
+const StyledFormGroup = styled('div')`
+ margin-bottom: ${({ theme }) => theme.gridUnit * 5}px;
+ .ant-form-item {
+ margin-bottom: 0;
+ }
+`;
const LabeledErrorBoundInput = ({
label,
@@ -68,9 +75,10 @@ const LabeledErrorBoundInput = ({
helpText,
required = false,
id,
+ className,
...props
}: LabeledErrorBoundInputProps) => (
- <>
+ <StyledFormGroup className={className}>
<FormLabel htmlFor={id} required={required}>
{label}
</FormLabel>
@@ -83,7 +91,7 @@ const LabeledErrorBoundInput = ({
>
<StyledInput {...props} {...validationMethods} />
</FormItem>
- </>
+ </StyledFormGroup>
);
export default LabeledErrorBoundInput;
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 f0542e4..5c7c729 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx
@@ -17,11 +17,14 @@
* under the License.
*/
import React, { FormEvent } from 'react';
-import cx from 'classnames';
+import { SupersetTheme, JsonObject } from '@superset-ui/core';
import { InputProps } from 'antd/lib/input';
-import { FormLabel, FormItem } from 'src/components/Form';
-import { Input } from 'src/common/components';
-import { StyledFormHeader, formScrollableStyles } from './styles';
+import ValidatedInput from 'src/components/Form/LabeledErrorBoundInput';
+import {
+ StyledFormHeader,
+ formScrollableStyles,
+ validatedFormStyles,
+} from './styles';
import { DatabaseForm } from '../types';
export const FormFieldOrder = [
@@ -33,64 +36,137 @@ export const FormFieldOrder = [
'database_name',
];
-const CHANGE_METHOD = {
- onChange: 'onChange',
- onPropertiesChange: 'onPropertiesChange',
-};
+interface FieldPropTypes {
+ required: boolean;
+ changeMethods: { onParametersChange: (value: any) => string } & {
+ onChange: (value: any) => string;
+ };
+ validationErrors: JsonObject | null;
+ getValidation: () => void;
+}
+
+const hostField = ({
+ required,
+ changeMethods,
+ getValidation,
+ validationErrors,
+}: FieldPropTypes) => (
+ <ValidatedInput
+ id="host"
+ name="host"
+ required={required}
+ validationMethods={{ onBlur: getValidation }}
+ errorMessage={validationErrors?.host}
+ placeholder="e.g. 127.0.0.1"
+ className="form-group-w-50"
+ label="Host"
+ onChange={changeMethods.onParametersChange}
+ />
+);
+const portField = ({
+ required,
+ changeMethods,
+ getValidation,
+ validationErrors,
+}: FieldPropTypes) => (
+ <ValidatedInput
+ id="port"
+ name="port"
+ required={required}
+ validationMethods={{ onBlur: getValidation }}
+ errorMessage={validationErrors?.port}
+ placeholder="e.g. 5432"
+ className="form-group-w-50"
+ label="Port"
+ onChange={changeMethods.onParametersChange}
+ />
+);
+const databaseField = ({
+ required,
+ changeMethods,
+ getValidation,
+ validationErrors,
+}: FieldPropTypes) => (
+ <ValidatedInput
+ id="database"
+ name="database"
+ required={required}
+ validationMethods={{ onBlur: getValidation }}
+ errorMessage={validationErrors?.database}
+ placeholder="e.g. world_population"
+ label="Database name"
+ onChange={changeMethods.onParametersChange}
+ helpText="Copy the name of the PostgreSQL database you are trying to connect to."
+ />
+);
+const usernameField = ({
+ required,
+ changeMethods,
+ getValidation,
+ validationErrors,
+}: FieldPropTypes) => (
+ <ValidatedInput
+ id="username"
+ name="username"
+ required={required}
+ validationMethods={{ onBlur: getValidation }}
+ errorMessage={validationErrors?.username}
+ placeholder="e.g. Analytics"
+ label="Username"
+ onChange={changeMethods.onParametersChange}
+ />
+);
+const passwordField = ({
+ required,
+ changeMethods,
+ getValidation,
+ validationErrors,
+}: FieldPropTypes) => (
+ <ValidatedInput
+ id="password"
+ name="password"
+ required={required}
+ validationMethods={{ onBlur: getValidation }}
+ errorMessage={validationErrors?.password}
+ placeholder="e.g. ********"
+ label="Password"
+ onChange={changeMethods.onParametersChange}
+ />
+);
+const displayField = ({
+ required,
+ changeMethods,
+ getValidation,
+ validationErrors,
+}: FieldPropTypes) => (
+ <ValidatedInput
+ id="database_name"
+ name="database_name"
+ required={required}
+ validationMethods={{ onBlur: getValidation }}
+ errorMessage={validationErrors?.database_name}
+ placeholder=""
+ label="Display Name"
+ onChange={changeMethods.onChange}
+ helpText="Pick a nickname for this database to display as in Superset."
+ />
+);
const FORM_FIELD_MAP = {
- host: {
- description: 'Host',
- type: 'text',
- className: 'w-50',
- placeholder: 'e.g. 127.0.0.1',
- changeMethod: CHANGE_METHOD.onPropertiesChange,
- },
- port: {
- description: 'Port',
- type: 'text',
- className: 'w-50',
- placeholder: 'e.g. 5432',
- changeMethod: CHANGE_METHOD.onPropertiesChange,
- },
- database: {
- description: 'Database name',
- type: 'text',
- label:
- 'Copy the name of the PostgreSQL database you are trying to connect to.',
- placeholder: 'e.g. world_population',
- changeMethod: CHANGE_METHOD.onPropertiesChange,
- },
- username: {
- description: 'Username',
- type: 'text',
- placeholder: 'e.g. Analytics',
- changeMethod: CHANGE_METHOD.onPropertiesChange,
- },
- password: {
- description: 'Password',
- type: 'text',
- placeholder: 'e.g. ********',
- changeMethod: CHANGE_METHOD.onPropertiesChange,
- },
- database_name: {
- description: 'Display Name',
- type: 'text',
- label: 'Pick a nickname for this database to display as in Superset.',
- changeMethod: CHANGE_METHOD.onChange,
- },
- query: {
- additionalProperties: {},
- description: 'Additional parameters',
- type: 'object',
- changeMethod: CHANGE_METHOD.onPropertiesChange,
- },
+ host: hostField,
+ port: portField,
+ database: databaseField,
+ username: usernameField,
+ password: passwordField,
+ database_name: displayField,
};
const DatabaseConnectionForm = ({
dbModel: { name, parameters },
onParametersChange,
onChange,
+ validationErrors,
+ getValidation,
}: {
dbModel: DatabaseForm;
onParametersChange: (
@@ -99,6 +175,8 @@ const DatabaseConnectionForm = ({
onChange: (
event: FormEvent<InputProps> | { target: HTMLInputElement },
) => void;
+ validationErrors: JsonObject | null;
+ getValidation: () => void;
}) => (
<>
<StyledFormHeader>
@@ -107,52 +185,30 @@ const DatabaseConnectionForm = ({
Need help? Learn more about connecting to {name}.
</p>
</StyledFormHeader>
- <div css={formScrollableStyles}>
+ <div
+ // @ts-ignore
+ css={(theme: SupersetTheme) => [
+ formScrollableStyles,
+ validatedFormStyles(theme),
+ ]}
+ >
{parameters &&
FormFieldOrder.filter(
(key: string) =>
Object.keys(parameters.properties).includes(key) ||
key === 'database_name',
- ).map(field => {
- const {
- className,
- description,
- type,
- placeholder,
- label,
- changeMethod,
- } = FORM_FIELD_MAP[field];
- const onEdit =
- changeMethod === CHANGE_METHOD.onChange
- ? onChange
- : onParametersChange;
- return (
- <FormItem
- className={cx(className, `form-group-${className}`)}
- key={field}
- >
- <FormLabel
- htmlFor={field}
- required={parameters.required.includes(field)}
- >
- {description}
- </FormLabel>
- <Input
- name={field}
- type={type}
- id={field}
- autoComplete="off"
- placeholder={placeholder}
- onChange={onEdit}
- />
- <p className="helper">{label}</p>
- </FormItem>
- );
- })}
+ ).map(field =>
+ FORM_FIELD_MAP[field]({
+ required: parameters.required.includes(field),
+ changeMethods: { onParametersChange, onChange },
+ validationErrors,
+ getValidation,
+ key: field,
+ }),
+ )}
</div>
</>
);
-
export const FormFieldMap = FORM_FIELD_MAP;
export default DatabaseConnectionForm;
diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx
index 8696725..3f10914 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx
@@ -336,7 +336,7 @@ describe('DatabaseModal', () => {
await screen.findByText(/display name/i);
// it does not fetch any databases if no id is passed in
- expect(fetchMock.calls().length).toEqual(0);
+ expect(fetchMock.calls(DATABASE_FETCH_ENDPOINT).length).toEqual(0);
// todo we haven't hooked this up to load dynamically yet so
// we can't currently test it
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 57104b8..265a856 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
@@ -33,6 +33,7 @@ import {
testDatabaseConnection,
useSingleViewResource,
useAvailableDatabases,
+ useDatabaseValidation,
} from 'src/views/CRUD/hooks';
import { useCommonConf } from 'src/views/CRUD/data/database/state';
import {
@@ -50,10 +51,9 @@ import {
antDModalStyles,
antDTabsStyles,
buttonLinkStyles,
- CreateHeader,
+ TabHeader,
CreateHeaderSubtitle,
CreateHeaderTitle,
- EditHeader,
EditHeaderSubtitle,
EditHeaderTitle,
formHelperStyles,
@@ -109,7 +109,7 @@ type DBReducerActionType =
| {
type: ActionType.dbSelected;
payload: {
- parameters: { engine?: string };
+ engine?: string;
configuration_method: CONFIGURATION_METHOD;
};
}
@@ -127,8 +127,6 @@ function dbReducer(
): Partial<DatabaseObject> | null {
const trimmedState = {
...(state || {}),
- database_name: state?.database_name?.trim() || '',
- sqlalchemy_uri: state?.sqlalchemy_uri || '',
};
switch (action.type) {
@@ -163,9 +161,7 @@ function dbReducer(
};
case ActionType.fetched:
return {
- parameters: {
- engine: trimmedState.parameters?.engine,
- },
+ engine: trimmedState.engine,
configuration_method: trimmedState.configuration_method,
...action.payload,
};
@@ -196,13 +192,16 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
>(dbReducer, null);
const [tabKey, setTabKey] = useState<string>(DEFAULT_TAB_KEY);
const [availableDbs, getAvailableDbs] = useAvailableDatabases();
+ const [validationErrors, getValidation] = useDatabaseValidation();
const [hasConnectedDb, setHasConnectedDb] = useState<boolean>(false);
+ const [dbName, setDbName] = useState('');
const conf = useCommonConf();
const isEditMode = !!databaseId;
const useSqlAlchemyForm =
db?.configuration_method === CONFIGURATION_METHOD.SQLALCHEMY_URI;
const useTabLayout = isEditMode || useSqlAlchemyForm;
+
// Database fetch logic
const {
state: { loading: dbLoading, resource: dbFetched },
@@ -302,7 +301,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
setDB({
type: ActionType.dbSelected,
payload: {
- parameters: {},
configuration_method: CONFIGURATION_METHOD.SQLALCHEMY_URI,
}, // todo hook this up to step 1
});
@@ -318,6 +316,9 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
type: ActionType.fetched,
payload: dbFetched,
});
+ // keep a copy of the name separate for display purposes
+ // because it shouldn't change when the form is updated
+ setDbName(dbFetched.database_name);
}
}, [dbFetched]);
@@ -328,7 +329,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
const dbModel: DatabaseForm =
availableDbs?.databases?.find(
(available: { engine: string | undefined }) =>
- available.engine === db?.parameters?.engine,
+ available.engine === db?.engine,
) || {};
const disableSave =
@@ -364,12 +365,12 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
}
>
{isEditMode ? (
- <EditHeader>
+ <TabHeader>
<EditHeaderTitle>{db?.backend}</EditHeaderTitle>
- <EditHeaderSubtitle>{db?.database_name}</EditHeaderSubtitle>
- </EditHeader>
+ <EditHeaderSubtitle>{dbName}</EditHeaderSubtitle>
+ </TabHeader>
) : (
- <CreateHeader>
+ <TabHeader>
<CreateHeaderTitle>Enter Primary Credentials</CreateHeaderTitle>
<CreateHeaderSubtitle>
Need help? Learn how to connect your database{' '}
@@ -382,7 +383,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
</a>
.
</CreateHeaderSubtitle>
- </CreateHeader>
+ </TabHeader>
)}
<hr />
<Tabs
@@ -514,6 +515,8 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
value: target.value,
})
}
+ getValidation={() => getValidation(db)}
+ validationErrors={validationErrors}
/>
<Button
buttonStyle="link"
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 4ae5629..a02a228 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts
@@ -164,13 +164,6 @@ export const formStyles = (theme: SupersetTheme) => css`
margin-left: ${theme.gridUnit * 8}px;
}
}
- .text-danger {
- color: ${theme.colors.error.base};
- font-size: ${theme.typography.sizes.s - 1}px;
- strong {
- font-weight: normal;
- }
- }
}
.control-label {
color: ${theme.colors.grayscale.dark1};
@@ -187,6 +180,14 @@ export const formStyles = (theme: SupersetTheme) => css`
}
`;
+export const validatedFormStyles = (theme: SupersetTheme) => css`
+ label {
+ color: ${theme.colors.grayscale.dark1};
+ font-size: ${theme.typography.sizes.s - 1}px;
+ margin-bottom: 0;
+ }
+`;
+
export const StyledInputContainer = styled.div`
margin-bottom: ${({ theme }) => theme.gridUnit * 6}px;
&.mb-0 {
@@ -306,23 +307,13 @@ export const buttonLinkStyles = css`
text-transform: initial;
`;
-export const EditHeader = styled.div`
- display: flex;
- flex-direction: column;
- justify-content: center;
- padding: 0px;
- margin: ${({ theme }) => theme.gridUnit * 4}px
- ${({ theme }) => theme.gridUnit * 4}px
- ${({ theme }) => theme.gridUnit * 9}px;
-`;
-
-export const CreateHeader = styled.div`
+export const TabHeader = styled.div`
display: flex;
flex-direction: column;
justify-content: center;
padding: 0px;
margin: 0 ${({ theme }) => theme.gridUnit * 4}px
- ${({ theme }) => theme.gridUnit * 6}px;
+ ${({ theme }) => theme.gridUnit * 8}px;
`;
export const CreateHeaderTitle = styled.div`
@@ -339,12 +330,12 @@ export const CreateHeaderSubtitle = styled.div`
export const EditHeaderTitle = styled.div`
color: ${({ theme }) => theme.colors.grayscale.light1};
- font-size: ${({ theme }) => theme.typography.sizes.s}px;
+ font-size: ${({ theme }) => theme.typography.sizes.s - 1}px;
text-transform: uppercase;
`;
export const EditHeaderSubtitle = styled.div`
color: ${({ theme }) => theme.colors.grayscale.dark1};
- font-size: ${({ theme }) => theme.typography.sizes.xl}px;
+ font-size: ${({ theme }) => theme.typography.sizes.l}px;
font-weight: bold;
`;
diff --git a/superset-frontend/src/views/CRUD/data/database/types.ts b/superset-frontend/src/views/CRUD/data/database/types.ts
index 2c386b5..1baac06 100644
--- a/superset-frontend/src/views/CRUD/data/database/types.ts
+++ b/superset-frontend/src/views/CRUD/data/database/types.ts
@@ -30,8 +30,9 @@ export type DatabaseObject = {
created_by?: null | DatabaseUser;
changed_on_delta_humanized?: string;
changed_on?: string;
- parameters?: { database_name?: string; engine?: string };
+ parameters?: { database_name?: string };
configuration_method: CONFIGURATION_METHOD;
+ engine?: string;
// Performance
cache_timeout?: string;
diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts
index 4275499..0c08030 100644
--- a/superset-frontend/src/views/CRUD/hooks.ts
+++ b/superset-frontend/src/views/CRUD/hooks.ts
@@ -657,3 +657,52 @@ export function useAvailableDatabases() {
return [availableDbs, getAvailable] as const;
}
+
+export function useDatabaseValidation() {
+ const [validationErrors, setValidationErrors] = useState<JsonObject | null>(
+ null,
+ );
+ const getValidation = useCallback(
+ (database: Partial<DatabaseObject> | null) => {
+ SupersetClient.post({
+ endpoint: '/api/v1/database/validate_parameters',
+ body: JSON.stringify(database),
+ headers: { 'Content-Type': 'application/json' },
+ })
+ .then(() => {
+ setValidationErrors(null);
+ })
+ .catch(e => {
+ 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',
+ )
+ .reduce(
+ (
+ obj: {},
+ {
+ extra,
+ message,
+ }: {
+ extra: { invalid: string[] };
+ message: string;
+ },
+ ) => ({ ...obj, [extra.invalid[0]]: message }),
+ {},
+ );
+ setValidationErrors(parsedErrors);
+ });
+ } else {
+ // eslint-disable-next-line no-console
+ console.error(e);
+ }
+ });
+ },
+ [setValidationErrors],
+ );
+
+ return [validationErrors, getValidation] as const;
+}
diff --git a/superset/databases/commands/validate.py b/superset/databases/commands/validate.py
index 93e32ef..2f084f6 100644
--- a/superset/databases/commands/validate.py
+++ b/superset/databases/commands/validate.py
@@ -78,7 +78,9 @@ class ValidateDatabaseParametersCommand(BaseCommand):
)
# perform initial validation
- errors = engine_spec.validate_parameters(self._properties["parameters"])
+ errors = engine_spec.validate_parameters(
+ self._properties.get("parameters", None)
+ )
if errors:
raise InvalidParametersError(errors)
@@ -90,7 +92,7 @@ class ValidateDatabaseParametersCommand(BaseCommand):
# try to connect
sqlalchemy_uri = engine_spec.build_sqlalchemy_uri(
- self._properties["parameters"], # type: ignore
+ self._properties.get("parameters", None), # type: ignore
encrypted_extra,
)
if self._model and sqlalchemy_uri == self._model.safe_sqlalchemy_uri():
diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py
index e952967..a9ea868 100644
--- a/superset/databases/schemas.py
+++ b/superset/databases/schemas.py
@@ -307,6 +307,12 @@ class DatabaseValidateParametersSchema(Schema):
allow_none=True,
validate=server_cert_validator,
)
+ configuration_method = EnumField(
+ ConfigurationMethod,
+ by_value=True,
+ allow_none=True,
+ description=configuration_method_description,
+ )
class DatabasePostSchema(Schema, DatabaseParametersSchemaMixin):
diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py
index 97321b8..593b17a 100644
--- a/superset/db_engine_specs/base.py
+++ b/superset/db_engine_specs/base.py
@@ -1400,7 +1400,7 @@ class BasicParametersMixin:
errors: List[SupersetError] = []
required = {"host", "port", "username", "database"}
- present = {key for key in parameters if parameters[key]} # type: ignore
+ present = {key for key in parameters if parameters.get(key, ())} # type: ignore
missing = sorted(required - present)
if missing:
@@ -1413,7 +1413,7 @@ class BasicParametersMixin:
),
)
- host = parameters["host"]
+ host = parameters.get("host", None)
if not host:
return errors
if not is_hostname_valid(host):
@@ -1427,7 +1427,7 @@ class BasicParametersMixin:
)
return errors
- port = parameters["port"]
+ port = parameters.get("port", None)
if not port:
return errors
if not is_port_open(host, port):
[superset] 01/03: split db modal file
Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
hugh pushed a commit to branch hugh/bg-validation-db-modal
in repository https://gitbox.apache.org/repos/asf/superset.git
commit e8cd4f4cab29ab7dfc9706c0dba6920f5115a513
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Wed Apr 21 12:18:55 2021 -0700
split db modal file
---
superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
index d1105e7..fda65ba 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
@@ -29,8 +29,9 @@ import { Tooltip } from 'src/components/Tooltip';
import Icons from 'src/components/Icons';
import ListView, { FilterOperator, Filters } from 'src/components/ListView';
import { commonMenuData } from 'src/views/CRUD/data/common';
-import DatabaseModal from 'src/views/CRUD/data/database/DatabaseModal';
import ImportModelsModal from 'src/components/ImportModal/index';
+import DatabaseModal from './DatabaseModal';
+
import { DatabaseObject } from './types';
const PAGE_SIZE = 25;
[superset] 02/03: hook up available databases
Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
hugh pushed a commit to branch hugh/bg-validation-db-modal
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 95917e40eb6d833ef783a59f80734b539cf4e5da
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Tue Apr 27 17:08:49 2021 -0700
hook up available databases
---
.../data/database/DatabaseModal/index.test.jsx | 67 ++++++++++++++++++++++
.../CRUD/data/database/DatabaseModal/index.tsx | 16 +++---
.../CRUD/data/database/DatabaseModal/styles.ts | 4 --
3 files changed, 76 insertions(+), 11 deletions(-)
diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx
index 1fc51e4..8696725 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx
@@ -319,5 +319,72 @@ describe('DatabaseModal', () => {
const todoText = screen.getAllByText(/todo/i);
expect(todoText[0]).toBeVisible();
});
+
+ describe('create database', () => {
+ it('should show a form when dynamic_form is selected', async () => {
+ const props = {
+ ...dbProps,
+ databaseId: null,
+ database_name: null,
+ sqlalchemy_uri: null,
+ };
+ render(<DatabaseModal {...props} />, { useRedux: true });
+ // it should have the correct header text
+ const headerText = screen.getByText(/connect a database/i);
+ expect(headerText).toBeVisible();
+
+ await screen.findByText(/display name/i);
+
+ // it does not fetch any databases if no id is passed in
+ expect(fetchMock.calls().length).toEqual(0);
+
+ // todo we haven't hooked this up to load dynamically yet so
+ // we can't currently test it
+ });
+ });
+
+ describe('edit database', () => {
+ it('renders the sqlalchemy form when the sqlalchemy_form configuration method is set', async () => {
+ render(<DatabaseModal {...dbProps} />, { useRedux: true });
+
+ // it should have tabs
+ const tabs = screen.getAllByRole('tab');
+ expect(tabs.length).toEqual(2);
+ expect(tabs[0]).toHaveTextContent('Basic');
+ expect(tabs[1]).toHaveTextContent('Advanced');
+
+ // it should have the correct header text
+ const headerText = screen.getByText(/edit database/i);
+ expect(headerText).toBeVisible();
+
+ // todo add more when this form is built out
+ });
+ it('renders the dynamic form when the dynamic_form configuration method is set', async () => {
+ fetchMock.get(DATABASE_FETCH_ENDPOINT, {
+ result: {
+ id: 10,
+ database_name: 'my database',
+ expose_in_sqllab: false,
+ allow_ctas: false,
+ allow_cvas: false,
+ configuration_method: 'dynamic_form',
+ parameters: {
+ database: 'mydatabase',
+ },
+ },
+ });
+ render(<DatabaseModal {...dbProps} />, { useRedux: true });
+
+ await screen.findByText(/todo/i);
+
+ // // it should have tabs
+ const tabs = screen.getAllByRole('tab');
+ expect(tabs.length).toEqual(2);
+
+ // it should show a TODO for now
+ const todoText = screen.getAllByText(/todo/i);
+ expect(todoText[0]).toBeVisible();
+ });
+ });
});
});
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 545cba0..57104b8 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
@@ -248,14 +248,16 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
// don't pass parameters if using the sqlalchemy uri
delete update.parameters;
}
- updateResource(db.id as number, update as DatabaseObject).then(result => {
- if (result) {
- if (onDatabaseAdd) {
- onDatabaseAdd();
- }
- onClose();
+ const result = await updateResource(
+ db.id as number,
+ update as DatabaseObject,
+ );
+ if (result) {
+ if (onDatabaseAdd) {
+ onDatabaseAdd();
}
- });
+ onClose();
+ }
} else if (db) {
// Create
const dbId = await createResource(update as DatabaseObject);
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 8c33756..4ae5629 100644
--- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts
+++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts
@@ -181,10 +181,6 @@ export const formStyles = (theme: SupersetTheme) => css`
font-size: ${theme.typography.sizes.s - 1}px;
margin-top: ${theme.gridUnit * 1.5}px;
}
- .ant-modal-body {
- padding-top: 0;
- margin-bottom: 0;
- }
.ant-tabs-content-holder {
overflow: auto;
max-height: 475px;