Fix number input validation bug

This commit is contained in:
Marliana Lara 2020-10-29 16:18:07 -04:00
parent e0feda780b
commit 79930347f9
No known key found for this signature in database
GPG Key ID: 38C73B40DFA809EE
17 changed files with 147 additions and 83 deletions

View File

@ -22,7 +22,7 @@ class Settings extends Base {
return this.http.options(`${this.baseUrl}${category}/`);
}
test(category, data) {
createTest(category, data) {
return this.http.post(`${this.baseUrl}${category}/test/`, data);
}
}

View File

@ -1,7 +1,10 @@
import React from 'react';
import { act } from 'react-dom/test-utils';
import { createMemoryHistory } from 'history';
import { mountWithContexts } from '../../../../testUtils/enzymeHelpers';
import {
mountWithContexts,
waitForElement,
} from '../../../../testUtils/enzymeHelpers';
import ActivityStream from './ActivityStream';
import { SettingsAPI } from '../../../api';
@ -56,4 +59,27 @@ describe('<ActivityStream />', () => {
});
expect(wrapper.find('ContentError').length).toBe(1);
});
test('should redirect to details for users without system admin permissions', async () => {
const history = createMemoryHistory({
initialEntries: ['/settings/activity_stream/edit'],
});
await act(async () => {
wrapper = mountWithContexts(<ActivityStream />, {
context: {
router: {
history,
},
config: {
me: {
is_superuser: false,
},
},
},
});
});
await waitForElement(wrapper, 'ContentLoading', el => el.length === 0);
expect(wrapper.find('ActivityStreamDetail').length).toBe(1);
expect(wrapper.find('ActivityStreamEdit').length).toBe(0);
});
});

View File

@ -51,24 +51,17 @@ function ActivityStreamEdit() {
request();
}, [request]);
const {
error: submitError,
request: submitForm,
result: submitResult,
} = useRequest(
useCallback(async values => {
const result = await SettingsAPI.updateAll(values);
return result;
}, []),
const { error: submitError, request: submitForm } = useRequest(
useCallback(
async values => {
await SettingsAPI.updateAll(values);
history.push('/settings/activity_stream/details');
},
[history]
),
null
);
useEffect(() => {
if (submitResult) {
history.push('/settings/activity_stream/details');
}
}, [submitResult, history]);
const handleSubmit = async form => {
await submitForm(form);
};

View File

@ -43,24 +43,17 @@ function AzureADEdit() {
fetchAzureAD();
}, [fetchAzureAD]);
const {
error: submitError,
request: submitForm,
result: submitResult,
} = useRequest(
useCallback(async values => {
const result = await SettingsAPI.updateAll(values);
return result;
}, []),
const { error: submitError, request: submitForm } = useRequest(
useCallback(
async values => {
await SettingsAPI.updateAll(values);
history.push('/settings/azure/details');
},
[history]
),
null
);
useEffect(() => {
if (submitResult) {
history.push('/settings/azure/details');
}
}, [submitResult, history]);
const handleSubmit = async form => {
await submitForm({
...form,
@ -90,7 +83,10 @@ function AzureADEdit() {
const initialValues = fields =>
Object.keys(fields).reduce((acc, key) => {
if (fields[key].type === 'list' || fields[key].type === 'nested object') {
acc[key] = JSON.stringify(fields[key].value, null, 2);
const emptyDefault = fields[key].type === 'list' ? '[]' : '{}';
acc[key] = fields[key].value
? JSON.stringify(fields[key].value, null, 2)
: emptyDefault;
} else {
acc[key] = fields[key].value ?? '';
}

View File

@ -80,4 +80,27 @@ describe('<Logging />', () => {
});
expect(wrapper.find('ContentError').length).toBe(1);
});
test('should redirect to details for users without system admin permissions', async () => {
const history = createMemoryHistory({
initialEntries: ['/settings/logging/edit'],
});
await act(async () => {
wrapper = mountWithContexts(<Logging />, {
context: {
router: {
history,
},
config: {
me: {
is_superuser: false,
},
},
},
});
});
await waitForElement(wrapper, 'ContentLoading', el => el.length === 0);
expect(wrapper.find('LoggingDetail').length).toBe(1);
expect(wrapper.find('LoggingEdit').length).toBe(0);
});
});

View File

@ -51,24 +51,17 @@ function LoggingEdit({ i18n }) {
fetchLogging();
}, [fetchLogging]);
const {
error: submitError,
request: submitForm,
result: submitResult,
} = useRequest(
useCallback(async values => {
const result = await SettingsAPI.updateAll(values);
return result;
}, []),
const { error: submitError, request: submitForm } = useRequest(
useCallback(
async values => {
await SettingsAPI.updateAll(values);
history.push('/settings/logging/details');
},
[history]
),
null
);
useEffect(() => {
if (submitResult) {
history.push('/settings/logging/details');
}
}, [submitResult, history]);
const handleSubmit = async form => {
await submitForm({
...form,
@ -95,7 +88,7 @@ function LoggingEdit({ i18n }) {
setValue: setTestLogging,
} = useRequest(
useCallback(async () => {
const result = await SettingsAPI.test('logging', {});
const result = await SettingsAPI.createTest('logging', {});
return result;
}, []),
null

View File

@ -212,15 +212,15 @@ describe('<LoggingEdit />', () => {
});
test('should display successful toast when test button is clicked', async () => {
SettingsAPI.test.mockResolvedValue({});
expect(SettingsAPI.test).toHaveBeenCalledTimes(0);
SettingsAPI.createTest.mockResolvedValue({});
expect(SettingsAPI.createTest).toHaveBeenCalledTimes(0);
expect(wrapper.find('LoggingTestAlert')).toHaveLength(0);
await act(async () => {
wrapper.find('button[aria-label="Test logging"]').invoke('onClick')();
});
wrapper.update();
await waitForElement(wrapper, 'LoggingTestAlert');
expect(SettingsAPI.test).toHaveBeenCalledTimes(1);
expect(SettingsAPI.createTest).toHaveBeenCalledTimes(1);
await act(async () => {
wrapper.find('AlertActionCloseButton button').invoke('onClick')();
});

View File

@ -58,4 +58,27 @@ describe('<MiscSystem />', () => {
});
expect(wrapper.find('ContentError').length).toBe(1);
});
test('should redirect to details for users without system admin permissions', async () => {
const history = createMemoryHistory({
initialEntries: ['/settings/miscellaneous_system/edit'],
});
await act(async () => {
wrapper = mountWithContexts(<MiscSystem />, {
context: {
router: {
history,
},
config: {
me: {
is_superuser: false,
},
},
},
});
});
await waitForElement(wrapper, 'ContentLoading', el => el.length === 0);
expect(wrapper.find('MiscSystemDetail').length).toBe(1);
expect(wrapper.find('MiscSystemEdit').length).toBe(0);
});
});

View File

@ -107,15 +107,14 @@ function MiscSystemEdit({ i18n }) {
fetchSystem();
}, [fetchSystem]);
const {
error: submitError,
request: submitForm,
result: submitResult,
} = useRequest(
useCallback(async values => {
const result = await SettingsAPI.updateAll(values);
return result;
}, []),
const { error: submitError, request: submitForm } = useRequest(
useCallback(
async values => {
await SettingsAPI.updateAll(values);
history.push('/settings/miscellaneous_system/details');
},
[history]
),
null
);
@ -138,12 +137,6 @@ function MiscSystemEdit({ i18n }) {
});
};
useEffect(() => {
if (submitResult) {
history.push('/settings/miscellaneous_system/details');
}
}, [submitResult, history]);
const handleRevertAll = async () => {
const {
ACCESS_TOKEN_EXPIRE_SECONDS,

View File

@ -46,7 +46,7 @@ describe('<Settings />', () => {
expect(wrapper.find('SettingList').length).toBe(0);
});
test('should render Settings for users with permissions system admin or auditor permissions', async () => {
test('should render Settings for users with system admin or auditor permissions', async () => {
const history = createMemoryHistory({
initialEntries: ['/settings'],
});

View File

@ -30,8 +30,8 @@ function RevertAllAlert({ i18n, onClose, onRevertAll }) {
</Button>,
]}
>
{i18n._(t`This will revert all configuration values to their
factory defaults. Are you sure you want to proceed?`)}
{i18n._(t`This will revert all configuration values on this page to
their factory defaults. Are you sure you want to proceed?`)}
</AlertModal>
);
}

View File

@ -18,7 +18,7 @@ describe('RevertButton', () => {
test_input: 'foo',
}}
>
{() => <RevertButton id="test_input" defaultValue="" />}
<RevertButton id="test_input" defaultValue="" />
</Formik>
);
expect(wrapper.find('button').text()).toEqual('Revert');
@ -34,7 +34,7 @@ describe('RevertButton', () => {
test_input: 'bar',
}}
>
{() => <RevertButton id="test_input" defaultValue="bar" />}
<RevertButton id="test_input" defaultValue="bar" />
</Formik>
);
expect(wrapper.find('button').text()).toEqual('Revert');
@ -47,7 +47,7 @@ describe('RevertButton', () => {
test_input: 'foo',
}}
>
{() => <RevertButton id="test_input" defaultValue="bar" />}
<RevertButton id="test_input" defaultValue="bar" />
</Formik>
);
expect(wrapper.find('button').text()).toEqual('Revert');
@ -68,7 +68,7 @@ describe('RevertButton', () => {
test_input: 'bar',
}}
>
{() => <RevertButton id="test_input" defaultValue="bar" />}
<RevertButton id="test_input" defaultValue="bar" />
</Formik>
);
expect(wrapper.find('button').text()).toEqual('Revert');

View File

@ -15,7 +15,13 @@ import CodeMirrorInput from '../../../components/CodeMirrorInput';
import { PasswordInput } from '../../../components/FormField';
import { FormFullWidthLayout } from '../../../components/FormLayout';
import Popover from '../../../components/Popover';
import { combine, required, url, minMaxValue } from '../../../util/validators';
import {
combine,
required,
url,
integer,
minMaxValue,
} from '../../../util/validators';
import RevertButton from './RevertButton';
const FormGroup = styled(PFFormGroup)`
@ -176,9 +182,11 @@ const InputField = withI18n()(
max_value = Number.MAX_SAFE_INTEGER,
} = config;
const validators = [
isRequired ? required(null, i18n) : null,
type === 'url' ? url(i18n) : null,
type === 'number' ? minMaxValue(min_value, max_value, i18n) : null,
...(isRequired ? [required(null, i18n)] : []),
...(type === 'url' ? [url(i18n)] : []),
...(type === 'number'
? [integer(i18n), minMaxValue(min_value, max_value, i18n)]
: []),
];
const [field, meta] = useField({ name, validate: combine(validators) });
const isValid = !(meta.touched && meta.error);
@ -197,7 +205,6 @@ const InputField = withI18n()(
id={name}
isRequired={isRequired}
placeholder={config.placeholder}
type={type}
validated={isValid ? 'default' : 'error'}
value={field.value}
onBlur={field.onBlur}

View File

@ -2,6 +2,7 @@ import { useState } from 'react';
/**
* useModal hook provides a way to read and update modal visibility
* Param: boolean that sets initial modal state
* Returns: {
* isModalOpen: boolean that indicates if modal is open
* toggleModal: function that toggles the modal open and close
@ -9,8 +10,8 @@ import { useState } from 'react';
* }
*/
export default function useModal() {
const [isModalOpen, setIsModalOpen] = useState(false);
export default function useModal(isOpen = false) {
const [isModalOpen, setIsModalOpen] = useState(isOpen);
function toggleModal() {
setIsModalOpen(!isModalOpen);

View File

@ -17,7 +17,7 @@ describe('useModal hook', () => {
let isModalOpen;
let toggleModal;
test('should return expected initial values', () => {
test('isModalOpen should return expected default value', () => {
testHook(() => {
({ isModalOpen, toggleModal, closeModal } = useModal());
});
@ -26,6 +26,15 @@ describe('useModal hook', () => {
expect(closeModal).toBeInstanceOf(Function);
});
test('isModalOpen should return expected initialized value', () => {
testHook(() => {
({ isModalOpen, toggleModal, closeModal } = useModal(true));
});
expect(isModalOpen).toEqual(true);
expect(toggleModal).toBeInstanceOf(Function);
expect(closeModal).toBeInstanceOf(Function);
});
test('should return expected isModalOpen value after modal toggle', () => {
testHook(() => {
({ isModalOpen, toggleModal, closeModal } = useModal());

View File

@ -69,7 +69,7 @@ export function noWhiteSpace(i18n) {
export function integer(i18n) {
return value => {
const str = String(value);
if (/[^0-9]/.test(str)) {
if (!Number.isInteger(value) && /[^0-9]/.test(str)) {
return i18n._(t`This field must be an integer`);
}
return undefined;