From 0ddd97b6031717ab5176f3fa5c9d8cfa4e435529 Mon Sep 17 00:00:00 2001 From: Justin Kambic Date: Thu, 23 Apr 2020 12:15:49 -0400 Subject: [PATCH] Implement PR feedback. --- .../common/constants/settings_defaults.ts | 2 +- .../common/runtime_types/dynamic_settings.ts | 16 +- .../__tests__/certificate_form.test.tsx | 3 +- .../settings/__tests__/indices_form.test.tsx | 3 +- .../components/settings/certificate_form.tsx | 246 +++++++++--------- .../components/settings/indices_form.tsx | 112 ++++---- .../plugins/uptime/public/pages/settings.tsx | 34 ++- .../public/state/reducers/dynamic_settings.ts | 5 +- .../uptime/public/state/selectors/index.ts | 4 +- .../uptime/server/lib/saved_objects.ts | 2 +- .../test/functional/apps/uptime/settings.ts | 12 +- .../functional/services/uptime/settings.ts | 2 +- 12 files changed, 230 insertions(+), 211 deletions(-) diff --git a/x-pack/legacy/plugins/uptime/common/constants/settings_defaults.ts b/x-pack/legacy/plugins/uptime/common/constants/settings_defaults.ts index f7eee27ba438d..b7986679a09ca 100644 --- a/x-pack/legacy/plugins/uptime/common/constants/settings_defaults.ts +++ b/x-pack/legacy/plugins/uptime/common/constants/settings_defaults.ts @@ -8,7 +8,7 @@ import { DynamicSettings } from '../runtime_types'; export const DYNAMIC_SETTINGS_DEFAULTS: DynamicSettings = { heartbeatIndices: 'heartbeat-8*', - certificatesThresholds: { + certThresholds: { expiration: 30, age: 365, }, diff --git a/x-pack/legacy/plugins/uptime/common/runtime_types/dynamic_settings.ts b/x-pack/legacy/plugins/uptime/common/runtime_types/dynamic_settings.ts index 1cd8682510d73..da887cc5055c1 100644 --- a/x-pack/legacy/plugins/uptime/common/runtime_types/dynamic_settings.ts +++ b/x-pack/legacy/plugins/uptime/common/runtime_types/dynamic_settings.ts @@ -6,19 +6,15 @@ import * as t from 'io-ts'; -export const CertificatesStatesThresholdType = t.partial({ +export const CertStateThresholdsType = t.type({ age: t.number, expiration: t.number, }); -export const DynamicSettingsType = t.intersection([ - t.type({ - heartbeatIndices: t.string, - }), - t.partial({ - certificatesThresholds: CertificatesStatesThresholdType, - }), -]); +export const DynamicSettingsType = t.type({ + heartbeatIndices: t.string, + certThresholds: CertStateThresholdsType, +}); export const DynamicSettingsSaveType = t.intersection([ t.type({ @@ -31,4 +27,4 @@ export const DynamicSettingsSaveType = t.intersection([ export type DynamicSettings = t.TypeOf; export type DynamicSettingsSaveResponse = t.TypeOf; -export type CertificatesStatesThreshold = t.TypeOf; +export type CertStateThresholds = t.TypeOf; diff --git a/x-pack/legacy/plugins/uptime/public/components/settings/__tests__/certificate_form.test.tsx b/x-pack/legacy/plugins/uptime/public/components/settings/__tests__/certificate_form.test.tsx index 9e8c722b72cc5..b396a92828d22 100644 --- a/x-pack/legacy/plugins/uptime/public/components/settings/__tests__/certificate_form.test.tsx +++ b/x-pack/legacy/plugins/uptime/public/components/settings/__tests__/certificate_form.test.tsx @@ -13,10 +13,11 @@ describe('CertificateForm', () => { expect( shallowWithRouter( { expect( shallowWithRouter( ) => void; +interface ChangedValues { + heartbeatIndices?: string; + certThresholds?: Partial; +} + +export type OnFieldChangeType = (changedValues: ChangedValues) => void; export interface SettingsFormProps { + loading: boolean; onChange: OnFieldChangeType; formFields: DynamicSettings | null; fieldErrors: any; @@ -31,135 +36,134 @@ export interface SettingsFormProps { } export const CertificateExpirationForm: React.FC = ({ + loading, onChange, formFields, fieldErrors, isDisabled, -}) => { - const dss = useSelector(selectDynamicSettings); - - return ( - <> - -

+}) => ( + <> + +

+ +

+ + + +

+ } + description={ + + } + > + {DYNAMIC_SETTINGS_DEFAULTS.certThresholds.expiration} + ), + }} /> - - - - - - } - description={ + isInvalid={!!fieldErrors?.certificatesThresholds?.errorState} + label={ } > - {dss.settings.certificatesThresholds?.expiration}, - }} + + + + onChange({ + certThresholds: { + expiration: Number(e.target.value), + }, + }) + } /> - } - isInvalid={!!fieldErrors?.certificatesThresholds?.errorState} - label={ - - } - > - - - - onChange({ - certificatesThresholds: { - expiration: Number(value), - }, - }) - } + + + + - - - - - - - - - {dss.settings.certificatesThresholds?.age}, - }} - /> - } - isInvalid={!!fieldErrors?.certificatesThresholds?.warningState} - label={ - + + + + {DYNAMIC_SETTINGS_DEFAULTS.certThresholds.age}, + }} + /> + } + isInvalid={!!fieldErrors?.certificatesThresholds?.warningState} + label={ + + } + > + + + + onChange({ + certThresholds: { age: Number(e.currentTarget.value) }, + }) + } /> - } - > - - - - onChange({ - certificatesThresholds: { age: Number(event.currentTarget.value) }, - }) - } + + + + - - - - - - - - - - - ); -}; + + + + + + +); diff --git a/x-pack/legacy/plugins/uptime/public/components/settings/indices_form.tsx b/x-pack/legacy/plugins/uptime/public/components/settings/indices_form.tsx index c3f0342ef3ea5..23c93da8896e2 100644 --- a/x-pack/legacy/plugins/uptime/public/components/settings/indices_form.tsx +++ b/x-pack/legacy/plugins/uptime/public/components/settings/indices_form.tsx @@ -6,7 +6,6 @@ import React from 'react'; import { FormattedMessage } from '@kbn/i18n/react'; -import { useSelector } from 'react-redux'; import { EuiDescribedFormGroup, EuiFormRow, @@ -15,75 +14,72 @@ import { EuiTitle, EuiSpacer, } from '@elastic/eui'; -import { selectDynamicSettings } from '../../state/selectors'; import { SettingsFormProps } from './certificate_form'; +import { DYNAMIC_SETTINGS_DEFAULTS } from '../../../common/constants'; export const IndicesForm: React.FC = ({ onChange, + loading, formFields, fieldErrors, isDisabled, -}) => { - const dss = useSelector(selectDynamicSettings); - - return ( - <> - -

+}) => ( + <> + +

+ +

+ + + + +

+ } + description={ + + } + > + {DYNAMIC_SETTINGS_DEFAULTS.heartbeatIndices}, + }} /> - - - - - - } - description={ + isInvalid={!!fieldErrors?.heartbeatIndices} + label={ } > - {dss.settings.heartbeatIndices}, - }} - /> - } - isInvalid={!!fieldErrors?.heartbeatIndices} - label={ - - } - > - onChange({ heartbeatIndices: event.currentTarget.value })} - /> - - - - ); -}; + disabled={isDisabled} + isLoading={loading} + value={formFields?.heartbeatIndices || ''} + onChange={(event: any) => onChange({ heartbeatIndices: event.currentTarget.value })} + /> + + + +); diff --git a/x-pack/legacy/plugins/uptime/public/pages/settings.tsx b/x-pack/legacy/plugins/uptime/public/pages/settings.tsx index 31f3433d3ccda..58e6b83d334fd 100644 --- a/x-pack/legacy/plugins/uptime/public/pages/settings.tsx +++ b/x-pack/legacy/plugins/uptime/public/pages/settings.tsx @@ -17,7 +17,7 @@ import { } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n/react'; import { useDispatch, useSelector } from 'react-redux'; -import { isEqual, merge } from 'lodash'; +import { isEqual } from 'lodash'; import { Link } from 'react-router-dom'; import { selectDynamicSettings } from '../state/selectors'; import { getDynamicSettings, setDynamicSettings } from '../state/actions/dynamic_settings'; @@ -36,7 +36,7 @@ import * as Translations from './translations'; const getFieldErrors = (formFields: DynamicSettings | null) => { if (formFields) { const blankStr = 'May not be blank'; - const { certificatesThresholds, heartbeatIndices } = formFields; + const { certThresholds: certificatesThresholds, heartbeatIndices } = formFields; const heartbeatIndErr = heartbeatIndices.match(/^\S+$/) ? '' : blankStr; const errorStateErr = certificatesThresholds?.expiration ? null : blankStr; const warningStateErr = certificatesThresholds?.age ? null : blankStr; @@ -67,21 +67,39 @@ export const SettingsPage = () => { dispatch(getDynamicSettings()); }, [dispatch]); - const [formFields, setFormFields] = useState({ ...dss.settings }); + const [formFields, setFormFields] = useState( + dss.settings ? { ...dss.settings } : null + ); + + if (!dss.loadError && formFields === null && dss.settings) { + setFormFields(Object.assign({}, { ...dss.settings })); + } const fieldErrors = getFieldErrors(formFields); const isFormValid = !(fieldErrors && Object.values(fieldErrors).find(v => !!v)); - const onChangeFormField: OnFieldChangeType = changedField => - setFormFields(Object.assign({ ...merge(formFields, changedField) })); + const onChangeFormField: OnFieldChangeType = changedField => { + if (formFields) { + setFormFields({ + heartbeatIndices: changedField.heartbeatIndices ?? formFields.heartbeatIndices, + certThresholds: Object.assign( + {}, + formFields.certThresholds, + changedField?.certThresholds ?? null + ), + }); + } + }; const onApply = (event: React.FormEvent) => { event.preventDefault(); - dispatch(setDynamicSettings(formFields)); + if (formFields) { + dispatch(setDynamicSettings(formFields)); + } }; - const resetForm = () => setFormFields({ ...dss.settings }); + const resetForm = () => setFormFields(dss.settings ? { ...dss.settings } : null); const isFormDirty = !isEqual(dss.settings, formFields); const canEdit: boolean = @@ -114,12 +132,14 @@ export const SettingsPage = () => {
( loading: true, }), [String(setDynamicSettingsSuccess)]: (state, action: Action) => ({ - ...state, settings: action.payload, saveSucceded: true, loading: false, diff --git a/x-pack/legacy/plugins/uptime/public/state/selectors/index.ts b/x-pack/legacy/plugins/uptime/public/state/selectors/index.ts index 7260c61f44147..15fc8b8a7b173 100644 --- a/x-pack/legacy/plugins/uptime/public/state/selectors/index.ts +++ b/x-pack/legacy/plugins/uptime/public/state/selectors/index.ts @@ -24,9 +24,7 @@ export const monitorLocationsSelector = (state: AppState, monitorId: string) => export const monitorStatusSelector = (state: AppState) => state.monitorStatus.status; -export const selectDynamicSettings = (state: AppState) => { - return state.dynamicSettings; -}; +export const selectDynamicSettings = (state: AppState) => state.dynamicSettings; export const selectIndexPattern = ({ indexPattern }: AppState) => { return { indexPattern: indexPattern.index_pattern, loading: indexPattern.loading }; diff --git a/x-pack/plugins/uptime/server/lib/saved_objects.ts b/x-pack/plugins/uptime/server/lib/saved_objects.ts index 0e52207e23408..d849fbd8ce0a8 100644 --- a/x-pack/plugins/uptime/server/lib/saved_objects.ts +++ b/x-pack/plugins/uptime/server/lib/saved_objects.ts @@ -26,7 +26,7 @@ export const umDynamicSettings: SavedObjectsType = { heartbeatIndices: { type: 'keyword', }, - certificatesThresholds: { + certThresholds: { properties: { expiration: { type: 'long', diff --git a/x-pack/test/functional/apps/uptime/settings.ts b/x-pack/test/functional/apps/uptime/settings.ts index 8f7aab7aa6e29..64b6300e0df63 100644 --- a/x-pack/test/functional/apps/uptime/settings.ts +++ b/x-pack/test/functional/apps/uptime/settings.ts @@ -60,7 +60,13 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await settings.go(); - const newFieldValues: DynamicSettings = { heartbeatIndices: 'new*' }; + const newFieldValues: DynamicSettings = { + heartbeatIndices: 'new*', + certThresholds: { + age: 365, + expiration: 30, + }, + }; await settings.changeHeartbeatIndicesInput(newFieldValues.heartbeatIndices); await settings.apply(); @@ -89,7 +95,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { // Verify that the settings page shows the value we previously saved await settings.go(); const fields = await settings.loadFields(); - expect(fields.certificatesThresholds?.expiration).to.eql(newErrorThreshold); + expect(fields.certThresholds?.expiration).to.eql(newErrorThreshold); }); it('changing certificate expiration warning threshold is reflected in settings page', async () => { @@ -106,7 +112,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { // Verify that the settings page shows the value we previously saved await settings.go(); const fields = await settings.loadFields(); - expect(fields.certificatesThresholds?.age).to.eql(newWarningThreshold); + expect(fields.certThresholds?.age).to.eql(newWarningThreshold); }); }); }; diff --git a/x-pack/test/functional/services/uptime/settings.ts b/x-pack/test/functional/services/uptime/settings.ts index 1eb19d2993e28..9719152b62d35 100644 --- a/x-pack/test/functional/services/uptime/settings.ts +++ b/x-pack/test/functional/services/uptime/settings.ts @@ -41,7 +41,7 @@ export function UptimeSettingsProvider({ getService }: FtrProviderContext) { return { heartbeatIndices, - certificatesThresholds: { + certThresholds: { age: parseInt(age, 10), expiration: parseInt(expiration, 10), },