From f80646c5987db4c228b00beda9549259021c2a40 Mon Sep 17 00:00:00 2001 From: Dan Ribbens Date: Wed, 25 Aug 2021 13:28:42 -0400 Subject: [PATCH] fix: allow save of collection with an undefined point --- demo/collections/Geolocation.ts | 21 +++++++++++++++++---- demo/collections/Localized.ts | 14 ++++++++++++++ src/collections/bindCollection.ts | 5 ++--- src/collections/buildSchema.ts | 2 +- src/collections/init.ts | 2 +- src/collections/tests/pointField.spec.js | 1 - src/express/types.ts | 2 +- src/fields/hookPromise.ts | 4 ++-- src/fields/performFieldOperations.ts | 21 ++++++++++++--------- src/fields/traverseFields.ts | 18 ++++++++++-------- src/mongoose/buildSchema.ts | 2 -- 11 files changed, 60 insertions(+), 32 deletions(-) diff --git a/demo/collections/Geolocation.ts b/demo/collections/Geolocation.ts index 1a4205ab5a2..b0e92e05354 100644 --- a/demo/collections/Geolocation.ts +++ b/demo/collections/Geolocation.ts @@ -1,6 +1,14 @@ /* eslint-disable no-param-reassign */ import { CollectionConfig } from '../../src/collections/config/types'; +const validateFieldTransformAction = (hook: string, value = null) => { + if (value !== null && !Array.isArray(value)) { + console.error(hook, value); + throw new Error('Field transformAction should convert value to array [x, y] and not { coordinates: [x, y] }'); + } + return value; +}; + const Geolocation: CollectionConfig = { slug: 'geolocation', labels: { @@ -24,7 +32,6 @@ const Geolocation: CollectionConfig = { afterRead: [ (operation) => { const { doc } = operation; - // console.log(doc); doc.afterReadHook = !doc.location?.coordinates; return doc; }, @@ -32,15 +39,15 @@ const Geolocation: CollectionConfig = { afterChange: [ (operation) => { const { doc } = operation; - operation.doc.afterChangeHook = !doc.location?.coordinates; - return operation.doc; + doc.afterChangeHook = !doc.location?.coordinates; + return doc; }, ], afterDelete: [ (operation) => { const { doc } = operation; operation.doc.afterDeleteHook = !doc.location?.coordinates; - return operation.doc; + return doc; }, ], }, @@ -49,6 +56,12 @@ const Geolocation: CollectionConfig = { name: 'location', type: 'point', label: 'Location', + hooks: { + beforeValidate: [({ value }) => validateFieldTransformAction('beforeValidate', value)], + beforeChange: [({ value }) => validateFieldTransformAction('beforeChange', value)], + afterChange: [({ value }) => validateFieldTransformAction('afterChange', value)], + afterRead: [({ value }) => validateFieldTransformAction('afterRead', value)], + }, }, { name: 'localizedPoint', diff --git a/demo/collections/Localized.ts b/demo/collections/Localized.ts index f222bb2a30e..87970f3d68d 100644 --- a/demo/collections/Localized.ts +++ b/demo/collections/Localized.ts @@ -1,6 +1,14 @@ import { CollectionConfig } from '../../src/collections/config/types'; import { Block } from '../../src/fields/config/types'; +const validateLocalizationTransform = (hook: string, value = null) => { + if (value !== null && typeof value !== 'string') { + console.error(hook, value); + throw new Error('Field text transformation in hook is wonky'); + } + return value; +}; + const RichTextBlock: Block = { slug: 'richTextBlock', labels: { @@ -46,6 +54,12 @@ const LocalizedPosts: CollectionConfig = { required: true, unique: true, localized: true, + hooks: { + beforeValidate: [({ value }) => validateLocalizationTransform('beforeValidate', value)], + beforeChange: [({ value }) => validateLocalizationTransform('beforeChange', value)], + afterChange: [({ value }) => validateLocalizationTransform('afterChange', value)], + afterRead: [({ value }) => validateLocalizationTransform('afterRead', value)], + }, }, { name: 'summary', diff --git a/src/collections/bindCollection.ts b/src/collections/bindCollection.ts index 6a50e04982e..2242b5fa4ca 100644 --- a/src/collections/bindCollection.ts +++ b/src/collections/bindCollection.ts @@ -1,7 +1,6 @@ -import { NextFunction, Response } from 'express'; -import { PayloadRequest } from '../express/types'; +import { NextFunction, Request, Response } from 'express'; -const bindCollectionMiddleware = (collection: string) => (req: PayloadRequest, res: Response, next: NextFunction) => { +const bindCollectionMiddleware = (collection: string) => (req: Request & { collection: string }, res: Response, next: NextFunction) => { req.collection = collection; next(); }; diff --git a/src/collections/buildSchema.ts b/src/collections/buildSchema.ts index 1cd583a285d..df5d3c0541f 100644 --- a/src/collections/buildSchema.ts +++ b/src/collections/buildSchema.ts @@ -1,6 +1,6 @@ import paginate from 'mongoose-paginate-v2'; import { Schema } from 'mongoose'; -import { SanitizedConfig } from '../../config'; +import { SanitizedConfig } from '../config/types'; import buildQueryPlugin from '../mongoose/buildQuery'; import buildSchema from '../mongoose/buildSchema'; import { SanitizedCollectionConfig } from './config/types'; diff --git a/src/collections/init.ts b/src/collections/init.ts index 1159c8da27e..b792fb75fee 100644 --- a/src/collections/init.ts +++ b/src/collections/init.ts @@ -8,7 +8,7 @@ import apiKeyStrategy from '../auth/strategies/apiKey'; import buildSchema from './buildSchema'; import bindCollectionMiddleware from './bindCollection'; import { SanitizedCollectionConfig } from './config/types'; -import { SanitizedConfig } from '../../config'; +import { SanitizedConfig } from '../config/types'; import { Payload } from '../index'; const LocalStrategy = Passport.Strategy; diff --git a/src/collections/tests/pointField.spec.js b/src/collections/tests/pointField.spec.js index 057c591ad17..a0d412baf73 100644 --- a/src/collections/tests/pointField.spec.js +++ b/src/collections/tests/pointField.spec.js @@ -39,7 +39,6 @@ describe('GeoJSON', () => { let doc; beforeAll(async (done) => { - // create document a const create = await fetch(`${serverURL}/api/geolocation`, { body: JSON.stringify({ location, localizedPoint }), headers, diff --git a/src/express/types.ts b/src/express/types.ts index 24d7763d8de..e8b6644519f 100644 --- a/src/express/types.ts +++ b/src/express/types.ts @@ -9,7 +9,7 @@ export type PayloadRequest = Request & { payload: Payload; locale?: string; fallbackLocale?: string; - collection?: Collection | string; + collection?: Collection; payloadAPI: 'REST' | 'local' | 'graphQL' file?: UploadedFile user: User | null diff --git a/src/fields/hookPromise.ts b/src/fields/hookPromise.ts index e3363987154..c03a9be78a7 100644 --- a/src/fields/hookPromise.ts +++ b/src/fields/hookPromise.ts @@ -12,7 +12,7 @@ type Arguments = { fullData: Record } -const hookPromise = async ({ +const hookPromise = ({ data, field, hook, @@ -20,7 +20,7 @@ const hookPromise = async ({ operation, fullOriginalDoc, fullData, -}: Arguments): Promise => { +}: Arguments) => async (): Promise => { const resultingData = data; if (field.hooks && field.hooks[hook]) { diff --git a/src/fields/performFieldOperations.ts b/src/fields/performFieldOperations.ts index 944fc63fad0..1875d0f6998 100644 --- a/src/fields/performFieldOperations.ts +++ b/src/fields/performFieldOperations.ts @@ -103,6 +103,18 @@ export default async function performFieldOperations(this: Payload, entityConfig docWithLocales, }); + if (hook === 'beforeChange') { + transformActions.forEach((action) => action()); + } + + unflattenLocaleActions.forEach((action) => action()); + + if (hook === 'afterRead') { + transformActions.forEach((action) => action()); + } + + hookPromises.forEach((promise) => promise()); + await Promise.all(hookPromises); validationPromises.forEach((promise) => promise()); @@ -113,15 +125,6 @@ export default async function performFieldOperations(this: Payload, entityConfig throw new ValidationError(errors); } - if (hook === 'beforeChange') { - transformActions.forEach((action) => action()); - } - - unflattenLocaleActions.forEach((action) => action()); - - if (hook === 'afterRead') { - transformActions.forEach((action) => action()); - } await Promise.all(accessPromises); diff --git a/src/fields/traverseFields.ts b/src/fields/traverseFields.ts index b2a74ef1538..744b018e3f8 100644 --- a/src/fields/traverseFields.ts +++ b/src/fields/traverseFields.ts @@ -24,7 +24,7 @@ type Arguments = { depth: number currentDepth: number hook: HookName - hookPromises: Promise[] + hookPromises: (() => Promise)[] fullOriginalDoc: Record fullData: Record validationPromises: (() => Promise)[] @@ -269,13 +269,15 @@ const traverseFields = (args: Arguments): void => { if (field.type === 'point' && data[field.name]) { transformActions.push(() => { - data[field.name] = { - type: 'Point', - coordinates: [ - parseFloat(data[field.name][0]), - parseFloat(data[field.name][1]), - ], - }; + if (Array.isArray(data[field.name]) && data[field.name][0] !== null && data[field.name][1] !== null) { + data[field.name] = { + type: 'Point', + coordinates: [ + parseFloat(data[field.name][0]), + parseFloat(data[field.name][1]), + ], + }; + } }); } diff --git a/src/mongoose/buildSchema.ts b/src/mongoose/buildSchema.ts index 9de2cde969b..ddc61531cf2 100644 --- a/src/mongoose/buildSchema.ts +++ b/src/mongoose/buildSchema.ts @@ -213,9 +213,7 @@ const fieldToSchemaMap = { const baseSchema = { type: { type: String, - default: 'Point', enum: ['Point'], - required: true, }, coordinates: { type: [Number],