EntityReferenceFieldItemList::referencedEntities might not return up-to-date entity objects

Created on 11 November 2016, about 8 years ago

Updated 8 April 2023, over 1 year ago

Problem/Motivation

EntityReferenceFieldItemList::referencedEntities in its current implementation will collect all the target ids from the field items and then collect the referenced entity objects by calling loadMultiple, but this is faulty in the following use cases:

An entity cloned object has been set on some of the field items -> load will not return this one but a different one.
resetCache is called after the entity objects have been initialized on some of the field items.

Proposed resolution

EntityReferenceFieldItemList::referencedEntities should first collect the entities which are already set on the field items and load the remaining ones through loadMultiple.

Remaining tasks

Add test coverage
Review

User interface changes

None.

API changes

None.

Data model changes

None.

🐛 Bug report

Status

Needs work

Version

10.1 ✨

Component

Entity →

Last updated 26 minutes ago

Maintained by
🇬🇧United Kingdom @catch
🇨🇭Switzerland @berdir
🇩🇪Germany @hchonov

Created by

🇩🇪Germany hchonov 🇪🇺🇩🇪🇧🇬

Live updates comments and jobs are added and updated live.

Bug Smash Initiative

Needs Review Queue Initiative
Used to track the progress of issues reviewed by the Drupal Needs Review Queue Initiative.

Needs tests
The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Incomplete comments

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

Comment over 1 year ago →
🇧🇪Belgium dieterholvoet Brussels
EntityReferenceFieldItemList::referencedEntities should just call $this->getValue($include_computed=TRUE) and then from the result leave only the entity objects.

Drupal\Core\TypedData\Plugin\DataType\ItemList::getValue() doesn't have an $include_computed parameter, so that option is off the table.

I'll create a MR implementing the first option.
Comment over 1 year ago →
🇧🇪Belgium dieterholvoet Brussels
@dieterholvoet opened merge request.
Comment over 1 year ago →
🇧🇪Belgium dieterholvoet Brussels
Comment over 1 year ago →
🇺🇸United States smustgrave
There are 2 options it appears in the issue summary proposed solution. Can it be highlighted which was chosen?

Also could use a test case showing the issue.
Comment over 1 year ago →
🇧🇪Belgium dieterholvoet Brussels

contrib.social Blog FAQ Discussions

Production build 0.71.5 2024