Problem/Motivation

In Drupal 7, if you write a module that uses field storage to write on entity_save() -- say, Taxonomy Access Control -- it is difficult to know what fields actually contain the data that you care about. In Drupal 8, we have Entity Reference in core, which compounds the issue because it should be used as the default entity-to-entity reference system.

Certainly, a module could provide its own field with a specific field name, but that approach defeats the purpose of having flexible field configuration and storage. What we need instead is a reusabe way for developers to know which fields they should be watching when an entity is saved, updated or deleted.

Proposed resolution

Add a function or method to the Field API that retrieves data related to a specific entity or module.

Pseudo-code:

function foo_entity_update(Drupal\Core\Entity\EntityInterface $entity) {
  $module = 'foo';
  $my_properties = $entity->getField($module);
  // Returns an array of property names.
  foreeach ($my_properties as $key) {
    $data = $entity->{$key};
    foo_save($data, $entity->id);
  }
}

Think about this in the context of access control, in particular.

Remaining tasks

Code to be written.

User interface changes

None.

API changes

New utility function or method for Field API.

CommentFileSizeAuthor
#8 entity_reference-1969082-lookup-fields-8.patch7.33 KBagentrickard
#6 entity_reference-1969082-lookup-fields-6.patch7.4 KBagentrickard
#5 entity_reference-1969082-lookup-fields.patch4.15 KBagentrickard
#3 entity_reference-1969082-lookup-fields.patch2.63 KBagentrickard
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Component: field system » entity_reference.module
Issue tags: -Field API

I think we probably want to add this in ER. :)

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
Status: Needs work » Active

Here's a patch that has the initial code. I actually made three functions. They can be used in combination for different purposes:

* entity_reference_get_fields($entity_type, $bundle)
Returns all entity_reference module fields for a bundle.

* entity_reference_get_target_fields($entity_type, $bundle, $target_entity_type)
Returns all entity reference fields that target a specific entity type.

* entity_reference_get_target_values($entity, $entity_type, $bundle, $target_entity_type)
Returns the field values of a target entity.

Proposed usage for a module like TAC or Domain Access:

function domain_node_access_records(\Drupal\Core\Entity\EntityInterface $node) {
  $values = entity_reference_get_target_values($node, 'node', $node->type,  'domain');
  // DO STUFF.
}

[Edit]: added $bundle to the get_target_values() function.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
Issue tags: +Needs tests
FileSize
entity_reference-1969082-lookup-fields.patch2.63 KB

And the patch.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
Status: Active » Needs work

/me can't remember to do two things at once today.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
FileSize
entity_reference-1969082-lookup-fields.patch4.15 KB

Here's a version with a test, but I can't figure out wtf is with the test entity. I would expect the $values array to return a simple array, but it returns a complex object that cannot be run through debug().

Worth noting that I amended an existing test because these are really just small helpers that leverage the features already being tested.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
Status: Needs work » Needs review
FileSize
entity_reference-1969082-lookup-fields-6.patch7.4 KB

The testing problem was PHPUnit v. WebTest behavior, AFAICT. Here's a version with working tests. The tests create a node type with three fields, two entity_reference and one text. Then we use the utility functions to see if they work as expected.

Could this be done in PHPUnit? I don't know.

Status: Needs review » Needs work

The last submitted patch, entity_reference-1969082-lookup-fields-6.patch, failed testing.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
Status: Active » Needs review
FileSize
entity_reference-1969082-lookup-fields-8.patch7.33 KB

Try again.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

Had a conversation with @swentel and @davereid in IRC about making this a method on the FieldInfo class which could return data about which module controls a field, or which database table it writes to.

However, I think that level of abstraction is a separate use-case.

At best, we would add the methods to FieldInfo and then leverage those to provide the API support for fields defined by Entity Reference, as in the current patch.

amateescu’s picture

amateescu CreditAttribution: amateescu commented

I would support having the first two helpers in FieldInfo, there's no reason why 'getting all fields of a certain type' needs to be something specific to Entity Reference. I said the first two because the second helper needs to wait for the outcome of #1847590: Fix UX of entity reference field type selection.

As for the third one, I'm not sure how helpful it is if you have the first two available..

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

True, you can use field_get_items() instead, but I'm looking at the actual code that I would have to write and I can already think of 4 places where I would have to duplicate this logic across different modules.

If ER is to be core and provide an API, having these kinds of helpers makes sense. I wish I had this in D7, too.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented 
+++ b/core/modules/entity_reference/entity_reference.moduleundefined
@@ -458,3 +458,77 @@ function entity_reference_create_instance($entity_type, $bundle, $field_name, $f
+ * @param string $entity
+ *   The entity being referenced.
+ * @param string $entity_type
+ *   The type of entity being referenced.
+ * @param string $bundle
+ *   The bundle name of the entity being referenced.
+ * @param string $target_entity_type

Is a bit confusing, it's hard to understand if the arguments belong to the entity that holds the reference or if they belong to the referenced entity.

Can't we get one of the *types* out of the entity?

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

In my tests, no, we couldn't count on the entity having a type. However, $entity->bundle() might do that consistently.

I don't find it confusing because it almost exactly matches the pattern of entity_reference_create_instance()

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

Spoke to @amateescu in IRC and he wants to ensure the functionality of the first two methods is present and provided by FieldAPI.

However, it is not clear to me why the second function (finding Target entities) should be pushed up a level. The "target_entity" concept is specific to Entity Reference.

Agree that we can drop the third function.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

It may just be a case where field_read_instance() needs to support what we're trying to do.

amateescu’s picture

amateescu CreditAttribution: amateescu commented

However, it is not clear to me why the second function (finding Target entities) should be pushed up a level. The "target_entity" concept is specific to Entity Reference.

That's because, pending the outcome of #1847590: Fix UX of entity reference field type selection, we might have an entity reference field type for each entity type (e.g. entity_reference:node, entity_reference:taxonomy_term, etc.) so the 'target_type' field setting would dissapear.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
Status: Needs review » Needs work

Got it. Thanks.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
Issue summary: View changes

Fixed a typo

amateescu’s picture

amateescu CreditAttribution: amateescu commented
Status: Needs work » Closed (duplicate)
Issue tags: -Needs tests

The first helper from #2 is now very easy to do because we store the field type as a property of the field_config entity type: \Drupal::entityManager()->getStorage('field_config')->loadByProperties(array('field_type' => 'entity_reference'))

The second one is being added in #1978714: Entity reference doesn't update its field settings when referenced entity bundles are deleted.

The third one is not that useful as a helper, as said in #14.

So I'm closing this issue as a duplicate. @agentrickard, please feel free to reopen if you disagree :)