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.

Found this while working on #1952394: Add configuration translation user interface module in core.

Files: 
CommentFileSizeAuthor
#14 field-instance-bundle-2134861-14.patch2.4 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 59,338 pass(es). View
#6 field-instance-bundle-2134861-6.patch2.38 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field-instance-bundle-2134861-6.patch. Unable to apply patch. See the log in the details link for more information. View

Comments

tstoeckler’s picture

Looks 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.

yched’s picture

Hm. 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 ?

Gábor Hojtsy’s picture

@yched: yes, agreed.

yched’s picture

Also, 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 ?)

Gábor Hojtsy’s picture

Yeah I did not know what to name it, I'm not attached to parentXYZ at all.

amateescu’s picture

FileSize
2.38 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field-instance-bundle-2134861-6.patch. Unable to apply patch. See the log in the details link for more information. View

Stumbled 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.

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

Awesome, 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.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: field-instance-bundle-2134861-6.patch, failed testing.

amateescu’s picture

Status: Needs work » Needs review
tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community
Gábor Hojtsy’s picture

Issue tags: +sprint
LinL’s picture

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: field-instance-bundle-2134861-6.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
2.4 KB
PASSED: [[SimpleTest]]: [MySQL] 59,338 pass(es). View

Rerolled.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Gábor Hojtsy’s picture

Issue tags: -sprint

Yay, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.