-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
9e7b1d0
update built-in fields to new validate hook syntax
acburdine b285e25
refactor resolve-hooks code
dcousens d9cfc5d
dedupe makeValidationHook for mode and validate hook
dcousens 556b941
fix select isNotNull when only validation.isRequired is set
dcousens ed77734
less meta at the front of error messages
dcousens 22e87fa
tidy up
dcousens 8dc51b9
revert password to db.isNullable: false default
dcousens 393dc93
unify validation and fix tests
dcousens f29f4ba
unify float to same as integer, bigInt et al
dcousens eadd3b0
add bigInt error when defaultValue and isRequired is set
dcousens 64069ff
add bigInt changeset
dcousens cbc2ece
fix example-test.ts
dcousens d1dba42
revert builtin.beforeOperation to happen first
dcousens a3c80b9
add multiselect nullable changeset + add tests
dcousens c2420fc
fix tests for non-nullable default fields
dcousens c45b153
fix bigInt changeset
dcousens File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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: { kind: 'autoincrement' }` and `validation.isRequired` is set |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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` | ||
) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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> | ||
} | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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