Allow the 'entity' property of entity reference fields to be aware of its position in the typed data tree

Created on 15 March 2024, 3 months ago
Updated 14 April 2024, 2 months ago

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 access check using incorrect revision of its parent, leading to issues editing and viewing paragraphs when content moderation is involved. β†’ :

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.

πŸ“Œ Task
Status

Needs work

Version

11.0 πŸ”₯

Component
EntityΒ  β†’

Last updated about 6 hours ago

Created by

πŸ‡·πŸ‡΄Romania amateescu

Live updates comments and jobs are added and updated live.
  • 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.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @amateescu
  • Status changed to Needs review 3 months ago
  • πŸ‡·πŸ‡΄Romania amateescu

    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.

  • Pipeline finished with Success
    3 months ago
    Total: 630s
    #120412
  • Status changed to Needs work 3 months ago
  • 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.

  • Status changed to Needs review 3 months ago
  • πŸ‡·πŸ‡΄Romania amateescu

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

  • Pipeline finished with Failed
    3 months ago
    Total: 601s
    #120719
  • Pipeline finished with Failed
    3 months ago
    Total: 1391s
    #120739
  • Status changed to Needs work 3 months ago
  • 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.

  • Status changed to Needs review 3 months ago
  • πŸ‡«πŸ‡·France nod_ Lille

    testbot is having issues

  • πŸ‡¬πŸ‡§United Kingdom catch

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

  • Status changed to Needs work 2 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    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.

Production build 0.69.0 2024