Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] [Uptime] Switch from EuiFieldNumber to EuiFieldText on settings page (#66425) #68044

Merged
merged 1 commit into from
Jun 3, 2020
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
6 changes: 5 additions & 1 deletion x-pack/plugins/uptime/common/translations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@

import { i18n } from '@kbn/i18n';

export const VALUE_MUST_BE_GREATER_THEN_ZEO = i18n.translate(
export const VALUE_MUST_BE_GREATER_THAN_ZERO = i18n.translate(
'xpack.uptime.settings.invalid.error',
{
defaultMessage: 'Value must be greater than 0.',
}
);

export const VALUE_MUST_BE_AN_INTEGER = i18n.translate('xpack.uptime.settings.invalid.nanError', {
defaultMessage: 'Value must be an integer.',
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import React from 'react';
import { CertificateExpirationForm } from '../certificate_form';
import { shallowWithRouter } from '../../../lib';
import { shallowWithRouter, mountWithRouter } from '../../../lib';

describe('CertificateForm', () => {
it('shallow renders expected elements for valid props', () => {
Expand All @@ -26,4 +26,110 @@ describe('CertificateForm', () => {
)
).toMatchSnapshot();
});

it('submits number values for certs settings fields', () => {
const onChangeMock = jest.fn();
const wrapper = mountWithRouter(
<CertificateExpirationForm
loading={false}
onChange={onChangeMock}
formFields={{
heartbeatIndices: 'heartbeat-8*',
certExpirationThreshold: 7,
certAgeThreshold: 36,
}}
fieldErrors={null}
isDisabled={false}
/>
);

const inputs = wrapper.find('input');

expect(inputs).toHaveLength(2);

// expiration threshold input
inputs.at(0).simulate('change', {
target: {
value: '23',
},
});

// age threshold input
inputs.at(1).simulate('change', {
target: {
value: '56',
},
});

expect(onChangeMock).toHaveBeenCalledTimes(2);

expect(onChangeMock.mock.calls[0]).toMatchInlineSnapshot(`
Array [
Object {
"certExpirationThreshold": 23,
},
]
`);

expect(onChangeMock.mock.calls[1]).toMatchInlineSnapshot(`
Array [
Object {
"certAgeThreshold": 56,
},
]
`);
});

it('submits undefined for NaN values', () => {
const onChangeMock = jest.fn();
const wrapper = mountWithRouter(
<CertificateExpirationForm
loading={false}
onChange={onChangeMock}
formFields={{
heartbeatIndices: 'heartbeat-8*',
certExpirationThreshold: 7,
certAgeThreshold: 36,
}}
fieldErrors={null}
isDisabled={false}
/>
);

const inputs = wrapper.find('input');

expect(inputs).toHaveLength(2);

// expiration threshold input
inputs.at(0).simulate('change', {
target: {
value: 'A',
},
});

// age threshold input
inputs.at(1).simulate('change', {
target: {
value: 'g',
},
});

expect(onChangeMock).toHaveBeenCalledTimes(2);

expect(onChangeMock.mock.calls[0]).toMatchInlineSnapshot(`
Array [
Object {
"certExpirationThreshold": undefined,
},
]
`);

expect(onChangeMock.mock.calls[1]).toMatchInlineSnapshot(`
Array [
Object {
"certAgeThreshold": undefined,
},
]
`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
EuiDescribedFormGroup,
EuiFormRow,
EuiCode,
EuiFieldNumber,
EuiFieldText,
EuiText,
EuiTitle,
EuiSpacer,
Expand Down Expand Up @@ -80,17 +80,17 @@ export const CertificateExpirationForm: React.FC<SettingsFormProps> = ({
>
<EuiFlexGroup>
<EuiFlexItem grow={2}>
<EuiFieldNumber
min={1}
<EuiFieldText
aria-label={certificateFormTranslations.expirationInputAriaLabel}
data-test-subj={`expiration-threshold-input-${loading ? 'loading' : 'loaded'}`}
fullWidth
disabled={isDisabled}
isInvalid={!!fieldErrors?.expirationThresholdError}
isLoading={loading}
value={formFields?.certExpirationThreshold ?? ''}
onChange={(e) =>
onChange({
certExpirationThreshold: Number(e.target.value),
certExpirationThreshold: Number(e.target.value) || undefined,
})
}
/>
Expand Down Expand Up @@ -128,17 +128,17 @@ export const CertificateExpirationForm: React.FC<SettingsFormProps> = ({
>
<EuiFlexGroup>
<EuiFlexItem grow={2}>
<EuiFieldNumber
min={1}
<EuiFieldText
aria-label={certificateFormTranslations.ageInputAriaLabel}
data-test-subj={`age-threshold-input-${loading ? 'loading' : 'loaded'}`}
fullWidth
disabled={isDisabled}
isInvalid={!!fieldErrors?.ageThresholdError}
isLoading={loading}
value={formFields?.certAgeThreshold ?? ''}
onChange={({ currentTarget: { value } }) =>
onChange={({ target: { value } }) =>
onChange({
certAgeThreshold: Number(value),
certAgeThreshold: Number(value) || undefined,
})
}
/>
Expand Down
35 changes: 35 additions & 0 deletions x-pack/plugins/uptime/public/pages/__tests__/settings.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { isValidCertVal } from '../settings';

describe('settings', () => {
describe('isValidCertVal', () => {
it('handles NaN values', () => {
expect(isValidCertVal(NaN)).toMatchInlineSnapshot(`"Must be a number."`);
});

it('handles undefined', () => {
expect(isValidCertVal(undefined)).toMatchInlineSnapshot(`"Must be a number."`);
});

it('handles non-integer numbers', () => {
expect(isValidCertVal(23.5)).toMatchInlineSnapshot(`"Value must be an integer."`);
});

it('handles values less than 0', () => {
expect(isValidCertVal(-1)).toMatchInlineSnapshot(`"Value must be greater than 0."`);
});

it('handles 0', () => {
expect(isValidCertVal(0)).toMatchInlineSnapshot(`"Value must be greater than 0."`);
});

it('allows valid integer numbers', () => {
expect(isValidCertVal(67)).toBeUndefined();
});
});
});
18 changes: 12 additions & 6 deletions x-pack/plugins/uptime/public/pages/settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ import {
OnFieldChangeType,
} from '../components/settings/certificate_form';
import * as Translations from './translations';
import { VALUE_MUST_BE_GREATER_THEN_ZEO } from '../../common/translations';
import {
VALUE_MUST_BE_GREATER_THAN_ZERO,
VALUE_MUST_BE_AN_INTEGER,
} from '../../common/translations';

interface SettingsPageFieldErrors {
heartbeatIndices: string | '';
Expand All @@ -47,12 +50,15 @@ export interface SettingsFormProps {
isDisabled: boolean;
}

const isValidCertVal = (val: string | number) => {
if (val === '') {
return Translations.BLANK_STR;
export const isValidCertVal = (val?: number): string | undefined => {
if (val === undefined || isNaN(val)) {
return Translations.settings.mustBeNumber;
}
if (val <= 0) {
return VALUE_MUST_BE_GREATER_THAN_ZERO;
}
if (val === 0) {
return VALUE_MUST_BE_GREATER_THEN_ZEO;
if (val % 1) {
return VALUE_MUST_BE_AN_INTEGER;
}
};

Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/uptime/public/pages/translations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ export const settings = {
returnToOverviewLinkLabel: i18n.translate('xpack.uptime.settings.returnToOverviewLinkLabel', {
defaultMessage: 'Return to overview',
}),
mustBeNumber: i18n.translate('xpack.uptime.settings.blankNumberField.error', {
defaultMessage: 'Must be a number.',
}),
};

export const BLANK_STR = i18n.translate('xpack.uptime.settings.blank.error', {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { validateCertsValues } from '../dynamic_settings';

describe('dynamic settings', () => {
describe('validateCertValues', () => {
it(`doesn't allow age threshold values less than 0`, () => {
expect(
validateCertsValues({
certAgeThreshold: -1,
certExpirationThreshold: 2,
heartbeatIndices: 'foo',
})
).toMatchInlineSnapshot(`
Object {
"certAgeThreshold": "Value must be greater than 0.",
}
`);
});

it(`doesn't allow non-integer age threshold values`, () => {
expect(
validateCertsValues({
certAgeThreshold: 10.2,
certExpirationThreshold: 2,
heartbeatIndices: 'foo',
})
).toMatchInlineSnapshot(`
Object {
"certAgeThreshold": "Value must be an integer.",
}
`);
});

it(`doesn't allow expiration threshold values less than 0`, () => {
expect(
validateCertsValues({
certAgeThreshold: 2,
certExpirationThreshold: -1,
heartbeatIndices: 'foo',
})
).toMatchInlineSnapshot(`
Object {
"certExpirationThreshold": "Value must be greater than 0.",
}
`);
});

it(`doesn't allow non-integer expiration threshold values`, () => {
expect(
validateCertsValues({
certAgeThreshold: 2,
certExpirationThreshold: 1.23,
heartbeatIndices: 'foo',
})
).toMatchInlineSnapshot(`
Object {
"certExpirationThreshold": "Value must be an integer.",
}
`);
});

it('allows valid values', () => {
expect(
validateCertsValues({
certAgeThreshold: 2,
certExpirationThreshold: 13,
heartbeatIndices: 'foo',
})
).toBeUndefined();
});
});
});
17 changes: 13 additions & 4 deletions x-pack/plugins/uptime/server/rest_api/dynamic_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import { UMServerLibs } from '../lib/lib';
import { DynamicSettings, DynamicSettingsType } from '../../common/runtime_types';
import { UMRestApiRouteFactory } from '.';
import { savedObjectsAdapter } from '../lib/saved_objects';
import { VALUE_MUST_BE_GREATER_THEN_ZEO } from '../../common/translations';
import {
VALUE_MUST_BE_GREATER_THAN_ZERO,
VALUE_MUST_BE_AN_INTEGER,
} from '../../common/translations';

export const createGetDynamicSettingsRoute: UMRestApiRouteFactory = (libs: UMServerLibs) => ({
method: 'GET',
Expand All @@ -24,13 +27,19 @@ export const createGetDynamicSettingsRoute: UMRestApiRouteFactory = (libs: UMSer
},
});

const validateCertsValues = (settings: DynamicSettings) => {
export const validateCertsValues = (
settings: DynamicSettings
): Record<string, string> | undefined => {
const errors: any = {};
if (settings.certAgeThreshold <= 0) {
errors.certAgeThreshold = VALUE_MUST_BE_GREATER_THEN_ZEO;
errors.certAgeThreshold = VALUE_MUST_BE_GREATER_THAN_ZERO;
} else if (settings.certAgeThreshold % 1) {
errors.certAgeThreshold = VALUE_MUST_BE_AN_INTEGER;
}
if (settings.certExpirationThreshold <= 0) {
errors.certExpirationThreshold = VALUE_MUST_BE_GREATER_THEN_ZEO;
errors.certExpirationThreshold = VALUE_MUST_BE_GREATER_THAN_ZERO;
} else if (settings.certExpirationThreshold % 1) {
errors.certExpirationThreshold = VALUE_MUST_BE_AN_INTEGER;
}
if (errors.certAgeThreshold || errors.certExpirationThreshold) {
return errors;
Expand Down