Context

Spin-off from #1992138: Helper issue for "field types as TypedData Plugins" / #1969728: Implement Field API "field types" as TypedData Plugins.

That issue adds actual validation on entity fields.
This means that implicit or explicit constraints contained in FieldItem::getPropertyDefinitions() (e.g 'type' => 'uri' in LinkItem, 'type' => 'integer' for 'tid' in TaxonomyTermReferenceItem...) that were just "dormant" so far in HEAD, are now fired.
This shows a couple problems, I'll open separate issues for them.

Issue

The "auto create / freetagging" feature of the autocomplete widgets (for entity_reference or taxo fields) break the constraint on 'target_id' / 'tid' being of type 'integer', because the items created by the "auto create" flow end up with a 'target_id' / 'tid' = FALSE, which is not an integer.

Patches coming up.

Files: 
CommentFileSizeAuthor
#4 e_r_constraints_autocreate-2012662-4-VALIDATE.patch1011 bytesyched
FAILED: [[SimpleTest]]: [MySQL] 54,330 pass(es), 493 fail(s), and 251 exception(s).
[ View ]
#4 e_r_constraints_autocreate-2012662-4-VALIDATE-FIX.patch3.66 KByched
FAILED: [[SimpleTest]]: [MySQL] 53,188 pass(es), 484 fail(s), and 251 exception(s).
[ View ]
#4 e_r_constraints_autocreate-2012662-4-FIX-ONLY.patch2.68 KByched
PASSED: [[SimpleTest]]: [MySQL] 55,327 pass(es).
[ View ]
#2 e_r_constraints_autocreate-2012662-1-PASS.patch2.37 KByched
FAILED: [[SimpleTest]]: [MySQL] 53,194 pass(es), 484 fail(s), and 251 exception(s).
[ View ]
#1 e_r_constraints_autocreate-2012662-1-FAIL.patch991 bytesyched
FAILED: [[SimpleTest]]: [MySQL] 54,244 pass(es), 507 fail(s), and 3,131 exception(s).
[ View ]
#1 e_r_constraints_autocreate-2012662-1-PASS.patch2.35 KByched
FAILED: [[SimpleTest]]: [MySQL] 52,572 pass(es), 496 fail(s), and 3,199 exception(s).
[ View ]
#1 e_r_constraints_autocreate-2012662-1-FIX-ONLY.patch1.38 KByched
FAILED: [[SimpleTest]]: [MySQL] 56,658 pass(es), 9 fail(s), and 0 exception(s).
[ View ]

Comments

yched’s picture

Status:Active» Needs review
StatusFileSize
new1.38 KB
FAILED: [[SimpleTest]]: [MySQL] 56,658 pass(es), 9 fail(s), and 0 exception(s).
[ View ]
new2.35 KB
FAILED: [[SimpleTest]]: [MySQL] 52,572 pass(es), 496 fail(s), and 3,199 exception(s).
[ View ]
new991 bytes
FAILED: [[SimpleTest]]: [MySQL] 54,244 pass(es), 507 fail(s), and 3,131 exception(s).
[ View ]

- "FAIL" patch adds a mock validation step in EntityFormController to demonstrate the buggy behavior when validation is actually performed. This should fail.
- "PASS" adds a proposed fix (standardize on '0' instead of FALSE to indicate "new entity being created"). This should pass.
- "FIX-ONLY" is just the fix, without the mock validate() calls. This should pass, and is the candidate patch to commit to HEAD.

yched’s picture

StatusFileSize
new2.37 KB
FAILED: [[SimpleTest]]: [MySQL] 53,194 pass(es), 484 fail(s), and 251 exception(s).
[ View ]

Oops, warnings in the "mock validation code". Updated version of the "PASS" patch.

Status:Needs review» Needs work

The last submitted patch, e_r_constraints_autocreate-2012662-1-PASS.patch, failed testing.

yched’s picture

Status:Needs work» Needs review
StatusFileSize
new2.68 KB
PASSED: [[SimpleTest]]: [MySQL] 55,327 pass(es).
[ View ]
new3.66 KB
FAILED: [[SimpleTest]]: [MySQL] 53,188 pass(es), 484 fail(s), and 251 exception(s).
[ View ]
new1011 bytes
FAILED: [[SimpleTest]]: [MySQL] 54,330 pass(es), 493 fail(s), and 251 exception(s).
[ View ]

So, ok:
- the "PASS" patch still has many fails despite including the fix for this issue, because triggering constraint validation causes a lot more stuff than just constraints on target_id / tid.
- in the fix, the taxonomy formatters need to be adjusted accordingly

New patches:
- the "VALIDATE" patch adds a mock validation step in EntityFormController to demonstrate the buggy behavior when validation is actually performed. This should fail.
- the "VALIDATE+FIX" patch adds a proposed fix (standardize on '0' instead of FALSE to indicate "new entity being created"). This still fails, but with less fails :-)
- the "FIX-ONLY" patch is just the fix, without the mock validate() calls. This should pass, and is the candidate patch to commit to HEAD.

yched’s picture

Issue tags:+Field API NG blocker

Tagging

Berdir’s picture

Issue tags:+Entity Field API

@yched: Note that all issues also need the Entity Field API tag to show up at http://entity.worldempire.ch/conversions

amateescu’s picture

Status:Needs review» Reviewed & tested by the community
Issue tags:-Entity Field API

Given that tests will come naturally when #1969728: Implement Field API "field types" as TypedData Plugins gets in and we'll actually use the constraints, I think it's fair to get this one committed before.

amateescu’s picture

Issue tags:+Entity Field API

Restoring tags..

alexpott’s picture

Status:Reviewed & tested by the community» Needs review
+++ b/core/lib/Drupal/Core/Entity/Field/Type/EntityWrapper.phpundefined
@@ -90,7 +90,7 @@ public function setValue($value, $notify = TRUE) {
-      $value = FALSE;
+      $value = 0;

This fix seems incomplete now all new values are 0 which is a int... what if the value being set is a string?

yched’s picture

@alexpott not sure what you mean. There should not be a case where 'target_id' gets assigned as a string.

alexpott’s picture

Status:Needs review» Reviewed & tested by the community

@yched oh - yep ingore me...

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed d3d704e and pushed to 8.x. Thanks!

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

Anonymous’s picture

Issue summary:View changes

Add structure