Skip to content

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

Open
dario-daza wants to merge 9 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 9 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 1m 46s —— View job


Code Review

  • Gather context
  • Review ContentTypeResource.java (new PATCH endpoint)
  • Review ContentTypeResourceUpdateMetadataTest.java (integration tests)
  • Review frontend changes
  • Review Postman collection changes
  • Post findings

Findings

1. idOrVar null/empty guard is dead code
ContentTypeResource.java, line 2090:

if (!UtilMethods.isSet(idOrVar)) {
    return Response.status(Response.Status.NOT_FOUND).build();
}

JAX-RS path parameters are never null or blank at this point — if the path segment is missing, the router returns 404 before the method is invoked. This block never executes and gives a false impression of safety. Either remove it or add a comment explaining the intent. It's not harmful, just misleading.

2. DOT_STYLE_EDITOR_SCHEMA string is not a constant
The magic string "DOT_STYLE_EDITOR_SCHEMA" appears five times in ContentTypeResource.java alone, and again in PageResourceHelper.java (line 901) and HTMLPageAssetRenderedBuilder.java (line 489). Any typo in the new endpoint silently writes to a different key that no other component reads. Extracting it to a shared constant would cost one line and prevent an entire class of silent bug.

Fix this →

3. normalizeStyleEditorSchemaToString only fires for one specific key
The method is called before the merge loop, so it normalises DOT_STYLE_EDITOR_SCHEMA sent as a JSON object. That is fine for the current use case, but the Javadoc says the endpoint is a generic metadata merge. If a future caller sends any other value as a nested object (e.g. CONTENT_EDITOR2_ENABLED as {"value": true}) it will be stored as a raw LinkedHashMap — whatever downstream cast that causes will surface silently at render time, not here. This is a design smell worth noting even if not blocking.

4. Frontend: DotCMSResponse type parameter is not provided
dot-style-editor-builder.component.ts, line 247:

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

DotCMSResponse<T> is generic. Using the raw form DotCMSResponse (no type argument) silently defaults entity to unknown. The next handler discards the response entirely, so this is not a runtime problem right now, but it is a type-safety gap. Consider DotCMSResponse<Record<string, unknown>> or just unknown if the response is intentionally ignored.

5. Integration test hardcodes admin credentials
ContentTypeResourceUpdateMetadataTest.java, line 291:

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

This pattern matches many other tests in the project, so it is not unique to this PR, but it is worth flagging: if the integration environment ever changes the default admin password this test silently authenticates as unauthenticated (returns 401 instead of 403/404, masking the actual assertion failures). Consider using the IntegrationTestInitService user lookup pattern used elsewhere.

6. Empty-patch early return re-loads and returns the content type unnecessarily
ContentTypeResource.java, line 2096–2099:

Logger.warn(this, "No metadata patch found for " + idOrVar);
final Map<String, Object> responseMap = new HashMap<>(
        contentTypeHelper.contentTypeToMap(existing, user)
);
return Response.ok(new ResponseEntityContentTypeDetailView(responseMap)).build();

An existing content-type fetch already happened before this guard. That is one DB round-trip wasted on what is arguably a bad-request scenario. A 400 response ("patch body must not be empty") would be more honest and avoid the useless contentTypeToMap call. Returning 200 with the unchanged state is not wrong, just odd — at minimum the Logger.warn level should be debug since this is an expected client behaviour (idempotent no-op), not a server warning.

WorkflowResource cleanup (.toList() replacement) is correct and unrelated housekeeping — no issues.

Rollback risk (already noted in the prior comment): the frontend switches to the new endpoint in the same PR, so a backend-only rollback leaves the N-frontend calling a 404 path. The old PUT endpoint is preserved, so the blast radius is contained to Style Editor Schema saves only, and only until the frontend asset cache expires.

Overall the implementation is clean and well-tested. The issues above are low-risk: #2 (missing constant) is the most likely to cause a future silent 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