Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update built-in fields to new validate hook syntax #9166

Merged
merged 16 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/add-isnullable-multiselect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystone-6/core': minor
---

Add `db.isNullable` support for multiselect field type, defaults to false
5 changes: 5 additions & 0 deletions .changeset/fix-bigint-validation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystone-6/core': patch
---

Fix bigInt field type to throw if `defaultValue` and `validation.isRequired` is set
5 changes: 5 additions & 0 deletions .changeset/update-field-hooks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@keystone-6/core": patch
---

Update built-in fields to use newer validate hook syntax
2 changes: 1 addition & 1 deletion examples/framework-astro/src/keystone/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ export const lists = {
title: text({ validation: { isRequired: true } }),
// we use this field to arbitrarily restrict Posts to only be viewed on a particular browser (using Post.access.filter)
browser: select({
validation: { isRequired: true },
options: [
{ label: 'Chrome', value: 'chrome' },
{ label: 'Firefox', value: 'firefox' },
],
validation: { isRequired: true },
}),
},
}),
Expand Down
2 changes: 1 addition & 1 deletion examples/testing/example-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ test('Check that trying to create user with no name (required field) fails', asy
})
},
{
message: 'You provided invalid data for this operation.\n - User.name: Name must not be empty',
message: 'You provided invalid data for this operation.\n - User.name: value must not be empty',
}
)
})
Expand Down
90 changes: 71 additions & 19 deletions packages/core/src/fields/non-null-graphql.ts
Original file line number Diff line number Diff line change
@@ -1,40 +1,92 @@
import { type BaseListTypeInfo, type CommonFieldConfig, type FieldData } from '../types'
import {
type BaseListTypeInfo,
type FieldData,
} from '../types'
import {
type ValidateFieldHook
} from '../types/config/hooks'

export function getResolvedIsNullable (
export function resolveDbNullable (
validation: undefined | { isRequired?: boolean },
db: undefined | { isNullable?: boolean }
): boolean {
if (db?.isNullable === false) {
return false
}
if (db?.isNullable === false) return false
if (db?.isNullable === undefined && validation?.isRequired) {
return false
}
return true
}

export function resolveHasValidation ({
db,
validation
}: {
db?: { isNullable?: boolean }
validation?: unknown
}) {
if (db?.isNullable === false) return true
if (validation !== undefined) return true
return false
export function makeValidateHook <ListTypeInfo extends BaseListTypeInfo> (
meta: FieldData,
config: {
label?: string,
db?: {
isNullable?: boolean
},
graphql?: {
isNonNull?: {
read?: boolean
}
},
validation?: {
isRequired?: boolean
[key: string]: unknown
},
},
f?: ValidateFieldHook<ListTypeInfo, 'create' | 'update' | 'delete', ListTypeInfo['fields']>
) {
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 = config.db?.isNullable === false || config.validation?.isRequired
if (addValidation) {
const validate = async function (args) {
const { operation, addValidationError, resolvedData } = args

if (valueRequired) {
const value = resolvedData?.[meta.fieldKey]
if (
(operation === 'create' && value === undefined)
|| ((operation === 'create' || operation === 'update') && (value === null))
) {
addValidationError(`missing value`)
}
}

await f?.(args)
} satisfies ValidateFieldHook<ListTypeInfo, 'create' | 'update' | 'delete', ListTypeInfo['fields']>

return {
mode,
validate,
}
}

return {
mode,
validate: f
}
}

export function assertReadIsNonNullAllowed<ListTypeInfo extends BaseListTypeInfo> (
meta: FieldData,
config: CommonFieldConfig<ListTypeInfo>,
resolvedIsNullable: boolean
config: {
graphql?: {
isNonNull?: {
read?: boolean
}
}
},
dbNullable: boolean
) {
if (!resolvedIsNullable) return
if (!dbNullable) return
if (!config.graphql?.isNonNull?.read) return

throw new Error(
`The field at ${meta.listKey}.${meta.fieldKey} sets graphql.isNonNull.read: true, but not validation.isRequired: true, or db.isNullable: false\n` +
`${meta.listKey}.${meta.fieldKey} sets graphql.isNonNull.read: true, but not validation.isRequired: true (or db.isNullable: false)\n` +
`Set validation.isRequired: true, or db.isNullable: false, or graphql.isNonNull.read: false`
)
}
71 changes: 71 additions & 0 deletions packages/core/src/fields/resolve-hooks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import {
type BaseListTypeInfo,
type FieldHooks,
type MaybePromise
} from '../types'

// force new syntax for built-in fields
// and block hooks from using resolveInput, they should use GraphQL resolvers
export type InternalFieldHooks<ListTypeInfo extends BaseListTypeInfo> =
Omit<FieldHooks<ListTypeInfo>, 'validateInput' | 'validateDelete' | 'resolveInput'>

/** @deprecated, TODO: remove in breaking change */
function resolveValidateHooks <ListTypeInfo extends BaseListTypeInfo> ({
validate,
validateInput,
validateDelete
}: FieldHooks<ListTypeInfo>): Exclude<FieldHooks<ListTypeInfo>["validate"], Function> | undefined {
if (validateInput || validateDelete) {
return {
create: validateInput,
update: validateInput,
delete: validateDelete,
}
}

if (!validate) return
if (typeof validate === 'function') {
return {
create: validate,
update: validate,
delete: validate
}
}

return validate
}

function merge <
R,
A extends (r: R) => MaybePromise<void>,
B extends (r: R) => MaybePromise<void>
> (a?: A, b?: B) {
if (!a && !b) return undefined
return async (args: R) => {
await a?.(args)
await b?.(args)
}
}

export function mergeFieldHooks <ListTypeInfo extends BaseListTypeInfo> (
builtin?: InternalFieldHooks<ListTypeInfo>,
hooks?: FieldHooks<ListTypeInfo>,
) {
if (hooks === undefined) return builtin
if (builtin === undefined) return hooks

const builtinValidate = resolveValidateHooks(builtin)
const hooksValidate = resolveValidateHooks(hooks)
return {
...hooks,
// WARNING: beforeOperation is _after_ a user beforeOperation hook, TODO: this is align with user expectations about when "operations" happen
// our *Operation hooks are built-in, and should happen nearest to the database
beforeOperation: merge(hooks.beforeOperation, builtin.beforeOperation),
afterOperation: merge(builtin.afterOperation, hooks.afterOperation),
validate: (builtinValidate || hooksValidate) ? {
create: merge(builtinValidate?.create, hooksValidate?.create),
update: merge(builtinValidate?.update, hooksValidate?.update),
delete: merge(builtinValidate?.delete, hooksValidate?.delete)
} : undefined,
} satisfies FieldHooks<ListTypeInfo>
}
Copy link
Member

@dcousens dcousens Jul 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acburdine I tidied this up a little to re-use a new merge function, but fundamentally it's the same

Loading
Loading