The commit of #1818556: Convert nodes to the new Entity Field API broke Edit module's metadata callback. Note that there is test coverage. But the whole business of moving from EntityPG to EntityNG (#1346214: [meta] Unified Entity Field API) has caused some of the tests to test EntityPG instead of EntityNG, which was the case here. Hence this could go on undetected.

The fix is easy:

-    $items = $entity->get($field_name);
-    $items = $items[$langcode];
+    $items = $entity->getTranslation($langcode)->get($field_name)->getValue();

However, it's the tests that also require updating to EntityNG. For that, I copied a subset of #1822000-26: Remove Drupal\field_test\Plugin\Entity\Type\TestEntity in favor of EntityTest verbatim — it was Berdir who wrote the fixed tests and hence should be credited here.

Related

#1939994: Complete conversion of nodes to the new Entity Field API

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers’s picture

Status: Active » Needs review
FileSize
6.82 KB
fago’s picture

+++ b/core/modules/edit/lib/Drupal/edit/MetadataGenerator.php
@@ -69,8 +69,7 @@ public function generate(EntityInterface $entity, FieldInstance $instance, $lang
-    $items = $entity->get($field_name);
-    $items = $items[$langcode];
+    $items = $entity->getTranslation($langcode)->get($field_name)->getValue();

I'd suggest to use getTranslation($langcode, FALSE) here, such that it works for untranslatable fields also. Else, patch looks good.

I guess this should work with the same data displayed, thus ideally we'd have a EntityLanguageDecorator that incorporates language fallbacks (as field API does now) here. But that's a separate task.

Wim Leers’s picture

Ok, made that one line change :)

fago’s picture

Status: Needs review » Reviewed & tested by the community

thanks, I think this is ready then.

Wim Leers’s picture

@fago: Thanks!

Dear core committer: please credit Berdir as well, he did the bulk of the work here.

Damien Tournoud’s picture

Does that mean that Edit only works with EntityNG now? Does it make sense to support both?

Berdir’s picture

Yes, that's what it means but I don't think it makes sense to support both. The only remaining public non-NG entities are now terms and users and they aren't supported yet anyway because not all node types are ported to the new entity access API yet so edit.module can't use that yet and afaik only supports nodes due to that right now.

Wim Leers’s picture

@Damien: Everything is being converted to EntityNG. Because node.module doesn't implement the new Entity Access API yet, Edit is currently confined to nodes anyway.

podarok’s picture

webchick’s picture

Issue tags: +sprint, +Spark

Tagging.

Dries’s picture

Committed to 8.x. Thanks for fixing this.

It seems wrong that this broke so badly. However, it sounds like we're working on improving or fixing our test coverage in #1822000: Remove Drupal\field_test\Plugin\Entity\Type\TestEntity in favor of EntityTest.

webchick’s picture

Status: Reviewed & tested by the community » Fixed
webchick’s picture

Issue tags: -sprint

x

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary. Add Related