DynamicEntityReferenceItem::getTargetTypes() requires 'entity_type_ids' setting array to be in the format: entity_type_id => entity_type_id because it is intersecting against keys. But DER has different definitions of this setting spread through its code.

Examples

Numeric (auto) IDs (DynamicEntityReferenceRelationshipTest):

      'settings' => array(
        'exclude_entity_types' => FALSE,
        'entity_type_ids' => array(
          'entity_test',
          'entity_test_mul',
        ),
      ),

Key/Value Mirroring (DynamicEntityReferenceWidgetTest):

      'settings' => array(
        'exclude_entity_types' => FALSE,
        'entity_type_ids' => array(
          'node' => 'node',
        ),
      ),

PHPDoc for getTargetTypes also mentions a return value of string[], which would should be an array of entity type IDs. However it actually returns Entity Type Labels keyed by Entity Type IDs.

I propose getTargetTypes should just diff/intersect against values. Keys should have no importance.

CommentFileSizeAuthor
#6 2553831-5-etsettings.patch6.02 KBdpi
#6 2553831-interdiff-2-5.diff5.28 KBdpi
#2 2553831-2-etsettings.patch1.03 KBdpi

Comments

dpi created an issue. See original summary.

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Active » Needs review
StatusFileSize
new 2553831-2-etsettings.patch1.03 KB

Use values instead of keys.

If this route is taken I would suggest that we remove existing key IDs in existing entity_type_ids definitions.

Status: Needs review » Needs work

The last submitted patch, 2: 2553831-2-etsettings.patch, failed testing.

dpi’s picture

dpi
Location Perth, Australia
commented
Issue summary: View changes
dpi’s picture

dpi
Location Perth, Australia
commented
Issue summary: View changes
dpi’s picture

dpi
Location Perth, Australia
commented
Status: Needs work » Needs review
StatusFileSize
new 2553831-interdiff-2-5.diff5.28 KB
new 2553831-5-etsettings.patch6.02 KB

Lets give this a shot.

The last submitted patch, 6: 2553831-interdiff-2-5.diff, failed testing.

Status: Needs review » Needs work

The last submitted patch, 6: 2553831-5-etsettings.patch, failed testing.

dpi’s picture

dpi
Location Perth, Australia
commented

All DER tests pass locally on beta14. Testbot is failing due to #2553915: Update class extend of DynamicEntityReferenceRelationshipTest.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented 
+++ b/src/Plugin/Field/FieldWidget/DynamicEntityReferenceWidget.php
@@ -84,7 +85,7 @@ class DynamicEntityReferenceWidget extends EntityReferenceAutocompleteWidget {
+      '#options' => array_intersect_key($labels, array_combine($available, $available)),

I think this was the reason it returns the KV but I can see this is the only place where we are doing this so I think this patch is an overall improvement. I'll commit this once HEAD is green.

Status: Needs work » Needs review

dpi queued 6: 2553831-5-etsettings.patch for re-testing.

jibran queued 6: 2553831-5-etsettings.patch for re-testing.

  • jibran committed 06fa264 on 8.x-1.x authored by dpi 
    Issue #2553831 by dpi: getTargetTypes has expectations that are not...
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Assigned: dpi » Unassigned
Category: Bug report » Task
Status: Needs review » Fixed

Pushed to 8.x-1.x. Thanks for the patch.

Status: Fixed » Closed (fixed)

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