Problem/Motivation

Once #2050843: Users 0 and 1 are created without a UUID is in, clean up the @todo added in #2090541: Avoid storing numeric target_id's for default values in field instance CMI files related to this issue in ConfigurableEntityReferenceFieldItemList

Proposed resolution

Remove the code related to the old exception with users 0 and 1. Maybe reuse code in this class and remove TaxonomyTermReferenceFieldItemList

Remaining tasks

Remove it.

User interface changes

None

API changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

plopesc’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
7.25 KB

While removing code exceptions for users in ConfigurableEntityReferenceFieldItemList I realized that resultant code is almost the same as TaxonomyTermReferenceFieldItemList, so the second class could be removed, using ConfigurableEntityReferenceFieldItemList as list_class for both TaxonomyTermReferenceItem and ConfigurableEntityReferenceItem.

This approach reuses the code, but forces to make enitty_reference module as a dependency for taxonomy module. Maybe, if you consider this approach, we could move this class where both modules can access it. Otherwise, we can just remove only some lines from ConfigurableEntityReferenceFieldItemList, leaving TaxonomyTermReferenceFieldItemList as is now.

Regards.

plopesc’s picture

Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 1: clean_user_uuid_exception-2162005-1.patch, failed testing.

yched’s picture

Thanks for following up on this, @plopesc.

The extra dependency is a bit unfortunate IMO.

But, in the same spirit than #2156337: merge ConfigEntityReferenceItemBase up into EntityReferenceItem, and fix inconsistencies, I think the methods could be moved to an EntityReferenceItem*List* class at the level of the \Core EntityRef field type (Core\Field\Plugin\Field\FieldType\EntityReferenceItem), that all other "reference" field types would use too.

(as a side note, I bumped #2073661: Add a EntityReferenceField::referencedEntities() method, which also has a case for introducing an EntityReferenceItemList class)

plopesc’s picture

Status: Needs work » Needs review
FileSize
11.26 KB
7.83 KB

Thanks for your review, @yched

It looks nicer now :)

The last submitted patch, 5: clean_user_uuid_exception-2162005-5.patch, failed testing.

yched’s picture

plopesc’s picture

Status: Needs review » Postponed
Related issues: +#2073661: Add a EntityReferenceField::referencedEntities() method

I believe we can postpone this on #2073661: Add a EntityReferenceField::referencedEntities() method where some we are taking some decisions that could affect this patch.

Pick it up again after fix this one will avoid some unnecessary re-rolls.

Regards

Berdir’s picture

Status: Postponed » Needs work

I think this can be unpostponed.

plopesc’s picture

Status: Needs work » Needs review
FileSize
11.31 KB

Hello

Here is a re-roll of the previous patch in #5.

I made some local tests and worked. Let's see what testbot says...

Status: Needs review » Needs work

The last submitted patch, 10: clean_user_uuid_exception-2162005-10.patch, failed testing.

plopesc’s picture

Status: Needs work » Needs review
FileSize
11.37 KB
785 bytes

Added check to avoid notices when fields return default values which does not include target_uuid

yched’s picture

Status: Needs review » Reviewed & tested by the community

Awesome !
+ removal of two specialized list classes : awesome++

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed f2015b7 and pushed to 8.x. Thanks!

  • alexpott committed f2015b7 on 8.0.x
    Issue #2162005 by plopesc: Clean up entity_reference default values once...
Berdir’s picture

Status: Fixed » Needs review
FileSize
898 bytes
+++ b/core/lib/Drupal/Core/Field/EntityReferenceFieldItemList.php
@@ -46,4 +49,69 @@ public function referencedEntities() {
+
+        foreach ($entities as $id => $entity) {
+          $entity_uuids[$entity->uuid()] = $id;
+        }
+        foreach ($uuids as $delta => $uuid) {
+          if ($entity_uuids[$uuid]) {
+            $default_value[$delta]['target_id'] = $entity_uuids[$uuid];
+            unset($default_value[$delta]['target_uuid']);
+          }

$entity_uuids is not defined and is not doing an isset() check, so if targets actually don't exist, this is throwing php notices.

Berdir’s picture

Title: Clean up entity_reference default values once users 0 and 1 provides UUID » [follow-up] Clean up entity_reference default values once users 0 and 1 provides UUID
swentel’s picture

Status: Needs review » Reviewed & tested by the community

Good to go when green.

alexpott’s picture

Title: [follow-up] Clean up entity_reference default values once users 0 and 1 provides UUID » Clean up entity_reference default values once users 0 and 1 provides UUID
Status: Reviewed & tested by the community » Fixed

Committed a5e7c59 and pushed to 8.0.x. Thanks!

  • alexpott committed a5e7c59 on 8.0.x
    Issue #2162005 followup by Berdir: Clean up entity_reference default...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.