Create method EntityReferenceFieldItemList::referencedIds()

Created on 30 October 2023, about 1 year ago
Updated 1 December 2023, about 1 year ago

Problem/Motivation

Provide an easy way to get referenced IDs from an entity reference field.

// Current solution.
$tids = array_column($node->field_tags->getValue(), 'target_id');

Steps to reproduce

-

Proposed resolution

// Proposed solution.
$tids = $node->field_tags->referencedIds();

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

โœจ Feature request
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Fieldย  โ†’

Last updated about 2 hours ago

Created by

๐Ÿ‡จ๐Ÿ‡ฆCanada jigarius Montrรฉal

Live updates comments and jobs are added and updated live.
  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

Sign in to follow issues

Comments & Activities

  • Issue created by @jigarius
  • @jigarius opened merge request.
  • Status changed to Needs review about 1 year ago
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada jigarius Montrรฉal

    Created an MR. Here's how ::referencedIds() works.

    1. Returns Target IDs, respecting deltas
    2. Invalid IDs are returned; there is no way to validate them, but that's normal.
    3. NULL items are not returned.

    Please let me know if you want me to change anything.

  • Status changed to Needs work about 1 year ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Same as the other, will need a CR.

  • Status changed to Needs review about 1 year ago
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada jigarius Montrรฉal

    Oops. Added a change record (unpublished).

  • Issue was unassigned.
  • Status changed to RTBC about 1 year ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Thanks, and good job adding examples to the CR.

  • Status changed to Needs review about 1 year ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States xjm

    This should have a subsystem maintainer review since it's altering the public API of entity references. Thanks!

  • Status changed to Needs work about 1 year ago
  • ๐Ÿ‡จ๐Ÿ‡ญSwitzerland berdir Switzerland

    Adding a new method to EntityReferenceFieldItemListInterface would break all modules implementing that, like entity_reference_revisions and I guess dynamic_entity_reference as well.

    Also, this method doesn't make sense for those other implementations, ERR has two ids (id and revision id), DER has entity type and id as the entity type can vary. So it would need to be a new interface only implemented by the core entity reference field. I'm not sure that's worth it, the array_column() approach isn't that complex.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bradjones1 Digital Nomad Life

    Came here to share similar sentiments as #8 and also, yeah, some implementations also would require an entity_type per reference to be meaningful. As proposed, if you used this on a DER field you would get an array of arrays (I think?) or at the very least IDs that were commingled among different entity types. I think the general idea here is good but we are trying to make entity references more robust, and this hard-codes in some assumptions around the core implementation.

  • ๐Ÿ‡จ๐Ÿ‡ฆCanada jigarius Montrรฉal

    Thanks for all the feedback. A different interface for a simple entity reference that just contains a target_id makes sense. However, I'll think aloud to get some more opinions.

    • All field types have can have multiple columns to store the "value", which is returned by "::getValue()".
    • So, if we want to check if a multi-value text field contains the value "foo", we'll have to do the same "array_column($entity->field->getValue())" exercise; which is perfectly fine, but looks kind of bulky (which is what motivated me to create this issue).
    • What do you opine about having a generic way of getting a particular column or columns from a field's "value"?
    • Say, something like ::getValue(array $columns = []);? If $columns is empty, then "::getValue()" behaves like it does right now; if it contains exactly one value, it returns just that column; if it contains multiple values, it returns an array of arrays containing just the specified columns?
    • Maybe it can be a separate method instead of "::getValue()", say "::getColumn()".

    Again, I'm just thinking aloud to see how you feel about providing a cleaner, easier-to-read way of getting an array of values in a field. A majority of simple fields have just one "value" column that contains the main value, so usually, devs would do something like, $entity->field->getColumn("value") to get something like, [0 => "foo", 1 => "bar"], etc.

    If this doesn't sound appealing and if the general opinion is that the dev should repeat "array_column()" for such cases, I will mark the issue as closed.

  • ๐Ÿ‡จ๐Ÿ‡ญSwitzerland berdir Switzerland

    getColumn() is an interesting thought, could be nice, but again, we need to think about BC. However, if we add it on FieldItemList then it will likely work for basically all fields. Not sure what to do with the interface, this is always awkward.

    Related but different is I'm thinking about not using magic methods, ๐Ÿ“Œ [meta] Deprecate __get/__set() on ContentEntityBase Needs work is for the entity/field level, unsure about the properties, also in relation to helping people understand the difference between getValue() vs ->value and so on (because there is so much unnecessarily convoluted code out there that makes me sad as an entity maintainer). So maybe we should have a plan that covers that too, so that we can come up with an API and method names that make sense combined as well and then implement it in smaller issues.

    Also somewhat related, there are some experiments with such methods on ๐Ÿ“Œ Load user entity in Cookie AuthenticationProvider instead of using manual queries Needs work although with the goal to bypass the field objects entirely, but still not sure if it's a good idea to do that.

Production build 0.71.5 2024