Skip to content

fix(edit-content): locale translation fixes — preserve HOST_FOLDER & RELATIONSHIP, resolve related items to target language#35738

Merged
adrianjm-dotCMS merged 5 commits into
mainfrom
35532-translate-locales-feature-fixes
May 21, 2026
Merged

fix(edit-content): locale translation fixes — preserve HOST_FOLDER & RELATIONSHIP, resolve related items to target language#35738
adrianjm-dotCMS merged 5 commits into
mainfrom
35532-translate-locales-feature-fixes

Conversation

@adrianjm-dotCMS
Copy link
Copy Markdown
Member

@adrianjm-dotCMS adrianjm-dotCMS commented May 18, 2026

Screen.Recording.2026-05-19.at.11.42.41.AM.mov

Summary

Fixes several bugs in the locale translation flow (populate & manually translate) for the edit content form.

  • Binary fields are now cleared on both Populate from current locale and Translate Manually — binary assets live on the filesystem and must be re-uploaded per locale; writeValue(null) alone is not enough, so the binary component is flushed (destroyed + recreated) to reset its internal visual state
  • HOST_FOLDER and RELATIONSHIP fields are preserved during Translate Manually — their values and component state survive the form reinit; captured from the FormControl before reinit and restored after
  • Relationship items now resolve to their target-language version on both populate and manual translate; if no translation exists, the original item is kept (per-item catchError fallback)
  • Language column in the relationship table now renders correctly after locale resolution — the LanguagePipe expects a DotLanguage object but getContentById returns language as a plain string; the resolved item is patched with the full DotLanguage object before it hits the store
  • Tag field is no longer cleared on Populate from current locale in the old (Dojo/JSP) edit content — setInode("") was called before setTags(), so the tag lookup by inode returned nothing; tags are now loaded from tag_inode before the inode is cleared

Root causes

Bug Root cause
Binary not clearing visually on populate #flushFieldsForRerender() was only called for manual translation, not populate
HOST_FOLDER cleared on manual translate Form reinit with contentlet = null overwrites all controls; no capture/restore
RELATIONSHIP cleared on manual translate Same as above; component was also being flushed unnecessarily
Language column empty after populate getContentById returns language as string, not DotLanguage; pipe returns '' for strings
Relationship items not updating on manual translate signalMethod was called with this.$inputs() (a value, one-shot) instead of this.$inputs (signal ref, reactive); component is preserved so ngOnInit doesn't re-run
Tag field cleared on populate (old editor) contentlet.setInode("") ran before contentlet.setTags(); tag lookup uses the inode so it returned nothing

Technical approach

  • prepareContentletForCopy accepts an optional fields param and nulls out all BINARY field values
  • dot-edit-content-form gains $shouldRenderPreservedFields signal and #preservedFieldTypesOnManualTranslation array (HOST_FOLDER, RELATIONSHIP); manual translation uses a selective flush that only destroys non-preserved field components
  • RelationshipFieldStore.initialize captures store.data() before the reset with a map operator, uses those items as fallback when contentlet is null (manual translation), and patches each fetched item's language to the full DotLanguage object
  • EditContentletAction.retrieveWebAsset now calls sibbling.setTags() before setInode("") so the tag-to-inode associations are loaded while the original inode is still available

Test plan

  • Open a contentlet in English that has a binary field, a site/folder field, a relationship field with items, and a tag field
  • Click the locale switcher → select an untranslated locale → Populate from current locale
    • Binary field should be empty (no preview, no file name)
    • Site/folder and relationship fields should be populated
    • Relationship items should show the target language in the language column (or fall back to English if no translation exists)
    • Tag field should show the same tags as the source locale (not empty)
  • Repeat with Translate Manually (new Angular editor)
    • All text/rich-text fields should be empty (flush resets them)
    • Binary field should be empty
    • Site/folder value should be preserved
    • Relationship items should be preserved and show the target language in the language column

This PR fixes: #35534

This PR fixes: #35532

…nual locale translation, resolve related items to target language

- Binary fields are now cleared on both populate and manual translate (re-upload required per locale)
- HOST_FOLDER and RELATIONSHIP fields are preserved on manual translate
- Related items resolve to their target-language version on both populate and manual translate; falls back to the original if no translation exists
- Language column in the relationship table now renders correctly after locale resolution (LanguagePipe expects DotLanguage object, not a string)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 18, 2026

Claude finished @adrianjm-dotCMS's task in 4m 2s —— View job


Code Review

  • Read changed files
  • Analyze Java change (EditContentletAction)
  • Analyze form component
  • Analyze relationship store
  • Analyze utility & feature changes
  • Check tests coverage

Overall the diagnosis is solid and the fixes match the root causes described. A few things worth a second look:


1. prepareContentletForCopy only clears BINARYFILE and IMAGE may need the same treatment

functions.util.ts:432

The filter only matches FIELD_TYPES.BINARY. The field enum also exposes FILE and IMAGE (dot-edit-content-field.enum.ts:12-14). In dot-edit-content-form-resolutions.ts:297-299 all three use defaultResolutionFn together, which is a hint that they're conceptually similar asset fields.

If FILE/IMAGE fields can hold an inline / per-locale value (rather than only an identifier reference to a DotAsset content), they'll suffer the same "stale asset from source locale" problem on populate. Worth confirming explicitly — if they only ever hold identifiers, leave it; if they can ever be value-bearing, add them to the filter.

2. N+1 calls on relationship resolution

relationship-field.store.ts:177-204

forkJoin(dataToProcess.map((item) => dotEditContentService.getContentById({ id: item.identifier, languageId: targetLanguageId })...))

One HTTP call per related item, in parallel. A relationship with 50+ items would fire 50 simultaneous requests on every locale copy. Not a regression (the previous code did nothing here), but worth tracking — a batch endpoint that resolves N identifiers to a target language in one call would be a clear win.

3. existingData capture has a subtle race window

relationship-field.store.ts:147-150

map((params) => ({ ...params, existingData: store.data() })),
tap(() => patchState(store, initialState)),

store.data() is captured synchronously before the reset, which works for the normal case. If initialize ever emits twice in rapid succession (two signal changes before the first switchMap completes), emission #2's map runs after emission #1's tap has already reset the store, so existingData becomes [] and items are lost. The current $inputs signal is unlikely to flap this way, but the invariant isn't enforced. A safer pattern is to capture in the switchMap only when contentlet == null (manual translation), or to use a dedicated previousData field in state populated by setData.

4. Java tag fix: catch is broad and a thrown exception still loses tags

EditContentletAction.java:1147-1153

try {
    contentlet.setTags();
} catch (Exception e) {
    Logger.warn(this, "Could not load tags for sibling " + sib, e);
}
contentlet.setInode("");
...
contentlet.setTags(); // line 1209, after inode is empty

Two notes:

  • setTags() declares throws DotDataException — catching plain Exception is broader than needed.
  • If the first call throws, loadedTags stays false (Contentlet.java:1580 only sets it after the loop completes successfully). The later setTags() at line 1209 then retries with inode = "" and the bug recurs silently. Probably acceptable for a "best-effort populate" flow, but a Logger.error on the warn path would make this observable.

5. Template change drops the outer @if ($shouldRenderFields())

dot-edit-content-form.component.html

The @if was moved from the wrapper to per-field. Row/column <div>s now render unconditionally — during the flush window the layout container stays mounted with no children. Two consequences:

  • Tests counting data-testId="row" / data-testId="column" will now find them during flush; the spec was updated for $shouldRenderFields but consumers depending on the old behavior could trip.
  • Brief visual artifact: padded but empty containers for one tick during populate. Probably imperceptible, but worth verifying in the recorded demo.

6. signalMethod reactive-vs-value pattern is brittle for future contributors

dot-relationship-field.component.ts:213

this.initialize(this.$inputs);   // signal ref → reactive
// vs
this.initialize(this.$inputs()); // value → one-shot

The doc comment explains the why, which is good. The footgun is that signalMethod's parameter type accepts both — the next contributor who refactors this is one keystroke away from breaking manual translation again. Consider either renaming the public surface (subscribeToInputs(signal) / initializeOnce(value)) or adding a type-level guard.

7. #capturePreservedFields assumes flat FormControls

dot-edit-content-form.component.ts:822-839

form.get(f.variable)?.value and setValue(value, { emitEvent: false }) work for HOST_FOLDER (string) and RELATIONSHIP (array of ids). If anyone later adds a preserved field type whose control is a FormGroup or FormArray, the capture/restore may lose nested state. A short comment near #preservedFieldTypesOnManualTranslation saying "only flat-valued field types may be added here" would lock the invariant in.

8. Minor nits

  • functions.util.ts:432-434reduce could be Object.fromEntries(... .map(f => [f.variable, null])). Style only.
  • dot-edit-content-form.component.ts:760-764#preservedFieldTypesOnManualTranslation is readonly and field-private but uses [FIELD_TYPES, FIELD_TYPES] value-typed; as const plus ReadonlySet<FIELD_TYPES> would make membership tests faster than Array.includes for hot paths (not actually hot here).
  • The locale resolution catchError per item is correct, but the comment says "language column will be blank for unresolvable items" — it'll actually contain the source locale's DotLanguage string from the API, which can render as an empty pipe output. Worth double-checking against the demo: does an item with no translation render empty, or render the source language?

Tests

The new tests in relationship-field.store.spec.ts cover the happy path, the empty-data short-circuit, the no-target-language short-circuit, the language-patch, the catchError fallback, and the existingData fallback. Good coverage. Missing:

  • No test for the form-component capture/restore of HOST_FOLDER/RELATIONSHIP (a unit test on #capturePreservedFields / #restorePreservedFields would lock that invariant in).
  • No test for the Java side (EditContentletAction#retrieveWebAsset tag preload). Hard to unit-test, but at least an integration test against a content type with a Tag field on populate would be valuable.

If FILE/IMAGE turn out to need the same null-out, that's the only item I'd block on. The rest are quality/maintainability points.
· Branch: 35532-translate-locales-feature-fixes

adrianjm-dotCMS and others added 2 commits May 18, 2026 16:55
…p field CI test failures

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…vent loss of data

- Added logic to load tag associations from the tag_inode table before clearing the inode in the EditContentletAction class. This ensures that the setTags() method can find the tags correctly, preventing potential data loss during the content editing process.
@github-actions github-actions Bot added the Area : Backend PR changes Java/Maven backend code label May 19, 2026
@adrianjm-dotCMS
Copy link
Copy Markdown
Member Author

@claude Code review here please

@claude

This comment was marked as resolved.

adrianjm-dotCMS and others added 2 commits May 19, 2026 13:23
…le-resolution tests, add fallback comment, fix Java style

- Rename $isPreservedField → isPreservedField ($ prefix reserved for signals/computed)
- Add test coverage for relationship store locale resolution branches:
  targetLanguageId + items calls getContentById per item, patches language to DotLanguage,
  catchError falls back to original item, null contentlet uses existingData,
  early-return when no targetLanguageId or empty data
- Add comment on catchError fallback explaining blank language column is expected
- Expand Java try/catch to multi-line block for consistency

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@adrianjm-dotCMS adrianjm-dotCMS marked this pull request as ready for review May 19, 2026 17:25
@adrianjm-dotCMS adrianjm-dotCMS added this pull request to the merge queue May 20, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks May 20, 2026
@adrianjm-dotCMS adrianjm-dotCMS added this pull request to the merge queue May 20, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks May 20, 2026
@adrianjm-dotCMS adrianjm-dotCMS added this pull request to the merge queue May 20, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks May 21, 2026
@adrianjm-dotCMS adrianjm-dotCMS added this pull request to the merge queue May 21, 2026
Merged via the queue into main with commit f9a5a20 May 21, 2026
52 checks passed
@adrianjm-dotCMS adrianjm-dotCMS deleted the 35532-translate-locales-feature-fixes branch May 21, 2026 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

3 participants