Problem/Motivation

The entity object obtained from an entity reference field (e.g. through $entity->get('ref_field')->entity) is not aware of its parent typed data object (EntityReferenceItem in this case).

For entity types that need to be aware of their parent/host entity (e.g. Paragraphs), this is a problem because they have to track the parent through an entity reference field, which, if it's not tracking a specific revision, can load the wrong parent revision in various cases (e.g. draft/pending revisions via Content Moderation, reverting the host entity to a previous revision, etc.)

Steps to reproduce

This is not easy to reproduce with core alone (I haven't yet tried the private file scenario described in #3047022-4: Layout builder fails to assign inline block access dependencies for the overrides section storage on entities with pending revisions), but many people bumped into this problem when using core's Layout Builder with Paragraphs. These are the easiest steps to reproduce with those modules, copied from #3090200-21: Paragraph view access check using incorrect revision of its parent, leading to issues viewing paragraphs when reverted host entities or content moderation is used:

1) Create LB controlled page
2) Add block with paragraphs (doesn't need to be nested)
3) Save the layout
4) Edit the layout, editing some of the data in the paragraphs, save the layout
5) Revert to the previous revision
6) Edit the block again
7) You will see the "You are not allowed to edit or remove this ." message

Proposed resolution

Set the typed data context on the target property of \Drupal\Core\Entity\Plugin\DataType\EntityReference and \Drupal\Core\TypedData\DataReferenceBase .

Remaining tasks

Agree that this feature is desirable, review the patch, write test coverage.

User interface changes

Nope.

API changes

Nope.

Data model changes

Entities retrieved from an entity reference field through the entity property will be aware of their position in the typed data tree.

Release notes snippet

TBD.

CommentFileSizeAuthor
#6 3428635-nr-bot.txt1.1 KBneeds-review-queue-bot
#4 3428635-nr-bot.txt1.9 KBneeds-review-queue-bot

Issue fork drupal-3428635

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

amateescu created an issue. See original summary.

amateescu’s picture

amateescu commented
Title: Allow the 'entity' property of entity reference fields to be aware of its hierarchy in the typed data tree » Allow the 'entity' property of entity reference fields to be aware of its position in the typed data tree
Status: Active » Needs review
Issue tags: +Needs architectural review, +Needs tests

Opened a MR with the proposed resolution.

Marking as NR for the architecture of this change. If we get agreement on that, I can follow up with test coverage.

amateescu opened merge request !7063

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3428635-nr-bot.txt1.9 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

amateescu’s picture

amateescu commented
Issue summary: View changes
Status: Needs work » Needs review

Turns out this can be done *a lot* easier by just setting the proper context after the typed data object is instantiated.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3428635-nr-bot.txt1.1 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs work » Needs review

testbot is having issues

catch’s picture

catch
he/him
Primary language English
commented

The fix looks really trivial and it's just exposing the information via existing APIs, so I can't really see a downside here.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: -Needs architectural review

Thanks @catch, removing Needs architectural review tag as this seems to have gotten a thumbs up to pursue.

Moving to NW for test failures though.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

This seems to be causing some kind of recursion or so?

    1)
    Drupal\Tests\field\Kernel\EntityReference\EntityReferenceFormatterTest::testEntityFormatterRecursiveRendering
    PHPUnit\Framework\Exception: Killed
amateescu’s picture

amateescu commented

@Berdir, yup! And I couldn't figure out a good way to prevent it..

steinmb’s picture

steinmb commented

Coming here from #3090200: Paragraph view access check using incorrect revision of its parent, leading to issues viewing paragraphs when reverted host entities or content moderation is used. Regarding the recursive problem, I am reading this right that the first suggested change did not have any failing tests https://git.drupalcode.org/project/drupal/-/merge_requests/7063/pipeline... as the simplified approach have?

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.