fix: duplicate handles locales with unique (#5600)

* fix: duplicate errors with localized and unique fields

* docs: beforeDuplicate hooks
This commit is contained in:
Dan Ribbens
2024-04-02 15:30:49 -04:00
committed by GitHub
parent be58f67115
commit 825ca94080
15 changed files with 262 additions and 193 deletions

View File

@@ -221,9 +221,13 @@ user-friendly.
### beforeDuplicate
The `beforeDuplicate` field hook is only called when duplicating a document. It may be used when documents having the
exact same properties may cause issue. This gives you a way to avoid duplicate names on `unique`, `required` fields or
to unset values by returning `null`. This is called immediately after `defaultValue` and before validation occurs.
The `beforeDuplicate` field hook is called on each locale (when using localization), when duplicating a document. It may be used when documents having the
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.
This hook gets called after `beforeChange` hooks are called and before the document is saved to the database.
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.
Here is an example of a number field with a hook that increments the number to avoid unique constraint errors when duplicating a document:
```ts
import { Field } from 'payload/types'

View File

@@ -36,6 +36,7 @@ export const duplicateOperation = async <TSlug extends keyof GeneratedTypes['col
incomingArgs: Arguments,
): Promise<GeneratedTypes['collections'][TSlug]> => {
let args = incomingArgs
const operation = 'create'
try {
const shouldCommit = await initTransaction(args.req)
@@ -52,7 +53,7 @@ export const duplicateOperation = async <TSlug extends keyof GeneratedTypes['col
args,
collection: args.collection.config,
context: args.req.context,
operation: 'update',
operation,
req: args.req,
})) || args
}, Promise.resolve())
@@ -63,11 +64,7 @@ export const duplicateOperation = async <TSlug extends keyof GeneratedTypes['col
depth,
draft: draftArg = true,
overrideAccess,
req: {
fallbackLocale,
payload: { config },
payload,
},
req: { fallbackLocale, locale: localeArg, payload },
req,
showHiddenFields,
} = args
@@ -107,38 +104,17 @@ export const duplicateOperation = async <TSlug extends keyof GeneratedTypes['col
if (!docWithLocales && !hasWherePolicy) throw new NotFound(req.t)
if (!docWithLocales && hasWherePolicy) throw new Forbidden(req.t)
// remove the createdAt timestamp and rely on the db to default it
// remove the createdAt timestamp and id to rely on the db to set the default it
delete docWithLocales.createdAt
delete docWithLocales.id
// for version enabled collections, override the current status with draft, unless draft is explicitly set to false
if (shouldSaveDraft) {
docWithLocales._status = 'draft'
}
// /////////////////////////////////////
// Iterate locales of document and call the db create or update functions
// /////////////////////////////////////
let locales = [undefined]
if (config.localization) {
// make sure the current request locale is the first locale to be handled to skip validation for other locales
locales = config.localization.locales.reduce(
(acc, { code }) => {
if (req.locale === code) return acc
acc.push(code)
return acc
},
[req.locale],
)
}
let result
await locales.reduce(async (previousPromise, locale: string | undefined, i) => {
await previousPromise
const operation = i === 0 ? 'create' : 'update'
const originalDoc = await afterRead({
collection: collectionConfig,
context: req.context,
@@ -146,31 +122,29 @@ export const duplicateOperation = async <TSlug extends keyof GeneratedTypes['col
doc: docWithLocales,
fallbackLocale: null,
global: null,
locale,
locale: req.locale,
overrideAccess: true,
req,
showHiddenFields: true,
})
let data = { ...originalDoc }
// /////////////////////////////////////
// Create Access
// /////////////////////////////////////
if (operation === 'create' && !overrideAccess) {
await executeAccess({ data, req }, collectionConfig.access.create)
if (!overrideAccess) {
await executeAccess({ data: originalDoc, req }, collectionConfig.access.create)
}
// /////////////////////////////////////
// beforeValidate - Fields
// /////////////////////////////////////
data = await beforeValidate<DeepPartial<GeneratedTypes['collections'][TSlug]>>({
let data = await beforeValidate<DeepPartial<GeneratedTypes['collections'][TSlug]>>({
id,
collection: collectionConfig,
context: req.context,
data,
data: originalDoc,
doc: originalDoc,
duplicate: true,
global: null,
@@ -226,12 +200,15 @@ export const duplicateOperation = async <TSlug extends keyof GeneratedTypes['col
data,
doc: originalDoc,
docWithLocales,
duplicate: true,
global: null,
operation,
req,
skipValidation: shouldSaveDraft || operation === 'update',
skipValidation: shouldSaveDraft,
})
}, Promise.resolve())
// set req.locale back to the original locale
req.locale = localeArg
// /////////////////////////////////////
// Create / Update
@@ -239,7 +216,7 @@ export const duplicateOperation = async <TSlug extends keyof GeneratedTypes['col
const versionDoc = await payload.db.create({
collection: collectionConfig.slug,
data: docWithLocales,
data: result,
req,
})
@@ -272,7 +249,7 @@ export const duplicateOperation = async <TSlug extends keyof GeneratedTypes['col
doc: versionDoc,
fallbackLocale,
global: null,
locale: req.locale,
locale: localeArg,
overrideAccess,
req,
showHiddenFields,
@@ -304,7 +281,7 @@ export const duplicateOperation = async <TSlug extends keyof GeneratedTypes['col
data: versionDoc,
doc: result,
global: null,
operation: 'create',
operation,
previousDoc: {},
req,
})
@@ -321,7 +298,7 @@ export const duplicateOperation = async <TSlug extends keyof GeneratedTypes['col
collection: collectionConfig,
context: req.context,
doc: result,
operation: 'create',
operation,
previousDoc: {},
req,
})) || result
@@ -334,7 +311,7 @@ export const duplicateOperation = async <TSlug extends keyof GeneratedTypes['col
result = await buildAfterOperation<GeneratedTypes['collections'][TSlug]>({
args,
collection: collectionConfig,
operation: 'create',
operation,
result,
})

View File

@@ -16,6 +16,10 @@ type Args<T> = {
req: PayloadRequest
}
/**
* This function is responsible for the following actions, in order:
* - Execute field hooks
*/
export const afterChange = async <T extends Record<string, unknown>>({
collection,
context,

View File

@@ -21,6 +21,16 @@ type Args = {
showHiddenFields: boolean
}
/**
* This function is responsible for the following actions, in order:
* - Remove hidden fields from response
* - Flatten locales into requested locale
* - Sanitize outgoing data (point field, etc.)
* - Execute field hooks
* - Execute read access control
* - Populate relationships
*/
export async function afterRead<T = any>(args: Args): Promise<T> {
const {
collection,

View File

@@ -0,0 +1,7 @@
import type { FieldHookArgs } from '../../config/types.js'
export const beforeDuplicate = async (args: FieldHookArgs) =>
await args.field.hooks.beforeDuplicate.reduce(async (priorHook, currentHook) => {
await priorHook
return await currentHook(args)
}, Promise.resolve())

View File

@@ -12,6 +12,7 @@ type Args<T> = {
data: Record<string, unknown> | T
doc: Record<string, unknown> | T
docWithLocales: Record<string, unknown>
duplicate?: boolean
global: SanitizedGlobalConfig | null
id?: number | string
operation: Operation
@@ -19,6 +20,15 @@ type Args<T> = {
skipValidation?: boolean
}
/**
* This function is responsible for the following actions, in order:
* - Run condition
* - Execute field hooks
* - Validate data
* - Transform data for storage
* - beforeDuplicate hooks (if duplicate)
* - Unflatten locales
*/
export const beforeChange = async <T extends Record<string, unknown>>({
id,
collection,
@@ -26,6 +36,7 @@ export const beforeChange = async <T extends Record<string, unknown>>({
data: incomingData,
doc,
docWithLocales,
duplicate = false,
global,
operation,
req,
@@ -42,6 +53,7 @@ export const beforeChange = async <T extends Record<string, unknown>>({
data,
doc,
docWithLocales,
duplicate,
errors,
fields: collection?.fields || global?.fields,
global,
@@ -59,7 +71,10 @@ export const beforeChange = async <T extends Record<string, unknown>>({
throw new ValidationError(errors, req.t)
}
mergeLocaleActions.forEach((action) => action())
await mergeLocaleActions.reduce(async (priorAction, action) => {
await priorAction
await action()
}, Promise.resolve())
return data
}

View File

@@ -3,9 +3,10 @@ import merge from 'deepmerge'
import type { SanitizedCollectionConfig } from '../../../collections/config/types.js'
import type { SanitizedGlobalConfig } from '../../../globals/config/types.js'
import type { Operation, PayloadRequest, RequestContext } from '../../../types/index.js'
import type { Field, TabAsField } from '../../config/types.js'
import type { Field, FieldHookArgs, TabAsField } from '../../config/types.js'
import { fieldAffectsData, tabHasName } from '../../config/types.js'
import { beforeDuplicate } from './beforeDuplicate.js'
import { getExistingRowDoc } from './getExistingRowDoc.js'
import { traverseFields } from './traverseFields.js'
@@ -15,11 +16,12 @@ type Args = {
data: Record<string, unknown>
doc: Record<string, unknown>
docWithLocales: Record<string, unknown>
duplicate: boolean
errors: { field: string; message: string }[]
field: Field | TabAsField
global: SanitizedGlobalConfig | null
id?: number | string
mergeLocaleActions: (() => void)[]
mergeLocaleActions: (() => Promise<void>)[]
operation: Operation
path: string
req: PayloadRequest
@@ -34,6 +36,7 @@ type Args = {
// - Execute field hooks
// - Validate data
// - Transform data for storage
// - beforeDuplicate hooks (if duplicate)
// - Unflatten locales
export const promise = async ({
@@ -43,6 +46,7 @@ export const promise = async ({
data,
doc,
docWithLocales,
duplicate,
errors,
field,
global,
@@ -59,10 +63,8 @@ export const promise = async ({
? Boolean(field.admin.condition(data, siblingData, { user: req.user }))
: true
let skipValidationFromHere = skipValidation || !passesCondition
const defaultLocale = req.payload.config?.localization
? req.payload.config.localization?.defaultLocale
: 'en'
const { localization } = req.payload.config
const defaultLocale = localization ? localization?.defaultLocale : 'en'
const operationLocale = req.locale || defaultLocale
if (fieldAffectsData(field)) {
@@ -131,17 +133,34 @@ export const promise = async ({
}
}
const beforeDuplicateArgs: FieldHookArgs = {
collection,
context,
data,
field,
global: undefined,
req,
siblingData,
value: siblingData[field.name],
}
// Push merge locale action if applicable
if (field.localized) {
mergeLocaleActions.push(() => {
if (req.payload.config.localization) {
const { localization } = req.payload.config
const localeData = localization.localeCodes.reduce((localizedValues, locale) => {
const fieldValue =
if (localization && field.localized) {
mergeLocaleActions.push(async () => {
const localeData = await localization.localeCodes.reduce(
async (localizedValuesPromise: Promise<Record<string, unknown>>, locale) => {
const localizedValues = await localizedValuesPromise
let fieldValue =
locale === req.locale
? siblingData[field.name]
: siblingDocWithLocales?.[field.name]?.[locale]
if (duplicate && field.hooks?.beforeDuplicate?.length) {
beforeDuplicateArgs.value = fieldValue
fieldValue = await beforeDuplicate(beforeDuplicateArgs)
}
// const result = await localizedValues
// update locale value if it's not undefined
if (typeof fieldValue !== 'undefined') {
return {
@@ -150,14 +169,19 @@ export const promise = async ({
}
}
return localizedValues
}, {})
return localizedValuesPromise
},
Promise.resolve({}),
)
// If there are locales with data, set the data
if (Object.keys(localeData).length > 0) {
siblingData[field.name] = localeData
}
}
})
} else if (duplicate && field.hooks?.beforeDuplicate?.length) {
mergeLocaleActions.push(async () => {
siblingData[field.name] = await beforeDuplicate(beforeDuplicateArgs)
})
}
}
@@ -195,6 +219,7 @@ export const promise = async ({
data,
doc,
docWithLocales,
duplicate,
errors,
fields: field.fields,
global,
@@ -225,6 +250,7 @@ export const promise = async ({
data,
doc,
docWithLocales,
duplicate,
errors,
fields: field.fields,
global,
@@ -267,6 +293,7 @@ export const promise = async ({
data,
doc,
docWithLocales,
duplicate,
errors,
fields: block.fields,
global,
@@ -298,6 +325,7 @@ export const promise = async ({
data,
doc,
docWithLocales,
duplicate,
errors,
fields: field.fields,
global,
@@ -339,6 +367,7 @@ export const promise = async ({
data,
doc,
docWithLocales,
duplicate,
errors,
fields: field.fields,
global,
@@ -363,6 +392,7 @@ export const promise = async ({
data,
doc,
docWithLocales,
duplicate,
errors,
fields: field.tabs.map((tab) => ({ ...tab, type: 'tab' })),
global,

View File

@@ -11,11 +11,12 @@ type Args = {
data: Record<string, unknown>
doc: Record<string, unknown>
docWithLocales: Record<string, unknown>
duplicate: boolean
errors: { field: string; message: string }[]
fields: (Field | TabAsField)[]
global: SanitizedGlobalConfig | null
id?: number | string
mergeLocaleActions: (() => void)[]
mergeLocaleActions: (() => Promise<void>)[]
operation: Operation
path: string
req: PayloadRequest
@@ -25,6 +26,14 @@ type Args = {
skipValidation?: boolean
}
/**
* This function is responsible for the following actions, in order:
* - Run condition
* - Execute field hooks
* - Validate data
* - Transform data for storage
* - Unflatten locales
*/
export const traverseFields = async ({
id,
collection,
@@ -32,6 +41,7 @@ export const traverseFields = async ({
data,
doc,
docWithLocales,
duplicate,
errors,
fields,
global,
@@ -55,6 +65,7 @@ export const traverseFields = async ({
data,
doc,
docWithLocales,
duplicate,
errors,
field,
global,

View File

@@ -18,13 +18,20 @@ type Args<T> = {
req: PayloadRequest
}
/**
* This function is responsible for the following actions, in order:
* - Sanitize incoming data
* - Execute field hooks
* - Execute field access control
* - Merge original document data into incoming data
* - Compute default values for undefined fields
*/
export const beforeValidate = async <T extends Record<string, unknown>>({
id,
collection,
context,
data: incomingData,
doc,
duplicate = false,
global,
operation,
overrideAccess,
@@ -38,7 +45,6 @@ export const beforeValidate = async <T extends Record<string, unknown>>({
context,
data,
doc,
duplicate,
fields: collection?.fields || global?.fields,
global,
operation,

View File

@@ -15,7 +15,6 @@ type Args<T> = {
context: RequestContext
data: T
doc: T
duplicate: boolean
field: Field | TabAsField
global: SanitizedGlobalConfig | null
id?: number | string
@@ -39,7 +38,6 @@ export const promise = async <T>({
context,
data,
doc,
duplicate,
field,
global,
operation,
@@ -261,32 +259,6 @@ export const promise = async <T>({
})
}
}
// Execute beforeDuplicate hook
if (duplicate && field.hooks?.beforeDuplicate) {
if (field.hooks?.beforeDuplicate) {
await field.hooks.beforeDuplicate.reduce(async (priorHook, currentHook) => {
await priorHook
const hookedValue = await currentHook({
collection,
context,
data,
field,
global,
operation,
originalDoc: doc,
req,
siblingData,
value: siblingData[field.name],
})
if (hookedValue !== undefined) {
siblingData[field.name] = hookedValue
}
}, Promise.resolve())
}
}
}
// Traverse subfields
@@ -304,7 +276,6 @@ export const promise = async <T>({
context,
data,
doc,
duplicate,
fields: field.fields,
global,
operation,
@@ -330,7 +301,6 @@ export const promise = async <T>({
context,
data,
doc,
duplicate,
fields: field.fields,
global,
operation,
@@ -366,7 +336,6 @@ export const promise = async <T>({
context,
data,
doc,
duplicate,
fields: block.fields,
global,
operation,
@@ -392,7 +361,6 @@ export const promise = async <T>({
context,
data,
doc,
duplicate,
fields: field.fields,
global,
operation,
@@ -425,7 +393,6 @@ export const promise = async <T>({
context,
data,
doc,
duplicate,
fields: field.fields,
global,
operation,
@@ -445,7 +412,6 @@ export const promise = async <T>({
context,
data,
doc,
duplicate,
fields: field.tabs.map((tab) => ({ ...tab, type: 'tab' })),
global,
operation,

View File

@@ -10,7 +10,6 @@ type Args<T> = {
context: RequestContext
data: T
doc: T
duplicate: boolean
fields: (Field | TabAsField)[]
global: SanitizedGlobalConfig | null
id?: number | string
@@ -27,7 +26,6 @@ export const traverseFields = async <T>({
context,
data,
doc,
duplicate,
fields,
global,
operation,
@@ -45,7 +43,6 @@ export const traverseFields = async <T>({
context,
data,
doc,
duplicate,
field,
global,
operation,

View File

@@ -1,10 +1,10 @@
// default beforeDuplicate hook for required and unique fields
import type { FieldAffectingData, FieldHook } from './config/types.js'
import { extractTranslations } from '../translations/extractTranslations.js'
const copyTranslations = extractTranslations(['general:copy'])
// default beforeDuplicate hook for required and unique fields
import type { FieldAffectingData, FieldHook } from './config/types.js'
const unique: FieldHook = ({ value }) => (typeof value === 'string' ? `${value} - Copy` : undefined)
const localizedUnique: FieldHook = ({ req, value }) =>
value ? `${value} - ${copyTranslations?.[req.locale]?.['general:copy'] ?? 'Copy'}` : undefined

View File

@@ -117,6 +117,11 @@ export class BasePayload<TGeneratedTypes extends GeneratedTypes> {
return find<T>(this, options)
}
/**
* @description Find document by ID
* @param options
* @returns document with specified ID
*/
findByID = async <T extends keyof TGeneratedTypes['collections']>(
options: FindByIDOptions<T>,
): Promise<TGeneratedTypes['collections'][T]> => {
@@ -194,12 +199,6 @@ export class BasePayload<TGeneratedTypes extends GeneratedTypes> {
logger: pino.Logger
/**
* @description Find document by ID
* @param options
* @returns document with specified ID
*/
login = async <T extends keyof TGeneratedTypes['collections']>(
options: LoginOptions<T>,
): Promise<LoginResult & { user: TGeneratedTypes['collections'][T] }> => {

View File

@@ -620,6 +620,7 @@ describe('Fields', () => {
it('should duplicate with unique fields', async () => {
const data = {
text: 'a',
// uniqueRequiredText: 'duplicate',
}
const doc = await payload.create({
collection: 'indexed-fields',

View File

@@ -1,15 +1,12 @@
import type { Payload } from 'payload'
import type { Where } from 'payload/types'
import { getPayload } from 'payload'
import type { NextRESTClient } from '../helpers/NextRESTClient.js'
import type { LocalizedPost, WithLocalizedRelationship } from './payload-types.js'
import { englishLocale } from '../globals/config.js'
import { idToString } from '../helpers/idToString.js'
import { initPayloadInt } from '../helpers/initPayloadInt.js'
import { startMemoryDB } from '../startMemoryDB.js'
import { arrayCollectionSlug } from './collections/Array/index.js'
import { nestedToArrayAndBlockCollectionSlug } from './collections/NestedToArrayAndBlock/index.js'
import configPromise from './config.js'
@@ -1013,6 +1010,51 @@ describe('Localization', () => {
expect(rowSpanish.textNotLocalized).toEqual('test')
})
})
describe('Duplicate Collection', () => {
it('should duplicate localized document', async () => {
const localizedPost = await payload.create({
collection: localizedPostsSlug,
data: {
localizedCheckbox: true,
title: englishTitle,
},
locale: defaultLocale,
})
const id = localizedPost.id.toString()
await payload.update({
id,
collection: localizedPostsSlug,
data: {
localizedCheckbox: false,
title: spanishTitle,
},
locale: spanishLocale,
})
const result = await payload.duplicate({
id,
collection: localizedPostsSlug,
locale: defaultLocale,
})
const allLocales = await payload.findByID({
id: result.id,
collection: localizedPostsSlug,
locale: 'all',
})
// check fields
expect(result.title).toStrictEqual(englishTitle)
expect(allLocales.title.es).toStrictEqual(spanishTitle)
expect(allLocales.localizedCheckbox.en).toBeTruthy()
expect(allLocales.localizedCheckbox.es).toBeFalsy()
})
})
})
async function createLocalizedPost(data: {