Skip to content

include style editor schemas in the GQL response#35528

Merged
dario-daza merged 18 commits intomainfrom
35454-return-schemas-in-GQL-to-avoid-unnecessary-network-calls
May 7, 2026
Merged

include style editor schemas in the GQL response#35528
dario-daza merged 18 commits intomainfrom
35454-return-schemas-in-GQL-to-avoid-unnecessary-network-calls

Conversation

@dario-daza
Copy link
Copy Markdown
Member

@dario-daza dario-daza commented Apr 30, 2026

Proposed Changes

Removed the ‎api/v1/page/{pageId}/contenttype-schema endpoint previously used to retrieve schemas for a page. This endpoint was only used by Headless pages, so we reused its logic to retrieve Style Editor Schemas and now expose them through GraphQL.

Checklist

  • Remove api/v1/page/{pageId}/contenttype-schema endpoint
  • Refactor Style Editor Schema logic in order to re use it in the GQL and REST Implementation.
  • Expose styleEditorSchemas via GQL
  • Update consumption in the SDK to now retrieve the Schemas from GQL instead of the endpoint.
  • Update integration / postman tests
  • Update FE tests

This PR fixes: #35454

This PR fixes: #35454

@github-actions github-actions Bot added Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code Area : SDK PR changes SDK libraries labels Apr 30, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 30, 2026

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


PR Review

Mostly solid refactor — the legacy /contenttype-schema endpoint goes, the REST path and GQL fetcher both share PageResourceHelper.getStyleEditorSchemas(...), and the SDK pulls the schemas through the page query in one round-trip. Tests cover both modes, including the EDIT_MODE gate. A few things worth a second look:

🔴 Material concerns

1. Rollback contract risk (already flagged twice in this PR — still real).
buildPageQuery always emits styleEditorSchemas on the DotPage fragment (utils.ts:108), and PageAPIGraphQLTypesProvider registers the field in N (line 173). On rollback to N-1, every page query from N's SDK will fail validation: Cannot query field 'styleEditorSchemas' on type 'DotPage'. graphql-java is strict — partial-tolerance won't save you. Either ship the schema field one release ahead of the SDK consumption, or make the field opt-in (e.g. only when a ?schemas=true flag is passed and the SDK queries it conditionally).

2. Extra DB round-trip on every EDIT_MODE GraphQL page load.
StyleEditorSchemasDataFetcher.safeGet calls getStyleEditorSchemasInPage(pageId), which re-queries MultiTreeAPI.getMultiTreesByPage and runs findContentletByIdentifierAnyLanguageAnyVariant per contentlet — even though the page's containers (and the contentlets inside them) are already loaded by ContainersDataFetcher for the same query. The REST path in HTMLPageAssetRenderedBuilder.resolveStyleEditorSchemas already reuses the loaded containers, which is why the comment there says "single computation point". The GQL fetcher should do the same — pull contentlets out of the already-resolved containers on the request context instead of re-loading from MultiTree. As-is, every EDIT_MODE GraphQL page load pays a multi-tree fetch + N findContentlet calls that the REST path avoids.

3. setPageAsset unconditionally clobbers editorStyleSchemas.
withPage.ts:153 writes editorStyleSchemas: payload.pageAsset.page.styleEditorSchemas ?? []. Any caller that passes a pageAsset without schemas (e.g. partial reload, optimistic state, lock/unlock refetch) will silently wipe the loaded schemas. The GQL query includes the field today, but this is a sharp edge — if a future code path passes a stripped-down asset, the style editor will quietly lose state until the next full GQL load. Consider only writing when payload.pageAsset.page.styleEditorSchemas !== undefined, or seeding from a dedicated path so the merge semantics are explicit.

🟡 Worth tightening (review feedback already on PR)

4. page-api.ts:250 — extract schemas after the null-check.
KevinDavilaDotCMS's comment is correct: const styleEditorSchemas = pageResponse ? pageResponse.page.styleEditorSchemas : [] runs before the if (!pageResponse) throw …. It's functionally fine but the pageResponse ? … : [] ternary is dead — by the time you'd use the value, pageResponse is non-null. Move the line below the throw and drop the ternary.

5. PREVIEW_MODE asymmetry — schemas land in injected script but not in PageView.
HTMLPageAssetRenderedBuilder.resolveStyleEditorSchemas returns schemas for any non-LIVE mode (used by injectUVEScript), but applyStyleEditorSchemas only writes them to the response in EDIT_MODE. Documented in the comments and consistent with the GQL fetcher gate, but it does mean PREVIEW_MODE pays the schema-resolution work and emits them inline in the script, while the API response omits them. Intentional, just confirm that's the contract you want before shipping — it's surprising for any client that reads the field from PREVIEW responses.

🟢 Minor

  • transforms.ts:77-78 comment says "GQL returns null when not in EDIT_MODE" but the data fetcher returns Collections.emptyList() (not null) in those cases. The ?? undefined normalization is still defensive, but the doc is slightly off.
  • The OpenAPI styleEditorSchemas type uses $ref: "#/components/schemas/JsonNode" — opaque to consumers. If the SDK already has a StyleEditorFormSchema type, surface it as a richer schema in OpenAPI too.
  • dot-uve.js diff is just a minified-variable rename (hD) — no semantic change. Make sure the editor build (nx run sdk-client:build:js) regenerated this and it's not stale or hand-edited.

Summary

The refactor itself is clean — single source of truth for schema resolution, fewer endpoints, less SDK round-tripping. The two things I'd block on are (1) the rollback contract issue (release-coordination, not code) and (2) the duplicate DB work in the GQL fetcher. The store wipe in setPageAsset is a latent foot-gun worth fixing now while it's still in your head.
| Branch

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 30, 2026

Pull Request Unsafe to Rollback!!!

  • Category: M-3 — REST / GraphQL / Headless API Contract Change
  • Risk Level: 🟡 MEDIUM
  • Why it's unsafe: N adds a new styleEditorSchemas field to the GraphQL Page schema AND updates the TypeScript SDK in the same PR to include styleEditorSchemas in every page query (via buildPageQuery). After rolling back to N-1, the field no longer exists in the GQL schema. Any client running N's SDK will send queries containing styleEditorSchemas, and the N-1 server will reject them with a GraphQL validation error: "Cannot query field 'styleEditorSchemas' on type 'Page'".
  • Code that makes it unsafe:
    • dotCMS/src/main/java/com/dotcms/graphql/business/PageAPIGraphQLTypesProvider.java lines 172-173 — registers the new field in the GQL schema
    • core-web/libs/sdk/client/src/lib/client/page/utils.ts line 115 — adds styleEditorSchemas to the query string emitted by buildPageQuery, meaning every SDK page query will include this field
  • Alternative (if possible): Ship the backend GQL field registration in Release N without updating the SDK query. In Release N+1 (once N-1 is outside the rollback window), update the SDK to query styleEditorSchemas. This decouples the schema addition from the client consumption, leaving N-1 able to serve all in-flight queries unmodified.

…sary-network-calls

# Conflicts:
#	dotCMS/src/main/webapp/ext/uve/dot-uve.js
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 5, 2026

Pull Request Unsafe to Rollback!!!

  • Category: M-3 — REST / GraphQL / Headless API Contract Change
  • Risk Level: 🟡 MEDIUM
  • Why it's unsafe: The PR adds a new styleEditorSchemas field to the GraphQL Page schema AND updates the TypeScript SDK in the same PR to include styleEditorSchemas in every page query (via buildPageQuery). After rolling back to N-1, the field no longer exists in the GQL schema. Any client running N's SDK will send queries containing styleEditorSchemas, and the N-1 server will reject them with a GraphQL validation error: "Cannot query field 'styleEditorSchemas' on type 'Page'".
  • Code that makes it unsafe:
    • dotCMS/src/main/java/com/dotcms/graphql/business/PageAPIGraphQLTypesProvider.java lines 172-173 — registers the new field in the GQL schema
    • core-web/libs/sdk/client/src/lib/client/page/utils.ts line 105 — adds styleEditorSchemas to the query string emitted by buildPageQuery, meaning every SDK page query will include this field
  • Alternative (if possible): Ship the backend GQL field registration in Release N without updating the SDK query. In Release N+1 (once N-1 is outside the rollback window), update the SDK to query styleEditorSchemas. This decouples the schema addition from the client consumption, leaving N-1 able to serve all in-flight queries unmodified.

@dario-daza dario-daza marked this pull request as ready for review May 5, 2026 15:22
Comment thread core-web/libs/sdk/client/src/lib/client/page/page-api.ts Outdated
…sary-network-calls

# Conflicts:
#	core-web/yarn.lock
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 6, 2026

Pull Request Unsafe to Rollback!!!

  • Category: M-3 — REST / GraphQL / Headless API Contract Change
  • Risk Level: 🟡 MEDIUM
  • Why it's unsafe: The PR adds a new styleEditorSchemas field to the GraphQL Page schema AND updates the TypeScript SDK in the same PR to include styleEditorSchemas in every page query (via buildPageQuery). After rolling back to N-1, the field no longer exists in the GQL schema. Any client running N's SDK will send queries containing styleEditorSchemas, and the N-1 server will reject them with a GraphQL validation error: "Cannot query field 'styleEditorSchemas' on type 'Page'".
  • Code that makes it unsafe:
    • dotCMS/src/main/java/com/dotcms/graphql/business/PageAPIGraphQLTypesProvider.java lines 172-173 — registers pageFields.put("styleEditorSchemas", new TypeFetcher(list(ExtendedScalars.Json), new StyleEditorSchemasDataFetcher())) in the GQL schema
    • core-web/libs/sdk/client/src/lib/client/page/utils.ts line 108 — adds styleEditorSchemas to the query string emitted by buildPageQuery, so every SDK page query will include this field
  • Alternative (if possible): Ship the backend GQL field registration in Release N without updating the SDK query. In Release N+1 (once N-1 is outside the rollback window), update the SDK to query styleEditorSchemas. This decouples the schema addition from the client consumption, leaving N-1 able to serve all in-flight queries unmodified.

Pull Request Unsafe to Rollback!!!

  • Category: M-3 — REST / GraphQL / Headless API Contract Change
  • Risk Level: 🟡 MEDIUM
  • Why it's unsafe: The PR removes the GET /api/v1/page/{pageId}/contenttype-schema REST endpoint and its ResponseEntityContentTypeSchemaView response wrapper. Any external/headless client (or N-1 SDK build still in the wild) that calls this endpoint will receive a 404 once N is deployed. Although the endpoint reappears after rollback to N-1, any third-party integration that adopted N's SDK and stopped sending these requests will not regain dependency on it — but more importantly, any consumer that updated to N's contract expectations (e.g., relying on styleEditorSchemas arriving via the page payload instead) will silently miss the data on N-1.
  • Code that makes it unsafe:
    • dotCMS/src/main/java/com/dotcms/rest/api/v1/page/PageResource.java — the entire getPageContentTypeSchemas JAX-RS method (@GET @Path("/{pageId}/contenttype-schema")) is deleted (~58 lines removed)
    • dotCMS/src/main/java/com/dotcms/rest/api/v1/page/ResponseEntityContentTypeSchemaView.java — file deleted entirely
    • dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml/v1/page/{pageId}/contenttype-schema path and ResponseEntityContentTypeSchemaView schema removed
  • Alternative (if possible): Deprecate the REST endpoint in Release N (mark it @Deprecated, log a warning when called) but keep it functional. Add the GQL field in N. Only delete the REST endpoint in Release N+1, once N-1 is outside the rollback window and any external consumers have had a release cycle to migrate.

@dario-daza dario-daza enabled auto-merge May 6, 2026 20:39
@dario-daza dario-daza disabled auto-merge May 6, 2026 20:40
@dario-daza dario-daza added this pull request to the merge queue May 6, 2026
@dario-daza dario-daza removed this pull request from the merge queue due to a manual request May 6, 2026
@dario-daza dario-daza added this pull request to the merge queue May 7, 2026
@dario-daza dario-daza removed this pull request from the merge queue due to a manual request May 7, 2026
@dario-daza dario-daza added this pull request to the merge queue May 7, 2026
Merged via the queue into main with commit 5ae4187 May 7, 2026
52 checks passed
@dario-daza dario-daza deleted the 35454-return-schemas-in-GQL-to-avoid-unnecessary-network-calls branch May 7, 2026 17:09
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 Area : SDK PR changes SDK libraries

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

feat(sdk-client): gate fetchStyleEditorSchemas on UVE edit mode to avoid unnecessary network calls

3 participants