Follow-up of #1957748: hook_field_prepare_view() is passed empty field values for EntityNG Entities

Right now, only field_attach_view() and field_attach_prepare_view() functions make sure they are working with a BC entity, the others have to be called with one, so the caller needs to make sure.

It would be easier to convert and later on remove it again when the other field attach functions would ensure this, as there are dozens if not hundreds of direct calls t those functions in the field tests, that I am converting in #1822000: Remove Drupal\field_test\Plugin\Entity\Type\TestEntity in favor of EntityTest.

Files: 
CommentFileSizeAuthor
#13 ensure-bc-entity-1974474-12.patch8.96 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 55,471 pass(es).
[ View ]
#5 ensure-bc-entity-1974474-5.patch7.37 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 54,604 pass(es).
[ View ]
#5 ensure-bc-entity-1974474-5-interdiff.txt851 bytesBerdir
#3 ensure-bc-1974474-3.patch7.48 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 54,244 pass(es), 150 fail(s), and 10 exception(s).
[ View ]
#3 ensure-bc-1974474-3-interdiff.txt594 bytesBerdir
#1 ensure-bc-1974474-1.patch7.48 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new7.48 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Easy as that... I think ;)

Status:Needs review» Needs work

The last submitted patch, ensure-bc-1974474-1.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new594 bytes
new7.48 KB
FAILED: [[SimpleTest]]: [MySQL] 54,244 pass(es), 150 fail(s), and 10 exception(s).
[ View ]

Well. me = fail.

Status:Needs review» Needs work

The last submitted patch, ensure-bc-1974474-3.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new851 bytes
new7.37 KB
PASSED: [[SimpleTest]]: [MySQL] 54,604 pass(es).
[ View ]

I've messed up field_attach_load(), this should be better.

Issue tags:+Entity Field API

Tagging.

+++ b/core/modules/field/field.attach.incundefined
@@ -821,6 +821,9 @@ function _field_invoke_widget_target() {
+  // Enable BC if necessary.
@@ -892,6 +895,11 @@ function field_attach_load($entity_type, $entities, $age = FIELD_LOAD_CURRENT, $
+  // Enable BC if necessary.
@@ -1028,6 +1036,9 @@ function field_attach_load_revision($entity_type, $entities, $options = array())
+  // Enable BC if necessary.
@@ -1077,6 +1088,9 @@ function field_attach_validate(EntityInterface $entity, array $options = array()
+  // Enable BC if necessary.
@@ -1114,6 +1128,9 @@ function field_attach_form_validate(EntityInterface $entity, $form, &$form_state
+  // Enable BC if necessary.
@@ -1135,6 +1152,9 @@ function field_attach_extract_form_values(EntityInterface $entity, $form, &$form
+  // Enable BC if necessary.
@@ -1156,6 +1176,9 @@ function field_attach_presave($entity) {
+  // Enable BC if necessary.
@@ -1197,6 +1220,9 @@ function field_attach_insert(EntityInterface $entity) {
+  // Enable BC if necessary.
...

I guess it would be helpful to explain why it's done here.

Can easily change that within the patch, but to what? I just took the existing comment and copied to all other places.

Title:Make sure within field_attach_*() that the are working with a BC entityMake sure within field_attach_*() that we are working with a BC entity

What about: Enable BC if the entity is NG.?

Doesn't seem to explain something either :)

"Field API needs a BC mode entity" or "Ensure we are working with a BC mode entity" maybe?

I like the second one.

StatusFileSize
new8.96 KB
PASSED: [[SimpleTest]]: [MySQL] 55,471 pass(es).
[ View ]

Here's a patch with that, then :) No interdiff, not really necessary I think ;)

Status:Needs review» Reviewed & tested by the community

Thanks!

A somehow related issue is #1977464: The custom field which add for comment con't show in views which caused a bug with all EntityNGs, though I think it's valid to have these two issues separated.

#13: ensure-bc-entity-1974474-12.patch queued for re-testing.

Status:Reviewed & tested by the community» Fixed

Committed 0be9849 and pushed to 8.x. Thanks!

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