[Batch Edit] Edit multiple languages of a localized data object field (#2492)#3895
Open
xIrusux wants to merge 10 commits into
Open
[Batch Edit] Edit multiple languages of a localized data object field (#2492)#3895xIrusux wants to merge 10 commits into
xIrusux wants to merge 10 commits into
Conversation
…fields (#2492) Batch-editing a localized data object field only saved the language selected at save time. The language selector swapped a single Form.Item's name between languages; antd unmounts the field on a name change and only collects values from currently-registered fields at submit, so every language except the last selected was dropped. Render one Form.Item per required language (all kept mounted, only the selected one visible) so every edited language is submitted. Localized items carry no initialValue, so languages the user did not touch are omitted from the payload instead of being sent as null (which would overwrite existing translations). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
xIrusux
changed the title
Changing a localizable row's locale remounted the field at a new localizedfields path and dropped the entered value on submit. Move the value from the old locale path to the new one when the locale changes (regular localizable fields; classification-store and object-brick keep their own name paths). Also match batch-edit entries by key + group (consistent with the fields-to-add filter) and use a Set for the used-locale lookup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the data-object batch edit modal to support editing multiple locales for a single localizable field by allowing one batch-edit entry per locale, using user.contentLanguages as the locale source and adjusting left-panel availability accordingly.
Changes:
- Updated batch-edit entry identity/selection logic to better account for field
groupand multiple locale rows. - Switched locale source from settings-required languages to the user’s permission-scoped
contentLanguages. - Updated the batch-edit list UI to use
PermissionBasedLanguageSelectionControland to keep per-locale form values in sync when the locale changes.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| assets/js/src/core/modules/data-object/listing/batch-actions/batch-edit-modal/utils/dropdown-filter.ts | Adjusts “already selected” logic to support multi-locale rows and hide fields once all locales are used. |
| assets/js/src/core/modules/data-object/listing/batch-actions/batch-edit-modal/hooks/use-batch-edit.ts | Updates add/update/remove behavior to treat entries as key+locale(+group/config) and to default locales from user.contentLanguages. |
| assets/js/src/core/modules/data-object/listing/batch-actions/batch-edit-modal/batch-edit-modal.tsx | Passes contentLanguages into the column-availability filter and sources locales from useUser(). |
| assets/js/src/core/modules/data-object/listing/batch-actions/batch-edit-modal/batch-edit-list-container.tsx | Renders locale pickers per row with exclusions and migrates form values when changing a row’s locale. |
Comments suppressed due to low confidence (1)
assets/js/src/core/modules/data-object/listing/batch-actions/batch-edit-modal/hooks/use-batch-edit.ts:97
- addOrUpdateBatchEdits() currently de-dupes/updates entries by key only and against the original batchEdits array. This will overwrite entries when multiple edits share the same key (e.g. classification-store items with different keyId/groupId, or multiple locale rows for the same field), and it can also assign locale=undefined when contentLanguages is empty. Use updatedEdits and match via isSameEntry(), and ensure the fallback locale is either a string or null.
locale: column.localizable ? column.locale ?? contentLanguages[0] : null,
value: undefined
}
const existingIndex = batchEdits.findIndex(edit => edit.key === newEdit.key)
Comment on lines
+55
to
59
| const isAlreadyInBatchEditList = item.mainType !== 'dataobject.classificationstore' && ( | ||
| item.localizable === true | ||
| ? contentLanguages.length > 0 && existingEntries.length >= contentLanguages.length | ||
| : existingEntries.length > 0 | ||
| ) |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
assets/js/src/core/modules/data-object/listing/batch-actions/batch-edit-modal/hooks/use-batch-edit.ts:97
addOrUpdateBatchEditsdetermines existing entries usingbatchEdits.findIndex(edit => edit.key === newEdit.key), which ignoresgroup,locale, and (for classification-store)config.keyId/groupId. This can overwrite an unrelated existing batch-edit entry that shares the same key (e.g. classification-store rows for different keyId/groupId, or same key in different groups). Use the same identity logic asremoveBatchEdit/updateLocale(key + locale + group, plus keyId/groupId for classification-store).
locale: column.localizable ? column.locale ?? contentLanguages[0] : null,
value: undefined
}
const existingIndex = batchEdits.findIndex(edit => edit.key === newEdit.key)
Comment on lines
+58
to
+70
| const usedLocales = new Set( | ||
| batchEdits | ||
| .filter(edit => edit.key === column.key && areGroupsEqual(edit.group, column.group)) | ||
| .map(edit => edit.locale) | ||
| ) | ||
| const nextLocale = contentLanguages.find(language => !usedLocales.has(language)) | ||
|
|
||
| if (nextLocale === undefined) { | ||
| return | ||
| } | ||
|
|
||
| setBatchEdits([...batchEdits, { ...column, locale: nextLocale, value }]) | ||
| return |
Comment on lines
+48
to
+52
| contentLanguages: string[] = [] | ||
| ): boolean => { | ||
| const isEditable: boolean = item.editable === true | ||
| const isAlreadyInBatchEditList = batchEdits.some(batchItem => | ||
| item.key === batchItem.key && areGroupsEqual(item.group, batchItem.group) && item.mainType !== 'dataobject.classificationstore' | ||
| const existingEntries = batchEdits.filter(batchItem => | ||
| item.key === batchItem.key && areGroupsEqual(item.group, batchItem.group) |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
+35
to
+37
| const isRegularLocalizable = batchEdit.localizable && | ||
| batchEdit.type !== 'dataobject.classificationstore' && | ||
| batchEdit.type !== 'dataobject.objectbrick' |
| // @todo infer selected language from grid config when available | ||
| locale: column.localizable ? column.locale ?? settings.requiredLanguages[0] : null, | ||
| value | ||
| if (column.localizable) { |
Comment on lines
+55
to
59
| const isAlreadyInBatchEditList = item.mainType !== 'dataobject.classificationstore' && ( | ||
| item.localizable === true | ||
| ? contentLanguages.length > 0 && existingEntries.length >= contentLanguages.length | ||
| : existingEntries.length > 0 | ||
| ) |
Comment on lines
+43
to
+46
| const oldPath = ['localizedfields', batchEdit.key, batchEdit.locale] | ||
| const newPath = ['localizedfields', batchEdit.key, nextLocale] | ||
| form.setFieldValue(newPath, form.getFieldValue(oldPath)) | ||
| form.setFieldValue(oldPath, undefined) |
| const batchEditTitle = hasFieldDefinition(batchEdit.config) ? (batchEdit.config.fieldDefinition as { title: string }).title : batchEdit.key | ||
| const key = batchEdit.type === 'dataobject.classificationstore' ? `${batchEdit.key}-${(batchEdit.config as { keyId: number }).keyId}-${(batchEdit.config as { groupId: number }).groupId}` : batchEdit.key | ||
| const baseKey = batchEdit.type === 'dataobject.classificationstore' ? `${batchEdit.key}-${(batchEdit.config as { keyId: number }).keyId}-${(batchEdit.config as { groupId: number }).groupId}` : batchEdit.key | ||
| const key = batchEdit.localizable ? `${baseKey}-${batchEdit.locale ?? ''}` : baseKey |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
assets/js/src/core/modules/data-object/listing/batch-actions/batch-edit-modal/hooks/use-batch-edit.ts:97
addOrUpdateBatchEditslooks up existing entries bykeyonly (batchEdits.findIndex(edit => edit.key === newEdit.key)), which can overwrite the wrong entry when multiple batch edits share the same key (e.g. classification-store adds multiple entries with the same base key but different keyId/groupId). Also,contentLanguages[0]can beundefinedat runtime, which would put an invalid locale into the model.
Consider matching via isSameEntry (key+group+locale(+keyId/groupId for classification-store)) against updatedEdits, and defaulting the locale to null when there is no language available.
locale: column.localizable ? column.locale ?? contentLanguages[0] : null,
value: undefined
}
const existingIndex = batchEdits.findIndex(edit => edit.key === newEdit.key)
user.contentLanguages is typed as object; guard with Array.isArray before spreading so a non-array value can't break adding rows or the completion check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| // @todo infer selected language from grid config when available | ||
| locale: column.localizable ? column.locale ?? settings.requiredLanguages[0] : null, | ||
| value | ||
| if (column.localizable) { |
Comment on lines
+55
to
59
| const isAlreadyInBatchEditList = item.mainType !== 'dataobject.classificationstore' && ( | ||
| item.localizable === true | ||
| ? contentLanguages.length > 0 && existingEntries.length >= contentLanguages.length | ||
| : existingEntries.length > 0 | ||
| ) |
Comment on lines
91
to
95
| const newEdit: BatchEdit = { | ||
| ...column, | ||
| // @todo infer selected language from grid config when available | ||
| locale: column.localizable ? column.locale ?? settings.requiredLanguages[0] : null, | ||
| locale: column.localizable ? column.locale ?? contentLanguages[0] : null, | ||
| value: undefined | ||
| } |
Comment on lines
+74
to
+76
| onChange={ (language) => { | ||
| migrateLocaleValue(batchEdit, language) | ||
| updateLocale(batchEdit, language) |
Comment on lines
+60
to
+61
| const baseKey = batchEdit.type === 'dataobject.classificationstore' ? `${batchEdit.key}-${(batchEdit.config as { keyId: number }).keyId}-${(batchEdit.config as { groupId: number }).groupId}` : batchEdit.key | ||
| const key = batchEdit.localizable ? `${baseKey}-${batchEdit.locale ?? ''}` : baseKey |
|
| // @todo infer selected language from grid config when available | ||
| locale: column.localizable ? column.locale ?? settings.requiredLanguages[0] : null, | ||
| value | ||
| if (column.localizable) { |
Comment on lines
+35
to
+37
| const isRegularLocalizable = batchEdit.localizable && | ||
| batchEdit.type !== 'dataobject.classificationstore' && | ||
| batchEdit.type !== 'dataobject.objectbrick' |
| .filter(edit => edit.key === column.key && areGroupsEqual(edit.group, column.group)) | ||
| .map(edit => edit.locale) | ||
| ) | ||
| const nextLocale = contentLanguages.find(language => !usedLocales.has(language)) |
| const batchEditTitle = hasFieldDefinition(batchEdit.config) ? (batchEdit.config.fieldDefinition as { title: string }).title : batchEdit.key | ||
| const key = batchEdit.type === 'dataobject.classificationstore' ? `${batchEdit.key}-${(batchEdit.config as { keyId: number }).keyId}-${(batchEdit.config as { groupId: number }).groupId}` : batchEdit.key | ||
| const baseKey = batchEdit.type === 'dataobject.classificationstore' ? `${batchEdit.key}-${(batchEdit.config as { keyId: number }).keyId}-${(batchEdit.config as { groupId: number }).groupId}` : batchEdit.key | ||
| const key = batchEdit.localizable ? `${baseKey}-${batchEdit.locale ?? ''}` : baseKey |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2492
Problem
Batch-editing a localized data object field only persisted a single language — the one selected at save time. Editing e.g. German + English still submitted only one locale.
What this changes (UX)
A localizable field now supports one input row per locale:
How it works
hooks/use-batch-edit.ts)Form.Itemwith a distinctlocalizedfields[key][locale]name, all mounted and visible. Every edited language is therefore submitted, and locales the user didn't add simply have no row (no accidental null-overwrite of other translations). This replaces the earlier hidden-item tracking approach.user.contentLanguagesviaPermissionBasedLanguageSelectionControl— the same permission-scoped set the grid's language switcher and grid column-config use (notrequiredLanguages, which is only the mandatory subset and was inconsistent with the grid).utils/dropdown-filter.ts,batch-edit-modal.tsx)Scope
Standard localizable data-object fields (the adapter path). Classification-store and object-brick keep their existing single-entry behavior (made locale-safe only).
Verification
tsc+ eslint clean.editableData.localizedfields.name = { en, fr }.🤖 Generated with Claude Code