From 39490a1a7f4f62123cdec776d67e538ee6095f43 Mon Sep 17 00:00:00 2001 From: Bernd Streppel Date: Sun, 7 Jul 2019 11:16:40 +0200 Subject: [PATCH 1/4] Fix handling of strings and numbers as strings in min and max rule --- src/index.js | 1 + src/rules.js | 14 +++++-- tests/rules.test.js | 92 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 3 deletions(-) diff --git a/src/index.js b/src/index.js index b9a3c61..458d507 100644 --- a/src/index.js +++ b/src/index.js @@ -132,6 +132,7 @@ function validateField(fieldData, formData) { ...rule, key: fieldData.key, value: fieldData.value, + rules: validation, values, } diff --git a/src/rules.js b/src/rules.js index 0d0f159..ccc282c 100644 --- a/src/rules.js +++ b/src/rules.js @@ -140,7 +140,7 @@ export default { lt: ({ value, values, params }) => value < values[params[0]], lte: ({ value, values, params }) => value <= values[params[0]], - max: ({ value, params }) => (b(value) || typeof value === 'number') && sizeOf(value) <= params[0], + max: ({ value, params, rules }) => (b(value) || typeof value === 'number') && sizeOf(value, rules) <= params[0], mimetypes: ({ value, params }) => { if (!value || !value.type) { @@ -166,7 +166,7 @@ export default { return false; }, - min: ({ value, params }) => (b(value) || typeof value === 'number') && sizeOf(value) >= params[0], + min: ({ value, params, rules }) => (b(value) || typeof value === 'number') && sizeOf(value, rules) >= params[0], not_in: ({ value, params }) => params.findIndex(param => deepEquals(param, value)) === -1, @@ -253,7 +253,11 @@ function isNotEmpty(value) { return typeof value === 'number' || typeof value === 'boolean' || !! value; } -function sizeOf(value) { +function sizeOf(value, rules) { + // If value is a string, but there is a numeric rule we parse the string as number. + if(rules !== undefined && typeof value === 'string' && hasNumericRule(rules)) { + return Number.parseFloat(value); + } //TODO files, images other things if (value.hasOwnProperty('length')) { value = value.length; @@ -261,6 +265,10 @@ function sizeOf(value) { return value; } +function hasNumericRule(rules) { + return rules.includes("number") || rules.includes("integer"); +} + function b(value) { return !! value; } diff --git a/tests/rules.test.js b/tests/rules.test.js index b1d0865..149abb5 100644 --- a/tests/rules.test.js +++ b/tests/rules.test.js @@ -1281,6 +1281,52 @@ describe('Rules', () => { value: null, result: false, }, + { + desc: 'string with numeric rule and small value', + value: "0", + params: [1], + ruleParams: { rules: ["integer", "max:1"] }, + result: true, + }, + { + desc: 'string with numeric rule and large value', + value: "80", + params: [10], + ruleParams: { rules: ["integer", "max:10"] }, + result: false, + }, + { + desc: 'string without numieric rule, but number value, long string', + value: "80000000000", + params: [10], + ruleParams: { rules: ["string", "max:10"] }, + result: false, + }, + { + desc: 'string without numieric rule, but number value, short string', + value: "80", + params: [10], + ruleParams: { rules: ["string", "max:10"] }, + result: true, + }, + { + desc: 'short string', + value: "abc", + params: [10], + result: true, + }, + { + desc: 'equal string', + value: "abcdefghij", + params: [10], + result: true, + }, + { + desc: 'too long string', + value: "abcdefghijk", + params: [10], + result: false, + } ]); createRuleTests('mimetypes', [ // File.type should be correct mime type @@ -1365,6 +1411,52 @@ describe('Rules', () => { value: null, result: false, }, + { + desc: 'string with numeric rule and small value', + value: "0", + params: [1], + ruleParams: { rules: ["integer", "min:1"] }, + result: false, + }, + { + desc: 'string with numeric rule and large value', + value: "80", + params: [10], + ruleParams: { rules: ["integer", "min:10"] }, + result: true, + }, + { + desc: 'string without numieric rule, but number value, long string', + value: "80000000000", + params: [10], + ruleParams: { rules: ["string", "min:10"] }, + result: true, + }, + { + desc: 'string without numieric rule, but number value, short string', + value: "80", + params: [10], + ruleParams: { rules: ["string", "min:10"] }, + result: false, + }, + { + desc: 'short string', + value: "abc", + params: [10], + result: false, + }, + { + desc: 'equal string', + value: "abcdefghij", + params: [10], + result: true, + }, + { + desc: 'too long string', + value: "abcdefghijk", + params: [10], + result: true, + } ]); createRuleTests('not_in', [ From 2a0b625ec5bcb49a0a9a9967c530c60ce419803a Mon Sep 17 00:00:00 2001 From: Bernd Streppel Date: Sun, 7 Jul 2019 22:00:38 +0200 Subject: [PATCH 2/4] Fix between rule to work with nmeric strings --- src/rules.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rules.js b/src/rules.js index ccc282c..5e00ea6 100644 --- a/src/rules.js +++ b/src/rules.js @@ -26,10 +26,10 @@ export default { before: ({ value, params }) => b(new Date(value) < new Date(params[0])), before_or_equal: ({ value, params }) => b(new Date(value) <= new Date(params[0])), - between: ({ value, params }) => { + between: ({ value, rules, params }) => { if (typeof value !== 'number' && !value) return false; const [min, max] = params; - value = sizeOf(value); + value = sizeOf(value, rules); return value > min && value < max; }, @@ -123,6 +123,7 @@ export default { if (!Array.isArray(array)) return false; return array.findIndex(arrayVal => deepEquals(arrayVal, value)) !== -1; }, + integer: ({ value }) => { return Number.isInteger(typeof value === 'string' ? parseInt(value) : value); }, From d9238f3c7d44a91a3af3d6a50d17b46d6ab09fb7 Mon Sep 17 00:00:00 2001 From: Bernd Streppel Date: Sun, 7 Jul 2019 22:03:35 +0200 Subject: [PATCH 3/4] Fix parameter for messages (e.g. for min and max) The rule params can now be used in the messages for replacement. They are collected together with the errors and forwarded to generateMessage to be used in DEFAULT_PLACEHOLDERS methods. --- src/index.js | 8 ++++---- src/messages.js | 6 +++--- src/placeholders.js | 13 +++++++++---- tests/index.test.js | 18 +++++++++--------- tests/placeholders.test.js | 9 +++++---- 5 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/index.js b/src/index.js index 458d507..8d6247e 100644 --- a/src/index.js +++ b/src/index.js @@ -50,10 +50,10 @@ function validateForm({ formData, includeMessages=true }) { if (result.errors) { fields.push(key); - errors.push(result.errors); + errors.push(result.errors.map(rule => rule.key)); if (includeMessages) { - const fieldMessages = result.errors.map(rule => generateMessage(rule, fieldData)); + const fieldMessages = result.errors.map(rule => generateMessage(rule.key, rule.params, fieldData)); messages.push(fieldMessages); } @@ -92,7 +92,7 @@ function parseRule(rule) { }; } -// {key, value, validation} +// Returns array of failed rules or false if all rules passed. function validateField(fieldData, formData) { const values = formData && Object.keys(formData).reduce((values, key) => { @@ -149,7 +149,7 @@ function validateField(fieldData, formData) { if (!overrideNullable && nullable && fieldData.value === null) { continue; } - errors.push(rule.key); + errors.push(rule); } } diff --git a/src/messages.js b/src/messages.js index 3ca0112..efc219a 100644 --- a/src/messages.js +++ b/src/messages.js @@ -21,7 +21,7 @@ export const messages = { before: () => "The :attribute must be a date before :date.", before_or_equal: () => "The :attribute must be a date before or equal to :date.", - between: () => "", // TODO this one is more complicated + between: () => "The :attribute must be between :min and :max.", boolean: () => "The :attribute field must be true or false.", @@ -66,12 +66,12 @@ export const messages = { lt: () => "", // TODO this is more complicated, and is it done with size? lte: () => "", // TODO this is more complicated, and is it done with size? - max: () => "", // TODO this is more complicated, and is it done with size? + max: () => "The :attribute must be maximal :max.", // TODO this is more complicated, and is it done with size? // mimes? // mimetypes? - min: () => "", // TODO this is more complicated, and is it done with size? + min: () => "The :attribute must be at least :min.", // TODO this is more complicated, and is it done with size? not_in: () => "The selected :attribute is invalid.", diff --git a/src/placeholders.js b/src/placeholders.js index 5fd886b..03abf2f 100644 --- a/src/placeholders.js +++ b/src/placeholders.js @@ -25,6 +25,13 @@ export const DEFAULT_PLACEHOLDERS = { "date_equals": { date: ({ params }) => params[0], }, + "digits": { + digits: ({ params }) => params[0], + }, + "digits_between": { + min: ({params}) => params[0], + max: ({params}) => params[1] + }, "dimensions": { // TODO }, @@ -86,10 +93,10 @@ export const DEFAULT_PLACEHOLDERS = { }, }; -export function generateMessage(rule, fieldData) { +export function generateMessage(rule, ruleParams, fieldData) { const message = getMessage(rule, fieldData); - return replacePlaceholders(message, { rule, ...fieldData }); + return replacePlaceholders(message, { rule, params: ruleParams, ...fieldData }); } function replacePlaceholders(message, fieldData) { @@ -104,8 +111,6 @@ function replacePlaceholders(message, fieldData) { const replacementValue = fieldData[placeholder]; if (replacementValue === null || replacementValue === undefined) { - - replacementValue = getDefaultPlaceholderValue(placeholder, fieldData) || ""; } diff --git a/tests/index.test.js b/tests/index.test.js index 448462d..41643e1 100644 --- a/tests/index.test.js +++ b/tests/index.test.js @@ -65,7 +65,7 @@ describe('Form Validator', () => { } const validateField = mockValidateField({ - errors: ['required'] + errors: [ { key: 'required' } ] }); expect(validateForm({ formData, includeMessages: false } )).toEqual({ @@ -88,7 +88,7 @@ describe('Form Validator', () => { } const validateField = mockValidateField({ - errors: ['testRule'] + errors: [ { key: 'testRule' } ] }); const messageMock = mockMessage('testRule', 'hello'); @@ -124,10 +124,10 @@ describe('Form Validator', () => { const validateField = mockValidateField(); validateField.mockReturnValueOnce({ - errors: ['required'], + errors: [ { key: 'required' } ], }); validateField.mockReturnValueOnce({ - errors: ['required'], + errors: [ { key: 'required' } ], }); expect(validateForm({ formData, includeMessages: false } )).toEqual({ @@ -149,7 +149,7 @@ describe('Form Validator', () => { const validateField = mockValidateField(); validateField.mockReturnValueOnce({ - errors: ['required', 'string'], + errors: [ { key: 'required' }, { key: 'string' } ], }); expect(validateForm({ formData, includeMessages: false } )).toEqual({ @@ -239,14 +239,14 @@ describe('Form Validator', () => { it('can detect an invalid field', () => { const result = validateField(createFieldData({ validation: ['required'] })) - expect(result.errors).toEqual(['required']); + expect(result.errors).toEqual([ { key: 'required', params: [] } ]); expect(console.warn).not.toHaveBeenCalled(); }); it('can detect multiple rules on one field', () => { const result = validateField(createFieldData({ validation: ['required', 'string'] })) - expect(result.errors).toEqual(['required', 'string']); + expect(result.errors).toEqual([ { key: 'required', params: [] }, { key: 'string', params: [] }]); expect(console.warn).not.toHaveBeenCalled(); }) @@ -277,7 +277,7 @@ describe('Form Validator', () => { throw new Error(); }); - expect(validateField(fieldData)).toEqual({ errors: ['testRule'] }); + expect(validateField(fieldData)).toEqual({ errors: [ { key: 'testRule', params: [] } ] }); expect(console.warn).toHaveBeenCalled(); expect(ruleMock).toHaveBeenCalled(); @@ -293,7 +293,7 @@ describe('Form Validator', () => { throw new Error(); }); - expect(validateField(fieldData)).toEqual({ errors: ['test'] }); + expect(validateField(fieldData)).toEqual({ errors: [ { key: 'test', params: [] } ] }); expect(console.warn).toHaveBeenCalled(); expect(ruleMock).toHaveBeenCalled(); diff --git a/tests/placeholders.test.js b/tests/placeholders.test.js index 4637962..8ce8617 100644 --- a/tests/placeholders.test.js +++ b/tests/placeholders.test.js @@ -5,6 +5,7 @@ const oldGetMessage = Messages.getMessage; describe('Placeholders', () => { const testRule = "foo"; + const testRuleParams = []; const testMessage = "hello :dog what's up with :cat"; const getMessage = jest.fn().mockReturnValue(testMessage); @@ -21,7 +22,7 @@ describe('Placeholders', () => { const cat = "Reginald"; const fieldData = { dog, cat }; - const message = generateMessage(testRule, fieldData); + const message = generateMessage(testRule, testRuleParams, fieldData); expect(message).toEqual("hello Suzy what's up with Reginald"); expect(getMessage).toHaveBeenCalledWith(testRule, fieldData); @@ -31,7 +32,7 @@ describe('Placeholders', () => { const dog = "Suzy"; const fieldData = { dog }; - const message = generateMessage(testRule, fieldData); + const message = generateMessage(testRule, testRuleParams, fieldData); expect(message).toEqual("hello Suzy what's up with "); expect(getMessage).toHaveBeenCalledWith(testRule, fieldData); @@ -41,7 +42,7 @@ describe('Placeholders', () => { const dog = "Suzy"; const fieldData = { dog }; - const message = generateMessage(testRule, fieldData); + const message = generateMessage(testRule, testRuleParams, fieldData); expect(message).toEqual("hello Suzy what's up with "); expect(getMessage).toHaveBeenCalledWith(testRule, fieldData); @@ -58,7 +59,7 @@ describe('Placeholders', () => { messageRules.forEach((rule) => { it(`will replace placeholders in message for ${rule} rule`, () => { const fieldData = { name: "some field", params }; - const message = generateMessage(rule, fieldData); + const message = generateMessage(rule, testRuleParams, fieldData); const placeholders = []; let placeholderMatch = message.match(PLACEHOLDER_REGEX); From c5a696dfa19687060ddedbd6ce0ae06cce1eb54d Mon Sep 17 00:00:00 2001 From: berndulum Date: Sat, 13 Jul 2019 19:37:13 +0200 Subject: [PATCH 4/4] Improve comment for parseRule method Co-Authored-By: Nik M --- src/index.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/index.js b/src/index.js index 8d6247e..9fb94e0 100644 --- a/src/index.js +++ b/src/index.js @@ -92,7 +92,12 @@ function parseRule(rule) { }; } -// Returns array of failed rules or false if all rules passed. +/** + * Returns array of failed rules or false if all rules passed. + * + * @param fieldData In the format { key, value, validation, ... } + * @param formData See `validateForm` + */ function validateField(fieldData, formData) { const values = formData && Object.keys(formData).reduce((values, key) => { @@ -162,4 +167,4 @@ toExport.validateForm = validateForm; toExport.validateField = validateField; toExport.parseRule = parseRule; -export const validate = toExport; \ No newline at end of file +export const validate = toExport;