Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Button from 'components/base/forms/Button'
import Input from 'components/base/forms/Input'
import Switch from 'components/Switch'
import ErrorMessage from 'components/ErrorMessage'
import FieldError from 'components/base/forms/FieldError'
import WarningMessage from 'components/WarningMessage'
import { ClickHouseConfig } from 'common/types/responses'
import { useTestWarehouseConnectionConfigMutation } from 'common/services/useWarehouseConnection'
Expand All @@ -14,6 +15,7 @@ import {
ClickHouseFormState,
isClickHouseConfigDirty,
isClickHouseFormValid,
isValidPort,
} from './clickhouseConfig'
import {
getButtonLabel,
Expand Down Expand Up @@ -70,6 +72,7 @@ const ClickHouseConfigForm: FC<ClickHouseConfigFormProps> = ({
const isValid = isClickHouseFormValid(form, isEdit)
const requiresTest = !isEdit || isClickHouseConfigDirty(form, initialConfig)
const canTest = canTestClickHouseConnection(form)
const hasInvalidPort = !!port && !isValidPort(port)
const canSave = isValid && (!requiresTest || testState !== 'idle')

const setField =
Expand Down Expand Up @@ -101,16 +104,11 @@ const ClickHouseConfigForm: FC<ClickHouseConfigFormProps> = ({
} else {
setTestState('errored')
setTestDetail(result.status_detail)
toast(
result.status_detail || 'Connection failed — check your credentials',
'danger',
)
}
} catch {
if (revision !== testRevision.current) return
setTestState('errored')
setTestDetail(null)
toast('Failed to test connection', 'danger')
}
}

Expand Down Expand Up @@ -168,6 +166,16 @@ const ClickHouseConfigForm: FC<ClickHouseConfigFormProps> = ({
setField(setPort)(e.target.value)
}
placeholder='9440'
aria-invalid={hasInvalidPort}
aria-describedby={
hasInvalidPort ? 'warehouse-config-port-error' : undefined
}
/>
<FieldError
id='warehouse-config-port-error'
error={
hasInvalidPort && 'Port must be a number between 1 and 65535.'
}
/>
</div>
<div className='wh-config-form__field'>
Expand Down Expand Up @@ -201,6 +209,7 @@ const ClickHouseConfigForm: FC<ClickHouseConfigFormProps> = ({
setField(setPassword)(e.target.value)
}
type='password'
autoComplete='new-password'
placeholder={isEdit ? '••••••••' : 'Password'}
/>
{isEdit && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ describe('isValidPort', () => {
expect(isValidPort('65535')).toBe(true)
})

it('accepts a port with surrounding whitespace', () => {
expect(isValidPort(' 9440 ')).toBe(true)
})

it('rejects out-of-range or non-numeric ports', () => {
expect(isValidPort('0')).toBe(false)
expect(isValidPort('65536')).toBe(false)
Expand All @@ -47,6 +51,12 @@ describe('isClickHouseFormValid', () => {
},
)

it('rejects a whitespace-only host', () => {
expect(isClickHouseFormValid({ ...validForm, host: ' ' }, false)).toBe(
false,
)
})

it('allows an empty password on edit (keeps the stored one)', () => {
expect(isClickHouseFormValid({ ...validForm, password: '' }, true)).toBe(
true,
Expand Down Expand Up @@ -99,6 +109,15 @@ describe('isClickHouseConfigDirty', () => {
).toBe(true)
})

it('is clean when the port differs only in formatting', () => {
expect(
isClickHouseConfigDirty(
{ ...validForm, password: '', port: '09440' },
initialConfig,
),
).toBe(false)
})

it('compares against defaults when there is no stored config', () => {
expect(
isClickHouseConfigDirty({ ...validForm, password: '' }, undefined),
Expand Down Expand Up @@ -126,4 +145,28 @@ describe('buildClickHousePayload', () => {
buildClickHousePayload({ ...validForm, password: '' }).credentials,
).toBeUndefined()
})

it('trims whitespace from pasted values, leaving the password intact', () => {
expect(
buildClickHousePayload({
...validForm,
database: 'flagsmith\n',
host: ' ch.example.com ',
name: ' Production ClickHouse ',
password: ' hunter2 ',
port: ' 9440 ',
username: 'default ',
}),
).toEqual({
config: {
database: 'flagsmith',
host: 'ch.example.com',
port: 9440,
secure: true,
username: 'default',
},
credentials: { password: ' hunter2 ' },
name: 'Production ClickHouse',
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ export type ClickHouseFormData = {
}

export const isValidPort = (port: string): boolean => {
const value = Number(port)
const trimmed = port.trim()
const value = Number(trimmed)
return (
/^\d+$/.test(port) &&
/^\d+$/.test(trimmed) &&
Number.isInteger(value) &&
value >= 1 &&
value <= 65535
Expand All @@ -38,34 +39,34 @@ export const isClickHouseFormValid = (
form: ClickHouseFormState,
isEdit: boolean,
): boolean =>
!!form.name &&
!!form.host &&
!!form.name.trim() &&
!!form.host.trim() &&
isValidPort(form.port) &&
!!form.database &&
!!form.username &&
!!form.database.trim() &&
!!form.username.trim() &&
(isEdit || !!form.password)

export const buildClickHousePayload = (
form: ClickHouseFormState,
): ClickHouseFormData => ({
config: {
database: form.database,
host: form.host,
port: Number(form.port),
database: form.database.trim(),
host: form.host.trim(),
port: Number(form.port.trim()),
secure: form.secure,
username: form.username,
username: form.username.trim(),
},
credentials: form.password ? { password: form.password } : undefined,
name: form.name,
name: form.name.trim(),
})

export const canTestClickHouseConnection = (
form: ClickHouseFormState,
): boolean =>
!!form.host &&
!!form.host.trim() &&
isValidPort(form.port) &&
!!form.database &&
!!form.username &&
!!form.database.trim() &&
!!form.username.trim() &&
!!form.password

export const isClickHouseConfigDirty = (
Expand All @@ -74,10 +75,10 @@ export const isClickHouseConfigDirty = (
): boolean => {
const initial = { ...CLICKHOUSE_DEFAULTS, ...initialConfig }
return (
form.host !== initial.host ||
form.port !== String(initial.port) ||
form.database !== initial.database ||
form.username !== initial.username ||
form.host.trim() !== initial.host ||
Number(form.port.trim()) !== initial.port ||
form.database.trim() !== initial.database ||
form.username.trim() !== initial.username ||
Comment thread
coderabbitai[bot] marked this conversation as resolved.
form.secure !== initial.secure ||
!!form.password
)
Expand Down
Loading