CommentFileSizeAuthor
#2 fix-HEAD.patch6.81 KBlarowlan

Comments

larowlan created an issue. See original summary.

larowlan’s picture

Status: Active » Needs review
StatusFileSize
new6.81 KB

Fixes for this and for the removed NEW_ENTITY_MARKER.

Tests all green locally.

  • jibran committed bc85333 on 8.x-1.x authored by larowlan
    Issue #2581609 by larowlan: field_field_config_presave expects there to...
yched’s picture

Yeah, kind of ugly :-)

The thing is, several e_r code in Core assume all ER fields have a Selection handler - DER bypasses the notion of "selection handler" completely ? How does it handle "what is referenceable and what is not" ?

Also, on that regard, maybe you guys would have a thing to say about the validation strategy considered in #2577963: Let entity_ref Selection handlers be in charge of the field validation (since it proposes to rely more heavily on the Selection handlers in the ValidReference constraint) ?

jibran’s picture

Status: Needs review » Needs work

We store the same settings as ER but keyed by entity type.
Something like

      'node' => [
        'handler' => "default",
        'handler_settings' => [],
      ],
      'user' => [
        'handler' => "default",
        'handler_settings' => [],
      ],

We don't use ValidReference constraint we have ValidDynamicReference constraint.

@larowlan I committed the patch but head still has some fails. :(

yched’s picture

I see, makes sense.

Still, the fact that the 'handler' setting expected from the parent is not present, is a bit worrying. Could it be left present but set to NULL ?

larowlan’s picture

Even null would fail, field module expects a particular format.
I will file a core issue. The guts of the problem is field item classes need a way to validate their settings in a configurable instance, I think field module knows too much about the nuances of an er item and the solution is to invert it. On leave for a few days, but will paste link here once I have something going.

larowlan’s picture

Jibran, fails are because head has moved again I think

larowlan’s picture

Yep http://cgit.drupalcode.org/drupal/diff/core/lib/Drupal/Core/Field/Plugin... wasn't in the version of HEAD it was passing on. New issue i think.

larowlan’s picture

larowlan’s picture

Also @jibran http://cgit.drupalcode.org/dynamic_entity_reference/tree/src/dynamic_ent... isn't what was in https://www.drupal.org/files/issues/fix-HEAD.patch - somehow the .module file ended up in /src folder.

Fixed and pushed that, should be back to green.

Status: Fixed » Needs work

The last submitted patch, 2: fix-HEAD.patch, failed testing.

The last submitted patch, 2: fix-HEAD.patch, failed testing.

jibran’s picture

Status: Needs work » Fixed
jibran’s picture

Assigned: larowlan » Unassigned
jibran’s picture

Status: Fixed » Closed (fixed)

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