Simplify validation hook code (#6667)

keystonejs · Sep 28, 2021 · 8631917 · 8631917
1 parent 2e95848
commit 8631917
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 57 deletions.
diff --git a/.changeset/three-emus-sit.md b/.changeset/three-emus-sit.md
@@ -0,0 +1,5 @@
+---
+'@keystone-next/keystone': patch
+---
+
+Simplified logic of validation hook execution code.
diff --git a/packages/keystone/src/lib/core/mutations/validation.ts b/packages/keystone/src/lib/core/mutations/validation.ts
@@ -1,22 +1,6 @@
 import { validationFailureError } from '../graphql-errors';
 import { InitialisedList } from '../types-for-lists';
 
-type AddValidationError = (msg: string) => void;
-
-async function validationHook(
-  _validationHook: (addValidationError: AddValidationError) => void | Promise<void>
-) {
-  const messages: string[] = [];
-
-  await _validationHook(msg => {
-    messages.push(msg);
-  });
-
-  if (messages.length) {
-    throw validationFailureError(messages);
-  }
-}
-
 type UpdateCreateHookArgs = Parameters<
   Exclude<InitialisedList['hooks']['validateInput'], undefined>
 >[0];
@@ -28,42 +12,45 @@ export async function validateUpdateCreate({
   hookArgs: Omit<UpdateCreateHookArgs, 'addValidationError'>;
 }) {
   const { operation, resolvedData } = hookArgs;
-  await validationHook(async _addValidationError => {
-    // Check isRequired
-    for (const [fieldKey, field] of Object.entries(list.fields)) {
-      const addValidationError = (msg: string) =>
-        _addValidationError(`${list.listKey}.${fieldKey}: ${msg}`);
-      // yes, this is a massive hack, it's just to make image and file fields work well enough
-      let val = resolvedData[fieldKey];
-      if (field.dbField.kind === 'multi') {
-        if (Object.values(resolvedData[fieldKey]).every(x => x === null)) {
-          val = null;
-        }
-        if (Object.values(resolvedData[fieldKey]).every(x => x === undefined)) {
-          val = undefined;
-        }
+  const messages: string[] = [];
+
+  // Check isRequired
+  for (const [fieldKey, field] of Object.entries(list.fields)) {
+    // yes, this is a massive hack, it's just to make image and file fields work well enough
+    let val = resolvedData[fieldKey];
+    if (field.dbField.kind === 'multi') {
+      if (Object.values(resolvedData[fieldKey]).every(x => x === null)) {
+        val = null;
       }
-      if (
-        field.__legacy?.isRequired &&
-        ((operation === 'create' && val == null) || (operation === 'update' && val === null))
-      ) {
-        addValidationError(`Required field "${fieldKey}" is null or undefined.`);
+      if (Object.values(resolvedData[fieldKey]).every(x => x === undefined)) {
+        val = undefined;
       }
     }
-
-    // Field validation hooks
-    for (const [fieldKey, field] of Object.entries(list.fields)) {
-      const addValidationError = (msg: string) =>
-        _addValidationError(`${list.listKey}.${fieldKey}: ${msg}`);
-      // @ts-ignore
-      await field.hooks.validateInput?.({ ...hookArgs, addValidationError, fieldKey });
+    if (
+      field.__legacy?.isRequired &&
+      ((operation === 'create' && val == null) || (operation === 'update' && val === null))
+    ) {
+      messages.push(
+        `${list.listKey}.${fieldKey}: Required field "${fieldKey}" is null or undefined.`
+      );
     }
+  }
 
-    // List validation hooks
-    const addValidationError = (msg: string) => _addValidationError(`${list.listKey}: ${msg}`);
+  // Field validation hooks
+  for (const [fieldKey, field] of Object.entries(list.fields)) {
+    const addValidationError = (msg: string) =>
+      messages.push(`${list.listKey}.${fieldKey}: ${msg}`);
     // @ts-ignore
-    await list.hooks.validateInput?.({ ...hookArgs, addValidationError });
-  });
+    await field.hooks.validateInput?.({ ...hookArgs, addValidationError, fieldKey });
+  }
+
+  // List validation hooks
+  const addValidationError = (msg: string) => messages.push(`${list.listKey}: ${msg}`);
+  // @ts-ignore
+  await list.hooks.validateInput?.({ ...hookArgs, addValidationError });
+  if (messages.length) {
+    throw validationFailureError(messages);
+  }
 }
 
 type DeleteHookArgs = Parameters<Exclude<InitialisedList['hooks']['validateDelete'], undefined>>[0];
@@ -74,16 +61,20 @@ export async function validateDelete({
   list: InitialisedList;
   hookArgs: Omit<DeleteHookArgs, 'addValidationError'>;
 }) {
-  await validationHook(async _addValidationError => {
-    // Field validation
-    for (const [fieldKey, field] of Object.entries(list.fields)) {
-      const addValidationError = (msg: string) =>
-        _addValidationError(`${list.listKey}.${fieldKey}: ${msg}`);
-      await field.hooks.validateDelete?.({ ...hookArgs, addValidationError, fieldKey });
-    }
+  const messages: string[] = [];
 
-    // List validation
-    const addValidationError = (msg: string) => _addValidationError(`${list.listKey}: ${msg}`);
-    await list.hooks.validateDelete?.({ ...hookArgs, addValidationError });
-  });
+  // Field validation
+  for (const [fieldKey, field] of Object.entries(list.fields)) {
+    const addValidationError = (msg: string) =>
+      messages.push(`${list.listKey}.${fieldKey}: ${msg}`);
+    await field.hooks.validateDelete?.({ ...hookArgs, addValidationError, fieldKey });
+  }
+
+  // List validation
+  const addValidationError = (msg: string) => messages.push(`${list.listKey}: ${msg}`);
+  await list.hooks.validateDelete?.({ ...hookArgs, addValidationError });
+
+  if (messages.length) {
+    throw validationFailureError(messages);
+  }
 }