[Data Object] Report whether a field can take part in inheritance by ValeriaMaltseva · Pull Request #2025 · pimcore/studio-backend-bundle

ValeriaMaltseva · 2026-09-03T11:45:20Z

Why

Related issue: pimcore/studio-ui-bundle#1045

The Studio UI wants to offer a Restore action on a field that carries its own value instead of the inherited one, so it can be given back to its origin object. The condition it needs is "this field takes part in inheritance, but this object holds its own value".

The second half is already in the response. The first half is not.

InheritanceService::processFieldDefinition() returns early for a field that cannot inherit at all:

if ($adapter === null || $fieldDefinition->supportsInheritance() === false) {
    return new InheritanceData($object->getId());
}

InheritanceData had no way to carry that, so the result is identical to a field with an own value. On the demo Car class that is 5 of the root fields — urlSlug (UrlSlug) and four DataQuality fields from DataQualityManagementBundle — all indistinguishable from a real override:

urlSlug : {objectId: 318, inherited: false}
carClass: {objectId: 318, inherited: false}

The frontend cannot fill this in itself. supportsInheritance() is open for extension — DataQuality above comes from a bundle — so no list held in the UI can ever be complete.

What

InheritanceData gains inheritable, and the early return above is the only place that turns it off. Everything past that point does take part in inheritance, so the origin walk in this service and the two constructions in ClassificationStoreAdapter keep the default and are unchanged. ObjectBricksAdapter delegates per leaf field to processFieldDefinition(), so bricks are covered too.

-urlSlug : {objectId: 318, inherited: false}
+urlSlug : {objectId: 318, inherited: false, inheritable: false}

-carClass: {objectId: 318, inherited: false}
+carClass: {objectId: 318, inherited: false, inheritable: true}

No tree walk, no extra queries — the flag is a value the service already computes and previously discarded.

Compatibility

Additive property with a default on an @internal final readonly DTO. The two existing properties are untouched, and a client that does not know the new one is unaffected. InheritanceData is also used by the grid column resolvers, where the addition is likewise additive.

Verified

tests/Unit/DataObject/Service/InheritanceServiceTest.php — 4 tests, 15 assertions, covering both branches that clear the flag (no adapter, supportsInheritance() === false), a field with an own value, and a field resolved from an ancestor. Run in Docker: vendor/bin/codecept run Unit — OK.
PHPStan on the three changed files — no errors.
Serialisation checked against the response shape: {"objectId":318,"inherited":false,"inheritable":false}. There are no serialization groups or custom normalizers on these DTOs, so the new getter is picked up like the existing two.
php-cs-fixer is not installed in this environment, so formatting is left to CI.

🤖 Generated with Claude Code

The inheritance data of a field carried only its origin object and whether the value currently comes from an ancestor. A field type that cannot inherit at all - UrlSlug, CalculatedValue, ReverseObjectRelation, Consent, Fieldcollections, and any type a bundle contributes without a Studio data adapter - is reported as not inherited, which is byte for byte what a field carrying its own value looks like. A client that wants to offer giving a field back to its origin object has to tell those apart, and cannot: the list of types that opt out of inheritance is open, so it is not something a frontend can hold. InheritanceData now carries the answer as `inheritable`. The service already computes it in order to take the early return, so the flag is only ever turned off there. Everything past that point does take part in inheritance, so the other places that build the data - the origin walk here and the two in ClassificationStoreAdapter - keep the default and stay unchanged. Additive on an @internal DTO with a default, so the two existing properties of the response are untouched and a client that does not know the new one is unaffected. Frontend counterpart: pimcore/studio-ui-bundle#1045 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

The new client-facing property is absent from the OpenAPI contract used by generated consumers.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds inheritance eligibility metadata needed by Studio UI’s Restore action.

Changes:

Adds inheritable to InheritanceData.
Marks unsupported fields as non-inheritable.
Adds focused service regression tests.

Review contract:

Root cause addressed at the owning service/DTO boundary (InheritanceService.php:78-82).
All constructor call sites remain compatible through the default value.
Change is additive and the DTO is internal.
Regression tests cover both false branches and inherited/own values.
OpenAPI response documentation remains incomplete (InheritanceData.php:24).

File summaries

File	Description
`src/DataObject/Data/Model/InheritanceData.php`	Adds inheritance eligibility metadata.
`src/DataObject/Service/InheritanceService.php`	Marks unsupported fields as non-inheritable.
`tests/Unit/DataObject/Service/InheritanceServiceTest.php`	Tests inheritance eligibility and origin resolution.

Review details

Files reviewed: 3/3 changed files
Comments generated: 2
Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

The inheritable flag was added to the InheritanceData response member without reaching the OpenAPI contract, so generated clients could not discover or type it. DataObjectDetail and grid ColumnData documented inheritance only as a generic object with a hand-written example. Declare InheritanceData as a component schema and reference it from both response shapes - as additionalProperties for the field-keyed DataObjectDetail map, and as a nullable ref on ColumnData. Co-Authored-By: Claude <noreply@anthropic.com>

Copilot

🟡 Changes recommended

The OpenAPI schemas currently misrepresent nested inheritance payloads.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 5/5 changed files
Comments generated: 2
Review effort level: Balanced

Referencing InheritanceData from additionalProperties constrained every top-level entry to the leaf DTO, but the service returns the data under a container key, and localized fields, object bricks and classification stores add further nested maps below that. The generated contract would have rejected the real response shape. Document the map as free-form and correct the description and example, which showed field keys at the top level instead of the container key. Co-Authored-By: Claude <noreply@anthropic.com>

Referencing the leaf InheritanceData schema documented only one of the two shapes the column can carry: AdapterResolver::getInheritanceData() passes through the nested maps built for localized fields, object bricks and classification stores, which the ref would reject. Document the property as anyOf the leaf schema and a free-form object, keeping the leaf typed for generated clients. anyOf rather than oneOf, because a leaf payload also matches the free-form branch and exactly-one matching would reject it. Co-Authored-By: Claude <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Custom inheritable fields without a registered Studio adapter are incorrectly reported as non-inheritable.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Balanced

…inherit A missing data adapter only means Studio has no adapter registered for the field type; it says nothing about whether the Pimcore field can take part in inheritance. A custom field can return true from supportsInheritance() without appearing in the adapter mapping, and was reported as not inheritable, contrary to the documented meaning of the flag. Turn the flag off only for supportsInheritance() === false and let a missing adapter fall through to the generic origin walk, which needs no adapter. Cover the missing-adapter field both with an own value and resolved from an ancestor, and the non-inheriting field type with no adapter present. Co-Authored-By: Claude <noreply@anthropic.com>

sonarqubecloud · 2026-09-03T13:02:20Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot

🟡 Changes recommended

Classification-store leaves can be misreported and the grid OpenAPI schema excludes an existing response shape.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/DataObject/Data/Model/InheritanceData.php:50

Defaulting omitted values to true misreports non-inheritable classification-store leaf types. src/DataObject/Data/Adapter/ClassificationStoreAdapter.php:175-185 computes an origin and constructs InheritanceData without this argument for every key, while classification stores support calculated-value keys whose Pimcore getter bypasses inheritance and whose definition reports supportsInheritance() === false. Those keys will now serialize inheritable: true, potentially exposing an invalid Restore action. Route each leaf through processFieldDefinition() or explicitly handle supportsInheritance() before relying on this default.

src/Grid/Schema/ColumnData.php:45

This schema excludes a response shape already produced by the declared array branch: ClassificationStoreAdapter::getFieldInheritance() returns a numerically indexed array at src/DataObject/Data/Adapter/ClassificationStoreAdapter.php:165-168, which serializes as a JSON array, not an object/map. Add an array alternative (with InheritanceData items) so generated clients do not reject valid grid responses.

            anyOf: [
                new Schema(ref: InheritanceData::class),
                new Schema(type: 'object', additionalProperties: true),
            ],

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Balanced

Copilot

🟡 Changes recommended

Classification-store child fields that do not support inheritance are still reported as inheritable.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Balanced

+            type: 'boolean',
+            example: true
+        )]
+        private bool $inheritable = true


Copilot AI balanced review requested due to automatic review settings September 3, 2026 11:45

Copilot started reviewing on behalf of ValeriaMaltseva September 3, 2026 11:45 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread src/DataObject/Data/Model/InheritanceData.php

Comment thread src/DataObject/Data/Model/InheritanceData.php Outdated

ValeriaMaltseva added this to the 2026.3.0 milestone Sep 3, 2026

ValeriaMaltseva and others added 2 commits September 3, 2026 13:52

Fix typo in inheritance documentation comment

369dffe

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

ValeriaMaltseva requested a balanced review from Copilot September 3, 2026 12:13

Copilot started reviewing on behalf of ValeriaMaltseva September 3, 2026 12:14 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread src/DataObject/Schema/DataObjectDetail.php Outdated

Comment thread src/Grid/Schema/ColumnData.php

ValeriaMaltseva and others added 2 commits September 3, 2026 14:38

ValeriaMaltseva requested a balanced review from Copilot September 3, 2026 12:51

Copilot started reviewing on behalf of ValeriaMaltseva September 3, 2026 12:52 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread src/DataObject/Service/InheritanceService.php Outdated

ValeriaMaltseva requested a balanced review from Copilot September 3, 2026 13:02

Copilot started reviewing on behalf of ValeriaMaltseva September 3, 2026 13:03 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread src/DataObject/Service/InheritanceService.php

Copilot started work on behalf of ValeriaMaltseva September 3, 2026 13:10 View session

Copilot finished work on behalf of ValeriaMaltseva September 3, 2026 13:11

ValeriaMaltseva requested a balanced review from Copilot September 3, 2026 13:12

Copilot started reviewing on behalf of ValeriaMaltseva September 3, 2026 13:12 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread src/DataObject/Data/Model/InheritanceData.php

type: 'boolean',

example: true

)]

private bool $inheritable = true

ValeriaMaltseva closed this Sep 3, 2026

github-actions Bot locked and limited conversation to collaborators Sep 3, 2026

Navigation Menu

Uh oh!

[Data Object] Report whether a field can take part in inheritance - #2025

[Data Object] Report whether a field can take part in inheritance#2025
ValeriaMaltseva wants to merge 6 commits into
2026.xfrom
feature/1045-report-field-inheritable-2026x

ValeriaMaltseva commented Sep 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

sonarqubecloud Bot commented Sep 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ValeriaMaltseva commented Sep 3, 2026

Why

What

Compatibility

Verified

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

sonarqubecloud Bot commented Sep 3, 2026

Quality Gate passed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants