From 5a99590caa3598de155eed8eddcc412790637ba4 Mon Sep 17 00:00:00 2001 From: SilviaAmAm Date: Mon, 4 Sep 2023 15:26:06 +0200 Subject: [PATCH 1/3] :sparkles: [open-formulieren/open-forms#1884] Add context to choose right error message after time validation --- src/formio/validators/MinMaxTimeValidator.js | 59 +++++++++++++++++--- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/src/formio/validators/MinMaxTimeValidator.js b/src/formio/validators/MinMaxTimeValidator.js index 2156ffc1d..cf6cc0d8b 100644 --- a/src/formio/validators/MinMaxTimeValidator.js +++ b/src/formio/validators/MinMaxTimeValidator.js @@ -1,5 +1,13 @@ +import set from 'lodash/set'; import moment from 'moment'; +const ERROR_CASES = { + ONLY_MIN: 'onlyMin', + ONLY_MAX: 'onlyMax', + MIN_SMALLER_THAN_MAX: 'minSmallerThanMax', + MAX_SMALLER_THAN_MIN: 'maxSmallerThanMin', +}; + const validateTimeBoundaries = (minBoundary, maxBoundary, timeValue) => { const minTime = minBoundary ? moment(minBoundary, 'HH:mm:ss') : null; const maxTime = maxBoundary ? moment(maxBoundary, 'HH:mm:ss') : null; @@ -7,20 +15,26 @@ const validateTimeBoundaries = (minBoundary, maxBoundary, timeValue) => { // Case 0: no boundaries given if (!minTime && !maxTime) { - return true; + return {isValid: true}; } // Case 1: only one boundary is given if (!minTime || !maxTime) { - if (minTime) return parsedValue >= minTime; - if (maxTime) return parsedValue < maxTime; + if (minTime) return {isValid: parsedValue >= minTime, error: ERROR_CASES.ONLY_MIN}; + if (maxTime) return {isValid: parsedValue < maxTime, error: ERROR_CASES.ONLY_MAX}; } else { // Case 2: min boundary is smaller than max boundary if (minTime < maxTime) { - return parsedValue >= minTime && parsedValue < maxTime; + return { + isValid: parsedValue >= minTime && parsedValue < maxTime, + error: ERROR_CASES.MIN_SMALLER_THAN_MAX, + }; } else { // Case 3: min boundary is bigger than max boundary (it's the next day. For example min = 08:00, max = 01:00) - return !(parsedValue >= maxTime && parsedValue < minTime); + return { + isValid: !(parsedValue >= maxTime && parsedValue < minTime), + error: ERROR_CASES.MAX_SMALLER_THAN_MIN, + }; } } }; @@ -31,7 +45,29 @@ const MinMaxTimeValidator = { const minTime = moment(component.component.minTime || '00:00:00', 'HH:mm:ss').format('HH:mm'); const maxTime = moment(component.component.maxTime || '23:59:59', 'HH:mm:ss').format('HH:mm'); - const errorMessage = component.component.errors?.invalid_time ?? 'invalid_time'; + let errorMessage; + const genericError = component.component.errors?.invalid_time ?? 'invalid_time'; + switch (component?.openForms?.validationErrorContext?.timeError) { + case ERROR_CASES.ONLY_MIN: { + errorMessage = component.component.errors?.minTime ?? genericError; + break; + } + case ERROR_CASES.ONLY_MAX: { + errorMessage = component.component.errors?.maxTime ?? genericError; + break; + } + case ERROR_CASES.MIN_SMALLER_THAN_MAX: + case ERROR_CASES.MAX_SMALLER_THAN_MIN: { + // Here we display the generic error, because it is ambiguous which error message should be + // shown between minTime/maxTime. + errorMessage = genericError; + break; + } + default: { + errorMessage = 'invalid_time'; + } + } + return component.t(errorMessage, { minTime: minTime, maxTime: maxTime, @@ -40,7 +76,16 @@ const MinMaxTimeValidator = { check(component, setting, value) { if (!value) return true; - return validateTimeBoundaries(component.component.minTime, component.component.maxTime, value); + const {isValid, error} = validateTimeBoundaries( + component.component.minTime, + component.component.maxTime, + value + ); + + if (!isValid) { + set(component, 'openForms.validationErrorContext', {timeError: error}); + } + return isValid; }, }; From 0c108534ab745dadd3433a3e5d26e68a9d4a70b5 Mon Sep 17 00:00:00 2001 From: SilviaAmAm Date: Mon, 4 Sep 2023 15:26:42 +0200 Subject: [PATCH 2/3] :white_check_mark: [open-formulieren/open-forms#1884] Test fine grained custom errors for time components --- src/jstests/formio/components/time.spec.js | 95 +++++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-) diff --git a/src/jstests/formio/components/time.spec.js b/src/jstests/formio/components/time.spec.js index 140cacf3d..53af0a0d8 100644 --- a/src/jstests/formio/components/time.spec.js +++ b/src/jstests/formio/components/time.spec.js @@ -200,12 +200,14 @@ describe('Time Component', () => { testValidity(invalidValues, false); }); - test('Time component with min/max validation and custom error', done => { + test('Time component with both min/max and max > min validation and custom error', done => { let formJSON = _.cloneDeep(timeForm); // Note: the backend dynamically updates the configuration so that `translatedErrors` are added to // `errors` in the correct language. formJSON.components[0].errors = { invalid_time: 'Custom error! Min time: {{ minTime }} Max time: {{ maxTime }}.', + minTime: 'Custom error! Min time {{ minTime }}', + maxTime: 'Custom error! max time {{ maxTime }}', }; formJSON.components[0].maxTime = '13:00:00'; formJSON.components[0].minTime = '12:00:00'; @@ -228,4 +230,95 @@ describe('Time Component', () => { }) .catch(done); }); + + test('Time component with both min/max and max < min validation and custom error', done => { + let formJSON = _.cloneDeep(timeForm); + // Note: the backend dynamically updates the configuration so that `translatedErrors` are added to + // `errors` in the correct language. + formJSON.components[0].errors = { + invalid_time: 'Custom error! Min time: {{ minTime }} Max time: {{ maxTime }}.', + minTime: 'Custom error! Min time {{ minTime }}', + maxTime: 'Custom error! max time {{ maxTime }}', + }; + formJSON.components[0].maxTime = '01:00:00'; // One o'clock in the night of the next day + formJSON.components[0].minTime = '08:00:00'; + + const element = document.createElement('div'); + + Formio.createForm(element, formJSON) + .then(form => { + form.setPristine(false); + const component = form.getComponent('time'); + const changed = component.setValue('07:00'); + expect(changed).toBeTruthy(); + + setTimeout(() => { + expect(!!component.error).toBeTruthy(); + expect(component.error.message).toEqual('Custom error! Min time: 08:00 Max time: 01:00.'); + + done(); + }, 300); + }) + .catch(done); + }); + + test('Time component with only min and custom error', done => { + let formJSON = _.cloneDeep(timeForm); + // Note: the backend dynamically updates the configuration so that `translatedErrors` are added to + // `errors` in the correct language. + formJSON.components[0].errors = { + invalid_time: 'Custom error! Min time: {{ minTime }} Max time: {{ maxTime }}.', + minTime: 'Custom error! Min time {{ minTime }}', + maxTime: 'Custom error! max time {{ maxTime }}', + }; + formJSON.components[0].minTime = '13:00:00'; + + const element = document.createElement('div'); + + Formio.createForm(element, formJSON) + .then(form => { + form.setPristine(false); + const component = form.getComponent('time'); + const changed = component.setValue('10:00'); + expect(changed).toBeTruthy(); + + setTimeout(() => { + expect(!!component.error).toBeTruthy(); + expect(component.error.message).toEqual('Custom error! Min time 13:00'); + + done(); + }, 300); + }) + .catch(done); + }); + + test('Time component with only max and custom error', done => { + let formJSON = _.cloneDeep(timeForm); + // Note: the backend dynamically updates the configuration so that `translatedErrors` are added to + // `errors` in the correct language. + formJSON.components[0].errors = { + invalid_time: 'Custom error! Min time: {{ minTime }} Max time: {{ maxTime }}.', + minTime: 'Custom error! Min time {{ minTime }}', + maxTime: 'Custom error! Max time {{ maxTime }}', + }; + formJSON.components[0].maxTime = '13:00:00'; + + const element = document.createElement('div'); + + Formio.createForm(element, formJSON) + .then(form => { + form.setPristine(false); + const component = form.getComponent('time'); + const changed = component.setValue('14:00'); + expect(changed).toBeTruthy(); + + setTimeout(() => { + expect(!!component.error).toBeTruthy(); + expect(component.error.message).toEqual('Custom error! Max time 13:00'); + + done(); + }, 300); + }) + .catch(done); + }); }); From 99732a79636d0c7f4c9a7c7a12d53f4c51238a0e Mon Sep 17 00:00:00 2001 From: SilviaAmAm Date: Wed, 6 Sep 2023 10:55:44 +0200 Subject: [PATCH 3/3] :ok_hand: [open-formulieren/open-forms#1884] PR Feedback --- src/formio/validators/MinMaxTimeValidator.js | 57 ++++++++------------ src/jstests/formio/components/time.spec.js | 30 ++++++++++- 2 files changed, 52 insertions(+), 35 deletions(-) diff --git a/src/formio/validators/MinMaxTimeValidator.js b/src/formio/validators/MinMaxTimeValidator.js index cf6cc0d8b..7c015ac92 100644 --- a/src/formio/validators/MinMaxTimeValidator.js +++ b/src/formio/validators/MinMaxTimeValidator.js @@ -1,13 +1,6 @@ import set from 'lodash/set'; import moment from 'moment'; -const ERROR_CASES = { - ONLY_MIN: 'onlyMin', - ONLY_MAX: 'onlyMax', - MIN_SMALLER_THAN_MAX: 'minSmallerThanMax', - MAX_SMALLER_THAN_MIN: 'maxSmallerThanMin', -}; - const validateTimeBoundaries = (minBoundary, maxBoundary, timeValue) => { const minTime = minBoundary ? moment(minBoundary, 'HH:mm:ss') : null; const maxTime = maxBoundary ? moment(maxBoundary, 'HH:mm:ss') : null; @@ -20,20 +13,22 @@ const validateTimeBoundaries = (minBoundary, maxBoundary, timeValue) => { // Case 1: only one boundary is given if (!minTime || !maxTime) { - if (minTime) return {isValid: parsedValue >= minTime, error: ERROR_CASES.ONLY_MIN}; - if (maxTime) return {isValid: parsedValue < maxTime, error: ERROR_CASES.ONLY_MAX}; + if (minTime) return {isValid: parsedValue >= minTime, errorKeys: ['minTime']}; + if (maxTime) return {isValid: parsedValue < maxTime, errorKeys: ['maxTime']}; } else { // Case 2: min boundary is smaller than max boundary if (minTime < maxTime) { + const isTooEarly = parsedValue < minTime; + const isTooLate = parsedValue >= maxTime; return { - isValid: parsedValue >= minTime && parsedValue < maxTime, - error: ERROR_CASES.MIN_SMALLER_THAN_MAX, + 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), - error: ERROR_CASES.MAX_SMALLER_THAN_MIN, + errorKeys: ['invalid_time'], }; } } @@ -45,26 +40,20 @@ const MinMaxTimeValidator = { const minTime = moment(component.component.minTime || '00:00:00', 'HH:mm:ss').format('HH:mm'); const maxTime = moment(component.component.maxTime || '23:59:59', 'HH:mm:ss').format('HH:mm'); - let errorMessage; - const genericError = component.component.errors?.invalid_time ?? 'invalid_time'; - switch (component?.openForms?.validationErrorContext?.timeError) { - case ERROR_CASES.ONLY_MIN: { - errorMessage = component.component.errors?.minTime ?? genericError; - break; - } - case ERROR_CASES.ONLY_MAX: { - errorMessage = component.component.errors?.maxTime ?? genericError; - break; - } - case ERROR_CASES.MIN_SMALLER_THAN_MAX: - case ERROR_CASES.MAX_SMALLER_THAN_MIN: { - // Here we display the generic error, because it is ambiguous which error message should be - // shown between minTime/maxTime. - errorMessage = genericError; - break; - } - default: { - errorMessage = 'invalid_time'; + let errorMessage = component.component.errors?.invalid_time || 'invalid_time'; + const errorKeys = component?.openForms?.validationErrorContext?.minMaxTimeValidatorErrorKeys; + const componentErrorMessages = component.component.errors; + + // The error keys are in order of priority: for example, if a time is below minTime, the + // errorKeys would be ['minTime', 'invalid_time']. If the form designer configured a custom minTime + // error message, it will be used. Otherwise, it will check if a 'invalid_time' custom error was used. + // If not, it will fall back on the default 'invalid_time'. + if (errorKeys && componentErrorMessages) { + for (const errorKey of errorKeys) { + if (componentErrorMessages[errorKey]) { + errorMessage = componentErrorMessages[errorKey]; + break; + } } } @@ -76,14 +65,14 @@ const MinMaxTimeValidator = { check(component, setting, value) { if (!value) return true; - const {isValid, error} = validateTimeBoundaries( + const {isValid, errorKeys} = validateTimeBoundaries( component.component.minTime, component.component.maxTime, value ); if (!isValid) { - set(component, 'openForms.validationErrorContext', {timeError: error}); + set(component, 'openForms.validationErrorContext.minMaxTimeValidatorErrorKeys', errorKeys); } return isValid; }, diff --git a/src/jstests/formio/components/time.spec.js b/src/jstests/formio/components/time.spec.js index 53af0a0d8..5ad4f683a 100644 --- a/src/jstests/formio/components/time.spec.js +++ b/src/jstests/formio/components/time.spec.js @@ -223,7 +223,7 @@ describe('Time Component', () => { setTimeout(() => { expect(!!component.error).toBeTruthy(); - expect(component.error.message).toEqual('Custom error! Min time: 12:00 Max time: 13:00.'); + expect(component.error.message).toEqual('Custom error! Min time 12:00'); done(); }, 300); @@ -321,4 +321,32 @@ describe('Time Component', () => { }) .catch(done); }); + + test('Time component with empty string error', done => { + let formJSON = _.cloneDeep(timeForm); + // Note: the backend dynamically updates the configuration so that `translatedErrors` are added to + // `errors` in the correct language. + formJSON.components[0].errors = { + invalid_time: '', + }; + formJSON.components[0].maxTime = '13:00:00'; + + const element = document.createElement('div'); + + Formio.createForm(element, formJSON) + .then(form => { + form.setPristine(false); + const component = form.getComponent('time'); + const changed = component.setValue('14:00'); + expect(changed).toBeTruthy(); + + setTimeout(() => { + expect(!!component.error).toBeTruthy(); + expect(component.error.message).toEqual('invalid_time'); + + done(); + }, 300); + }) + .catch(done); + }); });