Skip to content

fix(edit-content): parse block editor JSON strings when translating content#35247

Merged
oidacra merged 6 commits intomainfrom
issue-34025-block-editor-translate-text
Apr 13, 2026
Merged

fix(edit-content): parse block editor JSON strings when translating content#35247
oidacra merged 6 commits intomainfrom
issue-34025-block-editor-translate-text

Conversation

@oidacra
Copy link
Copy Markdown
Member

@oidacra oidacra commented Apr 7, 2026

Summary

  • Add blockEditorResolutionFn that JSON.parse()s block editor values when they come as JSON strings from the API (happens during content translation/copy to new language)
  • Previously, the block editor used defaultResolutionFn which returned the raw JSON string, causing the editor to display it as plain text instead of structured content

Closes #34025

Acceptance Criteria

  • Block editor content is preserved as structured data when translating to a new language
  • Object values (already parsed) pass through unchanged
  • Non-JSON strings are returned as-is (no error)
  • Null contentlet falls back to defaultValue

Test Plan

  • yarn nx test edit-content --testPathPattern=dot-edit-content-form-resolutions — all tests pass
  • 4 new tests for blockEditorResolutionFn
  • Manual: Create content with block editor, translate to new language, verify formatting preserved

Changed Files

File Change
libs/edit-content/.../dot-edit-content-form-resolutions.ts Added blockEditorResolutionFn, mapped to FIELD_TYPES.BLOCK_EDITOR
libs/edit-content/.../dot-edit-content-form-resolutions.spec.ts Added 4 tests, removed BLOCK_EDITOR from default fn group

This PR fixes: #34025

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 7, 2026

Claude finished @oidacra's task in 3m 39s —— View job


Code Review

  • Read changed files
  • Analyze for bugs, edge cases, type issues, design problems
  • Post findings

3 items worth a second look:

1. startsWith('{') heuristic silently skips non-object JSON

dot-edit-content-form-resolutions.ts:225

if (typeof value === 'string' && value.trim().startsWith('{')) {

Block editor documents ({type:'doc',...}) always start with {, so this works today. But the heuristic silently leaves a JSON string un-parsed if the translation API ever returns anything else (array root, quoted string, etc.). A more defensive alternative:

if (typeof value === 'string') {
    try {
        const parsed = JSON.parse(value);
        if (parsed !== null && typeof parsed === 'object') return parsed;
    } catch { /* not JSON */ }
}

This also removes the need for the startsWith pre-check entirely.


2. Round-trip contract is implicit — easy to break

blockEditorResolutionFn puts a parsed Record<string,unknown> into the FormControl. processFieldValue (in a different file) stringifies it back on submit. The only coupling is a comment. If processFieldValue is bypassed (different submit path, direct form patch, etc.) an object would reach the backend and be stored as [object Object]. This is the same bug the PR is fixing, just in a different code path.

Consider whether the stringify should live closer to where the block editor emits its value (it already calls JSON.stringify internally before emitting via onChangedot-block-editor.component.ts:294). The form then only ever holds strings, and nothing downstream needs to know about the round-trip.


3. processFieldValue's BLOCK_EDITOR guard catches Date and arrays

functions.util.ts:631-637

if (field.fieldType === FIELD_TYPES.BLOCK_EDITOR && fieldValue && typeof fieldValue === 'object') {
    return JSON.stringify(fieldValue);
}

typeof new Date() === 'object' and typeof [] === 'object' are both true. Arrays are already handled above (CATEGORY / flattened), so an array here is only possible for corrupted data — JSON.stringify([]) is fine. But a stray Date value would produce "\"2026-...\"". Worth adding && !Array.isArray(fieldValue) && !(fieldValue instanceof Date) to make the intent explicit, or accepting this as an unreachable edge case.


Not a bug but notable: blockEditorResolutionFn is typed FnResolutionValue<string | Record<string, unknown>>, but field.defaultValue (and contentlet field access) is any, so the function can silently return null, undefined, or other types that don't satisfy the declared return type. This is consistent with every other resolution function in the file, so it's not new risk.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes translated/copied Block Editor fields being displayed as plain text by parsing JSON-string values into structured objects during initial form value resolution.

Changes:

  • Added a dedicated blockEditorResolutionFn that attempts to JSON.parse() Block Editor values when they arrive as JSON strings.
  • Wired FIELD_TYPES.BLOCK_EDITOR to use the new resolution function instead of defaultResolutionFn.
  • Added Jest tests covering Block Editor resolution behavior and removed BLOCK_EDITOR from the “default resolution fn” group.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
core-web/libs/edit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form-resolutions.ts Introduces blockEditorResolutionFn and maps it to FIELD_TYPES.BLOCK_EDITOR so block editor receives structured data when API returns JSON strings.
core-web/libs/edit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form-resolutions.spec.ts Adds unit tests for Block Editor resolution and updates the default-mapping assertions accordingly.

@github-actions github-actions bot added the Area : Backend PR changes Java/Maven backend code label Apr 8, 2026
@oidacra oidacra force-pushed the issue-34025-block-editor-translate-text branch from e92a2c2 to 3062ffb Compare April 8, 2026 14:02
@github-actions github-actions bot removed the Area : Backend PR changes Java/Maven backend code label Apr 8, 2026
@oidacra oidacra requested review from hmoreras and nicobytes April 8, 2026 14:17
@oidacra oidacra self-assigned this Apr 8, 2026
oidacra added 3 commits April 13, 2026 11:29
…ontent

Add blockEditorResolutionFn that JSON.parse()s string values returned
by the API when copying/translating content to a new language.

Closes #34025
Use Record<string, unknown> instead of object for blockEditorResolutionFn
type and add test for invalid JSON catch path.
… Map.toString() persistence

After blockEditorResolutionFn parses the JSON string to an object so the
editor can render translated content, the FormControl holds an object.
If the user saves without interacting with the editor (which is what
emits the JSON.stringify via onChange), the object was sent to the
backend and persisted as Map.toString(), corrupting the field on
subsequent loads.

Stringify BLOCK_EDITOR object values in processFieldValue so the submit
payload matches what the editor's own onChange emits.

Closes #34025
@oidacra oidacra force-pushed the issue-34025-block-editor-translate-text branch from 3062ffb to 0630dc7 Compare April 13, 2026 16:07
@oidacra oidacra requested a review from Copilot April 13, 2026 16:07
@oidacra oidacra marked this pull request as ready for review April 13, 2026 16:07
@oidacra oidacra requested a review from adrianjm-dotCMS April 13, 2026 16:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

core-web/libs/edit-content/src/lib/utils/functions.util.ts:614

  • processFieldValue now accepts Record<string, unknown>, but it calls isFlattenedField(fieldValue, field) where isFlattenedField is still typed to accept only string | string[] | Date | number | null | undefined. This makes the call site type-incompatible and can fail TypeScript compilation. Consider widening isFlattenedField's fieldValue parameter to include the new object type (or unknown) so it matches processFieldValue's input union.
export const processFieldValue = (
    fieldValue: string | string[] | Date | number | Record<string, unknown> | null | undefined,
    field: DotCMSContentTypeField
): string | number | null | undefined => {
    // Handle flattened fields (multi-select, etc.)
    if (isFlattenedField(fieldValue, field)) {
        return (fieldValue as string[]).join(',');
    }

oidacra added 3 commits April 13, 2026 12:17
…ly primitives

processFieldValue widened its input type to include Record<string, unknown>
for the block editor path, but downstream helpers (isFlattenedField,
processCalendarFieldValue) still declare primitive-only signatures.

Handle the block editor branch up front and cast the remaining value to
the primitive union, so helper signatures remain accurate. Also guard
against arrays and Date instances (typeof 'object' in JS) to make the
intent explicit.
…hrough processFieldValue

Replace the previous early-return-with-as-cast approach with wider helper
signatures. isFlattenedField and processCalendarFieldValue already guard
internally against unexpected inputs at runtime (type guard / explicit
fallthrough), so accepting unknown honestly reflects their defensive
behavior and removes the need for casts in processFieldValue.
…lue to keep FormValues inference intact

Widening processFormValue's input to include Record<string, unknown>
caused TS to infer that the !field branch could return an object,
breaking FormValues assignability. At runtime the input never carries
that shape to this function's output (processFieldValue stringifies
BLOCK_EDITOR objects before they reach Object.fromEntries), so the
widening was never necessary here. processFieldValue keeps its widened
input since it legitimately handles the object path internally.
@oidacra oidacra enabled auto-merge April 13, 2026 16:37
@oidacra oidacra added this pull request to the merge queue Apr 13, 2026
Merged via the queue into main with commit 0934851 Apr 13, 2026
39 of 42 checks passed
@oidacra oidacra deleted the issue-34025-block-editor-translate-text branch April 13, 2026 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[DEFECT] When translating to a new language, content in the block editor is copied as text not as an object

3 participants