Skip to content

Commit 5873a3d

Browse files
jmikrutjmikrut
authored
fix: duplicating localized nested arrays (#8220)
Fixes an issue where duplicating documents in Postgres / SQLite would crash because of a foreign key constraint / unique ID issue when you have nested arrays / blocks within localized arrays / blocks. We now run `beforeDuplicate` against all locales prior to `beforeValidate` and `beforeChange` hooks. This PR also fixes a series of issues in Postgres / SQLite where you have localized groups / named tabs, and then arrays / blocks within the localized groups / named tabs.
1 parent 8fc2c43 commit 5873a3d

File tree

19 files changed

+592
-84
lines changed

19 files changed

+592
-84
lines changed

‎docs/hooks/fields.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ user-friendly.
200200
The `beforeDuplicate` field hook is called on each locale (when using localization), when duplicating a document. It may be used when documents having the
201201
exact same properties may cause issue. This gives you a way to avoid duplicate names on `unique`, `required` fields or when external systems expect non-repeating values on documents.
202202

203-
This hook gets called after `beforeChange` hooks are called and before the document is saved to the database.
203+
This hook gets called before the `beforeValidate` and `beforeChange` hooks are called.
204204

205205
By Default, unique and required text fields Payload will append "- Copy" to the original document value. The default is not added if your field has its own, you must return non-unique values from your beforeDuplicate hook to avoid errors or enable the `disableDuplicate` option on the collection.
206206
Here is an example of a number field with a hook that increments the number to avoid unique constraint errors when duplicating a document:

‎packages/db-sqlite/src/schema/traverseFields.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,8 @@ export const traverseFields = ({
166166
if (field.hasMany) {
167167
const isLocalized =
168168
Boolean(field.localized && adapter.payload.config.localization) ||
169-
withinLocalizedArrayOrBlock
169+
withinLocalizedArrayOrBlock ||
170+
forceLocalized
170171

171172
if (isLocalized) {
172173
hasLocalizedManyTextField = true
@@ -199,7 +200,8 @@ export const traverseFields = ({
199200
if (field.hasMany) {
200201
const isLocalized =
201202
Boolean(field.localized && adapter.payload.config.localization) ||
202-
withinLocalizedArrayOrBlock
203+
withinLocalizedArrayOrBlock ||
204+
forceLocalized
203205

204206
if (isLocalized) {
205207
hasLocalizedManyNumberField = true
@@ -279,7 +281,8 @@ export const traverseFields = ({
279281

280282
const isLocalized =
281283
Boolean(field.localized && adapter.payload.config.localization) ||
282-
withinLocalizedArrayOrBlock
284+
withinLocalizedArrayOrBlock ||
285+
forceLocalized
283286

284287
if (isLocalized) {
285288
baseColumns.locale = text('locale', { enum: locales }).notNull()
@@ -365,7 +368,8 @@ export const traverseFields = ({
365368

366369
const isLocalized =
367370
Boolean(field.localized && adapter.payload.config.localization) ||
368-
withinLocalizedArrayOrBlock
371+
withinLocalizedArrayOrBlock ||
372+
forceLocalized
369373

370374
if (isLocalized) {
371375
baseColumns._locale = text('_locale', { enum: locales }).notNull()
@@ -503,7 +507,8 @@ export const traverseFields = ({
503507

504508
const isLocalized =
505509
Boolean(field.localized && adapter.payload.config.localization) ||
506-
withinLocalizedArrayOrBlock
510+
withinLocalizedArrayOrBlock ||
511+
forceLocalized
507512

508513
if (isLocalized) {
509514
baseColumns._locale = text('_locale', { enum: locales }).notNull()

‎packages/drizzle/src/postgres/schema/traverseFields.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,8 @@ export const traverseFields = ({
172172
if (field.hasMany) {
173173
const isLocalized =
174174
Boolean(field.localized && adapter.payload.config.localization) ||
175-
withinLocalizedArrayOrBlock
175+
withinLocalizedArrayOrBlock ||
176+
forceLocalized
176177

177178
if (isLocalized) {
178179
hasLocalizedManyTextField = true
@@ -205,7 +206,8 @@ export const traverseFields = ({
205206
if (field.hasMany) {
206207
const isLocalized =
207208
Boolean(field.localized && adapter.payload.config.localization) ||
208-
withinLocalizedArrayOrBlock
209+
withinLocalizedArrayOrBlock ||
210+
forceLocalized
209211

210212
if (isLocalized) {
211213
hasLocalizedManyNumberField = true
@@ -300,7 +302,8 @@ export const traverseFields = ({
300302

301303
const isLocalized =
302304
Boolean(field.localized && adapter.payload.config.localization) ||
303-
withinLocalizedArrayOrBlock
305+
withinLocalizedArrayOrBlock ||
306+
forceLocalized
304307

305308
if (isLocalized) {
306309
baseColumns.locale = adapter.enums.enum__locales('locale').notNull()
@@ -382,7 +385,8 @@ export const traverseFields = ({
382385

383386
const isLocalized =
384387
Boolean(field.localized && adapter.payload.config.localization) ||
385-
withinLocalizedArrayOrBlock
388+
withinLocalizedArrayOrBlock ||
389+
forceLocalized
386390

387391
if (isLocalized) {
388392
baseColumns._locale = adapter.enums.enum__locales('_locale').notNull()
@@ -516,7 +520,8 @@ export const traverseFields = ({
516520

517521
const isLocalized =
518522
Boolean(field.localized && adapter.payload.config.localization) ||
519-
withinLocalizedArrayOrBlock
523+
withinLocalizedArrayOrBlock ||
524+
forceLocalized
520525

521526
if (isLocalized) {
522527
baseColumns._locale = adapter.enums.enum__locales('_locale').notNull()

‎packages/drizzle/src/transform/read/traverseFields.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,7 @@ export const traverseFields = <T extends Record<string, unknown>>({
489489
valuesToTransform.push({
490490
ref: localizedFieldData,
491491
table: {
492+
...table,
492493
...localeRow,
493494
},
494495
})
@@ -526,7 +527,7 @@ export const traverseFields = <T extends Record<string, unknown>>({
526527
relationships,
527528
table,
528529
texts,
529-
withinArrayOrBlockLocale,
530+
withinArrayOrBlockLocale: locale || withinArrayOrBlockLocale,
530531
})
531532

532533
if ('_order' in ref) {

‎packages/payload/src/collections/operations/duplicate.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { APIError, Forbidden, NotFound } from '../../errors/index.js'
1414
import { afterChange } from '../../fields/hooks/afterChange/index.js'
1515
import { afterRead } from '../../fields/hooks/afterRead/index.js'
1616
import { beforeChange } from '../../fields/hooks/beforeChange/index.js'
17+
import { beforeDuplicate } from '../../fields/hooks/beforeDuplicate/index.js'
1718
import { beforeValidate } from '../../fields/hooks/beforeValidate/index.js'
1819
import { commitTransaction } from '../../utilities/commitTransaction.js'
1920
import { initTransaction } from '../../utilities/initTransaction.js'
@@ -93,7 +94,7 @@ export const duplicateOperation = async <TSlug extends CollectionSlug>(
9394
where: combineQueries({ id: { equals: id } }, accessResults),
9495
}
9596

96-
const docWithLocales = await getLatestCollectionVersion({
97+
let docWithLocales = await getLatestCollectionVersion({
9798
id,
9899
config: collectionConfig,
99100
payload,
@@ -112,6 +113,15 @@ export const duplicateOperation = async <TSlug extends CollectionSlug>(
112113
delete docWithLocales.createdAt
113114
delete docWithLocales.id
114115

116+
docWithLocales = await beforeDuplicate({
117+
id,
118+
collection: collectionConfig,
119+
context: req.context,
120+
doc: docWithLocales,
121+
overrideAccess,
122+
req,
123+
})
124+
115125
// for version enabled collections, override the current status with draft, unless draft is explicitly set to false
116126
if (shouldSaveDraft) {
117127
docWithLocales._status = 'draft'
@@ -205,7 +215,6 @@ export const duplicateOperation = async <TSlug extends CollectionSlug>(
205215
data,
206216
doc: originalDoc,
207217
docWithLocales,
208-
duplicate: true,
209218
global: null,
210219
operation,
211220
req,

‎packages/payload/src/fields/baseFields/baseBeforeDuplicateArrays.ts

Lines changed: 0 additions & 18 deletions
This file was deleted.

‎packages/payload/src/fields/baseFields/baseIDField.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ export const baseIDField: TextField = {
2525
return value
2626
},
2727
],
28+
beforeDuplicate: [
29+
() => {
30+
return new ObjectId().toHexString()
31+
},
32+
],
2833
},
2934
label: 'ID',
3035
}

‎packages/payload/src/fields/config/sanitize.ts

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import {
1212
} from '../../errors/index.js'
1313
import { MissingEditorProp } from '../../errors/MissingEditorProp.js'
1414
import { formatLabels, toWords } from '../../utilities/formatLabels.js'
15-
import { baseBeforeDuplicateArrays } from '../baseFields/baseBeforeDuplicateArrays.js'
1615
import { baseBlockFields } from '../baseFields/baseBlockFields.js'
1716
import { baseIDField } from '../baseFields/baseIDField.js'
1817
import { setDefaultBeforeDuplicate } from '../setDefaultBeforeDuplicate.js'
@@ -131,15 +130,6 @@ export const sanitizeFields = async ({
131130

132131
if (field.type === 'array' && field.fields) {
133132
field.fields.push(baseIDField)
134-
if (field.localized) {
135-
if (!field.hooks) {
136-
field.hooks = {}
137-
}
138-
if (!field.hooks.beforeDuplicate) {
139-
field.hooks.beforeDuplicate = []
140-
}
141-
field.hooks.beforeDuplicate.push(baseBeforeDuplicateArrays)
142-
}
143133
}
144134

145135
if ((field.type === 'blocks' || field.type === 'array') && field.label) {
@@ -220,15 +210,6 @@ export const sanitizeFields = async ({
220210
}
221211

222212
if (field.type === 'blocks' && field.blocks) {
223-
if (field.localized) {
224-
if (!field.hooks) {
225-
field.hooks = {}
226-
}
227-
if (!field.hooks.beforeDuplicate) {
228-
field.hooks.beforeDuplicate = []
229-
}
230-
field.hooks.beforeDuplicate.push(baseBeforeDuplicateArrays)
231-
}
232213
for (const block of field.blocks) {
233214
if (block._sanitized === true) {
234215
continue

‎packages/payload/src/fields/hooks/beforeChange/index.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ type Args<T extends JsonObject> = {
1212
data: T
1313
doc: T
1414
docWithLocales: JsonObject
15-
duplicate?: boolean
1615
global: null | SanitizedGlobalConfig
1716
id?: number | string
1817
operation: Operation
@@ -26,7 +25,6 @@ type Args<T extends JsonObject> = {
2625
* - Execute field hooks
2726
* - Validate data
2827
* - Transform data for storage
29-
* - beforeDuplicate hooks (if duplicate)
3028
* - Unflatten locales. The input `data` is the normal document for one locale. The output result will become the document with locales.
3129
*/
3230
export const beforeChange = async <T extends JsonObject>({
@@ -36,7 +34,6 @@ export const beforeChange = async <T extends JsonObject>({
3634
data: incomingData,
3735
doc,
3836
docWithLocales,
39-
duplicate = false,
4037
global,
4138
operation,
4239
req,
@@ -53,7 +50,6 @@ export const beforeChange = async <T extends JsonObject>({
5350
data,
5451
doc,
5552
docWithLocales,
56-
duplicate,
5753
errors,
5854
fields: collection?.fields || global?.fields,
5955
global,

‎packages/payload/src/fields/hooks/beforeChange/promise.ts

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { MissingEditorProp } from '../../../errors/index.js'
88
import { deepMergeWithSourceArrays } from '../../../utilities/deepMerge.js'
99
import { fieldAffectsData, tabHasName } from '../../config/types.js'
1010
import { getFieldPaths } from '../../getFieldPaths.js'
11-
import { beforeDuplicate } from './beforeDuplicate.js'
1211
import { getExistingRowDoc } from './getExistingRowDoc.js'
1312
import { traverseFields } from './traverseFields.js'
1413

@@ -18,7 +17,6 @@ type Args = {
1817
data: JsonObject
1918
doc: JsonObject
2019
docWithLocales: JsonObject
21-
duplicate: boolean
2220
errors: { field: string; message: string }[]
2321
field: Field | TabAsField
2422
global: null | SanitizedGlobalConfig
@@ -55,7 +53,6 @@ export const promise = async ({
5553
data,
5654
doc,
5755
docWithLocales,
58-
duplicate,
5956
errors,
6057
field,
6158
global,
@@ -176,16 +173,11 @@ export const promise = async ({
176173
const localeData = await localization.localeCodes.reduce(
177174
async (localizedValuesPromise: Promise<JsonObject>, locale) => {
178175
const localizedValues = await localizedValuesPromise
179-
let fieldValue =
176+
const fieldValue =
180177
locale === req.locale
181178
? siblingData[field.name]
182179
: siblingDocWithLocales?.[field.name]?.[locale]
183180

184-
if (duplicate && field.hooks?.beforeDuplicate?.length) {
185-
beforeDuplicateArgs.value = fieldValue
186-
fieldValue = await beforeDuplicate(beforeDuplicateArgs)
187-
}
188-
189181
// const result = await localizedValues
190182
// update locale value if it's not undefined
191183
if (typeof fieldValue !== 'undefined') {
@@ -205,10 +197,6 @@ export const promise = async ({
205197
siblingData[field.name] = localeData
206198
}
207199
})
208-
} else if (duplicate && field.hooks?.beforeDuplicate?.length) {
209-
mergeLocaleActions.push(async () => {
210-
siblingData[field.name] = await beforeDuplicate(beforeDuplicateArgs)
211-
})
212200
}
213201
}
214202

@@ -250,7 +238,6 @@ export const promise = async ({
250238
data,
251239
doc,
252240
docWithLocales,
253-
duplicate,
254241
errors,
255242
fields: field.fields,
256243
global,
@@ -282,7 +269,6 @@ export const promise = async ({
282269
data,
283270
doc,
284271
docWithLocales,
285-
duplicate,
286272
errors,
287273
fields: field.fields,
288274
global,
@@ -332,7 +318,6 @@ export const promise = async ({
332318
data,
333319
doc,
334320
docWithLocales,
335-
duplicate,
336321
errors,
337322
fields: block.fields,
338323
global,
@@ -365,7 +350,6 @@ export const promise = async ({
365350
data,
366351
doc,
367352
docWithLocales,
368-
duplicate,
369353
errors,
370354
fields: field.fields,
371355
global,
@@ -411,7 +395,6 @@ export const promise = async ({
411395
data,
412396
doc,
413397
docWithLocales,
414-
duplicate,
415398
errors,
416399
fields: field.fields,
417400
global,
@@ -437,7 +420,6 @@ export const promise = async ({
437420
data,
438421
doc,
439422
docWithLocales,
440-
duplicate,
441423
errors,
442424
fields: field.tabs.map((tab) => ({ ...tab, type: 'tab' })),
443425
global,
@@ -474,7 +456,6 @@ export const promise = async ({
474456
context,
475457
data,
476458
docWithLocales,
477-
duplicate,
478459
errors,
479460
field,
480461
global,

0 commit comments

Comments
 (0)