Skip to content

Fix relations/properties pointing to document folders - #1105

Merged
robertSt7 merged 3 commits into
pimcore:2026.2from
blankse:fix_document_folder_relation
Sep 7, 2026
Merged

Fix relations/properties pointing to document folders#1105
robertSt7 merged 3 commits into
pimcore:2026.2from
blankse:fix_document_folder_relation

Conversation

@blankse

@blankse blankse commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Bug

When a relation, an object/document property or an editable relation targets a document folder, the GraphQL query fails with:

unknown document type: document_folder   (GraphQL/Service.php, getDocumentTypeDefinition)

The resolveType() methods of the relation/property resolvers handle the document element type via getDocumentTypeDefinition('document_' . $documentType). For a folder, $documentType is folderdocument_folder, which is not a registered type (only _document_folder is). This makes any query that can resolve a document folder through such a field error out.

Fix

Add a folder guard that returns the registered _document_folder type — mirroring how object folders are already handled (_object_folder).

Affected resolvers:

  • src/GraphQL/DataObjectType/AbstractRelationsType.php
  • src/GraphQL/General/AnyTargetType.php
  • src/GraphQL/General/AnyDocumentTargetType.php
  • src/GraphQL/PropertyType/ObjectsType.php

Notes

Analogous to the object-folder fix. php -l clean on all four files.

Related

Complementary to #1126. This PR only adds the resolveType() folder guard (so a folder value resolves to _document_folder). For that resolved type to be a valid member of the relation/property unions, it also needs #1126, which makes the document union folder-inclusive. Both are required for relations/properties targeting a document folder — please merge together.

@blankse
blankse force-pushed the fix_document_folder_relation branch from 1697967 to 72cc832 Compare June 9, 2026 12:50
@blankse
blankse changed the base branch from 1.9 to 2026.1 June 9, 2026 12:50
@sonarqubecloud

sonarqubecloud Bot commented Jun 9, 2026

Copy link
Copy Markdown

1 similar comment
@sonarqubecloud

sonarqubecloud Bot commented Jun 9, 2026

Copy link
Copy Markdown

@robertSt7
robertSt7 changed the base branch from 2026.1 to 2026.2 July 9, 2026 13:14
Copilot AI review requested due to automatic review settings July 22, 2026 16:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes GraphQL type resolution for document-folder relation and property targets. The PR also includes two competing generated Studio builds, unrelated to the stated fix.

Changes:

  • Maps document folders to _document_folder in four resolvers.
  • Adds two Studio build artifact sets and removes legacy manifests.
  • Updates generated public paths and entrypoints.

Reviewed changes

Copilot reviewed 26 out of 48 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/GraphQL/DataObjectType/AbstractRelationsType.php Resolves relation document folders.
src/GraphQL/General/AnyTargetType.php Resolves generic document-folder targets.
src/GraphQL/General/AnyDocumentTargetType.php Resolves document-only folder targets.
src/GraphQL/PropertyType/ObjectsType.php Resolves property document folders.
.../987f95c1.../static/js/remoteEntry.js.LICENSE.txt Adds generated license notices.
.../987f95c1.../static/js/main.9c3ad8a2.js.LICENSE.txt Adds generated license notice.
.../987f95c1.../static/js/async/840.4693a4bb.js.LICENSE.txt Adds dependency licenses.
.../987f95c1.../static/js/async/696.3b1d6da3.js.LICENSE.txt Adds generated license notice.
.../987f95c1.../static/js/async/696.3b1d6da3.js Adds compiled Studio chunk.
.../987f95c1.../static/js/async/__federation_expose_plugins.c4cf01bd.js.LICENSE.txt Adds generated license notice.
.../987f95c1.../static/js/async/__federation_expose_plugins.c4cf01bd.js Adds compiled plugin exposure.
.../987f95c1.../static/js/async/__federation_expose_default_export.87553f32.js.LICENSE.txt Adds generated license notice.
.../987f95c1.../static/js/async/__federation_expose_default_export.87553f32.js Adds compiled SDK exposure.
.../987f95c1.../static/js/109.62a4c31b.js.LICENSE.txt Adds generated license notices.
.../987f95c1.../mf-stats.json Updates generated public path.
.../987f95c1.../mf-manifest.json Updates generated public path.
.../987f95c1.../manifest.json Adds build manifest.
.../987f95c1.../main.html Adds generated HTML entry.
.../987f95c1.../exposeRemote.js Registers the generated remote.
.../987f95c1.../entrypoints.json Adds build entrypoints.
.../785fd5b2.../manifest.json Removes legacy build manifest.
.../785fd5b2.../entrypoints.json Removes legacy entrypoints.
.../04ab6b52.../static/js/remoteEntry.js.LICENSE.txt Adds generated license notices.
.../04ab6b52.../static/js/main.9c3ad8a2.js.LICENSE.txt Adds generated license notice.
.../04ab6b52.../static/js/async/840.b584be96.js.LICENSE.txt Adds dependency licenses.
.../04ab6b52.../static/js/async/499.4aead98c.js.LICENSE.txt Adds generated license notice.
.../04ab6b52.../static/js/async/499.4aead98c.js Adds compiled Studio chunk.
.../04ab6b52.../static/js/async/__federation_expose_plugins.85a68c1a.js.LICENSE.txt Adds generated license notice.
.../04ab6b52.../static/js/async/__federation_expose_default_export.062d3a0b.js.LICENSE.txt Adds generated license notice.
.../04ab6b52.../static/js/async/__federation_expose_default_export.062d3a0b.js Adds compiled SDK exposure.
.../04ab6b52.../static/js/109.62a4c31b.js.LICENSE.txt Adds generated license notices.
.../04ab6b52.../mf-stats.json Adds federation build statistics.
.../04ab6b52.../mf-manifest.json Adds federation manifest.
.../04ab6b52.../manifest.json Adds build manifest.
.../04ab6b52.../main.html Updates generated script paths.
.../04ab6b52.../exposeRemote.js Updates remote registration path.
.../04ab6b52.../entrypoints.json Adds competing build entrypoints.
Files not reviewed (3)
  • src/Resources/public/studio/build/04ab6b52-dfd5-4e8c-9d1e-8e789ac66855/static/js/async/499.4aead98c.js: Generated file
  • src/Resources/public/studio/build/04ab6b52-dfd5-4e8c-9d1e-8e789ac66855/static/js/async/__federation_expose_default_export.062d3a0b.js: Generated file
  • src/Resources/public/studio/build/04ab6b52-dfd5-4e8c-9d1e-8e789ac66855/static/js/async/__federation_expose_plugins.85a68c1a.js: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/GraphQL/DataObjectType/AbstractRelationsType.php
Comment thread src/GraphQL/PropertyType/ObjectsType.php
All document types have type and properties fields, but the relation
type resolvers did not handle folders, pointing them to a non-existing
'document_folder' type definition instead of the registered
'_document_folder'.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@blankse
blankse force-pushed the fix_document_folder_relation branch from fe6c60d to 8b2619f Compare July 22, 2026 17:06
@blankse
blankse force-pushed the fix_document_folder_relation branch from 6b8034b to 2273097 Compare July 22, 2026 17:40
@robertSt7 robertSt7 self-assigned this Aug 7, 2026
@robertSt7 robertSt7 added the Bug label Aug 7, 2026
@robertSt7
robertSt7 force-pushed the fix_document_folder_relation branch from f43acf4 to 2273097 Compare August 7, 2026 09:47
@robertSt7 robertSt7 added this to the 2026.2.4 milestone Aug 7, 2026
@robertSt7

robertSt7 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@blankse Could you please add a regression test for this fix? tests/GraphQL/ResolveTest.php doesn't currently cover relation/property/editable resolution, so nothing would catch this case. Ideally it'd test resolving a relation/property pointing at a document folder, together with #1126 since both are needed for it to actually work end-to-end.

@jcPimcore jcPimcore modified the milestones: 2026.2.4, 2026.2.5, 2026.2.6 Aug 11, 2026
@astapc

astapc commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

@blankse ping

@berfinyuksel
berfinyuksel requested review from robertSt7 and removed request for berfinyuksel and bluvulture August 21, 2026 08:24
@jcPimcore jcPimcore modified the milestones: 2026.2.6, 2026.2.7 Aug 25, 2026
Covers all four resolvers touched by this fix (AbstractRelationsType via
HrefType, PropertyType\ObjectsType, AnyTargetType, AnyDocumentTargetType):
a relation/property/hotspot metadata value pointing at a document folder
resolves to the '_document_folder' type, while a page still resolves to
'document_page'. Without the fix the folder case looked up
'document_folder' and threw "unknown document type".

Service::getDocumentDataTypes() is added as the counterpart of the existing
getDataObjectDataTypes() so the test can restore the registered document
types after temporarily registering its own.
@blankse

blankse commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Added in f1b9205: tests/GraphQL/DocumentFolderResolveTypeTest.php.

It creates a real Document\Folder and a Document\Page (via TestHelper) and asserts resolveType() on all four resolvers this PR touches — AbstractRelationsType (through HrefType), PropertyType\ObjectsType, AnyTargetType and AnyDocumentTargetType — returns the _document_folder type for the folder and still document_page for the page. Without the fix the folder case looks up document_folder and throws "unknown document type", so the test fails on the old code.

Two notes:

  • I added Service::getDocumentDataTypes() as the counterpart of the existing getDataObjectDataTypes(), so the test can restore the registered document types after temporarily registering its own instead of reaching into the object with reflection (SonarCloud flags that as php:S3011). Say the word if you would rather not widen the API and I will find another way.
  • The end-to-end half you mentioned really does need Make getDocument folder-safe (resolve document folders) #1126: this PR only makes resolveType() return the folder type, whereas the union has to contain _document_folder for a query to succeed — that is DocumentType::getTypes() in Make getDocument folder-safe (resolve document folders) #1126. So a test that runs an actual query cannot be green on this branch alone. I can add the union-membership test (DocumentType::getTypes() contains _document_folder, no duplicate members in AnyTargetType/AnyDocumentTargetType) to Make getDocument folder-safe (resolve document folders) #1126, and if you want a real query-level test once both are merged, I would add it as a follow-up on the target branch. Just tell me which you prefer.

@sonarqubecloud

sonarqubecloud Bot commented Sep 1, 2026

Copy link
Copy Markdown

@robertSt7
robertSt7 merged commit ec0c5b7 into pimcore:2026.2 Sep 7, 2026
24 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 7, 2026
@robertSt7

Copy link
Copy Markdown
Contributor

@blankse Thanks for fixing this pointing

@blankse
blankse deleted the fix_document_folder_relation branch September 7, 2026 11:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants