Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Field instances have no method to return the bundle the instance is attached to. EntityInterface::bundle() always returns 'field_instance' for field instances, as they have no bundles of their own. The 'bundle' property stores the information we need, but it should be a) a method on an interface and b) not so easy to confuse with EntityInterface::bundle().
Proposed resolution
Add a method to get the parent bundle, instead of resorting to direct property access (or extracting bundle name from the field instance ID with explode()).
Remaining tasks
- Discuss.
- Review.
- Commit.
User interface changes
None.
API changes
Added method to access the parent bundle name.
Related Issues
Found this while working on #1952394: Add configuration translation user interface module in core.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | field-instance-bundle-2134861-14.patch | 2.4 KB | Gábor Hojtsy |
| #6 | field-instance-bundle-2134861-6.patch | 2.38 KB | amateescu |
Comments
Comment #1
tstoecklerLooks great. We should also add the method to the interface, though.
Leaving at needs review, for some input from some of the field API folks.
Comment #2
yched CreditAttribution: yched commentedHm. Until recently, $field and $instance properties were supposed to be accessed directly as public properties. Then #1953408: Remove ArrayAccess BC layer from field config entities moved most of those accesses to methods from FieldDefinitionInterface, I was semi-ok with that :-) (and writing is still done through direct property access, FieldDefinitionInterface currently only provides getters, no setters)
Thing is: FieldDefinitionInterface doesn't include getBundle(): not all "field definitions" are assigned to a specific entity bundle. So this would be a method for FieldInstanceInterface ?
Note that it's also true of 'entity_type', it's only accessible through the property right now - although that could/should now be part of FieldDefinitionInterface - I opened #2134967: FieldDefinitionInterface should include a getTargetEntityTypeId() for that.
So more generally, this is about the properties in $field / $instance that are not covered by FieldDefinitionInterface getters ?
Comment #3
Gábor Hojtsy@yched: yes, agreed.
Comment #4
yched CreditAttribution: yched commentedAlso, I think in similar cases (config entities pointing to a given bundle, and bundle() being already taken by EntityInterface) we tend to use targetBundle() rather than parentBundle().
(although more generally, I don't really get why config entity types have bundles, that's useless and confusing IMO - well, that's a different issue, opened #2134985: Limit the concept of 'bundle' to ContentEntities ?)
Comment #5
Gábor HojtsyYeah I did not know what to name it, I'm not attached to parentXYZ at all.
Comment #6
amateescu CreditAttribution: amateescu commentedStumbled upon this as a @todo while I was working on another patch, and I agree that targetBundle() is more inline with our current naming strategy.
Comment #7
tstoecklerAwesome, so @Gábor Hojtsy and @yched agree on this. Code looks good. And due to the change in config_translation.module this actually has some test coverage.
Comment #9
amateescu CreditAttribution: amateescu commented6: field-instance-bundle-2134861-6.patch queued for re-testing.
Comment #10
tstoecklerComment #11
Gábor HojtsyComment #12
LinL CreditAttribution: LinL commented6: field-instance-bundle-2134861-6.patch queued for re-testing.
Comment #14
Gábor HojtsyRerolled.
Comment #15
webchickCommitted and pushed to 8.x. Thanks!
Comment #16
Gábor HojtsyYay, thanks!