Problem/Motivation

Custom plugin for entity_reference field type is following the ReverseEntityReference example:

public function getFieldDefinitionSettings(): array {
  return [
    'target_type' => 'media',
  ];
}

However on the Manage Fields screen the field still says "Reference type: Content", and on Manage Display none of the media-specific formatters show up. It seems that something is missing to get the field definition settings into the right spot so the rest of the field system can see them.

Issue fork computed_field-3436047

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

KarlShea created an issue. See original summary.

karlshea’s picture

karlshea
he/him
Primary language English
Location Minneapolis 🇺🇸
commented

I wonder if returning target_type from getFieldDefinitionSettings() is a red herring? It looks like EntityReferenceItem from core is returning target_type from defaultStorageSettings(), maybe there needs to be a way of influencing that from within a computed field plugin?

karlshea’s picture

karlshea
he/him
Primary language English
Location Minneapolis 🇺🇸
commented

Yes, looks like it, e.g. EntityReferenceItem::storageSettingsSummary.

$storage_definition is \Drupal\computed_field\Field\FieldStorageDefinition, which in its create() is initializing the defaults for the field type, which is "node" target_type. It doesn't seems like there's any way to alter that either.

karlshea’s picture

karlshea
he/him
Primary language English
Location Minneapolis 🇺🇸
commented

Looks like ComputedFieldSettingsTrait is also conflating storage settings vs field settings as well which I don't think is hurting anything but I'm also not sure it's helping anything.

Drupal\computed_field\Field\FieldStorageDefinition might need a way of talking to the computed field plugin to get field storage settings? Or possibly getFieldStorageDefinition() in the ComputedField entity might do it? hook_entity_bundle_field_info_alter can't help because all you can do is set the settings on the storage definition which is just recreated each time it's accessed.

KarlShea opened merge request !11

karlshea’s picture

karlshea
he/him
Primary language English
Location Minneapolis 🇺🇸
commented

Looks like ComputedFieldSettingsTrait is also conflating storage settings vs field settings as well which I don't think is hurting anything but I'm also not sure it's helping anything.

Nope, it needs those settings there too.

karlshea’s picture

karlshea
he/him
Primary language English
Location Minneapolis 🇺🇸
commented
Title: Plugin for entity_reference field doesn't set target_type » Field storage target_type isn't getting set for plugin handling entity_reference fields
Category: Support request » Bug report
Status: Active » Needs review

Would definitely appreciate a look at this, seems to have fixed the entity_reference issue for me! Media-specific field formatters are now appearing for my computed field.

It still feels like having combined settings for storage and the field is weird, but I don't know enough about field internals to know if that's normal or not 🤷🏻‍♂️

karlshea’s picture

karlshea
he/him
Primary language English
Location Minneapolis 🇺🇸
commented

Looks like this interacts with the cardinality patch, some of the media formatters look at the field storage definition cardinality in their isApplicable method.

When that patch is in this will also need to be added to ComputedField::getFieldStorageDefinition():

// Set cardinality
$definition->setCardinality($plugin->getFieldCardinality());
karlshea’s picture

karlshea
he/him
Primary language English
Location Minneapolis 🇺🇸
commented

Rebased MR!11 on top of the work in #3350715: Computed field should specify the cardinality of the field

joachim’s picture

joachim commented
Status: Needs review » Needs work

I've committed #3350715: Computed field should specify the cardinality of the field so this needs rebasing again.

karlshea’s picture

karlshea
he/him
Primary language English
Location Minneapolis 🇺🇸
commented
Status: Needs work » Needs review

Rebased!

joachim’s picture

joachim commented

Made a few tweaks.

Could you check to see if it still fixes the problem for you?

joachim’s picture

joachim commented
Status: Needs review » Fixed

Tested it locally and all works. Committed.

Thanks for reporting and working on this!

  • joachim committed 2177ee1b on 4.0.x 
    Issue #3436047 by KarlShea, joachim: Field storage doesn’t get settings...
joachim’s picture

joachim commented

I'm going to file a follow-up for splitting the settings method into field settings & storage settings.

karlshea’s picture

karlshea
he/him
Primary language English
Location Minneapolis 🇺🇸
commented

Thank you! It all appears to be working for me as well.

Status: Fixed » Closed (fixed)

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

jonmcl’s picture

jonmcl commented

If I am following along with this change, am I correct that this

public function getFieldDefinitionSettings(): array {
  return [
    'target_type' => 'media',
    'handler' => 'default:media',
    'handler_settings' => [
      'target_bundles' => [
        'image',
      ],
    ],
  ];
}

needs to be changed to


public function getStorageDefinitionSettings(): array {
  return [
    'target_type' => 'media',
  ];
}

public function getFieldDefinitionSettings(): array {
  return [
    'handler' => 'default:media',
    'handler_settings' => [
      'target_bundles' => [
        'image',
      ],
    ],
  ];
}

Am I understanding this change correctly?

karlshea’s picture

karlshea
he/him
Primary language English
Location Minneapolis 🇺🇸
commented

@jonmcl yes, that's correct! My computed media field has exactly the same code.