Resolve renderlet parent document id from DocumentContext - #3994
Conversation
|
One or more custom setup steps configured for this repository failed during this Copilot code review run: Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review. Note You can configure setup steps for Copilot code review separately from Copilot cloud agent with a |
There was a problem hiding this comment.
Pull request overview
Updates renderlet requests to resolve the parent document ID from DocumentContext, supporting iframe-mounted editables.
Changes:
- Replaces router parameter lookup with document context.
- Adds focused Jest coverage for router-independent rendering.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
renderlet-content.tsx |
Sources parentDocumentId from document context. |
renderlet-content.test.tsx |
Tests context-derived API parameters. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
assets/js/src/core/modules/element/dynamic-types/definitions/document/editable/components/renderlet-editable/renderlet-content.test.tsx:14
- The generated client contract is still optional (
document-api-slice.gen.ts:375declaresparentDocumentId?: number). Typing this mock with that interface does not detect the drift—an optional field remains accepted—and the runtime assertion only verifies this component's current value. This contradicts the PR's required-parameter goal and the companion backend contract; keep the generated field required (or regenerate after the backend schema is available).
import { type DocumentRenderletRenderApiArg } from '@Pimcore/modules/document/document-api-slice.gen'
|
One or more custom setup steps configured for this repository failed during this Copilot code review run: Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review. Note You can configure setup steps for Copilot code review separately from Copilot cloud agent with a |
ValeriaMaltseva
left a comment
ValeriaMaltseva
left a comment
There was a problem hiding this comment.
@robertSt7 LGTM, let's merge it :)
RenderletContent used useParams() to read the open document's id, but that hook only resolves inside the main app's router tree. The document editor canvas mounts editables in a separate React root (inside an iframe) that has no router, so the id came back undefined there and the renderlet render request lost its parent document context. Read the id from DocumentContext instead, which is provided in both mounting locations. Backport of #3994 (2026.2) onto 2025.4. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
393eb36 to
7cf158e
Compare
|
RenderletContentuseduseParams()to read the open document's id, but that hook only resolves inside the main app's router tree. The document editor canvas mounts editables in a separate React root (inside an iframe) with no router, so the id came backundefinedthere. This reads the id fromDocumentContextinstead, which is provided in both mounting locations, and makesparentDocumentIdrequired to match the now-mandatory backend parameter.Companion fix: pimcore/studio-backend-bundle#1999
Related: pimcore/service-operations#848
Test plan
RenderletContentwith onlyDocumentContext(no router) and assertingparentDocumentIdis sentjestpasses on the touched test file