make the 'group' property on EntityReferenceSelection plugins implicitly derived

Created on 13 June 2024, 15 days ago
Updated 24 June 2024, 3 days ago

Problem/Motivation

The interplay between the plugin ID and the group property on EntityReferenceSelection plugins is weird.

Despite improving the docs for this #2945789: Document quirks of Entity reference selection plugins while a fix is figured out , it's still confusing 🐛 Misleading documentation in \Drupal\Core\Entity\Annotation\EntityReferenceSelection Active . There is a long-standing issue to fix it 🐛 Entity reference selection plugins break when not following a weird ID pattern Needs work but no way forward.

I have an alternative suggestion: since the group property MUST conform to a certain pattern, remove it from the plugin attribute and derive it in the plugin manager.

The current rule is:

- group and id must be identical, OR
- if id is of the form foo:bar, group must be foo

Therefore, we can remove the group property from the attribute and set it in the plugin manager like this:

- if id does not contain a ':', set group to the same as the id
- if id is of the form foo:bar, set group to foo

We keep the concept of group, because it's used by EntityReferenceItem (see my comments on 🐛 Misleading documentation in \Drupal\Core\Entity\Annotation\EntityReferenceSelection Active for an explanation), but it's now impossible to get it wrong.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

📌 Task
Status

Needs review

Version

11.0 🔥

Component
Entity 

Last updated 1 day ago

Created by

🇬🇧United Kingdom joachim

Live updates comments and jobs are added and updated live.
  • Contributed project blocker

    It denotes an issue that prevents porting of a contributed project to the stable version of Drupal due to missing APIs, regressions, and so on.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @joachim
  • 🇬🇧United Kingdom joachim
  • Status changed to Needs review 15 days ago
  • 🇬🇧United Kingdom joachim
  • Pipeline finished with Failed
    15 days ago
    Total: 189s
    #197982
  • 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium

    Can we still reach out to whoever wrote this to hear why the whole concept of groups was introduced to begin with? I feel like another solution might be to just drop the whole idea of groups.

  • 🇬🇧United Kingdom joachim

    In theory we could do that. The field settings would instead let you pick all the plugins that declare they work with the current entity type.

    But in the SelectionPluginManager there's a lot of logic for selecting the 'best' plugin, so of which seems irrelevant given what EntityReferenceItem does, but which could be in use in the wild. In particular, someone could have added a default:node_extra plugin and relied in the weight to get that selected automatically in a custom form that uses the entity_autocomplete element. There'd be a lot of behaviour to first DOCUMENT (!!!!) then write tests for (there are none!! SURPRISE!!!) and then deprecate with BC. I'm not against doing that, but the MR here is a nice quick solution which means no more headdesking for devs trying to write a plugin.

  • Status changed to Needs work 15 days 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 days ago
  • 🇬🇧United Kingdom joachim
  • Pipeline finished with Failed
    3 days ago
    Total: 603s
    #206862
  • Something not mentioned in the summary is that there can be more than one colon. The ID could be foo:bar:baz

  • I'm pretty sure that when it comes to selection handlers, there can be multiple colons.

  • 🇬🇧United Kingdom joachim

    True, but that won't make any difference. It's the base plugin ID that has to match the group.

Production build 0.69.0 2024