-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix(ui): preserve localized blocks and arrays when using CopyToLocale #13216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5e65c88
dbf9a72
98faa43
43fe251
da88137
dc37449
caaed27
86f472d
833385f
fdd75ce
9ab2464
8b5a3af
d4566e9
30d3c57
35d37b8
639d394
88bf152
8739944
d4a442b
f634aba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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)) && | ||
Comment on lines
+74
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MongoDB and PostgreSQL behave differently when querying documents that don't exist in a specific locale:
This difference caused |
||
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, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
}) | ||
|
||
Comment on lines
-434
to
-459
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test was migrated to int. See comment below |
||
test('should default source locale to current locale', async () => { | ||
await changeLocale(page, spanishLocale) | ||
await createAndSaveDoc(page, url, { title }) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We were forgetting about forward
parentPath
here