From 957867f6e2a762ab909ab60f46ebe57e7e107233 Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Fri, 20 Dec 2024 15:14:23 -0500 Subject: [PATCH] fix: ensures generated IDs persist on create (#10089) IDs that are supplied directly through the API, such as client-side generated IDs when adding new blocks and array rows, are overwritten on create. This is because when adding blocks or array rows on the client, their IDs are generated first before being sent to the server for processing. Then when the server receives this data, it incorrectly overrides them to ensure they are unique when using relational DBs. But this only needs to happen when no ID was supplied on create, or specifically when duplicating documents via the `beforeDuplicate` hook. --- .../src/fields/baseFields/baseIDField.ts | 19 +- test/database/config.ts | 25 + test/database/int.spec.ts | 59 ++- test/database/payload-types.ts | 31 ++ test/fields/int.spec.ts | 17 - test/fields/payload-types.ts | 455 ++++++------------ tsconfig.base.json | 87 +--- 7 files changed, 291 insertions(+), 402 deletions(-) diff --git a/packages/payload/src/fields/baseFields/baseIDField.ts b/packages/payload/src/fields/baseFields/baseIDField.ts index 708ad8d44ad..f83f0b7b0af 100644 --- a/packages/payload/src/fields/baseFields/baseIDField.ts +++ b/packages/payload/src/fields/baseFields/baseIDField.ts @@ -13,23 +13,8 @@ export const baseIDField: TextField = { }, defaultValue: () => new ObjectId().toHexString(), hooks: { - beforeChange: [ - ({ operation, value }) => { - // If creating new doc, need to disregard any - // ids that have been passed in because they will cause - // primary key unique conflicts in relational DBs - if (!value || (operation === 'create' && value)) { - return new ObjectId().toHexString() - } - - return value - }, - ], - beforeDuplicate: [ - () => { - return new ObjectId().toHexString() - }, - ], + beforeChange: [({ value }) => value || new ObjectId().toHexString()], + beforeDuplicate: [() => new ObjectId().toHexString()], }, label: 'ID', } diff --git a/test/database/config.ts b/test/database/config.ts index db731fbc5ba..7c6f06c03bd 100644 --- a/test/database/config.ts +++ b/test/database/config.ts @@ -54,6 +54,31 @@ export default buildConfigWithDefaults({ ], }, }, + { + name: 'arrayWithIDs', + type: 'array', + fields: [ + { + name: 'text', + type: 'text', + }, + ], + }, + { + name: 'blocksWithIDs', + type: 'blocks', + blocks: [ + { + slug: 'block', + fields: [ + { + name: 'text', + type: 'text', + }, + ], + }, + ], + }, ], hooks: { beforeOperation: [ diff --git a/test/database/int.spec.ts b/test/database/int.spec.ts index 4c5cb2b0e7f..806e8e14694 100644 --- a/test/database/int.spec.ts +++ b/test/database/int.spec.ts @@ -7,7 +7,7 @@ import { migrateRelationshipsV2_V3, migrateVersionsV1_V2, } from '@payloadcms/db-mongodb/migration-utils' -import { desc, type Table } from 'drizzle-orm' +import { type Table } from 'drizzle-orm' import * as drizzlePg from 'drizzle-orm/pg-core' import * as drizzleSqlite from 'drizzle-orm/sqlite-core' import fs from 'fs' @@ -136,6 +136,63 @@ describe('database', () => { it('should not accidentally treat nested id fields as custom id', () => { expect(payload.collections['fake-custom-ids'].customIDType).toBeUndefined() }) + + it('should not overwrite supplied block and array row IDs on create', async () => { + const arrayRowID = '67648ed5c72f13be6eacf24e' + const blockID = '6764de9af79a863575c5f58c' + + const doc = await payload.create({ + collection: 'posts', + data: { + title: 'test', + arrayWithIDs: [ + { + id: arrayRowID, + }, + ], + blocksWithIDs: [ + { + blockType: 'block', + id: blockID, + }, + ], + }, + }) + + expect(doc.arrayWithIDs[0].id).toStrictEqual(arrayRowID) + expect(doc.blocksWithIDs[0].id).toStrictEqual(blockID) + }) + + it('should overwrite supplied block and array row IDs on duplicate', async () => { + const arrayRowID = '6764deb5201e9e36aeba3b6c' + const blockID = '6764dec58c68f337a758180c' + + const doc = await payload.create({ + collection: 'posts', + data: { + title: 'test', + arrayWithIDs: [ + { + id: arrayRowID, + }, + ], + blocksWithIDs: [ + { + blockType: 'block', + id: blockID, + }, + ], + }, + }) + + const duplicate = await payload.duplicate({ + collection: 'posts', + id: doc.id, + }) + + expect(duplicate.arrayWithIDs[0].id).not.toStrictEqual(arrayRowID) + expect(duplicate.blocksWithIDs[0].id).not.toStrictEqual(blockID) + }) }) describe('timestamps', () => { diff --git a/test/database/payload-types.ts b/test/database/payload-types.ts index c778e5d9e71..4cc83ae7052 100644 --- a/test/database/payload-types.ts +++ b/test/database/payload-types.ts @@ -90,6 +90,20 @@ export interface Post { title: string; hasTransaction?: boolean | null; throwAfterChange?: boolean | null; + arrayWithIDs?: + | { + text?: string | null; + id?: string | null; + }[] + | null; + blocksWithIDs?: + | { + text?: string | null; + id?: string | null; + blockName?: string | null; + blockType: 'block'; + }[] + | null; updatedAt: string; createdAt: string; } @@ -428,6 +442,23 @@ export interface PostsSelect { title?: T; hasTransaction?: T; throwAfterChange?: T; + arrayWithIDs?: + | T + | { + text?: T; + id?: T; + }; + blocksWithIDs?: + | T + | { + block?: + | T + | { + text?: T; + id?: T; + blockName?: T; + }; + }; updatedAt?: T; createdAt?: T; } diff --git a/test/fields/int.spec.ts b/test/fields/int.spec.ts index 16f5b533879..251f8f73d6f 100644 --- a/test/fields/int.spec.ts +++ b/test/fields/int.spec.ts @@ -2285,23 +2285,6 @@ describe('Fields', () => { expect(blockFieldsFail.docs).toHaveLength(0) }) - it('should create when existing block ids are used', async () => { - const blockFields = await payload.find({ - collection: 'block-fields', - limit: 1, - }) - const [doc] = blockFields.docs - - const result = await payload.create({ - collection: 'block-fields', - data: { - ...doc, - }, - }) - - expect(result.id).toBeDefined() - }) - it('should filter based on nested block fields', async () => { await payload.create({ collection: 'block-fields', diff --git a/test/fields/payload-types.ts b/test/fields/payload-types.ts index 02d905d57ef..1ab83d80f20 100644 --- a/test/fields/payload-types.ts +++ b/test/fields/payload-types.ts @@ -2157,257 +2157,42 @@ export interface BlockFieldsSelect { blocks?: | T | { - content?: - | T - | { - text?: T; - richText?: T; - id?: T; - blockName?: T; - }; - number?: - | T - | { - number?: T; - id?: T; - blockName?: T; - }; - subBlocks?: - | T - | { - subBlocks?: - | T - | { - text?: - | T - | { - text?: T; - id?: T; - blockName?: T; - }; - number?: - | T - | { - number?: T; - id?: T; - blockName?: T; - }; - }; - id?: T; - blockName?: T; - }; - tabs?: - | T - | { - textInCollapsible?: T; - textInRow?: T; - id?: T; - blockName?: T; - }; + content?: T | ContentBlockSelect; + number?: T | NumberBlockSelect; + subBlocks?: T | SubBlocksBlockSelect; + tabs?: T | TabsBlockSelect; }; duplicate?: | T | { - content?: - | T - | { - text?: T; - richText?: T; - id?: T; - blockName?: T; - }; - number?: - | T - | { - number?: T; - id?: T; - blockName?: T; - }; - subBlocks?: - | T - | { - subBlocks?: - | T - | { - text?: - | T - | { - text?: T; - id?: T; - blockName?: T; - }; - number?: - | T - | { - number?: T; - id?: T; - blockName?: T; - }; - }; - id?: T; - blockName?: T; - }; - tabs?: - | T - | { - textInCollapsible?: T; - textInRow?: T; - id?: T; - blockName?: T; - }; + content?: T | ContentBlockSelect; + number?: T | NumberBlockSelect; + subBlocks?: T | SubBlocksBlockSelect; + tabs?: T | TabsBlockSelect; }; collapsedByDefaultBlocks?: | T | { - localizedContent?: - | T - | { - text?: T; - richText?: T; - id?: T; - blockName?: T; - }; - localizedNumber?: - | T - | { - number?: T; - id?: T; - blockName?: T; - }; - localizedSubBlocks?: - | T - | { - subBlocks?: - | T - | { - text?: - | T - | { - text?: T; - id?: T; - blockName?: T; - }; - number?: - | T - | { - number?: T; - id?: T; - blockName?: T; - }; - }; - id?: T; - blockName?: T; - }; - localizedTabs?: - | T - | { - textInCollapsible?: T; - textInRow?: T; - id?: T; - blockName?: T; - }; + localizedContent?: T | LocalizedContentBlockSelect; + localizedNumber?: T | LocalizedNumberBlockSelect; + localizedSubBlocks?: T | LocalizedSubBlocksBlockSelect; + localizedTabs?: T | LocalizedTabsBlockSelect; }; disableSort?: | T | { - localizedContent?: - | T - | { - text?: T; - richText?: T; - id?: T; - blockName?: T; - }; - localizedNumber?: - | T - | { - number?: T; - id?: T; - blockName?: T; - }; - localizedSubBlocks?: - | T - | { - subBlocks?: - | T - | { - text?: - | T - | { - text?: T; - id?: T; - blockName?: T; - }; - number?: - | T - | { - number?: T; - id?: T; - blockName?: T; - }; - }; - id?: T; - blockName?: T; - }; - localizedTabs?: - | T - | { - textInCollapsible?: T; - textInRow?: T; - id?: T; - blockName?: T; - }; + localizedContent?: T | LocalizedContentBlockSelect; + localizedNumber?: T | LocalizedNumberBlockSelect; + localizedSubBlocks?: T | LocalizedSubBlocksBlockSelect; + localizedTabs?: T | LocalizedTabsBlockSelect; }; localizedBlocks?: | T | { - localizedContent?: - | T - | { - text?: T; - richText?: T; - id?: T; - blockName?: T; - }; - localizedNumber?: - | T - | { - number?: T; - id?: T; - blockName?: T; - }; - localizedSubBlocks?: - | T - | { - subBlocks?: - | T - | { - text?: - | T - | { - text?: T; - id?: T; - blockName?: T; - }; - number?: - | T - | { - number?: T; - id?: T; - blockName?: T; - }; - }; - id?: T; - blockName?: T; - }; - localizedTabs?: - | T - | { - textInCollapsible?: T; - textInRow?: T; - id?: T; - blockName?: T; - }; + localizedContent?: T | LocalizedContentBlockSelect; + localizedNumber?: T | LocalizedNumberBlockSelect; + localizedSubBlocks?: T | LocalizedSubBlocksBlockSelect; + localizedTabs?: T | LocalizedTabsBlockSelect; }; i18nBlocks?: | T @@ -2545,6 +2330,116 @@ export interface BlockFieldsSelect { updatedAt?: T; createdAt?: T; } +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "ContentBlock_select". + */ +export interface ContentBlockSelect { + text?: T; + richText?: T; + id?: T; + blockName?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "NumberBlock_select". + */ +export interface NumberBlockSelect { + number?: T; + id?: T; + blockName?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "SubBlocksBlock_select". + */ +export interface SubBlocksBlockSelect { + subBlocks?: + | T + | { + text?: + | T + | { + text?: T; + id?: T; + blockName?: T; + }; + number?: + | T + | { + number?: T; + id?: T; + blockName?: T; + }; + }; + id?: T; + blockName?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "TabsBlock_select". + */ +export interface TabsBlockSelect { + textInCollapsible?: T; + textInRow?: T; + id?: T; + blockName?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "localizedContentBlock_select". + */ +export interface LocalizedContentBlockSelect { + text?: T; + richText?: T; + id?: T; + blockName?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "localizedNumberBlock_select". + */ +export interface LocalizedNumberBlockSelect { + number?: T; + id?: T; + blockName?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "localizedSubBlocksBlock_select". + */ +export interface LocalizedSubBlocksBlockSelect { + subBlocks?: + | T + | { + text?: + | T + | { + text?: T; + id?: T; + blockName?: T; + }; + number?: + | T + | { + number?: T; + id?: T; + blockName?: T; + }; + }; + id?: T; + blockName?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "localizedTabsBlock_select". + */ +export interface LocalizedTabsBlockSelect { + textInCollapsible?: T; + textInRow?: T; + id?: T; + blockName?: T; +} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "checkbox-fields_select". @@ -3127,53 +3022,10 @@ export interface TabsFieldsSelect { blocks?: | T | { - content?: - | T - | { - text?: T; - richText?: T; - id?: T; - blockName?: T; - }; - number?: - | T - | { - number?: T; - id?: T; - blockName?: T; - }; - subBlocks?: - | T - | { - subBlocks?: - | T - | { - text?: - | T - | { - text?: T; - id?: T; - blockName?: T; - }; - number?: - | T - | { - number?: T; - id?: T; - blockName?: T; - }; - }; - id?: T; - blockName?: T; - }; - tabs?: - | T - | { - textInCollapsible?: T; - textInRow?: T; - id?: T; - blockName?: T; - }; + content?: T | ContentBlockSelect; + number?: T | NumberBlockSelect; + subBlocks?: T | SubBlocksBlockSelect; + tabs?: T | TabsBlockSelect; }; group?: | T @@ -3183,24 +3035,7 @@ export interface TabsFieldsSelect { textInRow?: T; numberInRow?: T; json?: T; - tab?: - | T - | { - array?: - | T - | { - text?: T; - id?: T; - }; - text?: T; - defaultValue?: T; - arrayInRow?: - | T - | { - textInArrayInRow?: T; - id?: T; - }; - }; + tab?: T | TabWithNameSelect; namedTabWithDefaultValue?: | T | { @@ -3250,6 +3085,26 @@ export interface TabsFieldsSelect { updatedAt?: T; createdAt?: T; } +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "TabWithName_select". + */ +export interface TabWithNameSelect { + array?: + | T + | { + text?: T; + id?: T; + }; + text?: T; + defaultValue?: T; + arrayInRow?: + | T + | { + textInArrayInRow?: T; + id?: T; + }; +} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "text-fields_select". diff --git a/tsconfig.base.json b/tsconfig.base.json index 4a6b98fad29..0ea64097ce5 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -12,21 +12,13 @@ "esModuleInterop": true, "forceConsistentCasingInFileNames": true, "jsx": "preserve", - "lib": [ - "DOM", - "DOM.Iterable", - "ES2022" - ], + "lib": ["DOM", "DOM.Iterable", "ES2022"], "outDir": "${configDir}/dist", "resolveJsonModule": true, "skipLibCheck": true, "emitDeclarationOnly": true, "sourceMap": true, - "types": [ - "jest", - "node", - "@types/jest" - ], + "types": ["jest", "node", "@types/jest"], "incremental": true, "isolatedModules": true, "strict": false, @@ -36,72 +28,33 @@ } ], "paths": { - "@payload-config": [ - "./test/_community/config.ts" - ], - "@payloadcms/live-preview": [ - "./packages/live-preview/src" - ], - "@payloadcms/live-preview-react": [ - "./packages/live-preview-react/src/index.ts" - ], - "@payloadcms/live-preview-vue": [ - "./packages/live-preview-vue/src/index.ts" - ], - "@payloadcms/ui": [ - "./packages/ui/src/exports/client/index.ts" - ], - "@payloadcms/ui/shared": [ - "./packages/ui/src/exports/shared/index.ts" - ], - "@payloadcms/ui/scss": [ - "./packages/ui/src/scss.scss" - ], - "@payloadcms/ui/scss/app.scss": [ - "./packages/ui/src/scss/app.scss" - ], - "@payloadcms/next/*": [ - "./packages/next/src/exports/*.ts" - ], + "@payload-config": ["./test/database/config.ts"], + "@payloadcms/live-preview": ["./packages/live-preview/src"], + "@payloadcms/live-preview-react": ["./packages/live-preview-react/src/index.ts"], + "@payloadcms/live-preview-vue": ["./packages/live-preview-vue/src/index.ts"], + "@payloadcms/ui": ["./packages/ui/src/exports/client/index.ts"], + "@payloadcms/ui/shared": ["./packages/ui/src/exports/shared/index.ts"], + "@payloadcms/ui/scss": ["./packages/ui/src/scss.scss"], + "@payloadcms/ui/scss/app.scss": ["./packages/ui/src/scss/app.scss"], + "@payloadcms/next/*": ["./packages/next/src/exports/*.ts"], "@payloadcms/richtext-lexical/client": [ "./packages/richtext-lexical/src/exports/client/index.ts" ], - "@payloadcms/richtext-lexical/rsc": [ - "./packages/richtext-lexical/src/exports/server/rsc.ts" - ], - "@payloadcms/richtext-slate/rsc": [ - "./packages/richtext-slate/src/exports/server/rsc.ts" - ], + "@payloadcms/richtext-lexical/rsc": ["./packages/richtext-lexical/src/exports/server/rsc.ts"], + "@payloadcms/richtext-slate/rsc": ["./packages/richtext-slate/src/exports/server/rsc.ts"], "@payloadcms/richtext-slate/client": [ "./packages/richtext-slate/src/exports/client/index.ts" ], - "@payloadcms/plugin-seo/client": [ - "./packages/plugin-seo/src/exports/client.ts" - ], - "@payloadcms/plugin-sentry/client": [ - "./packages/plugin-sentry/src/exports/client.ts" - ], - "@payloadcms/plugin-stripe/client": [ - "./packages/plugin-stripe/src/exports/client.ts" - ], - "@payloadcms/plugin-search/client": [ - "./packages/plugin-search/src/exports/client.ts" - ], + "@payloadcms/plugin-seo/client": ["./packages/plugin-seo/src/exports/client.ts"], + "@payloadcms/plugin-sentry/client": ["./packages/plugin-sentry/src/exports/client.ts"], + "@payloadcms/plugin-stripe/client": ["./packages/plugin-stripe/src/exports/client.ts"], + "@payloadcms/plugin-search/client": ["./packages/plugin-search/src/exports/client.ts"], "@payloadcms/plugin-form-builder/client": [ "./packages/plugin-form-builder/src/exports/client.ts" ], - "@payloadcms/next": [ - "./packages/next/src/exports/*" - ] + "@payloadcms/next": ["./packages/next/src/exports/*"] } }, - "include": [ - "${configDir}/src" - ], - "exclude": [ - "${configDir}/dist", - "${configDir}/build", - "${configDir}/temp", - "**/*.spec.ts" - ] + "include": ["${configDir}/src"], + "exclude": ["${configDir}/dist", "${configDir}/build", "${configDir}/temp", "**/*.spec.ts"] }