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.

Sign up for GitHub

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

[open-formulieren/open-forms#3611] Make time bounds validation inclusive #596

Merged
merged 4 commits into from
Nov 22, 2023
Merged

[open-formulieren/open-forms#3611] Make time bounds validation inclusive #596

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e8fa867
[open-formulieren/open-forms#3611] Make time bounds validation inclusive
Viicos Nov 20, 2023
d245e80
[open-formulieren/open-forms#3611] Refactor tests to be parametrized
Viicos Nov 20, 2023
d18c7ac
[open-formulieren/open-forms#3611] Actually let jest run the expectat…
Viicos Nov 21, 2023
3598e1a
[open-formulieren/open-forms#3611] Add missing sleep call in test
Viicos Nov 22, 2023
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: 3 additions & 3 deletions src/formio/validators/MinMaxTimeValidator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,20 @@
// Case 1: only one boundary is given
if (!minTime || !maxTime) {
if (minTime) return {isValid: parsedValue >= minTime, errorKeys: ['minTime']};
if (maxTime) return {isValid: parsedValue < maxTime, errorKeys: ['maxTime']};
if (maxTime) return {isValid: parsedValue <= maxTime, errorKeys: ['maxTime']};

Check warning on line 17 in src/formio/validators/MinMaxTimeValidator.js

View check run for this annotation

Codecov / codecov/patch

src/formio/validators/MinMaxTimeValidator.js#L17 

Added line #L17 was not covered by tests
} else {
// Case 2: min boundary is smaller than max boundary
if (minTime < maxTime) {
const isTooEarly = parsedValue < minTime;
const isTooLate = parsedValue >= maxTime;
const isTooLate = parsedValue > maxTime;

Check warning on line 22 in src/formio/validators/MinMaxTimeValidator.js

View check run for this annotation

Codecov / codecov/patch

src/formio/validators/MinMaxTimeValidator.js#L22 

Added line #L22 was not covered by tests
return {
isValid: !isTooEarly && !isTooLate,
errorKeys: [isTooEarly ? 'minTime' : 'maxTime', 'invalid_time'],
};
} else {
// Case 3: min boundary is bigger than max boundary (it's the next day. For example min = 08:00, max = 01:00)
return {
isValid: !(parsedValue >= maxTime && parsedValue < minTime),
isValid: !(parsedValue > maxTime && parsedValue < minTime), // Basically, the opposite of 01:00 < `parsedValue` < 08:00

Check warning on line 30 in src/formio/validators/MinMaxTimeValidator.js

View check run for this annotation

Codecov / codecov/patch

src/formio/validators/MinMaxTimeValidator.js#L30 

Added line #L30 was not covered by tests
errorKeys: ['invalid_time'],
};
}
Expand Down
285 changes: 111 additions & 174 deletions src/jstests/formio/components/time.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import _ from 'lodash';
import React from 'react';
import {Formio} from 'react-formio';

import OpenFormsModule from 'formio/module';
Expand All @@ -10,195 +9,133 @@ import {timeForm} from './fixtures/time';
Formio.use(OpenFormsModule);

describe('Time Component', () => {
test('Time component with min/max time validation', done => {
it.each([
['09:00:00', true],
['10:30:00', true],
['11:11:11', true],
['17:00:00', true],
['17:30:00', false],
['08:30:00', false],
])('Time component with min/max time validation', async (value, valid) => {
let formJSON = _.cloneDeep(timeForm);
formJSON.components[0].validate.minTime = '09:00:00';
formJSON.components[0].validate.maxTime = '17:00:00';

const validValues = ['09:00:00', '10:30:00', '11:11:11'];

const invalidValues = ['17:00:00', '17:30:00', '08:30:00'];

const testValidity = (values, valid) => {
values.forEach(value => {
const element = document.createElement('div');

Formio.createForm(element, formJSON)
.then(form => {
form.setPristine(false);
const component = form.getComponent('time');
const changed = component.setValue(value);
expect(changed).toBeTruthy();

setTimeout(() => {
if (valid) {
expect(!!component.error).toBeFalsy();
} else {
expect(!!component.error).toBeTruthy();
expect(component.error.message).toEqual('invalid_time');
}

if (value === invalidValues[2]) {
done();
}
}, 300);
})
.catch(done);
});
};

testValidity(validValues, true);
testValidity(invalidValues, false);
});

test('Time component without min/max time validation', done => {
let formJSON = _.cloneDeep(timeForm);

const validValues = ['00:00:00', '23:59:59', '11:11:11'];

const testValidity = values => {
values.forEach(value => {
const element = document.createElement('div');

Formio.createForm(element, formJSON)
.then(form => {
form.setPristine(false);
const component = form.getComponent('time');
const changed = component.setValue(value);
expect(changed).toBeTruthy();

setTimeout(() => {
expect(!!component.error).toBeFalsy();

if (value === validValues[2]) {
done();
}
}, 300);
})
.catch(done);
});
};

testValidity(validValues);
const element = document.createElement('div');
const form = await Formio.createForm(element, formJSON);
form.setPristine(false);

const component = form.getComponent('time');
const changed = component.setValue(value);
expect(changed).toBeTruthy();
setTimeout(() => {
if (valid) {
expect(!!component.error).toBeFalsy();
} else {
expect(!!component.error).toBeTruthy();
expect(component.error.message).toEqual('invalid_time');
}
}, 300);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should also be made async, I think we have a sleep utility somewhere. Now the end of test execution is never signalled and the block likely exits before the assertions are made.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, feels really weird from a UX perspective. I guess this is the current state of JS frontend tests

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, it has everything to do with Formio's event dispatching and debouncing. With a pure react implementation, we shouldn't need setTimeout anymore

});

test('Time component with only min time validation', done => {
it.each(['00:00:00', '23:59:59', '11:11:11'])(
'Time component without min/max time validation',
async value => {
let formJSON = _.cloneDeep(timeForm);

const element = document.createElement('div');
const form = await Formio.createForm(element, formJSON);
form.setPristine(false);

const component = form.getComponent('time');
const changed = component.setValue(value);

expect(changed).toBeTruthy();
setTimeout(() => {
expect(!!component.error).toBeFalsy();
}, 300);
Viicos marked this conversation as resolved.
Show resolved Hide resolved
}
);

it.each([
['17:00:00', true],
['08:00:00', false],
])('Time component with only min time validation', async (value, valid) => {
let formJSON = _.cloneDeep(timeForm);
formJSON.components[0].validate.minTime = '09:00:00';

const validValues = ['17:00:00'];

const invalidValues = ['08:00:00'];

const testValidity = (values, valid) => {
values.forEach(value => {
const element = document.createElement('div');

Formio.createForm(element, formJSON)
.then(form => {
form.setPristine(false);
const component = form.getComponent('time');
const changed = component.setValue(value);
expect(changed).toBeTruthy();

setTimeout(() => {
if (valid) {
expect(!!component.error).toBeFalsy();
} else {
expect(!!component.error).toBeTruthy();
}

if (value === invalidValues[0]) {
done();
}
}, 300);
})
.catch(done);
});
};

testValidity(validValues, true);
testValidity(invalidValues, false);
const element = document.createElement('div');
const form = await Formio.createForm(element, formJSON);
form.setPristine(false);

const component = form.getComponent('time');
const changed = component.setValue(value);
expect(changed).toBeTruthy();
setTimeout(() => {
if (valid) {
expect(!!component.error).toBeFalsy();
} else {
expect(!!component.error).toBeTruthy();
expect(component.error.message).toEqual('invalid_time');
}
}, 300);
});

test('Time component with only max time validation', done => {
it.each([
['08:00:00', true],
['09:00:00', true],
['17:00:00', false],
])('Time component with only max time validation', async (value, valid) => {
let formJSON = _.cloneDeep(timeForm);
formJSON.components[0].validate.maxTime = '09:00:00';

const validValues = ['08:00:00'];

const invalidValues = ['17:00:00'];

const testValidity = (values, valid) => {
values.forEach(value => {
const element = document.createElement('div');

Formio.createForm(element, formJSON)
.then(form => {
form.setPristine(false);
const component = form.getComponent('time');
const changed = component.setValue(value);
expect(changed).toBeTruthy();

setTimeout(() => {
if (valid) {
expect(!!component.error).toBeFalsy();
} else {
expect(!!component.error).toBeTruthy();
}

if (value === invalidValues[0]) {
done();
}
}, 300);
})
.catch(done);
});
};

testValidity(validValues, true);
testValidity(invalidValues, false);
const element = document.createElement('div');
const form = await Formio.createForm(element, formJSON);
form.setPristine(false);

const component = form.getComponent('time');
const changed = component.setValue(value);
expect(changed).toBeTruthy();
setTimeout(() => {
if (valid) {
expect(!!component.error).toBeFalsy();
} else {
expect(!!component.error).toBeTruthy();
expect(component.error.message).toEqual('invalid_time');
}
}, 300);
});

test('Time component with min time boundary larger than max time boundary', done => {
let formJSON = _.cloneDeep(timeForm);
formJSON.components[0].validate.maxTime = '01:00:00';
formJSON.components[0].validate.minTime = '08:00:00';

const validValues = ['09:00:00', '00:30:00'];

const invalidValues = ['02:00:00'];

const testValidity = (values, valid) => {
values.forEach(value => {
const element = document.createElement('div');

Formio.createForm(element, formJSON)
.then(form => {
form.setPristine(false);
const component = form.getComponent('time');
const changed = component.setValue(value);
expect(changed).toBeTruthy();

setTimeout(() => {
if (valid) {
expect(!!component.error).toBeFalsy();
} else {
expect(!!component.error).toBeTruthy();
}

if (value === invalidValues[0]) {
done();
}
}, 300);
})
.catch(done);
});
};

testValidity(validValues, true);
testValidity(invalidValues, false);
});
it.each([
['09:00:00', true],
['00:30:00', true],
['01:00:00', true],
['08:00:00', true],
['02:00:00', false],
])(
'Time component with min time boundary larger than max time boundary',
async (value, valid) => {
let formJSON = _.cloneDeep(timeForm);
formJSON.components[0].validate.maxTime = '01:00:00';
formJSON.components[0].validate.minTime = '08:00:00';

const element = document.createElement('div');
const form = await Formio.createForm(element, formJSON);
form.setPristine(false);

const component = form.getComponent('time');
const changed = component.setValue(value);
expect(changed).toBeTruthy();
setTimeout(() => {
if (valid) {
expect(!!component.error).toBeFalsy();
} else {
expect(!!component.error).toBeTruthy();
expect(component.error.message).toEqual('invalid_time');
}
}, 300);
}
);

test('Time component with both min/max and max > min validation and custom error', done => {
let formJSON = _.cloneDeep(timeForm);
Expand Down
Loading