In line 66:
$this->derivatives[$definition['id']]['entity_types'] = $definition['targetEntityType'];

But the annotation says that entity_types is an array. The name also suggests that it's an array. I think the code should change to

$this->derivatives[$definition['id']]['entity_types'] = [$definition['targetEntityType']];

There is some code added, perhaps to address the bug, that we might be able to remove when this is fixed.


  /**
   * Overrides DefaultPluginManager::processDefinition().
   */
  public function processDefinition(&$definition, $plugin_id) {
    $definition += [
      'entity_types' => FALSE,
    ];

    if ($definition['entity_types'] !== FALSE && !is_array($definition['entity_types'])) {
      $definition['entity_types'] = [$definition['entity_types']];
    }
  }

Or perhaps it's better to leave ::processDefinition() in, just in case someone else makes the same mistake?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

oknate created an issue. See original summary.

oknate’s picture

Title: ViewModeDeriver sets entity_types which should be array to single entity machine name » ViewModeDeriver sets entity_types which should be array to string
oknate’s picture

Issue summary: View changes
oknate’s picture

Issue summary: View changes
oknate’s picture

Here are two patches for the issue. A one-liner, and a more elaborate one.

Wim Leers’s picture

Priority: Normal » Minor
Status: Needs review » Reviewed & tested by the community
+++ b/src/EntityEmbedDisplay/EntityEmbedDisplayManager.php
@@ -37,19 +37,6 @@ class EntityEmbedDisplayManager extends DefaultPluginManager {
-  public function processDefinition(&$definition, $plugin_id) {

Less custom code! 🥳

Will commit if green.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 3059394-5-full-bodied.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

oknate’s picture

Status: Needs work » Needs review
FileSize
2.29 KB

I had eyeballed it earlier and not tested manually. It turns out the base field definition already had the keys 'label' and 'entity_types', so it didn't set those for any of the plugins. This led to some very unexpected behavior. I checked and we can flip it around, so you add the $base_plugin_definition to our custom settings and that works.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

D'oh, right!

I agree this new patch iteration matches the original array merging behavior exactly.

Wim Leers’s picture

oknate’s picture

Status: Needs work » Needs review
FileSize
2.72 KB

Here's the reroll.

  • Wim Leers committed 79e59ca on 8.x-1.x authored by oknate
    Issue #3059394 by oknate: ViewModeDeriver sets entity_types which should...
Wim Leers’s picture

Status: Needs review » Fixed

🚢

Status: Fixed » Closed (fixed)

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