From 803a37eaa947397fa0a93b9f4f7d702c6b94ceaa Mon Sep 17 00:00:00 2001 From: Jessica Chowdhury <67977755+JessChowdhury@users.noreply.github.com> Date: Fri, 10 Nov 2023 17:06:21 +0000 Subject: [PATCH] fix: simplifies block/array/hasMany-number field validations (#4052) Co-authored-by: Jarrod Flesch --- .../payload/src/fields/validations.spec.ts | 6 +- packages/payload/src/fields/validations.ts | 179 ++++++++---------- test/fields/collections/Array/index.ts | 11 ++ test/fields/collections/Blocks/index.ts | 16 ++ test/fields/collections/Number/index.ts | 6 + test/fields/collections/Relationship/index.ts | 7 + test/fields/e2e.spec.ts | 93 +++++++++ 7 files changed, 216 insertions(+), 102 deletions(-) diff --git a/packages/payload/src/fields/validations.spec.ts b/packages/payload/src/fields/validations.spec.ts index d6d51a513..6bfae847d 100644 --- a/packages/payload/src/fields/validations.spec.ts +++ b/packages/payload/src/fields/validations.spec.ts @@ -431,7 +431,7 @@ describe('Field Validations', () => { it('should handle required value', () => { const val = '' const result = number(val, { ...numberOptions, required: true }) - expect(result).toBe('validation:enterNumber') + expect(result).toBe('validation:required') }) it('should validate minValue', () => { const val = 2.4 @@ -461,12 +461,12 @@ describe('Field Validations', () => { it('should validate an array of numbers using minRows', async () => { const val = [1.25, 2.5] const result = number(val, { ...numberOptions, hasMany: true, minRows: 4 }) - expect(result).toBe('validation:lessThanMin') + expect(result).toBe('validation:requiresAtLeast') }) it('should validate an array of numbers using maxRows', async () => { const val = [1.25, 2.5, 3.5] const result = number(val, { ...numberOptions, hasMany: true, maxRows: 2 }) - expect(result).toBe('validation:greaterThanMax') + expect(result).toBe('validation:requiresNoMoreThan') }) }) }) diff --git a/packages/payload/src/fields/validations.ts b/packages/payload/src/fields/validations.ts index 1afb9e584..b9cdbad16 100644 --- a/packages/payload/src/fields/validations.ts +++ b/packages/payload/src/fields/validations.ts @@ -22,67 +22,13 @@ import type { import canUseDOM from '../utilities/canUseDOM' import { getIDType } from '../utilities/getIDType' +import { isNumber } from '../utilities/isNumber' import { isValidID } from '../utilities/isValidID' import { fieldAffectsData } from './config/types' -export const number: Validate = ( - value: number | number[], - { hasMany, max, maxRows, min, minRows, required, t }, -) => { - const toValidate: number[] = Array.isArray(value) ? value : [value] - - // eslint-disable-next-line no-restricted-syntax - for (const valueToValidate of toValidate) { - const floatValue = parseFloat(valueToValidate as unknown as string) - if ( - (value && typeof floatValue !== 'number') || - (required && Number.isNaN(floatValue)) || - (value && Number.isNaN(floatValue)) - ) { - return t('validation:enterNumber') - } - - if (typeof max === 'number' && floatValue > max) { - return t('validation:greaterThanMax', { label: t('value'), max, value }) - } - - if (typeof min === 'number' && floatValue < min) { - return t('validation:lessThanMin', { label: t('value'), min, value }) - } - - if (required && typeof floatValue !== 'number') { - return t('validation:required') - } - } - - if (required && toValidate.length === 0) { - return t('validation:required') - } - - if (hasMany === true) { - if (minRows && toValidate.length < minRows) { - return t('validation:lessThanMin', { - label: t('rows'), - min: minRows, - value: toValidate.length, - }) - } - - if (maxRows && toValidate.length > maxRows) { - return t('validation:greaterThanMax', { - label: t('rows'), - max: maxRows, - value: toValidate.length, - }) - } - } - - return true -} - export const text: Validate = ( value: string, - { config, maxLength: fieldMaxLength, minLength, payload, required, t }, + { config, maxLength: fieldMaxLength, minLength, required, t }, ) => { let maxLength: number @@ -220,6 +166,83 @@ export const richText: Validate = return await editor.validate(value, options) } +const validateArrayLength: any = ( + value, + options: { + maxRows?: number + minRows?: number + required?: boolean + t: (key: string, options?: { [key: string]: number | string }) => string + }, +) => { + const { maxRows, minRows, required, t } = options + if (value?.length === 3) { + console.log(value) + } + const arrayLength = Array.isArray(value) ? value.length : 0 + + if (!required && arrayLength === 0) return true + + if (minRows && arrayLength < minRows) { + return t('validation:requiresAtLeast', { count: minRows, label: t('rows') }) + } + + if (maxRows && arrayLength > maxRows) { + return t('validation:requiresNoMoreThan', { count: maxRows, label: t('rows') }) + } + + if (required && !arrayLength) { + return t('validation:requiresAtLeast', { count: 1, label: t('row') }) + } + + return true +} + +export const number: Validate = ( + value: number | number[], + { hasMany, max, maxRows, min, minRows, required, t }, +) => { + if (hasMany === true) { + const lengthValidationResult = validateArrayLength(value, { maxRows, minRows, required, t }) + if (typeof lengthValidationResult === 'string') return lengthValidationResult + } + + if (!value && required) return t('validation:required') + if (!value && !required) return true + + const numbersToValidate: number[] = Array.isArray(value) ? value : [value] + + for (const number of numbersToValidate) { + if (!isNumber(number)) return t('validation:enterNumber') + + const numberValue = parseFloat(number as unknown as string) + + if (typeof max === 'number' && numberValue > max) { + return t('validation:greaterThanMax', { label: t('value'), max, value }) + } + + if (typeof min === 'number' && numberValue < min) { + return t('validation:lessThanMin', { label: t('value'), min, value }) + } + } + + return true +} + +export const array: Validate = ( + value, + { maxRows, minRows, required, t }, +) => { + return validateArrayLength(value, { maxRows, minRows, required, t }) +} + +export const blocks: Validate = ( + value, + { maxRows, minRows, required, t }, +) => { + return validateArrayLength(value, { maxRows, minRows, required, t }) +} + const validateFilterOptions: Validate = async ( value, { id, data, filterOptions, payload, relationTo, siblingData, t, user, req }, @@ -344,7 +367,7 @@ export const relationship: Validate = async return t('validation:required') } - if (Array.isArray(value)) { + if (Array.isArray(value) && value.length > 0) { if (minRows && value.length < minRows) { return t('validation:lessThanMin', { label: t('rows'), min: minRows, value: value.length }) } @@ -398,27 +421,6 @@ export const relationship: Validate = async return validateFilterOptions(value, options) } -export const array: Validate = ( - value, - { maxRows, minRows, required, t }, -) => { - const arrayLength = Array.isArray(value) ? value.length : 0 - - if (minRows && arrayLength < minRows) { - return t('validation:requiresAtLeast', { count: minRows, label: t('rows') }) - } - - if (maxRows && arrayLength > maxRows) { - return t('validation:requiresNoMoreThan', { count: maxRows, label: t('rows') }) - } - - if (!arrayLength && required) { - return t('validation:requiresAtLeast', { count: 1, label: t('row') }) - } - - return true -} - export const select: Validate = ( value, { hasMany, options, required, t }, @@ -467,27 +469,6 @@ export const radio: Validate = (value, { options, return required ? t('validation:required') : true } -export const blocks: Validate = ( - value, - { maxRows, minRows, required, t }, -) => { - const arrayLength = Array.isArray(value) ? value.length : 0 - - if (minRows && arrayLength < minRows) { - return t('validation:requiresAtLeast', { count: minRows, label: t('rows') }) - } - - if (maxRows && arrayLength > maxRows) { - return t('validation:requiresNoMoreThan', { count: maxRows, label: t('rows') }) - } - - if (!arrayLength && required) { - return t('validation:requiresAtLeast', { count: 1, label: t('row') }) - } - - return true -} - export const point: Validate = ( value: [number | string, number | string] = ['', ''], { required, t }, diff --git a/test/fields/collections/Array/index.ts b/test/fields/collections/Array/index.ts index f42aaf42e..94226543f 100644 --- a/test/fields/collections/Array/index.ts +++ b/test/fields/collections/Array/index.ts @@ -125,6 +125,17 @@ const ArrayFields: CollectionConfig = { }, }, }, + { + name: 'arrayWithMinRows', + type: 'array', + minRows: 2, + fields: [ + { + name: 'text', + type: 'text', + }, + ], + }, ], } diff --git a/test/fields/collections/Blocks/index.ts b/test/fields/collections/Blocks/index.ts index ec8e15665..d56150c4c 100644 --- a/test/fields/collections/Blocks/index.ts +++ b/test/fields/collections/Blocks/index.ts @@ -208,6 +208,22 @@ const BlockFields: CollectionConfig = { }, ], }, + { + name: 'blocksWithMinRows', + type: 'blocks', + minRows: 2, + blocks: [ + { + slug: 'block', + fields: [ + { + name: 'blockTitle', + type: 'text', + }, + ], + }, + ], + }, { name: 'customBlocks', type: 'blocks', diff --git a/test/fields/collections/Number/index.ts b/test/fields/collections/Number/index.ts index e7a60d7b8..34eee8e7f 100644 --- a/test/fields/collections/Number/index.ts +++ b/test/fields/collections/Number/index.ts @@ -73,6 +73,12 @@ const NumberFields: CollectionConfig = { hasMany: true, localized: true, }, + { + name: 'withMinRows', + type: 'number', + hasMany: true, + minRows: 2, + }, ], } diff --git a/test/fields/collections/Relationship/index.ts b/test/fields/collections/Relationship/index.ts index ca7975443..25f419ec0 100644 --- a/test/fields/collections/Relationship/index.ts +++ b/test/fields/collections/Relationship/index.ts @@ -82,6 +82,13 @@ const RelationshipFields: CollectionConfig = { ], type: 'array', }, + { + name: 'relationshipWithMinRows', + relationTo: ['text-fields'], + hasMany: true, + minRows: 2, + type: 'relationship', + }, ], slug: relationshipFieldsSlug, } diff --git a/test/fields/e2e.spec.ts b/test/fields/e2e.spec.ts index 544dd4045..848175ddf 100644 --- a/test/fields/e2e.spec.ts +++ b/test/fields/e2e.spec.ts @@ -186,6 +186,25 @@ describe('fields', () => { await saveDocAndAssert(page) await expect(field.locator('.rs__value-container')).toContainText(String(input)) }) + + test('should bypass min rows validation when no rows present and field is not required', async () => { + await page.goto(url.create) + await saveDocAndAssert(page) + await expect(page.locator('.Toastify')).toContainText('successfully') + }) + + test('should fail min rows validation when rows are present', async () => { + const input = 5 + + await page.goto(url.create) + await page.locator('.field-withMinRows').click() + + await page.keyboard.type(String(input)) + await page.keyboard.press('Enter') + await page.click('#action-save', { delay: 100 }) + + await expect(page.locator('.Toastify')).toContainText('Please correct invalid fields') + }) }) describe('indexed', () => { @@ -564,6 +583,38 @@ describe('fields', () => { ).toHaveValue('items>1>title') }) + test('should bypass min rows validation when no rows present and field is not required', async () => { + await page.goto(url.create) + await saveDocAndAssert(page) + await expect(page.locator('.Toastify')).toContainText('successfully') + }) + + test('should fail min rows validation when rows are present', async () => { + await page.goto(url.create) + + await page + .locator('#field-blocksWithMinRows') + .getByRole('button', { name: 'Add Blocks With Min Row' }) + .click() + + const blocksDrawer = page.locator('[id^=drawer_1_blocks-drawer-]') + await expect(blocksDrawer).toBeVisible() + + const firstBlockSelector = blocksDrawer + .locator('.blocks-drawer__blocks .blocks-drawer__block') + .first() + + await firstBlockSelector.click() + + const firstRow = page.locator('input[name="blocksWithMinRows.0.blockTitle"]') + await expect(firstRow).toBeVisible() + await firstRow.fill('first row') + await expect(firstRow).toHaveValue('first row') + + await page.click('#action-save', { delay: 100 }) + await expect(page.locator('.Toastify')).toContainText('Please correct invalid fields') + }) + describe('row manipulation', () => { describe('react hooks', () => { test('should add 2 new block rows', async () => { @@ -639,6 +690,20 @@ describe('fields', () => { await expect(customRowLabel).toHaveCSS('text-transform', 'uppercase') }) + test('should bypass min rows validation when no rows present and field is not required', async () => { + await page.goto(url.create) + await saveDocAndAssert(page) + await expect(page.locator('.Toastify')).toContainText('successfully') + }) + + test('should fail min rows validation when rows are present', async () => { + await page.goto(url.create) + await page.locator('#field-arrayWithMinRows >> .array-field__add-row').click() + + await page.click('#action-save', { delay: 100 }) + await expect(page.locator('.Toastify')).toContainText('Please correct invalid fields') + }) + describe('row manipulation', () => { test('should add, remove and duplicate rows', async () => { const assertText0 = 'array row 1' @@ -1545,6 +1610,34 @@ describe('fields', () => { // but the relationship document should NOT exist, as the hotkey should have saved the drawer and not the parent page expect(relationshipDocuments.docs.length).toEqual(0) }) + + test('should bypass min rows validation when no rows present and field is not required', async () => { + await page.goto(url.create) + // First fill out the relationship field, as it's required + await page.locator('#relationship-add-new .relationship-add-new__add-button').click() + await page.locator('#field-relationship .value-container').click() + await page.getByText('Seeded text document', { exact: true }).click() + + await saveDocAndAssert(page) + await expect(page.locator('.Toastify')).toContainText('successfully') + }) + + test('should fail min rows validation when rows are present', async () => { + await page.goto(url.create) + + // First fill out the relationship field, as it's required + await page.locator('#relationship-add-new .relationship-add-new__add-button').click() + await page.locator('#field-relationship .value-container').click() + await page.getByText('Seeded text document', { exact: true }).click() + + await page.locator('#field-relationshipWithMinRows .value-container').click() + await page + .locator('#field-relationshipWithMinRows .rs__option:has-text("Seeded text document")') + .click() + + await page.click('#action-save', { delay: 100 }) + await expect(page.locator('.Toastify')).toContainText('Please correct invalid fields') + }) }) describe('upload', () => {