fix: simplifies block/array/hasMany-number field validations (#4052)

Co-authored-by: Jarrod Flesch <jarrodmflesch@gmail.com>

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')
     })
   })
 })

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<unknown, unknown, NumberField> = (
-  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<unknown, unknown, TextField> = (
   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<object, unknown, RichTextField, RichTextField> =
   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<unknown, unknown, NumberField> = (
+  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<unknown, unknown, ArrayField> = (
+  value,
+  { maxRows, minRows, required, t },
+) => {
+  return validateArrayLength(value, { maxRows, minRows, required, t })
+}
+
+export const blocks: Validate<unknown, unknown, BlockField> = (
+  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<unknown, unknown, RelationshipField> = 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<unknown, unknown, RelationshipField> = async
   return validateFilterOptions(value, options)
 }
 
-export const array: Validate<unknown, unknown, ArrayField> = (
-  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<unknown, unknown, SelectField> = (
   value,
   { hasMany, options, required, t },
@@ -467,27 +469,6 @@ export const radio: Validate<unknown, unknown, RadioField> = (value, { options,
   return required ? t('validation:required') : true
 }
 
-export const blocks: Validate<unknown, unknown, BlockField> = (
-  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<unknown, unknown, PointField> = (
   value: [number | string, number | string] = ['', ''],
   { required, t },

test/fields/collections/Array/index.ts

@@ -125,6 +125,17 @@ const ArrayFields: CollectionConfig = {
         },
       },
     },
+    {
+      name: 'arrayWithMinRows',
+      type: 'array',
+      minRows: 2,
+      fields: [
+        {
+          name: 'text',
+          type: 'text',
+        },
+      ],
+    },
   ],
 }
 

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',

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,
+    },
   ],
 }
 

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,
 }

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', () => {