tabshift/payloadcms
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

fix(ui): preserve localized blocks and arrays when using CopyToLocale (#13216)

Browse Source 
## Problem:
In PR #11887, a bug fix for `copyToLocale` was introduced to address
issues with copying content between locales in Postgres. However, an
incorrect algorithm was used, which removed all "id" properties from
documents being copied. This led to bug #12536, where `copyToLocale`
would mistakenly delete the document in the source language, affecting
not only Postgres but any database.

## Cause and Solution:

When copying documents with localized arrays or blocks, Postgres throws
errors if there are two blocks with the same ID. This is why PR #11887
removed all IDs from the document to avoid conflicts. However, this
removal was too broad and caused issues in cases where it was
unnecessary.


The correct solution should remove the IDs only in nested fields whose
ancestors are localized. The reasoning is as follows:
- When an array/block is **not localized** (`localized: false`), if it
contains localized fields, these fields share the same ID across
different locales.
- When an array/block **is localized** (`localized: true`), its
descendant fields cannot share the same ID across different locales if
Postgres is being used. This wouldn't be an issue if the table
containing localized blocks had a composite primary key of `locale +
id`. However, since the primary key is just `id`, we need to assign a
new ID for these fields.

This PR properly removes IDs **only for nested fields** whose ancestors
are localized.

Fixes #12536

## Example:
### Before Fix:
```js
// Original document (en)
array: [{
  id: "123",
  text: { en: "English text" }
}]

// After copying to 'es' locale, a new ID was created instead of updating the existing item
array: [{
  id: "456",  // 🐛 New ID created!
  text: { es: "Spanish text" } // 🐛 'en' locale is missing
}]
```
### After fix:
```js
// After fix
array: [{
  id: "123",  // ✅ Same ID maintained
  text: {
    en: "English text",
    es: "Spanish text"  // ✅ Properly merged with existing item
  }
}]
```


## Additional fixes:

### TraverseFields

In the process of designing an appropriate solution, I detected a couple
of bugs in traverseFields that are also addressed in this PR.

### Fixed MongoDB Empty Array Handling

During testing, I discovered that MongoDB and PostgreSQL behave
differently when querying documents that don't exist in a specific
locale:
- PostgreSQL: Returns the document with data from the fallback locale
- MongoDB: Returns the document with empty arrays for localized fields

This difference caused `copyToLocale` to fail in MongoDB because the
merge algorithm only checked for `null` or `undefined` values, but not
empty arrays. When MongoDB returned `content: []` for a non-existent
locale, the algorithm would attempt to iterate over the empty array
instead of using the source locale's data.

### Move test e2e to int

The test introduced in #11887 didn't catch the bug because our e2e suite
doesn't run on Postgres. I migrated the test to an integration test that
does run on Postgres and MongoDB.
This commit is contained in:
German Jablonski
2025-07-24 20:37:13 +01:00
committed by GitHub
parent fa7d209cc9
commit 4a712b3483
6 changed files with 316 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
packages/payload/src/utilities/traverseFields.ts
View File

@@ -28,7 +28,7 @@ const traverseArrayOrBlocksField = ({
fillEmpty: boolean
leavesFirst: boolean
parentIsLocalized: boolean
parentPath?: string
parentPath: string
parentRef?: unknown
}) => {
if (fillEmpty) {
@@ -403,7 +403,18 @@ export const traverseFields = ({
currentRef &&
typeof currentRef === 'object'
) {
if (fieldShouldBeLocalized({ field, parentIsLocalized: parentIsLocalized! })) {
// TODO: `?? field.localized ?? false` shouldn't be necessary, but right now it
// is so that all fields are correctly traversed in copyToLocale and
// therefore pass the localization integration tests.
// I tried replacing the `!parentIsLocalized` condition with `parentIsLocalized === false`
// in `fieldShouldBeLocalized`, but several tests failed. We must be calling it with incorrect
// parameters somewhere.
if (
fieldShouldBeLocalized({
field,
parentIsLocalized: parentIsLocalized ?? field.localized ?? false,
})
) {
if (Array.isArray(currentRef)) {
return
}
@@ -423,6 +434,7 @@ export const traverseFields = ({
fillEmpty,
leavesFirst,
parentIsLocalized: true,
parentPath,
parentRef: currentParentRef,
})
}
@@ -436,6 +448,7 @@ export const traverseFields = ({
fillEmpty,
leavesFirst,
parentIsLocalized: parentIsLocalized!,
parentPath,
parentRef: currentParentRef,
})
}

64
packages/ui/src/utilities/copyDataFromLocale.ts
View File

@@ -7,6 +7,7 @@ import {
formatErrors,
type PayloadRequest,
type ServerFunction,
traverseFields,
} from 'payload'
import { fieldAffectsData, fieldShouldBeLocalized, tabHasName } from 'payload/shared'
@@ -70,8 +71,9 @@ function iterateFields(
// if the field has no value, take the source value
if (
field.name in toLocaleData &&
// only replace if the target value is null or undefined
[null, undefined].includes(toLocaleData[field.name]) &&
// only replace if the target value is null, undefined, or empty array
([null, undefined].includes(toLocaleData[field.name]) ||
(Array.isArray(toLocaleData[field.name]) && toLocaleData[field.name].length === 0)) &&
field.name in fromLocaleData
) {
toLocaleData[field.name] = fromLocaleData[field.name]
@@ -190,14 +192,22 @@ function mergeData(
return toLocaleData
}
function removeIds(data: Data): Data {
if (Array.isArray(data)) {
return data.map(removeIds)
}
if (typeof data === 'object' && data !== null) {
const { id: _id, ...rest } = data
return Object.fromEntries(Object.entries(rest).map(([key, value]) => [key, removeIds(value)]))
}
/**
* We don't have to recursively remove all ids,
* just the ones from the fields inside a localized array or block.
*/
function removeIdIfParentIsLocalized(data: Data, fields: Field[]): Data {
traverseFields({
callback: ({ parentIsLocalized, ref }) => {
if (parentIsLocalized) {
delete (ref as { id: unknown }).id
}
},
fields,
fillEmpty: false,
ref: data,
})
return data
}
@@ -306,21 +316,23 @@ export const copyDataFromLocale = async (args: CopyDataFromLocaleArgs) => {
throw new Error(`Error fetching data from locale "${toLocale}"`)
}
const fromLocaleDataWithoutID = removeIds(fromLocaleData.value)
const toLocaleDataWithoutID = removeIds(toLocaleData.value)
const fields = globalSlug
? globals[globalSlug].config.fields
: collections[collectionSlug].config.fields
const fromLocaleDataWithoutID = fromLocaleData.value
const toLocaleDataWithoutID = toLocaleData.value
const dataWithID = overrideData
? fromLocaleDataWithoutID
: mergeData(fromLocaleDataWithoutID, toLocaleDataWithoutID, fields, req, false)
const data = removeIdIfParentIsLocalized(dataWithID, fields)
return globalSlug
? await payload.updateGlobal({
slug: globalSlug,
data: overrideData
? fromLocaleDataWithoutID
: mergeData(
fromLocaleDataWithoutID,
toLocaleDataWithoutID,
globals[globalSlug].config.fields,
req,
false,
),
data,
locale: toLocale,
overrideAccess: false,
req,
@@ -329,15 +341,7 @@ export const copyDataFromLocale = async (args: CopyDataFromLocaleArgs) => {
: await payload.update({
id: docID,
collection: collectionSlug,
data: overrideData
? fromLocaleDataWithoutID
: mergeData(
fromLocaleDataWithoutID,
toLocaleDataWithoutID,
collections[collectionSlug].config.fields,
req,
false,
),
data,
locale: toLocale,
overrideAccess: false,
req,

5
test/localization/collections/NestedToArrayAndBlock/index.ts
View File

@@ -12,6 +12,11 @@ export const NestedToArrayAndBlock: CollectionConfig = {
{
slug: 'block',
fields: [
{
name: 'someText',
type: 'text',
localized: true,
},
{
name: 'array',
type: 'array',

26
test/localization/e2e.spec.ts
View File

@@ -431,32 +431,6 @@ describe('Localization', () => {
await expect(arrayField).toHaveValue(sampleText)
})
test('should copy block to locale', async () => {
const sampleText = 'Copy this text'
const blocksCollection = new AdminUrlUtil(serverURL, blocksCollectionSlug)
await page.goto(blocksCollection.create)
await changeLocale(page, 'pt')
const addBlock = page.locator('.blocks-field__drawer-toggler')
await addBlock.click()
const selectBlock = page.locator('.blocks-drawer__block button')
await selectBlock.click()
const addContentButton = page
.locator('#field-content__0__content')
.getByRole('button', { name: 'Add Content' })
await addContentButton.click()
await selectBlock.click()
const textField = page.locator('#field-content__0__content__0__text')
await expect(textField).toBeVisible()
await textField.fill(sampleText)
await saveDocAndAssert(page)
await openCopyToLocaleDrawer(page)
await setToLocale(page, 'English')
await runCopy(page)
await expect(textField).toHaveValue(sampleText)
})
test('should default source locale to current locale', async () => {
await changeLocale(page, spanishLocale)
await createAndSaveDoc(page, url, { title })

251
test/localization/int.spec.ts
View File

@@ -6,6 +6,7 @@ import { fileURLToPath } from 'url'
import type { NextRESTClient } from '../helpers/NextRESTClient.js'
import type {
BlocksField,
LocalizedPost,
LocalizedSort,
Nested,
@@ -508,8 +509,6 @@ describe('Localization', () => {
sort: 'date',
})
console.log({ sortByTitleQuery })
expect(sortByTitleQuery.totalDocs).toEqual(expectedTotalDocs)
expect(sortByDateQuery.totalDocs).toEqual(expectedTotalDocs)
})
@@ -2177,6 +2176,195 @@ describe('Localization', () => {
})
describe('nested fields', () => {
it('should update localized block', async () => {
const doc = await payload.create({
collection: 'blocks-fields',
locale: 'en',
data: {
content: [
{
blockType: 'blockInsideBlock',
content: [
{
blockType: 'textBlock',
text: 'some-text',
},
],
},
],
},
})
const updated = await payload.update({
id: doc.id,
collection: 'blocks-fields',
data: {
id: doc.id,
content: [
{
// This can't be added in Postgres because you'd get a duplicate ID error
// since the parent is localized, and the primary key in the block table
// consists only of the ID. That's why it's removed in `copyToLocale`.
// id: doc.content?.[0]?.id,
blockName: null,
array: [],
blockType: 'blockInsideBlock',
content: [
{
// Same as above.
// id: doc.content?.[0]?.content?.[0]?.id,
text: 'some-text',
blockName: null,
blockType: 'textBlock',
},
],
},
],
},
locale: 'es',
})
console.dir(updated, { depth: null })
expect(updated.content?.[0]?.content?.[0]?.text).toBe('some-text')
})
it('update specific locale should not erease the others in blocks and arrays', async () => {
const doc = await payload.create({
collection: 'nested',
locale: 'en',
data: {
blocks: [
{
blockType: 'block',
someText: 'some-block-text-en',
},
],
topLevelArray: [
{
localizedText: 'some-localized-text',
notLocalizedText: 'some-not-localized-text',
},
],
},
})
expect(doc.blocks?.[0]?.someText).toBe('some-block-text-en')
expect(doc.topLevelArray?.[0]?.localizedText).toBe('some-localized-text')
expect(doc.topLevelArray?.[0]?.notLocalizedText).toBe('some-not-localized-text')
expect(doc.topLevelArray).toHaveLength(1)
const findAllLocales = await payload.findByID({
id: doc.id,
collection: 'nested',
locale: 'all',
})
expect(findAllLocales.blocks?.[0]?.someText).toStrictEqual({
en: 'some-block-text-en',
})
expect(findAllLocales.topLevelArray?.[0]?.localizedText).toStrictEqual({
en: 'some-localized-text',
})
const updatedDoc = await payload.update({
id: doc.id,
collection: 'nested',
locale: 'es',
data: {
blocks: [
{
id: doc.blocks?.[0]?.id,
blockType: 'block',
someText: 'some-block-text-es',
},
],
topLevelArray: [
{
id: doc.topLevelArray?.[0]?.id,
localizedText: 'some-localized-text-es',
notLocalizedText: 'some-not-localized-text-es',
},
],
},
})
expect(updatedDoc.blocks?.[0]?.someText).toBe('some-block-text-es')
expect(updatedDoc.topLevelArray?.[0]?.localizedText).toBe('some-localized-text-es')
expect(updatedDoc.topLevelArray?.[0]?.notLocalizedText).toBe('some-not-localized-text-es')
const refreshedDoc = await payload.findByID({
id: doc.id,
collection: 'nested',
locale: 'all',
})
expect(refreshedDoc.blocks?.[0]?.someText).toStrictEqual({
en: 'some-block-text-en',
es: 'some-block-text-es',
})
expect(refreshedDoc.topLevelArray?.[0]?.localizedText).toStrictEqual({
en: 'some-localized-text',
es: 'some-localized-text-es',
})
})
it('update specific locale should not erease the others in simple fields', async () => {
const doc = await payload.create({
collection: 'localized-posts',
locale: 'en',
data: {
title: 'some-localized-title',
description: 'some-not-localized-description',
localizedDescription: 'some-localized-description',
},
})
expect(doc.title).toBe('some-localized-title')
expect(doc.localizedDescription).toBe('some-localized-description')
const findAllLocales = await payload.findByID({
id: doc.id,
collection: 'localized-posts',
locale: 'all',
})
expect(findAllLocales.title).toStrictEqual({
en: 'some-localized-title',
})
expect(findAllLocales.localizedDescription).toStrictEqual({
en: 'some-localized-description',
})
const updatedDoc = await payload.update({
id: doc.id,
collection: 'localized-posts',
locale: 'es',
data: {
title: 'some-localized-title-es',
description: 'some-not-localized-description-es',
localizedDescription: 'some-localized-description-es',
},
})
expect(updatedDoc.title).toBe('some-localized-title-es')
expect(updatedDoc.localizedDescription).toBe('some-localized-description-es')
const refreshedDoc = await payload.findByID({
id: doc.id,
collection: 'localized-posts',
locale: 'all',
})
expect(refreshedDoc.title).toStrictEqual({
en: 'some-localized-title',
es: 'some-localized-title-es',
})
expect(refreshedDoc.localizedDescription).toStrictEqual({
en: 'some-localized-description',
es: 'some-localized-description-es',
})
})
it('should allow for fields which could contain new tables within localized arrays to be stored', async () => {
const randomDoc = (
await payload.find({
@@ -2621,6 +2809,41 @@ describe('Localization', () => {
expect(res.localizedCheckbox).toBe(true)
})
it('should copy block to locale', async () => {
// This was previously an e2e test but it was migrated to int
// because at the moment only int tests run in Postgres in CI,
// and that's where the bug occurs.
const doc = await payload.create({
collection: 'blocks-fields',
locale: 'en',
data: {
content: [
{
blockType: 'blockInsideBlock',
content: [
{
blockType: 'textBlock',
text: 'some-text',
},
],
},
],
},
})
const req = await createLocalReq({ user }, payload)
const res = (await copyDataFromLocaleHandler({
fromLocale: 'en',
req,
toLocale: 'es',
docID: doc.id,
collectionSlug: 'blocks-fields',
})) as BlocksField
expect(res.content?.[0]?.content?.[0]?.text).toBe('some-text')
})
it('should copy localized nested to arrays', async () => {
const doc = await payload.create({
collection: 'nested',
@@ -2645,8 +2868,18 @@ describe('Localization', () => {
collectionSlug: 'nested',
})) as Nested
expect(res.topLevelArray[0].localizedText).toBe('some-localized-text')
expect(res.topLevelArray[0].notLocalizedText).toBe('some-not-localized-text')
expect(res.topLevelArray?.[0]?.localizedText).toBe('some-localized-text')
expect(res.topLevelArray?.[0]?.notLocalizedText).toBe('some-not-localized-text')
const refreshedDoc = await payload.findByID({
id: doc.id,
collection: 'nested',
})
// The source data should remain unchanged
expect(refreshedDoc.topLevelArray?.[0]?.localizedText).toBe('some-localized-text')
expect(refreshedDoc.topLevelArray?.[0]?.notLocalizedText).toBe('some-not-localized-text')
expect(refreshedDoc.topLevelArray).toHaveLength(1)
})
it('should copy localized arrays', async () => {
@@ -2672,7 +2905,15 @@ describe('Localization', () => {
collectionSlug: 'nested',
})) as Nested
expect(res.topLevelArrayLocalized[0].text).toBe('some-text')
expect(res.topLevelArrayLocalized?.[0]?.text).toBe('some-text')
const refreshedDoc = await payload.findByID({
id: doc.id,
collection: 'nested',
})
// The source data should remain unchanged
expect(refreshedDoc.topLevelArrayLocalized?.[0]?.text).toBe('some-text')
})
})
})

16
test/localization/payload-types.ts
View File

@@ -361,6 +361,13 @@ export interface User {
hash?: string | null;
loginAttempts?: number | null;
lockUntil?: string | null;
sessions?:
| {
id: string;
createdAt?: string | null;
expiresAt: string;
}[]
| null;
password?: string | null;
}
/**
@@ -536,6 +543,7 @@ export interface Nested {
id: string;
blocks?:
| {
someText?: string | null;
array?:
| {
text?: string | null;
@@ -997,6 +1005,13 @@ export interface UsersSelect<T extends boolean = true> {
hash?: T;
loginAttempts?: T;
lockUntil?: T;
sessions?:
| T
| {
id?: T;
createdAt?: T;
expiresAt?: T;
};
}
/**
* This interface was referenced by `Config`'s JSON-Schema
@@ -1159,6 +1174,7 @@ export interface NestedSelect<T extends boolean = true> {
block?:
| T
| {
someText?: T;
array?:
| T
| {