Skip to content

Commit

Permalink
fix: ensures generated IDs persist on create (#10089)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jacobsfletch authored Dec 20, 2024
1 parent 4e95353 commit 957867f
Show file tree
Hide file tree
Showing 7 changed files with 291 additions and 402 deletions.
19 changes: 2 additions & 17 deletions packages/payload/src/fields/baseFields/baseIDField.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
}
25 changes: 25 additions & 0 deletions test/database/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down
59 changes: 58 additions & 1 deletion test/database/int.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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', () => {
Expand Down
31 changes: 31 additions & 0 deletions test/database/payload-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -428,6 +442,23 @@ export interface PostsSelect<T extends boolean = true> {
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;
}
Expand Down
17 changes: 0 additions & 17 deletions test/fields/int.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Loading

0 comments on commit 957867f

Please sign in to comment.