Skip to content

exclude fields from the payload when saving style editor schemas#35981

Open
dario-daza wants to merge 2 commits into
mainfrom
35979-fields-are-deleted-when-saving-style-editor-schema-on-a-content-type
Open

exclude fields from the payload when saving style editor schemas#35981
dario-daza wants to merge 2 commits into
mainfrom
35979-fields-are-deleted-when-saving-style-editor-schema-on-a-content-type

Conversation

@dario-daza
Copy link
Copy Markdown
Member

Proposed Changes

  • Strip fields and systemActionMappings from the content type payload when saving the Style Editor schema
  • Add tests verifying neither property is included in the PUT request

Closes #35979

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Jun 4, 2026

Claude finished @fabrizzio-dotCMS's task in 1m 4s —— View job


Claude finished @fabrizzio-dotCMS's task in 34s —— View job


Review

  • Read changed files
  • Analyze for issues
  • Post findings

Clean fix. Two minor observations worth considering:

1. Two tests do the same setup and assertion pattern — merge or share

dot-style-editor-builder.component.spec.ts:273-293 — Both new tests call setup(MOCK_CONTENT_TYPE_WITH_FIELDS), click save, and inspect mock.calls[0][1]. They could be a single it asserting both properties, or at minimum extract the payload retrieval into a shared helper. As-is the duplication is minor but fragile: mock.calls[0][1] will silently break if call ordering ever shifts.

2. payload type is widened at the call site

dot-style-editor-builder.component.ts:226 — The putData<DotCMSContentType> generic still says DotCMSContentType, but payload is now typed as Omit<DotCMSContentType, 'fields' | 'systemActionMappings'>. TypeScript will accept this (structural subtype), but it means the generic is misleading — the response type and the body type are conflated. Not a bug, but consider putData<DotCMSContentType, Omit<DotCMSContentType, ...>>(url, payload) if DotCrudService supports a second type param, or a brief cast/comment noting the intent.

3. No risk of the fields key being undefined vs absent — confirmed safe. Destructuring removes the key entirely regardless of its value, so the API will never receive fields: undefined.

Overall logic is correct — stripping both keys via destructure before spread is the right approach.

@github-actions github-actions Bot added the Area : Frontend PR changes Angular/TypeScript frontend code label Jun 4, 2026
@dario-daza dario-daza marked this pull request as ready for review June 4, 2026 19:30
@dario-daza dario-daza requested review from KevinDavilaDotCMS and zJaaal and removed request for zJaaal June 4, 2026 19:30
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.

Fields are deleted when saving Style Editor schema on a Content Type

2 participants