From 2cf6c8c2157a431cbaa2cbc28efbead8eaddc91c Mon Sep 17 00:00:00 2001 From: Daniel Cousens <413395+dcousens@users.noreply.github.com> Date: Mon, 8 Jul 2024 20:24:05 +1000 Subject: [PATCH] unify validation and fix tests --- packages/core/src/fields/non-null-graphql.ts | 27 ++-- .../core/src/fields/types/bigInt/index.ts | 75 ++++++----- .../src/fields/types/calendarDay/index.ts | 10 +- .../core/src/fields/types/decimal/index.ts | 19 +-- packages/core/src/fields/types/float/index.ts | 54 ++++---- .../core/src/fields/types/integer/index.ts | 76 +++++------ .../src/fields/types/multiselect/index.ts | 8 +- .../core/src/fields/types/password/index.ts | 99 ++++++++------- packages/core/src/fields/types/text/index.ts | 119 +++++++++--------- .../core/src/fields/types/timestamp/index.ts | 4 +- tests/api-tests/auth.test.ts | 2 +- tests/api-tests/fields/crud.test.ts | 3 +- tests/api-tests/fields/required.test.ts | 17 ++- .../types/fixtures/decimal/test-fixtures.ts | 4 +- .../types/fixtures/password/test-fixtures.ts | 12 +- 15 files changed, 268 insertions(+), 261 deletions(-) diff --git a/packages/core/src/fields/non-null-graphql.ts b/packages/core/src/fields/non-null-graphql.ts index b099bf82736..f2438b7311b 100644 --- a/packages/core/src/fields/non-null-graphql.ts +++ b/packages/core/src/fields/non-null-graphql.ts @@ -17,15 +17,6 @@ export function resolveDbNullable ( return true } -function shouldAddValidation ( - db?: { isNullable?: boolean }, - validation?: unknown -) { - if (db?.isNullable === false) return true - if (validation !== undefined) return true - return false -} - export function makeValidateHook ( meta: FieldData, config: { @@ -40,22 +31,28 @@ export function makeValidateHook ( }, validation?: { isRequired?: boolean + [key: string]: unknown }, }, f?: ValidateFieldHook ) { const dbNullable = resolveDbNullable(config.validation, config.db) const mode = dbNullable ? ('optional' as const) : ('required' as const) + const valueRequired = config.validation?.isRequired || !dbNullable assertReadIsNonNullAllowed(meta, config, dbNullable) - const addValidation = shouldAddValidation(config.db, config.validation) + const addValidation = config.db?.isNullable === false || config.validation?.isRequired if (addValidation) { const validate = async function (args) { const { operation, addValidationError, resolvedData } = args - if (operation !== 'delete') { - const value = resolvedData[meta.fieldKey] - if ((config.validation?.isRequired || dbNullable === false) && value === null) { - addValidationError(`Missing value`) + + if (valueRequired) { + const value = resolvedData?.[meta.fieldKey] + if ( + (operation === 'create' && value === undefined) + || ((operation === 'create' || operation === 'update') && (value === null)) + ) { + addValidationError(`missing value`) } } @@ -70,7 +67,7 @@ export function makeValidateHook ( return { mode, - validate: undefined + validate: f } } diff --git a/packages/core/src/fields/types/bigInt/index.ts b/packages/core/src/fields/types/bigInt/index.ts index 3f117ebec36..857441d3e7f 100644 --- a/packages/core/src/fields/types/bigInt/index.ts +++ b/packages/core/src/fields/types/bigInt/index.ts @@ -6,11 +6,11 @@ import { orderDirectionEnum, } from '../../../types' import { graphql } from '../../..' +import { filters } from '../../filters' import { resolveDbNullable, makeValidateHook } from '../../non-null-graphql' -import { filters } from '../../filters' import { mergeFieldHooks } from '../../resolve-hooks' export type BigIntFieldConfig = @@ -33,15 +33,19 @@ export type BigIntFieldConfig = const MAX_INT = 9223372036854775807n const MIN_INT = -9223372036854775808n -export function bigInt ( - config: BigIntFieldConfig = {} -): FieldTypeFunc { +export function bigInt (config: BigIntFieldConfig = {}): FieldTypeFunc { const { - isIndexed, defaultValue: _defaultValue, - validation: validation_, + isIndexed, + validation = {}, } = config + const { + isRequired = false, + min, + max + } = validation + return (meta) => { const defaultValue = _defaultValue ?? null const hasAutoIncDefault = @@ -49,12 +53,11 @@ export function bigInt ( defaultValue !== null && defaultValue.kind === 'autoincrement' - const isNullable = resolveDbNullable(validation_, config.db) - if (hasAutoIncDefault) { if (meta.provider === 'sqlite' || meta.provider === 'mysql') { throw new Error(`${meta.listKey}.${meta.fieldKey} specifies defaultValue: { kind: 'autoincrement' }, this is not supported on ${meta.provider}`) } + const isNullable = resolveDbNullable(validation, config.db) if (isNullable !== false) { throw new Error( `${meta.listKey}.${meta.fieldKey} specifies defaultValue: { kind: 'autoincrement' } but doesn't specify db.isNullable: false.\n` + @@ -63,45 +66,50 @@ export function bigInt ( ) } } - - const validation = { - isRequired: validation_?.isRequired ?? false, - min: validation_?.min ?? MIN_INT, - max: validation_?.max ?? MAX_INT, + if (min !== undefined && !Number.isInteger(min)) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.min: ${min} but it must be an integer`) } - - for (const type of ['min', 'max'] as const) { - if (validation[type] > MAX_INT || validation[type] < MIN_INT) { - throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.${type}: ${validation[type]} which is outside of the range of a 64bit signed integer(${MIN_INT}n - ${MAX_INT}n) which is not allowed`) - } + if (max !== undefined && !Number.isInteger(max)) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.max: ${max} but it must be an integer`) } - if (validation.min > validation.max) { + if (min !== undefined && (min > MAX_INT || min < MIN_INT)) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.min: ${min} which is outside of the range of a 64-bit signed integer`) + } + if (max !== undefined && (max > MAX_INT || max < MIN_INT)) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.max: ${max} which is outside of the range of a 64-bit signed integer`) + } + if ( + min !== undefined && + max !== undefined && + min > max + ) { throw new Error(`${meta.listKey}.${meta.fieldKey} specifies a validation.max that is less than the validation.min, and therefore has no valid options`) } + const hasAdditionalValidation = min !== undefined || max !== undefined const { mode, validate, - } = makeValidateHook(meta, config, ({ resolvedData, operation, addValidationError }) => { + } = makeValidateHook(meta, config, hasAdditionalValidation ? ({ resolvedData, operation, addValidationError }) => { if (operation === 'delete') return const value = resolvedData[meta.fieldKey] if (typeof value === 'number') { - if (validation?.min !== undefined && value < validation.min) { - addValidationError(`value must be greater than or equal to ${validation.min}`) + if (min !== undefined && value < min) { + addValidationError(`value must be greater than or equal to ${min}`) } - if (validation?.max !== undefined && value > validation.max) { - addValidationError(`value must be less than or equal to ${validation.max}`) + if (max !== undefined && value > max) { + addValidationError(`value must be less than or equal to ${max}`) } } - }) + } : undefined) return fieldType({ kind: 'scalar', mode, scalar: 'BigInt', - // This will resolve to 'index' if the boolean is true, otherwise other values - false will be converted to undefined + // this will resolve to 'index' if the boolean is true, otherwise other values - false will be converted to undefined index: isIndexed === true ? 'index' : isIndexed || undefined, default: typeof defaultValue === 'bigint' @@ -135,19 +143,20 @@ export function bigInt ( update: { arg: graphql.arg({ type: graphql.BigInt }) }, orderBy: { arg: graphql.arg({ type: orderDirectionEnum }) }, }, - output: graphql.field({ - type: graphql.BigInt, - }), + output: graphql.field({ type: graphql.BigInt, }), __ksTelemetryFieldTypeName: '@keystone-6/bigInt', views: '@keystone-6/core/fields/types/bigInt/views', getAdminMeta () { return { validation: { - min: validation.min.toString(), - max: validation.max.toString(), - isRequired: validation.isRequired, + min: min?.toString() ?? `${MIN_INT}`, + max: max?.toString() ?? `${MAX_INT}`, + isRequired, }, - defaultValue: typeof defaultValue === 'bigint' ? defaultValue.toString() : defaultValue, + defaultValue: + typeof defaultValue === 'bigint' + ? defaultValue.toString() + : defaultValue, } }, }) diff --git a/packages/core/src/fields/types/calendarDay/index.ts b/packages/core/src/fields/types/calendarDay/index.ts index ac6184b89f8..89e37f182c9 100644 --- a/packages/core/src/fields/types/calendarDay/index.ts +++ b/packages/core/src/fields/types/calendarDay/index.ts @@ -25,14 +25,13 @@ export type CalendarDayFieldConfig = } } -export const calendarDay = - ({ +export function calendarDay (config: CalendarDayFieldConfig = {}): FieldTypeFunc { + const { isIndexed, validation, defaultValue, - ...config - }: CalendarDayFieldConfig = {}): FieldTypeFunc => - meta => { + } = config + return (meta) => { if (typeof defaultValue === 'string') { try { graphql.CalendarDay.graphQLType.parseValue(defaultValue) @@ -126,6 +125,7 @@ export const calendarDay = }, }) } +} function dateStringToDateObjectInUTC (value: string) { return new Date(`${value}T00:00Z`) diff --git a/packages/core/src/fields/types/decimal/index.ts b/packages/core/src/fields/types/decimal/index.ts index 8fb956e4e98..b3f9d6f1d5d 100644 --- a/packages/core/src/fields/types/decimal/index.ts +++ b/packages/core/src/fields/types/decimal/index.ts @@ -48,16 +48,16 @@ function parseDecimalValueOption (meta: FieldData, value: string, name: string) return decimal } -export const decimal = - ({ +export function decimal (config: DecimalFieldConfig = {}): FieldTypeFunc { + const { isIndexed, precision = 18, scale = 4, validation, defaultValue, - ...config - }: DecimalFieldConfig = {}): FieldTypeFunc => - meta => { + } = config + + return (meta) => { if (meta.provider === 'sqlite') { throw new Error('The decimal field does not support sqlite') } @@ -107,13 +107,13 @@ export const decimal = } = makeValidateHook(meta, config, ({ resolvedData, operation, addValidationError }) => { if (operation === 'delete') return - const val: Decimal | null | undefined = resolvedData[meta.fieldKey] - if (val != null) { - if (min !== undefined && val.lessThan(min)) { + const value: Decimal | null | undefined = resolvedData[meta.fieldKey] + if (value != null) { + if (min !== undefined && value.lessThan(min)) { addValidationError(`value must be greater than or equal to ${min}`) } - if (max !== undefined && val.greaterThan(max)) { + if (max !== undefined && value.greaterThan(max)) { addValidationError(`value must be less than or equal to ${max}`) } } @@ -184,3 +184,4 @@ export const decimal = }), }) } +} diff --git a/packages/core/src/fields/types/float/index.ts b/packages/core/src/fields/types/float/index.ts index 1f944e9f00f..7e3dd86b106 100644 --- a/packages/core/src/fields/types/float/index.ts +++ b/packages/core/src/fields/types/float/index.ts @@ -27,14 +27,13 @@ export type FloatFieldConfig = } } -export const float = - ({ - isIndexed, - validation, +export function float (config: FloatFieldConfig = {}): FieldTypeFunc { + const { defaultValue, - ...config - }: FloatFieldConfig = {}): FieldTypeFunc => - meta => { + isIndexed, + validation: v = {}, + } = config + return (meta) => { if ( defaultValue !== undefined && (typeof defaultValue !== 'number' || !Number.isFinite(defaultValue)) @@ -43,45 +42,46 @@ export const float = } if ( - validation?.min !== undefined && - (typeof validation.min !== 'number' || !Number.isFinite(validation.min)) + v.min !== undefined && + (typeof v.min !== 'number' || !Number.isFinite(v.min)) ) { - throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.min: ${validation.min} but it must be a valid finite number`) + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.min: ${v.min} but it must be a valid finite number`) } if ( - validation?.max !== undefined && - (typeof validation.max !== 'number' || !Number.isFinite(validation.max)) + v.max !== undefined && + (typeof v.max !== 'number' || !Number.isFinite(v.max)) ) { - throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.max: ${validation.max} but it must be a valid finite number`) + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.max: ${v.max} but it must be a valid finite number`) } if ( - validation?.min !== undefined && - validation?.max !== undefined && - validation.min > validation.max + v.min !== undefined && + v.max !== undefined && + v.min > v.max ) { throw new Error(`${meta.listKey}.${meta.fieldKey} specifies a validation.max that is less than the validation.min, and therefore has no valid options`) } + const hasAdditionalValidation = v.min !== undefined || v.max !== undefined const { mode, validate, - } = makeValidateHook(meta, config, ({ resolvedData, operation, addValidationError }) => { + } = makeValidateHook(meta, config, hasAdditionalValidation ? ({ resolvedData, operation, addValidationError }) => { if (operation === 'delete') return const value = resolvedData[meta.fieldKey] if (typeof value === 'number') { - if (validation?.max !== undefined && value > validation.max) { - addValidationError(`value must be less than or equal to ${validation.max}` + if (v.max !== undefined && value > v.max) { + addValidationError(`value must be less than or equal to ${v.max}` ) } - if (validation?.min !== undefined && value < validation.min) { - addValidationError(`value must be greater than or equal to ${validation.min}`) + if (v.min !== undefined && value < v.min) { + addValidationError(`value must be greater than or equal to ${v.min}`) } } - }) + } : undefined) return fieldType({ kind: 'scalar', @@ -95,8 +95,7 @@ export const float = ...config, hooks: mergeFieldHooks({ validate }, config.hooks), input: { - uniqueWhere: - isIndexed === 'unique' ? { arg: graphql.arg({ type: graphql.Float }) } : undefined, + uniqueWhere: isIndexed === 'unique' ? { arg: graphql.arg({ type: graphql.Float }) } : undefined, where: { arg: graphql.arg({ type: filters[meta.provider].Float[mode] }), resolve: mode === 'optional' ? filters.resolveCommon : undefined, @@ -124,12 +123,13 @@ export const float = getAdminMeta () { return { validation: { - min: validation?.min || null, - max: validation?.max || null, - isRequired: validation?.isRequired ?? false, + isRequired: v.isRequired ?? false, + min: v.min ?? null, + max: v.max ?? null, }, defaultValue: defaultValue ?? null, } }, }) } +} diff --git a/packages/core/src/fields/types/integer/index.ts b/packages/core/src/fields/types/integer/index.ts index 6a38f63a29f..43e6a3499f6 100644 --- a/packages/core/src/fields/types/integer/index.ts +++ b/packages/core/src/fields/types/integer/index.ts @@ -29,31 +29,35 @@ export type IntegerFieldConfig = } } -// These are the max and min values available to a 32 bit signed integer +// these are the lowest and highest values for a signed 32-bit integer const MAX_INT = 2147483647 const MIN_INT = -2147483648 -export function integer ({ - isIndexed, - defaultValue: _defaultValue, - validation, - ...config -}: IntegerFieldConfig = {}): FieldTypeFunc { - return meta => { +export function integer (config: IntegerFieldConfig = {}): FieldTypeFunc { + const { + defaultValue: _defaultValue, + isIndexed, + validation = {}, + } = config + + const { + isRequired = false, + min, + max + } = validation + + return (meta) => { const defaultValue = _defaultValue ?? null const hasAutoIncDefault = typeof defaultValue == 'object' && defaultValue !== null && defaultValue.kind === 'autoincrement' - const isNullable = resolveDbNullable(validation, config.db) - if (hasAutoIncDefault) { if (meta.provider === 'sqlite' || meta.provider === 'mysql') { - throw new Error( - `${meta.listKey}.${meta.fieldKey} specifies defaultValue: { kind: 'autoincrement' }, this is not supported on ${meta.provider}` - ) + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies defaultValue: { kind: 'autoincrement' }, this is not supported on ${meta.provider}`) } + const isNullable = resolveDbNullable(validation, config.db) if (isNullable !== false) { throw new Error( `${meta.listKey}.${meta.fieldKey} specifies defaultValue: { kind: 'autoincrement' } but doesn't specify db.isNullable: false.\n` + @@ -62,52 +66,50 @@ export function integer ({ ) } } - - if (validation?.min !== undefined && !Number.isInteger(validation.min)) { - throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.min: ${validation.min} but it must be an integer`) + if (min !== undefined && !Number.isInteger(min)) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.min: ${min} but it must be an integer`) } - if (validation?.max !== undefined && !Number.isInteger(validation.max)) { - throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.max: ${validation.max} but it must be an integer`) + if (max !== undefined && !Number.isInteger(max)) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.max: ${max} but it must be an integer`) } - - if (validation?.min !== undefined && (validation?.min > MAX_INT || validation?.min < MIN_INT)) { - throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.min: ${validation.min} which is outside of the range of a 32bit signed integer (${MIN_INT} - ${MAX_INT}) which is not allowed`) + if (min !== undefined && (min > MAX_INT || min < MIN_INT)) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.min: ${min} which is outside of the range of a 32-bit signed integer`) } - if (validation?.max !== undefined && (validation?.max > MAX_INT || validation?.max < MIN_INT)) { - throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.max: ${validation.max} which is outside of the range of a 32bit signed integer (${MIN_INT} - ${MAX_INT}) which is not allowed`) + if (max !== undefined && (max > MAX_INT || max < MIN_INT)) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.max: ${max} which is outside of the range of a 32-bit signed integer`) } - if ( - validation?.min !== undefined && - validation?.max !== undefined && - validation.min > validation.max + min !== undefined && + max !== undefined && + min > max ) { throw new Error(`${meta.listKey}.${meta.fieldKey} specifies a validation.max that is less than the validation.min, and therefore has no valid options`) } + const hasAdditionalValidation = min !== undefined || max !== undefined const { mode, validate, - } = makeValidateHook(meta, config, ({ resolvedData, operation, addValidationError }) => { + } = makeValidateHook(meta, config, hasAdditionalValidation ? ({ resolvedData, operation, addValidationError }) => { if (operation === 'delete') return const value = resolvedData[meta.fieldKey] if (typeof value === 'number') { - if (validation?.min !== undefined && value < validation.min) { - addValidationError(`value must be greater than or equal to ${validation.min}`) + if (min !== undefined && value < min) { + addValidationError(`value must be greater than or equal to ${min}`) } - if (validation?.max !== undefined && value > validation.max) { - addValidationError(`value must be less than or equal to ${validation.max}`) + if (max !== undefined && value > max) { + addValidationError(`value must be less than or equal to ${max}`) } } - }) + } : undefined) return fieldType({ kind: 'scalar', mode, scalar: 'Int', - // This will resolve to 'index' if the boolean is true, otherwise other values - false will be converted to undefined + // this will resolve to 'index' if the boolean is true, otherwise other values - false will be converted to undefined index: isIndexed === true ? 'index' : isIndexed || undefined, default: typeof defaultValue === 'number' @@ -147,9 +149,9 @@ export function integer ({ getAdminMeta () { return { validation: { - min: validation?.min ?? MIN_INT, - max: validation?.max ?? MAX_INT, - isRequired: validation?.isRequired ?? false, + min: min ?? MIN_INT, + max: max ?? MAX_INT, + isRequired, }, defaultValue: defaultValue === null || typeof defaultValue === 'number' diff --git a/packages/core/src/fields/types/multiselect/index.ts b/packages/core/src/fields/types/multiselect/index.ts index 46154682184..53fe9ad1830 100644 --- a/packages/core/src/fields/types/multiselect/index.ts +++ b/packages/core/src/fields/types/multiselect/index.ts @@ -40,7 +40,7 @@ export type MultiselectFieldConfig = } } -// These are the max and min values available to a 32 bit signed integer +// these are the lowest and highest values for a signed 32-bit integer const MAX_INT = 2147483647 const MIN_INT = -2147483648 @@ -91,14 +91,14 @@ export function multiselect ( const { mode, validate, - } = makeValidateHook(meta, config, ({ resolvedData, operation, addValidationError }) => { + } = makeValidateHook(meta, config, ({ inputData, operation, addValidationError }) => { if (operation === 'delete') return - const values: readonly (string | number)[] | undefined = resolvedData[meta.fieldKey] + const values: readonly (string | number)[] | undefined = inputData[meta.fieldKey] // resolvedData is JSON if (values !== undefined) { for (const value of values) { if (!accepted.has(value)) { - addValidationError(`value is not an accepted option`) + addValidationError(`'${value}' is not an accepted option`) } } if (new Set(values).size !== values.length) { diff --git a/packages/core/src/fields/types/password/index.ts b/packages/core/src/fields/types/password/index.ts index 575016dcd47..1324ca89136 100644 --- a/packages/core/src/fields/types/password/index.ts +++ b/packages/core/src/fields/types/password/index.ts @@ -57,40 +57,41 @@ export function password (config: Passwor const { bcrypt = bcryptjs, workFactor = 10, - validation: _validation, + validation = {}, } = config + const { + isRequired = false, + rejectCommon = false, + match, + length: { + max + } = {}, + } = validation + const min = isRequired ? validation.length?.min ?? 8 : validation.length?.min return (meta) => { if ((config as any).isIndexed === 'unique') { throw Error("isIndexed: 'unique' is not a supported option for field type password") } - - const validation = { - isRequired: _validation?.isRequired ?? false, - rejectCommon: _validation?.rejectCommon ?? false, - match: _validation?.match - ? { - regex: _validation.match.regex, - explanation: _validation.match.explanation ?? `value must match ${_validation.match.regex}`, - } - : null, - length: { - min: _validation?.length?.min ?? 8, - max: _validation?.length?.max ?? null, - }, + if (min !== undefined && (!Number.isInteger(min) || min < 0)) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.length.min: ${min} but it must be a positive integer`) } - - for (const type of ['min', 'max'] as const) { - const val = validation.length[type] - if (val !== null && (!Number.isInteger(val) || val < 1)) { - throw new Error(`${meta.listKey}.${meta.fieldKey}: validation.length.${type} must be a positive integer >= 1`) - } + if (max !== undefined && (!Number.isInteger(max) || max < 0)) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.length.max: ${max} but it must be a positive integer`) } - - if (validation.length.max !== null && validation.length.min > validation.length.max) { - throw new Error(`${meta.listKey}.${meta.fieldKey}: validation.length.max cannot be less than validation.length.min`) + if (isRequired && min !== undefined && min === 0) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.isRequired: true and validation.length.min: 0, this is not allowed because validation.isRequired implies at least a min length of 1`) + } + if (isRequired && max !== undefined && max === 0) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.isRequired: true and validation.length.max: 0, this is not allowed because validation.isRequired implies at least a max length of 1`) + } + if ( + min !== undefined && + max !== undefined && + min > max + ) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies a validation.length.max that is less than the validation.length.min, and therefore has no valid options`) } - if (workFactor < 6 || workFactor > 31 || !Number.isInteger(workFactor)) { throw new Error(`${meta.listKey}.${meta.fieldKey}: workFactor must be an integer between 6 and 31`) } @@ -100,32 +101,33 @@ export function password (config: Passwor return bcrypt.hash(val, workFactor) } + const hasAdditionalValidation = match || rejectCommon || min !== undefined || max !== undefined const { mode, validate, - } = makeValidateHook(meta, config, ({ resolvedData, operation, addValidationError }) => { + } = makeValidateHook(meta, config, hasAdditionalValidation ? ({ inputData, operation, addValidationError }) => { if (operation === 'delete') return - const value = resolvedData[meta.fieldKey] + const value = inputData[meta.fieldKey] // we use inputData, as resolveData is hashed if (value != null) { - if (value.length < validation.length.min) { - if (validation.length.min === 1) { + if (min !== undefined && value.length < min) { + if (min === 1) { addValidationError(`value must not be empty`) } else { - addValidationError(`value must be at least ${validation.length.min} characters long`) + addValidationError(`value must be at least ${min} characters long`) } } - if (validation.length.max !== null && value.length > validation.length.max) { - addValidationError(`Value must be no longer than ${validation.length.max} characters`) + if (max !== undefined && value.length > max) { + addValidationError(`value must be no longer than ${max} characters`) } - if (validation.match && !validation.match.regex.test(value)) { - addValidationError(validation.match.explanation) + if (match && !match.regex.test(value)) { + addValidationError(match.explanation ?? `value must match ${match.regex}`) } - if (validation.rejectCommon && dumbPasswords.check(value)) { - addValidationError(`Value is too common and is not allowed`) + if (rejectCommon && dumbPasswords.check(value)) { + addValidationError(`value is too common and is not allowed`) } } - }) + } : undefined) return fieldType({ kind: 'scalar', @@ -173,16 +175,19 @@ export function password (config: Passwor getAdminMeta: (): PasswordFieldMeta => ({ isNullable: mode === 'optional', validation: { - ...validation, - match: validation.match - ? { - regex: { - source: validation.match.regex.source, - flags: validation.match.regex.flags, - }, - explanation: validation.match.explanation, - } - : null, + isRequired, + rejectCommon, + match: match ? { + regex: { + source: match.regex.source, + flags: match.regex.flags, + }, + explanation: match.explanation ?? `value must match ${match.regex}`, + } : null, + length: { + max: max ?? null, + min: min ?? 8 + }, }, }), output: graphql.field({ diff --git a/packages/core/src/fields/types/text/index.ts b/packages/core/src/fields/types/text/index.ts index 5dadfffd27f..426908a2763 100644 --- a/packages/core/src/fields/types/text/index.ts +++ b/packages/core/src/fields/types/text/index.ts @@ -53,72 +53,83 @@ export type TextFieldConfig = } } +export type TextFieldMeta = { + displayMode: 'input' | 'textarea' + shouldUseModeInsensitive: boolean + isNullable: boolean + validation: { + isRequired: boolean + match: { regex: { source: string, flags: string }, explanation: string | null } | null + length: { min: number | null, max: number | null } + } + defaultValue: string | null +} + export function text ( config: TextFieldConfig = {} ): FieldTypeFunc { const { - isIndexed, defaultValue: defaultValue_, - validation: validation_ + isIndexed, + validation = {} } = config config.db ??= {} config.db.isNullable ??= false // TODO: sigh, remove in breaking change? + const isRequired = validation.isRequired ?? false + const match = validation.match + const min = validation.isRequired ? validation.length?.min ?? 1 : validation.length?.min + const max = validation.length?.max + return (meta) => { - for (const type of ['min', 'max'] as const) { - const val = validation_?.length?.[type] - if (val !== undefined && (!Number.isInteger(val) || val < 0)) { - throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.length.${type}: ${val} but it must be a positive integer`) - } - if (validation_?.isRequired && val !== undefined && val === 0) { - throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.isRequired: true and validation.length.${type}: 0, this is not allowed because validation.isRequired implies at least a min length of 1`) - } + if (min !== undefined && (!Number.isInteger(min) || min < 0)) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.length.min: ${min} but it must be a positive integer`) + } + if (max !== undefined && (!Number.isInteger(max) || max < 0)) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.length.max: ${max} but it must be a positive integer`) + } + if (isRequired && min !== undefined && min === 0) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.isRequired: true and validation.length.min: 0, this is not allowed because validation.isRequired implies at least a min length of 1`) + } + if (isRequired && max !== undefined && max === 0) { + throw new Error(`${meta.listKey}.${meta.fieldKey} specifies validation.isRequired: true and validation.length.max: 0, this is not allowed because validation.isRequired implies at least a max length of 1`) } - if ( - validation_?.length?.min !== undefined && - validation_?.length?.max !== undefined && - validation_?.length?.min > validation_?.length?.max + min !== undefined && + max !== undefined && + min > max ) { throw new Error(`${meta.listKey}.${meta.fieldKey} specifies a validation.length.max that is less than the validation.length.min, and therefore has no valid options`) } - const validation = validation_ ? { - ...validation_, - length: { - min: validation_?.isRequired ? validation_?.length?.min ?? 1 : validation_?.length?.min, - max: validation_?.length?.max, - }, - } : undefined - // defaulted to false as a zero length string is preferred to null const isNullable = config.db?.isNullable ?? false const defaultValue = isNullable ? (defaultValue_ ?? null) : (defaultValue_ ?? '') - + const hasAdditionalValidation = match || min !== undefined || max !== undefined const { mode, validate, - } = makeValidateHook(meta, config, ({ resolvedData, operation, addValidationError }) => { + } = makeValidateHook(meta, config, hasAdditionalValidation ? ({ resolvedData, operation, addValidationError }) => { if (operation === 'delete') return const value = resolvedData[meta.fieldKey] if (value != null) { - if (validation?.length?.min !== undefined && value.length < validation.length.min) { - if (validation.length.min === 1) { + if (min !== undefined && value.length < min) { + if (min === 1) { addValidationError(`value must not be empty`) } else { - addValidationError(`value must be at least ${validation.length.min} characters long`) + addValidationError(`value must be at least ${min} characters long`) } } - if (validation?.length?.max !== undefined && value.length > validation.length.max) { - addValidationError(`value must be no longer than ${validation.length.max} characters`) + if (max !== undefined && value.length > max) { + addValidationError(`value must be no longer than ${max} characters`) } - if (validation?.match && !validation.match.regex.test(value)) { - addValidationError(validation.match.explanation || `value must match ${validation.match.regex}`) + if (match && !match.regex.test(value)) { + addValidationError(match.explanation ?? `value must match ${match.regex}`) } } - }) + } : undefined) return fieldType({ kind: 'scalar', @@ -133,8 +144,7 @@ export function text ( ...config, hooks: mergeFieldHooks({ validate }, config.hooks), input: { - uniqueWhere: - isIndexed === 'unique' ? { arg: graphql.arg({ type: graphql.String }) } : undefined, + uniqueWhere: isIndexed === 'unique' ? { arg: graphql.arg({ type: graphql.String }) } : undefined, where: { arg: graphql.arg({ type: filters[meta.provider].String[mode], @@ -147,10 +157,8 @@ export function text ( defaultValue: typeof defaultValue === 'string' ? defaultValue : undefined, }), resolve (val) { - if (val === undefined) { - return defaultValue ?? null - } - return val + if (val !== undefined) return val + return defaultValue ?? null }, }, update: { arg: graphql.arg({ type: graphql.String }) }, @@ -166,17 +174,18 @@ export function text ( displayMode: config.ui?.displayMode ?? 'input', shouldUseModeInsensitive: meta.provider === 'postgresql', validation: { - isRequired: validation?.isRequired ?? false, - match: validation?.match - ? { - regex: { - source: validation.match.regex.source, - flags: validation.match.regex.flags, - }, - explanation: validation.match.explanation ?? null, - } - : null, - length: { max: validation?.length?.max ?? null, min: validation?.length?.min ?? null }, + isRequired, + match: match ? { + regex: { + source: match.regex.source, + flags: match.regex.flags, + }, + explanation: match.explanation ?? `value must match ${match.regex}`, + } : null, + length: { + max: max ?? null, + min: min ?? null + }, }, defaultValue: defaultValue ?? (isNullable ? null : ''), isNullable, @@ -185,15 +194,3 @@ export function text ( }) } } - -export type TextFieldMeta = { - displayMode: 'input' | 'textarea' - shouldUseModeInsensitive: boolean - isNullable: boolean - validation: { - isRequired: boolean - match: { regex: { source: string, flags: string }, explanation: string | null } | null - length: { min: number | null, max: number | null } - } - defaultValue: string | null -} diff --git a/packages/core/src/fields/types/timestamp/index.ts b/packages/core/src/fields/types/timestamp/index.ts index 74776c108f2..d2cdd55d8ac 100644 --- a/packages/core/src/fields/types/timestamp/index.ts +++ b/packages/core/src/fields/types/timestamp/index.ts @@ -41,9 +41,7 @@ export function timestamp ( try { graphql.DateTime.graphQLType.parseValue(defaultValue) } catch (err) { - throw new Error( - `${meta.listKey}.${meta.fieldKey}.defaultValue is required to be an ISO8601 date-time string such as ${new Date().toISOString()}` - ) + throw new Error(`${meta.listKey}.${meta.fieldKey}.defaultValue is required to be an ISO8601 date-time string such as ${new Date().toISOString()}`) } } diff --git a/tests/api-tests/auth.test.ts b/tests/api-tests/auth.test.ts index 6efd0783c8f..4c1ab5f3a40 100644 --- a/tests/api-tests/auth.test.ts +++ b/tests/api-tests/auth.test.ts @@ -233,7 +233,7 @@ describe('Auth testing', () => { expectValidationError(body.errors, [ { path: ['createUser'], // I don't like this! - messages: ['User.email: Email must not be empty'], + messages: ['User.email: value must not be empty'], }, ]) expect(body.data).toEqual(null) diff --git a/tests/api-tests/fields/crud.test.ts b/tests/api-tests/fields/crud.test.ts index 70996d5cb76..bcfbe5e6b5d 100644 --- a/tests/api-tests/fields/crud.test.ts +++ b/tests/api-tests/fields/crud.test.ts @@ -4,7 +4,6 @@ import { text } from '@keystone-6/core/fields' import { type KeystoneContext } from '@keystone-6/core/types' import { setupTestRunner } from '@keystone-6/api-tests/test-runner' import { allowAll } from '@keystone-6/core/access' -import { humanize } from '../../../packages/core/src/lib/utils' import { dbProvider, expectSingleResolverError, @@ -221,7 +220,7 @@ for (const modulePath of testModules) { expectValidationError(errors, [ { path: [updateMutationName], - messages: [`Test.${fieldName}: ${humanize(fieldName)} is required`], + messages: [`Test.${fieldName}: missing value`], }, ]) } diff --git a/tests/api-tests/fields/required.test.ts b/tests/api-tests/fields/required.test.ts index 48f07ab2c91..5f14b2151b0 100644 --- a/tests/api-tests/fields/required.test.ts +++ b/tests/api-tests/fields/required.test.ts @@ -6,7 +6,6 @@ import { list } from '@keystone-6/core' import { text } from '@keystone-6/core/fields' import { setupTestRunner } from '@keystone-6/api-tests/test-runner' import { allowAll } from '@keystone-6/core/access' -import { humanize } from '../../../packages/core/src/lib/utils' import { dbProvider, expectValidationError @@ -85,23 +84,23 @@ for (const modulePath of testModules) { }, }) - const messages = [`Test.testField: ${humanize('testField')} is required`] + const messages = [`Test.testField: missing value`] test( 'Create an object without the required field', runner(async ({ context }) => { const { data, errors } = await context.graphql.raw({ query: ` - mutation { - createTest(data: { name: "test entry" } ) { id } - }`, + mutation { + createTest(data: { name: "test entry" } ) { id } + }`, }) expect(data).toEqual({ createTest: null }) expectValidationError(errors, [ { path: ['createTest'], messages: - mod.name === 'Text' ? ['Test.testField: Test Field must not be empty'] : messages, + mod.name === 'Text' ? ['Test.testField: value must not be empty'] : messages, }, ]) }) @@ -112,9 +111,9 @@ for (const modulePath of testModules) { runner(async ({ context }) => { const { data, errors } = await context.graphql.raw({ query: ` - mutation { - createTest(data: { name: "test entry", testField: null } ) { id } - }`, + mutation { + createTest(data: { name: "test entry", testField: null } ) { id } + }`, }) expect(data).toEqual({ createTest: null }) expectValidationError(errors, [ diff --git a/tests/api-tests/fields/types/fixtures/decimal/test-fixtures.ts b/tests/api-tests/fields/types/fixtures/decimal/test-fixtures.ts index 9c8ea2bd0c1..0cef149b424 100644 --- a/tests/api-tests/fields/types/fixtures/decimal/test-fixtures.ts +++ b/tests/api-tests/fields/types/fixtures/decimal/test-fixtures.ts @@ -55,7 +55,7 @@ export const crudTests = (keystoneTestWrapper: any) => { expect(result.errors).toHaveLength(1) expect(result.errors![0].message).toMatchInlineSnapshot(` "You provided invalid data for this operation. - - Test.price: Price must be greater than or equal to -300" + - Test.price: value must be greater than or equal to -300" `) }) ) @@ -76,7 +76,7 @@ export const crudTests = (keystoneTestWrapper: any) => { expect(result.errors).toHaveLength(1) expect(result.errors![0].message).toMatchInlineSnapshot(` "You provided invalid data for this operation. - - Test.price: Price must be less than or equal to 50000000" + - Test.price: value must be less than or equal to 50000000" `) }) ) diff --git a/tests/api-tests/fields/types/fixtures/password/test-fixtures.ts b/tests/api-tests/fields/types/fixtures/password/test-fixtures.ts index 02cbbab34ff..4465a1d3fdf 100644 --- a/tests/api-tests/fields/types/fixtures/password/test-fixtures.ts +++ b/tests/api-tests/fields/types/fixtures/password/test-fixtures.ts @@ -48,9 +48,9 @@ export const crudTests = (keystoneTestWrapper: any) => { data: { password: '123' }, }) ).rejects.toMatchInlineSnapshot(` - [GraphQLError: You provided invalid data for this operation. - - Test.password: Password must be at least 4 characters long] - `) + [GraphQLError: You provided invalid data for this operation. + - Test.password: value must be at least 4 characters long] + `) }) ) test( @@ -61,9 +61,9 @@ export const crudTests = (keystoneTestWrapper: any) => { data: { passwordRejectCommon: 'password' }, }) ).rejects.toMatchInlineSnapshot(` - [GraphQLError: You provided invalid data for this operation. - - Test.passwordRejectCommon: Password Reject Common is too common and is not allowed] - `) + [GraphQLError: You provided invalid data for this operation. + - Test.passwordRejectCommon: value is too common and is not allowed] + `) const data = await context.query.Test.createOne({ data: { passwordRejectCommon: 'sdfinwedvhweqfoiuwdfnvjiewrijnf' }, query: `passwordRejectCommon {isSet}`,