Contrib.social

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

Open on Drupal.org →
Created on 15 March 2024, 10 months ago
Updated 5 August 2024, 5 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 17 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 10 months ago
  • Comment 10 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.

  • Merge request !7063Make the 'entity' property of ER fields aware of its typed data hierarchy tree. → (Open) created by amateescu
  • Pipeline finished with Success
    10 months ago
    Total: 630s
    #120412
  • Status changed to Needs work 10 months ago
  • Comment 10 months ago →
    needs-review-queue-bot

    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 10 months ago
  • Comment 10 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
    10 months ago
    Total: 601s
    #120719
  • Pipeline finished with Failed
    10 months ago
    Total: 1391s
    #120739
  • Status changed to Needs work 9 months ago
  • Comment 9 months ago →
    needs-review-queue-bot

    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 9 months ago
  • Comment 9 months ago →
    🇫🇷France nod_ Lille

    testbot is having issues

  • Comment 9 months ago →
    🇬🇧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 9 months ago
  • Comment 9 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.

  • Comment 5 months ago →
    🇨🇭Switzerland berdir Switzerland

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

        1)
    Drupal\Tests\field\Kernel\EntityReference\EntityReferenceFormatterTest::testEntityFormatterRecursiveRendering
    PHPUnit\Framework\Exception: Killed
  • Comment 5 months ago →
    🇷🇴Romania amateescu

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

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024