This is an automated email from the ASF dual-hosted git repository. diegopucci pushed a commit to branch diego/ch78628/fix-disabled-ssh-toggle in repository https://gitbox.apache.org/repos/asf/superset.git
commit 1ec5cf8b6ec2f1dd43c8a57d7904cbe077490ed2 Author: geido <[email protected]> AuthorDate: Thu Feb 15 18:23:10 2024 +0200 Merge SSHTunnelSwitch components --- .../superset-ui-core/src/ui-overrides/types.ts | 10 --- .../DatabaseConnectionForm/CommonParameters.tsx | 35 +-------- .../DatabaseConnectionForm/EncryptedField.tsx | 2 +- .../DatabaseConnectionForm/TableCatalog.tsx | 3 +- .../DatabaseConnectionForm/ValidatedInputField.tsx | 2 +- .../DatabaseModal/DatabaseConnectionForm/index.tsx | 42 +++------- .../databases/DatabaseModal/SSHTunnelSwitch.tsx | 89 +++++++++++++++++----- .../src/features/databases/DatabaseModal/index.tsx | 85 +++++++++++++++------ superset-frontend/src/features/databases/types.ts | 43 +++++++++++ 9 files changed, 186 insertions(+), 125 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts b/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts index 45ec06e90e..96b573389f 100644 --- a/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts +++ b/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts @@ -44,16 +44,6 @@ interface MenuObjectChildProps { disable?: boolean; } -export interface SwitchProps { - isEditMode: boolean; - dbFetched: any; - disableSSHTunnelingForEngine?: boolean; - useSSHTunneling: boolean; - setUseSSHTunneling: React.Dispatch<React.SetStateAction<boolean>>; - setDB: React.Dispatch<any>; - isSSHTunneling: boolean; -} - type ConfigDetailsProps = { embeddedId: string; }; diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx index 7b52eab26c..3f1f5f9625 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx @@ -17,12 +17,11 @@ * under the License. */ import React from 'react'; -import { isEmpty } from 'lodash'; import { SupersetTheme, t } from '@superset-ui/core'; import { AntdSwitch } from 'src/components'; import InfoTooltip from 'src/components/InfoTooltip'; import ValidatedInput from 'src/components/Form/LabeledErrorBoundInput'; -import { FieldPropTypes } from '.'; +import { FieldPropTypes } from '../../types'; import { toggleStyle, infoTooltip } from '../styles'; export const hostField = ({ @@ -252,35 +251,3 @@ export const forceSSLField = ({ /> </div> ); - -export const SSHTunnelSwitch = ({ - isEditMode, - changeMethods, - clearValidationErrors, - db, -}: FieldPropTypes) => ( - <div css={(theme: SupersetTheme) => infoTooltip(theme)}> - <AntdSwitch - disabled={isEditMode && !isEmpty(db?.ssh_tunnel)} - checked={db?.parameters?.ssh} - onChange={changed => { - changeMethods.onParametersChange({ - target: { - type: 'toggle', - name: 'ssh', - checked: true, - value: changed, - }, - }); - clearValidationErrors(); - }} - data-test="ssh-tunnel-switch" - /> - <span css={toggleStyle}>{t('SSH Tunnel')}</span> - <InfoTooltip - tooltip={t('SSH Tunnel configuration parameters')} - placement="right" - viewBox="0 -5 24 24" - /> - </div> -); diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx index c5e268e569..009afc84ef 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx @@ -22,7 +22,7 @@ import { AntdButton, AntdSelect } from 'src/components'; import InfoTooltip from 'src/components/InfoTooltip'; import FormLabel from 'src/components/Form/FormLabel'; import Icons from 'src/components/Icons'; -import { FieldPropTypes } from '.'; +import { FieldPropTypes } from '../../types'; import { infoTooltip, labelMarginBottom, CredentialInfoForm } from '../styles'; enum CredentialInfoOptions { diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx index ed5cc94903..47a0ec1579 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx @@ -21,9 +21,8 @@ import { css, SupersetTheme, t } from '@superset-ui/core'; import ValidatedInput from 'src/components/Form/LabeledErrorBoundInput'; import FormLabel from 'src/components/Form/FormLabel'; import Icons from 'src/components/Icons'; -import { FieldPropTypes } from '.'; import { StyledFooterButton, StyledCatalogTable } from '../styles'; -import { CatalogObject } from '../../types'; +import { CatalogObject, FieldPropTypes } from '../../types'; export const TableCatalog = ({ required, diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/ValidatedInputField.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/ValidatedInputField.tsx index ec2e239ac4..d6794f9a21 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/ValidatedInputField.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/ValidatedInputField.tsx @@ -19,7 +19,7 @@ import React from 'react'; import { t } from '@superset-ui/core'; import ValidatedInput from 'src/components/Form/LabeledErrorBoundInput'; -import { FieldPropTypes } from '.'; +import { FieldPropTypes } from '../../types'; const FIELD_TEXT_MAP = { account: { diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx index e747b3c895..dc4ce10dab 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx @@ -17,7 +17,11 @@ * under the License. */ import React, { FormEvent } from 'react'; -import { SupersetTheme, JsonObject } from '@superset-ui/core'; +import { + SupersetTheme, + JsonObject, + getExtensionsRegistry, +} from '@superset-ui/core'; import { InputProps } from 'antd/lib/input'; import { Form } from 'src/components/Form'; import { @@ -31,13 +35,13 @@ import { portField, queryField, usernameField, - SSHTunnelSwitch, } from './CommonParameters'; import { validatedInputField } from './ValidatedInputField'; import { EncryptedField } from './EncryptedField'; import { TableCatalog } from './TableCatalog'; import { formScrollableStyles, validatedFormStyles } from '../styles'; import { DatabaseForm, DatabaseObject } from '../../types'; +import SSHTunnelSwitch from '../SSHTunnelSwitch'; export const FormFieldOrder = [ 'host', @@ -59,34 +63,10 @@ export const FormFieldOrder = [ 'ssh', ]; -export interface FieldPropTypes { - required: boolean; - hasTooltip?: boolean; - tooltipText?: (value: any) => string; - placeholder?: string; - onParametersChange: (value: any) => string; - onParametersUploadFileChange: (value: any) => string; - changeMethods: { onParametersChange: (value: any) => string } & { - onChange: (value: any) => string; - } & { - onQueryChange: (value: any) => string; - } & { onParametersUploadFileChange: (value: any) => string } & { - onAddTableCatalog: () => void; - onRemoveTableCatalog: (idx: number) => void; - } & { - onExtraInputChange: (value: any) => void; - onSSHTunnelParametersChange: (value: any) => string; - }; - validationErrors: JsonObject | null; - getValidation: () => void; - clearValidationErrors: () => void; - db?: DatabaseObject; - field: string; - isEditMode?: boolean; - sslForced?: boolean; - defaultDBName?: string; - editNewDb?: boolean; -} +const extensionsRegistry = getExtensionsRegistry(); + +const SSHTunnelSwitchComponent = + extensionsRegistry.get('ssh_tunnel.form.switch') ?? SSHTunnelSwitch; const FORM_FIELD_MAP = { host: hostField, @@ -105,7 +85,7 @@ const FORM_FIELD_MAP = { warehouse: validatedInputField, role: validatedInputField, account: validatedInputField, - ssh: SSHTunnelSwitch, + ssh: SSHTunnelSwitchComponent, }; interface DatabaseConnectionFormProps { diff --git a/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelSwitch.tsx b/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelSwitch.tsx index 388e3c83b1..d541390483 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelSwitch.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/SSHTunnelSwitch.tsx @@ -16,35 +16,80 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; -import { t, SupersetTheme, SwitchProps } from '@superset-ui/core'; +import React, { useEffect, useState } from 'react'; +import { t, SupersetTheme } from '@superset-ui/core'; import { AntdSwitch } from 'src/components'; import InfoTooltip from 'src/components/InfoTooltip'; import { isEmpty } from 'lodash'; -import { ActionType } from '.'; import { infoTooltip, toggleStyle } from './styles'; +import { DatabaseObject, FieldPropTypes } from '../types'; + +type ChangeMethodsType = FieldPropTypes['changeMethods']; + +// changeMethods compatibility with dynamic forms +type SwitchPropsChangeMethodsType = { + onParametersChange: ChangeMethodsType['onParametersChange']; +}; + +type SwitchProps = { + db: DatabaseObject; + changeMethods: SwitchPropsChangeMethodsType; + isSSHTunnelEnabled?: boolean; +}; const SSHTunnelSwitch = ({ - isEditMode, - dbFetched, - useSSHTunneling, - setUseSSHTunneling, - setDB, - isSSHTunneling, -}: SwitchProps) => - isSSHTunneling ? ( + // true by default for compatibility with dynamic forms + isSSHTunnelEnabled = true, + changeMethods, + db, +}: SwitchProps) => { + const [isChecked, setChecked] = useState(false); + + const handleOnChange = (changed: boolean) => { + setChecked(changed); + + changeMethods.onParametersChange({ + target: { + type: 'toggle', + name: 'ssh', + checked: true, + value: changed, + }, + }); + }; + + useEffect(() => { + if ( + isSSHTunnelEnabled && + db?.parameters?.ssh !== undefined && + db.parameters.ssh !== isChecked + ) { + setChecked(db.parameters.ssh); + } + }, [db?.parameters?.ssh]); + + useEffect(() => { + if ( + isSSHTunnelEnabled && + !db?.parameters?.ssh && + !isEmpty(db?.ssh_tunnel) + ) { + changeMethods.onParametersChange({ + target: { + type: 'toggle', + name: 'ssh', + checked: true, + value: true, + }, + }); + } + }, [db?.ssh_tunnel]); + + return isSSHTunnelEnabled ? ( <div css={(theme: SupersetTheme) => infoTooltip(theme)}> <AntdSwitch - disabled={isEditMode && !isEmpty(dbFetched?.ssh_tunnel)} - checked={useSSHTunneling} - onChange={changed => { - setUseSSHTunneling(changed); - if (!changed) { - setDB({ - type: ActionType.RemoveSSHTunnelConfig, - }); - } - }} + checked={isChecked} + onChange={handleOnChange} data-test="ssh-tunnel-switch" /> <span css={toggleStyle}>{t('SSH Tunnel')}</span> @@ -55,4 +100,6 @@ const SSHTunnelSwitch = ({ /> </div> ) : null; +}; + export default SSHTunnelSwitch; diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.tsx index add5218123..98e7cab415 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/index.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/index.tsx @@ -23,6 +23,7 @@ import { FeatureFlag, isFeatureEnabled, getExtensionsRegistry, + SupersetClient, } from '@superset-ui/core'; import React, { FunctionComponent, @@ -208,8 +209,8 @@ export type DBReducerActionType = | { type: | ActionType.Reset - | ActionType.AddTableCatalogSheet - | ActionType.RemoveSSHTunnelConfig; + | ActionType.RemoveSSHTunnelConfig + | ActionType.AddTableCatalogSheet; } | { type: ActionType.RemoveTableCatalogSheet; @@ -659,7 +660,8 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ extra: db?.extra, masked_encrypted_extra: db?.masked_encrypted_extra || '', server_cert: db?.server_cert || undefined, - ssh_tunnel: db?.ssh_tunnel || undefined, + ssh_tunnel: + !isEmpty(db?.ssh_tunnel) && useSSHTunneling ? db.ssh_tunnel : undefined, }; setTestInProgress(true); testDatabaseConnection( @@ -687,10 +689,27 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ return false; }; + const handleClearValidationErrors = () => { + setValidationErrors(null); + }; + + const handleClearSSHTunnelConfig = () => { + setDB({ type: ActionType.RemoveSSHTunnelConfig }); + }; + + const handleParametersChange = ({ target }: { target: HTMLInputElement }) => { + onChange(ActionType.ParametersChange, { + type: target.type, + name: target.name, + checked: target.checked, + value: target.value, + }); + }; + const onClose = () => { setDB({ type: ActionType.Reset }); setHasConnectedDb(false); - setValidationErrors(null); // reset validation errors on close + handleClearValidationErrors(); // reset validation errors on close clearError(); setEditNewDb(false); setFileList([]); @@ -763,7 +782,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ } // only do validation for non ssh tunnel connections - if (!dbToUpdate?.ssh_tunnel) { + if (!dbToUpdate?.parameters?.ssh) { // make sure that button spinner animates setLoading(true); const errors = await getValidation(dbToUpdate, true); @@ -830,6 +849,30 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ } setLoading(true); + + // tunnel config is kept until db is saved for UX convenience (toggling on and off) + // strictly checking for false as an indication that the toggle got unchecked + if (isSSHTunneling && dbToUpdate?.parameters?.ssh === false) { + if (db?.id && dbFetched?.ssh_tunnel) { + // the db and ssh tunnel exist, should be removed + try { + await SupersetClient.delete({ + endpoint: `/api/v1/database/${db.id}/ssh_tunnel/`, + }); + } catch (e) { + addDangerToast( + t('There was an error removing the SSH tunnel configuration'), + ); + setLoading(false); + console.error(e); + return; + } + } + handleClearSSHTunnelConfig(); + // remove ssh tunnel from payload + dbToUpdate.ssh_tunnel = undefined; + } + if (db?.id) { const result = await updateResource( db.id as number, @@ -1282,10 +1325,11 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ }, [sshPrivateKeyPasswordNeeded]); useEffect(() => { - if (db && isSSHTunneling) { - setUseSSHTunneling(!isEmpty(db?.ssh_tunnel)); + if (isSSHTunneling) { + setUseSSHTunneling(!!db?.parameters?.ssh); + handleClearValidationErrors(); } - }, [db, isSSHTunneling]); + }, [db?.parameters?.ssh]); const onDbImport = async (info: UploadChangeParam) => { setImportingErrorMessage(''); @@ -1623,14 +1667,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ payload: { indexToDelete: idx }, }); }} - onParametersChange={({ target }: { target: HTMLInputElement }) => - onChange(ActionType.ParametersChange, { - type: target.type, - name: target.name, - checked: target.checked, - value: target.value, - }) - } + onParametersChange={handleParametersChange} onChange={({ target }: { target: HTMLInputElement }) => onChange(ActionType.TextChange, { name: target.name, @@ -1640,9 +1677,9 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ getValidation={() => getValidation(db)} validationErrors={validationErrors} getPlaceholder={getPlaceholder} - clearValidationErrors={() => setValidationErrors(null)} + clearValidationErrors={handleClearValidationErrors} /> - {db?.parameters?.ssh && ( + {useSSHTunneling && ( <SSHTunnelContainer>{renderSSHTunnelForm()}</SSHTunnelContainer> )} </> @@ -1792,13 +1829,11 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ testInProgress={testInProgress} > <SSHTunnelSwitchComponent - isEditMode={isEditMode} - dbFetched={dbFetched} - disableSSHTunnelingForEngine={disableSSHTunnelingForEngine} - useSSHTunneling={useSSHTunneling} - setUseSSHTunneling={setUseSSHTunneling} - setDB={setDB} - isSSHTunneling={isSSHTunneling} + db={db as DatabaseObject} + changeMethods={{ + onParametersChange: handleParametersChange, + }} + isSSHTunnelEnabled={isSSHTunneling} /> {useSSHTunneling && renderSSHTunnelForm()} </SqlAlchemyForm> diff --git a/superset-frontend/src/features/databases/types.ts b/superset-frontend/src/features/databases/types.ts index 50e535f9b1..dbbd661890 100644 --- a/superset-frontend/src/features/databases/types.ts +++ b/superset-frontend/src/features/databases/types.ts @@ -1,3 +1,7 @@ +import { JsonObject } from '@superset-ui/core'; +import { InputProps } from 'antd/lib/input'; +import { FormEvent } from 'react'; + /** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -231,3 +235,42 @@ export interface ExtraJson { }; version?: string; } + +type ParametersChangeValueType = HTMLInputElement & { + value: string | boolean; +}; + +type ParametersChangeType<T = ParametersChangeValueType> = + | FormEvent<InputProps> + | { target: T }; + +export interface FieldPropTypes { + required: boolean; + hasTooltip?: boolean; + tooltipText?: (value: any) => string; + placeholder?: string; + onParametersChange: (event: ParametersChangeType) => void; + onParametersUploadFileChange: (value: any) => string; + changeMethods: { + onParametersChange: (event: ParametersChangeType) => void; + } & { + onChange: (value: any) => string; + } & { + onQueryChange: (value: any) => string; + } & { onParametersUploadFileChange: (value: any) => string } & { + onAddTableCatalog: () => void; + onRemoveTableCatalog: (idx: number) => void; + } & { + onExtraInputChange: (value: any) => void; + onSSHTunnelParametersChange: (value: any) => string; + }; + validationErrors: JsonObject | null; + getValidation: () => void; + clearValidationErrors: () => void; + db?: DatabaseObject; + field: string; + isEditMode?: boolean; + sslForced?: boolean; + defaultDBName?: string; + editNewDb?: boolean; +}
