Skip to content

Create new endpoint to save style editor schema per content type#36070

Open
dario-daza wants to merge 8 commits into
mainfrom
35781-create-new-endpoint-to-save-style-editor-schema-per-content-type
Open

Create new endpoint to save style editor schema per content type#36070
dario-daza wants to merge 8 commits into
mainfrom
35781-create-new-endpoint-to-save-style-editor-schema-per-content-type

Conversation

@dario-daza

@dario-daza dario-daza commented Jun 9, 2026

Copy link
Copy Markdown
Member

Proposed Changes

Create the new PATCH /v1/contenttype/id/{idOrVar}/metadata endpoint to avoid editing other Content Type information when creating and saving a Style Editor Schema for a Content Type.

BE

  • Create the new PATCH /v1/contenttype/id/{idOrVar}/metadata endpoint to handle the metadata of the Content Types, specifically the DOT_STYLE_EDITOR_SCHEMA that is related to the Style Editor Schema.
  • Fix Postman tests using the new endpoint.
  • Create an integration test for the new endpoint.

FE

  • Call the new endpoint when saving Style Editor Schemas per Content Type.
  • Replace the use of DotCrudService with http.
  • Fix tests.

This PR fixes: #35781
This PR fixes: #35979

@claude

claude Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @dario-daza's task in 3m 14s —— View job


PR Review

  • Gather context and read changed files
  • Review backend changes (ContentTypeResource.java)
  • Review frontend changes (dot-style-editor-builder component)
  • Review integration tests
  • Review Postman collection changes
  • Final summary

Issues Found

1. BadRequestException swallowed as 500 — ContentTypeResource.java:2132

normalizeStyleEditorSchemaToString (line 2155) throws BadRequestException, which extends WebApplicationException. However, the catch block at line 2132 is catch (final Exception e), which intercepts WebApplicationException subclasses before JAX-RS can rethrow them with their intended status. The result: an invalid DOT_STYLE_EDITOR_SCHEMA value returns HTTP 500 instead of 400.

} catch (final Exception e) {
    Logger.error(this, ...);
    return ExceptionMapperUtil.createResponse(e, Response.Status.INTERNAL_SERVER_ERROR); // ← eats BadRequestException
}

Fix: add an explicit catch (final BadRequestException e) above the generic Exception catch, or rethrow WebApplicationException subclasses:

} catch (final BadRequestException e) {
    throw e; // let JAX-RS handle it as 400
} catch (final Exception e) {
    ...

Fix this →


2. Empty-patch path skips normalizeStyleEditorSchemaToString early-return logic but still logs a warning — ContentTypeResource.java:2095-2100

An empty body ({}) returns 200 silently but logs a Logger.warn. An empty PATCH is a perfectly valid no-op; this produces noise in logs and misleads operators. Either return 200 silently or document it as intentional. Fix this →


3. normalizeStyleEditorSchemaToString uses a raw ObjectMapperContentTypeResource.java:2161

The method creates a new ObjectMapper via DotObjectMapperProvider.getInstance().getDefaultObjectMapper() on every call. This is the shared default mapper, but it's invoked inside a private static method that could serialize arbitrary user-supplied Object values. If the value happens to be something complex (e.g., a LinkedHashMap from Jackson deserialization), this should be fine — but the defensive comment in the javadoc ("Page-rendering code downstream casts the stored value directly to String") suggests this normalization is a workaround for a deeper contract violation. Worth a note on whether the API contract should simply reject non-string values rather than silently coercing them.


4. Integration test uses hardcoded admin credentials — ContentTypeResourceUpdateMetadataTest.java:292

"Basic " + Base64.getEncoder().encodeToString("admin@dotcms.com:admin".getBytes())

This is consistent with other integration tests in this codebase, so not a blocker, but worth noting that the test won't run if the default admin password has been changed.


5. Frontend: HTTP URL is relative without leading slash — dot-style-editor-builder.component.ts:213

this.#http.patch<DotCMSResponse>(`v1/contenttype/id/${contentType.id}/metadata`, ...)

No leading /. This works because Angular's dev proxy is configured to handle relative paths, but it creates a subtle dependency on base-href configuration and could break in embedded or non-root deployments. Other dotCMS services typically use the full /api/v1/... prefix. If the existing pattern for this app omits /api/ for backend calls, this is fine — but it should at minimum be consistent with how other HTTP calls in this component or sibling services are constructed.


Minor

  • The $isFormValid computed (line 153) allows sections with zero fields to pass validation since the check is section.fields.length > 0 && .... An empty section (title only, no fields) is blocked. That seems intentional but isn't immediately obvious from the guard structure — the empty-section case silently prevents saving, which could confuse users.
  • No test covers the 403 (forbidden) path in the integration test suite — only 200 and 404 are tested.

Overall the change is well-structured. The new endpoint is additive-only, the merge semantics are correct, and the test coverage is solid. The BadRequestException catch ordering is the only actual bug.

@dario-daza dario-daza marked this pull request as ready for review June 9, 2026 14:57
@dario-daza dario-daza changed the title 35781 create new endpoint to save style editor schema per content type Create new endpoint to save style editor schema per content type Jun 9, 2026
@claude

claude Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Pull Request Unsafe to Rollback!!!

  • Category: M-3 — REST / GraphQL / Headless API Contract Change
  • Risk Level: 🟡 MEDIUM
  • Why it's unsafe: This PR adds a new PATCH /v1/contenttype/id/{idOrVar}/metadata endpoint and updates the Angular frontend component in the same PR to call it (replacing the previous PUT /v1/contenttype/id/{idOrVar} call). If the backend is rolled back to N-1 while a browser or CDN has cached the N-version Angular frontend, any attempt to save a Style Editor Schema will call the new endpoint which does not exist on N-1, resulting in a 404 error. The user will be unable to save Style Editor Schemas until the browser cache clears.
  • Code that makes it unsafe:
    • dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResource.java — new @PATCH @Path("/id/{idOrVar}/metadata") handler added at line 2022+
    • core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/style-editor/dot-style-editor-builder.component.ts — line 247: this.#http.patch\<DotCMSResponse\>(v1/contenttype/id/${contentType.id}/metadata, metadataPatch) replaces the prior DotCrudService.putData call to the old PUT endpoint
    • dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml — new path /v1/contenttype/id/{idOrVar}/metadata added (lines 8148+)
  • Alternative (if possible): Since the new endpoint is purely additive (a new path, no existing path removed or changed), the rollback risk is bounded: only the Style Editor Schema save fails with 404, and only while the N-frontend is cached. If the old PUT /v1/contenttype/id/{idOrVar} endpoint is preserved (which it is — this PR does not remove it), operators can restore full functionality by clearing the CDN cache and/or forcing a frontend refresh after rollback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

Status: No status

1 participant