diff --git a/superset-frontend/cypress-base/cypress/e2e/database/modal.test.ts b/superset-frontend/cypress-base/cypress/e2e/database/modal.test.ts index 87a0bb92304..340fb7392c5 100644 --- a/superset-frontend/cypress-base/cypress/e2e/database/modal.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/database/modal.test.ts @@ -32,53 +32,87 @@ describe('Add database', () => { }); beforeEach(() => { + cy.intercept('POST', '**/api/v1/database/validate_parameters/**').as( + 'validateParams', + ); + cy.intercept('POST', '**/api/v1/database/').as('createDb'); + closeModal(); cy.getBySel('btn-create-database').click(); }); it('should open dynamic form', () => { - // click postgres dynamic form cy.get('.preferred > :nth-child(1)').click(); - // make sure all the fields are rendering cy.get('input[name="host"]').should('have.value', ''); cy.get('input[name="port"]').should('have.value', ''); cy.get('input[name="database"]').should('have.value', ''); + cy.get('input[name="username"]').should('have.value', ''); cy.get('input[name="password"]').should('have.value', ''); cy.get('input[name="database_name"]').should('have.value', ''); }); it('should open sqlalchemy form', () => { - // click postgres dynamic form cy.get('.preferred > :nth-child(1)').click(); - cy.getBySel('sqla-connect-btn').click(); - // check if the sqlalchemy form is showing up cy.getBySel('database-name-input').should('be.visible'); cy.getBySel('sqlalchemy-uri-input').should('be.visible'); }); it('show error alerts on dynamic form for bad host', () => { - // click postgres dynamic form cy.get('.preferred > :nth-child(1)').click(); - cy.get('input[name="host"]').focus(); - cy.focused().type('badhost', { force: true }); - cy.get('input[name="port"]').focus(); - cy.focused().type('5432', { force: true }); - cy.get('.ant-form-item-explain-error').contains( - "The hostname provided can't be resolved", - ); + + cy.get('input[name="host"]').type('badhost', { force: true }); + cy.get('input[name="port"]').type('5432', { force: true }); + cy.get('input[name="username"]').type('testusername', { force: true }); + cy.get('input[name="database"]').type('testdb', { force: true }); + cy.get('input[name="password"]').type('testpass', { force: true }); + + cy.get('body').click(0, 0); + + cy.wait('@validateParams', { timeout: 30000 }); + + cy.getBySel('btn-submit-connection').should('not.be.disabled'); + cy.getBySel('btn-submit-connection').click({ force: true }); + + cy.wait('@validateParams', { timeout: 30000 }).then(() => { + cy.wait('@createDb', { timeout: 60000 }).then(() => { + cy.contains( + '.ant-form-item-explain-error', + "The hostname provided can't be resolved", + ).should('exist'); + }); + }); }); it('show error alerts on dynamic form for bad port', () => { - // click postgres dynamic form cy.get('.preferred > :nth-child(1)').click(); - cy.get('input[name="host"]').focus(); - cy.focused().type('localhost', { force: true }); - cy.get('input[name="port"]').focus(); - cy.focused().type('123', { force: true }); - cy.get('input[name="database"]').focus(); - cy.get('.ant-form-item-explain-error').contains('The port is closed'); + + cy.get('input[name="host"]').type('localhost', { force: true }); + cy.get('body').click(0, 0); + cy.wait('@validateParams', { timeout: 30000 }); + + cy.get('input[name="port"]').type('5430', { force: true }); + cy.get('input[name="database"]').type('testdb', { force: true }); + cy.get('input[name="username"]').type('testusername', { force: true }); + + cy.wait('@validateParams', { timeout: 30000 }); + + cy.get('input[name="password"]').type('testpass', { force: true }); + cy.wait('@validateParams'); + + cy.getBySel('btn-submit-connection').should('not.be.disabled'); + cy.getBySel('btn-submit-connection').click({ force: true }); + cy.wait('@validateParams', { timeout: 30000 }).then(() => { + cy.get('body').click(0, 0); + cy.getBySel('btn-submit-connection').click({ force: true }); + cy.wait('@createDb', { timeout: 60000 }).then(() => { + cy.contains( + '.ant-form-item-explain-error', + 'The port is closed', + ).should('exist'); + }); + }); }); }); diff --git a/superset-frontend/packages/superset-ui-core/src/components/Form/LabeledErrorBoundInput.tsx b/superset-frontend/packages/superset-ui-core/src/components/Form/LabeledErrorBoundInput.tsx index 77d5a1c5550..b768c764cce 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Form/LabeledErrorBoundInput.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Form/LabeledErrorBoundInput.tsx @@ -16,9 +16,8 @@ * specific language governing permissions and limitations * under the License. */ -import { styled, css, SupersetTheme, t } from '../..'; -import { error as errorIcon } from '../assets/svgs'; -import { Button, Icons, InfoTooltip, Tooltip } from '..'; +import { styled, t } from '@superset-ui/core'; +import { Button, Icons, InfoTooltip, Tooltip, Flex } from '..'; import { Input } from '../Input'; import { FormLabel } from './FormLabel'; import { FormItem } from './FormItem'; @@ -32,28 +31,6 @@ const StyledInputPassword = styled(Input.Password)` margin: ${({ theme }) => `${theme.sizeUnit}px 0 ${theme.sizeUnit * 2}px`}; `; -const alertIconStyles = (theme: SupersetTheme, hasError: boolean) => css` - .ant-form-item-children-icon { - display: none; - } - ${hasError && - `.ant-form-item-control-input-content { - position: relative; - &:after { - content: ' '; - display: inline-block; - background: ${theme.colorError}; - mask: url(${errorIcon}); - mask-size: cover; - width: ${theme.sizeUnit * 4}px; - height: ${theme.sizeUnit * 4}px; - position: absolute; - right: ${theme.sizeUnit * 1.25}px; - top: ${theme.sizeUnit * 2.75}px; - } - }`} -`; - const StyledFormGroup = styled('div')` input::-webkit-outer-spin-button, input::-webkit-inner-spin-button { @@ -66,21 +43,10 @@ const StyledFormGroup = styled('div')` } `; -const StyledAlignment = styled.div` - display: flex; - align-items: center; -`; - const StyledFormLabel = styled(FormLabel)` margin-bottom: 0; `; -const iconReset = css` - &.anticon > * { - line-height: 0; - } -`; - export const LabeledErrorBoundInput = ({ label, validationMethods, @@ -94,60 +60,62 @@ export const LabeledErrorBoundInput = ({ visibilityToggle, get_url, description, + isValidating = false, ...props -}: LabeledErrorBoundInputProps) => ( - - - - {label} - - {hasTooltip && } - - alertIconStyles(theme, !!errorMessage)} - validateTrigger={Object.keys(validationMethods)} - validateStatus={errorMessage ? 'error' : 'success'} - help={errorMessage || helpText} - hasFeedback={!!errorMessage} - > - {visibilityToggle || props.name === 'password' ? ( - - visible ? ( - - - - ) : ( - - - - ) - } - role="textbox" - /> - ) : ( - - )} - {get_url && description ? ( - - ) : ( -
- )} -
-
-); +}: LabeledErrorBoundInputProps) => { + const hasError = !!errorMessage; + return ( + + + + {label} + + {hasTooltip && } + + + {visibilityToggle || props.name === 'password' ? ( + + visible ? ( + + + + ) : ( + + + + ) + } + role="textbox" + /> + ) : ( + + )} + {get_url && description ? ( + + ) : ( +
+ )} +
+
+ ); +}; +export default LabeledErrorBoundInput; diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx index e650276444e..e40a1dcd12a 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx @@ -31,8 +31,10 @@ export const hostField = ({ getValidation, validationErrors, db, + isValidating, }: FieldPropTypes) => ( ( <> { const extraJson = JSON.parse(db?.extra || '{}'); return ( ( ( ( ( ( ( ( <> ( ( <> { getValidation: jest.fn(), clearValidationErrors: jest.fn(), field: 'test', + isValidating: false, db: { configuration_method: 'dynamic_form', database_name: 'test', diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx index 94c1871f072..423427354db 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx @@ -40,6 +40,7 @@ const DatabaseConnectionForm = ({ sslForced, validationErrors, clearValidationErrors, + isValidating, }: DatabaseConnectionFormProps) => { const parameters = dbModel?.parameters as { properties: { @@ -90,6 +91,7 @@ const DatabaseConnectionForm = ({ isEditMode, sslForced, editNewDb, + isValidating, placeholder: getPlaceholder ? getPlaceholder(field) : undefined, }), )} diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx index 6de8f55f003..8b75e15467d 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx @@ -1353,12 +1353,16 @@ describe('DatabaseModal', () => { expect(usernameField).toHaveValue(''); expect(passwordField).toHaveValue(''); + expect(connectButton).toBeDisabled(); + userEvent.type(hostField, 'localhost'); userEvent.type(portField, '5432'); userEvent.type(databaseNameField, 'postgres'); userEvent.type(usernameField, 'testdb'); userEvent.type(passwordField, 'demoPassword'); + await waitFor(() => expect(connectButton).toBeEnabled()); + expect(await screen.findByDisplayValue(/5432/i)).toBeInTheDocument(); expect(hostField).toHaveValue('localhost'); expect(portField).toHaveValue(5432); @@ -1366,9 +1370,10 @@ describe('DatabaseModal', () => { expect(usernameField).toHaveValue('testdb'); expect(passwordField).toHaveValue('demoPassword'); + expect(connectButton).toBeEnabled(); userEvent.click(connectButton); await waitFor(() => { - expect(fetchMock.calls(VALIDATE_PARAMS_ENDPOINT).length).toEqual(6); + expect(fetchMock.calls(VALIDATE_PARAMS_ENDPOINT).length).toEqual(5); }); }); }); diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.tsx index c2b789e7692..cbb6ced2a98 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/index.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/index.tsx @@ -594,8 +594,14 @@ const DatabaseModal: FunctionComponent = ({ const [tabKey, setTabKey] = useState(DEFAULT_TAB_KEY); const [availableDbs, getAvailableDbs] = useAvailableDatabases(); - const [validationErrors, getValidation, setValidationErrors] = - useDatabaseValidation(); + const [ + validationErrors, + getValidation, + setValidationErrors, + isValidating, + hasValidated, + setHasValidated, + ] = useDatabaseValidation(); const [hasConnectedDb, setHasConnectedDb] = useState(false); const [showCTAbtns, setShowCTAbtns] = useState(false); const [dbName, setDbName] = useState(''); @@ -754,7 +760,8 @@ const DatabaseModal: FunctionComponent = ({ const handleClearValidationErrors = useCallback(() => { setValidationErrors(null); - }, [setValidationErrors]); + setHasValidated(false); + }, [setValidationErrors, setHasValidated]); const handleParametersChange = useCallback( ({ target }: { target: HTMLInputElement }) => { @@ -812,7 +819,6 @@ const DatabaseModal: FunctionComponent = ({ const onSave = async () => { let dbConfigExtraExtensionOnSaveError; - setLoading(true); dbConfigExtraExtension @@ -1208,10 +1214,18 @@ const DatabaseModal: FunctionComponent = ({ {t('Back')} 0) + ) + } > {t('Connect')} @@ -1452,6 +1466,7 @@ const DatabaseModal: FunctionComponent = ({ onChange={(event: ChangeEvent) => setPasswords({ ...passwords, [database]: event.target.value }) } + isValidating={isValidating} validationMethods={{ onBlur: () => {} }} errorMessage={validationErrors?.password_needed} label={t('%s PASSWORD', database.slice(10))} @@ -1460,6 +1475,7 @@ const DatabaseModal: FunctionComponent = ({ )} {sshTunnelPasswordFields?.indexOf(database) >= 0 && ( = ({ ) => @@ -1498,6 +1515,7 @@ const DatabaseModal: FunctionComponent = ({ ) => @@ -1553,6 +1571,7 @@ const DatabaseModal: FunctionComponent = ({ {} }} errorMessage={validationErrors?.confirm_overwrite} @@ -1688,6 +1707,7 @@ const DatabaseModal: FunctionComponent = ({ const renderDatabaseConnectionForm = () => ( <> | null; + isValidating: boolean; onParametersChange: ( event: FormEvent | { target: HTMLInputElement }, ) => void; diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index a9cf4eb7fc0..5a5721205ee 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -760,137 +760,98 @@ export function useDatabaseValidation() { const [validationErrors, setValidationErrors] = useState( null, ); + const [isValidating, setIsValidating] = useState(false); + const [hasValidated, setHasValidated] = useState(false); + const getValidation = useCallback( - (database: Partial | null, onCreate = false) => { + async (database: Partial | null, onCreate = false) => { if (database?.parameters?.ssh) { - // TODO: /validate_parameters/ and related utils should support ssh tunnel setValidationErrors(null); - return []; + setIsValidating(false); + setHasValidated(true); + return Promise.resolve([]); } - return ( - SupersetClient.post({ + setIsValidating(true); + + try { + await SupersetClient.post({ endpoint: '/api/v1/database/validate_parameters/', body: JSON.stringify(transformDB(database)), headers: { 'Content-Type': 'application/json' }, - }) - .then(() => { - setValidationErrors(null); - }) - // eslint-disable-next-line consistent-return - .catch(e => { - if (typeof e.json === 'function') { - return 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; - }) - .reduce( - ( - obj: {}, - { - error_type, - extra, - message, - }: { - error_type: string; - extra: { - invalid?: string[]; - missing?: string[]; - name: string; - catalog: { - name: string; - url: string; - idx: number; - }; - issue_codes?: { - code?: number; - message?: string; - }[]; - }; - message: string; - }, - ) => { - if (extra.catalog) { - if (extra.catalog.name) { - return { - ...obj, - error_type, - [extra.catalog.idx]: { - name: message, - }, - }; - } - if (extra.catalog.url) { - return { - ...obj, - error_type, - [extra.catalog.idx]: { - url: message, - }, - }; - } + }); + setValidationErrors(null); + setIsValidating(false); + setHasValidated(true); + return []; + } catch (error) { + if (typeof error.json === 'function') { + return error.json().then(({ errors = [] }) => { + const parsedErrors = errors + .filter((err: { error_type: string }) => { + const allowed = [ + 'CONNECTION_MISSING_PARAMETERS_ERROR', + 'CONNECTION_ACCESS_DENIED_ERROR', + 'INVALID_PAYLOAD_SCHEMA_ERROR', + ]; + return allowed.includes(err.error_type) || onCreate; + }) + .reduce((acc: JsonObject, err_2: any) => { + const { message, extra } = err_2; - return { - ...obj, - error_type, - [extra.catalog.idx]: { - name: message, - url: message, - }, - }; - } - // if extra.invalid doesn't exist then the - // error can't be mapped to a parameter - // so leave it alone - if (extra.invalid) { - return { - ...obj, - [extra.invalid[0]]: message, - error_type, - }; - } - if (extra.missing) { - return { - ...obj, - error_type, - ...Object.assign( - {}, - ...extra.missing.map(field => ({ - [field]: 'This is a required field', - })), - ), - }; - } - if (extra.issue_codes?.length) { - return { - ...obj, - error_type, - description: message || extra.issue_codes[0]?.message, - }; - } + if (extra?.catalog) { + const { idx } = extra.catalog; + acc[idx] = { + ...acc[idx], + ...(extra.catalog.name ? { name: message } : {}), + ...(extra.catalog.url ? { url: message } : {}), + }; + return acc; + } - return obj; - }, - {}, - ); - setValidationErrors(parsedErrors); - return parsedErrors; - }); - } - // eslint-disable-next-line no-console - console.error(e); - }) - ); + if (extra?.invalid) { + extra.invalid.forEach((field: string) => { + acc[field] = message; + }); + } + + if (extra?.missing) { + extra.missing.forEach((field_1: string) => { + acc[field_1] = 'This is a required field'; + }); + } + + if (extra?.issue_codes?.length) { + acc.description = message || extra.issue_codes[0]?.message; + } + + return acc; + }, {}); + + setValidationErrors(parsedErrors); + setIsValidating(false); + setHasValidated(true); + return parsedErrors; + }); + } + + console.error('Unexpected error during validation:', error); + setIsValidating(false); + setHasValidated(true); + return {}; + } }, [setValidationErrors], ); - return [validationErrors, getValidation, setValidationErrors] as const; + return [ + validationErrors, + getValidation, + setValidationErrors, + isValidating, + hasValidated, + setHasValidated, + ] as const; } export const reportSelector = (