feat(style-editor): implement duplicate identifier validation for fields

[KevinDavilaDotCMS]

2026-04-06.12-33-01.mov

This PR fixes: #34703

This PR fixes: #34703

[claude]

Claude finished @KevinDavilaDotCMS's task in 2m 43s —— View job

---

PR Review: feat(style-editor): implement duplicate identifier validation for fields

• Read changed files
• Analyze code and write review

---

Issues Found

1. Untrimmed identifier saved to backend (new)

dot-style-editor-field-form.component.ts:261-263 — setIdentifier stores the raw typed value and #emitChange passes it up unchanged. getDuplicateIdentifiers trims identifiers before comparing (models.ts:73), so two fields with "myField" and "myField " are correctly detected as duplicates — but if the user saves, #toStyleEditorField writes field.identifier verbatim (builder.component.ts:339), so the stored schema ends up with a trailing-space identifier. Fix: trim in setIdentifier or in #toStyleEditorField.

2. Duplicated validation logic

dot-style-editor-field-form.component.ts:179-204 re-implements the same logic as fieldHasErrors in models.ts:90-116, with the only additions being the $showErrors() short-circuit and the $isDuplicateIdentifier() check. If a new validation rule is added to fieldHasErrors, the panel's red-border signal $hasErrors will silently miss it. Consider passing isDuplicate into fieldHasErrors and inverting the $showErrors guard at the call site.

3. console.warn fires on every new content type

dot-style-editor-builder.component.ts:261-263 logs [StyleEditorBuilder] Invalid schema in metadata when !raw || typeof raw !== 'string', which is the expected state for any content type that has never had a schema saved. Every first load of a new content type hits this branch.

4. typeInIdentifierInput integration test is fragile

dot-style-editor-builder.component.spec.ts:80-86 dispatches a native DOM input event deep inside a grandchild component and relies on the full cascade (DefaultValueAccessor → ngModelChange → setIdentifier → fieldChange → updateField) firing synchronously. If the identifier input ever moves to reactive forms, or if OnPush blocks synchronous propagation, the test at line 310 will silently pass without actually calling updateField. A direct call to spectator.component.updateField(0, modifiedField) would be more robust.

5. Missing test: error state clears after duplicate is resolved

The new tests verify that a duplicate blocks saving, but none assert that $duplicateIdentifiers() becomes empty once the user gives the conflicting field a unique identifier. The existing test at line 310 only asserts the API was called, not that the error state cleared.

---

The core logic in getDuplicateIdentifiers, $isFormValid, $hasFieldErrors, and the [isDuplicateIdentifier] template binding is correct. Item #1 (untrimmed identifier) is a new bug; items #2–5 carry over from the previous review cycle and remain unaddressed.

[claude]

Claude finished @KevinDavilaDotCMS's task in 4m 2s —— View job

---

PR Review: feat(style-editor): implement duplicate identifier validation for fields

• Read changed files
• Analyze code and write review

---

Issues Found

1. Duplicated validation logic

dot-style-editor-field-form.component.ts:179-204 reimplements virtually the same logic as fieldHasErrors in models.ts:90-116, with the only difference being the this.$isDuplicateIdentifier() check. If a new validation rule is added to fieldHasErrors, it must also be mirrored in $hasErrors; otherwise the field-level error indicator and the builder's save gate silently diverge. Consider adding an isDuplicate param to fieldHasErrors so it stays the single source of truth.

2. typeInIdentifierInput integration test is fragile

dot-style-editor-builder.component.spec.ts:80-86 dispatches native DOM input events to drive state changes deep inside a child component. This relies on an implicit cascade: DefaultValueAccessor picks up the native event -> ngModelChange fires -> field-form updates its state -> emits fieldChange -> builder calls updateField(). This chain can break silently if the identifier input moves from ngModel to reactive forms, or if OnPush prevents synchronous propagation. The test at line 310-321 is only reliable if the full cascade fires synchronously. A more robust approach is to call spectator.component.updateField() directly with a modified field.

3. console.warn fires on normal first load

dot-style-editor-builder.component.ts:258-260 logs '[StyleEditorBuilder] Invalid schema in metadata' when a content type has no stored schema yet, which is the expected state for every new content type. Pre-existing, but produces noise in logs for every new content type.

4. Missing test: error clears after duplicate is resolved

The new tests verify that duplicates block saving, but none verify that $duplicateIdentifiers() becomes empty once the user gives the conflicting field a unique identifier. The test at line 310 only asserts the API was called, not that the error state cleared.

---

The core logic in getDuplicateIdentifiers and its wiring into $isFormValid, $hasFieldErrors (section), and the [isDuplicateIdentifier] binding in the template is correct. Main risk is item #1: validation logic split across two near-identical code paths.