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.

Issue

When those constraints fail, the generated violation objects do not point to any specific delta: the propertyPath is just the name of the property, e.g. 'url', not '0[url]'. It is thus impossible to assign them back to the right form element.

TypedDataManager::getConstraints() has a "@todo: Figure out how to handle nested constraint structures as collections.", which seems to be about exactly that ? But fixing this is above my head right now...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

To help reproduce this, the patch https://drupal.org/files/e_r_constraints_autocreate-2012662-1-FAIL.patch from #2012662: Constraints on 'target_id' / 'tid' properties break autocomplete if applied adds a "mock validation step" to EntityFormController, that spits out the constraint violations on a node form submit.

Steps to reproduce:
- add a "link" field with cardinality 2 to article nodes.
- create or edit a node, put "http://drupal.org" in the 1st field, "not-a-url" in the second field
--> there is no way to tell in which delta the error occurred.

yched’s picture

Issue summary: View changes

Add structure

fago’s picture

The 0 should be part of the property path, not sure why it isn't. I'll take a look later on.

fago’s picture

Status: Active » Needs review
FileSize
2.13 KB
2.18 KB

ok, found the problem. empty() is FALSE for '0'.. -> added test coverage for that.

Attached is a patch with the test only (should fail) and one including the fix.

fago’s picture

Component: entity system » typed data system
Issue tags: +Entity Field API, +Entity validation

Status: Needs review » Needs work

The last submitted patch, d8_validate_0.test_only-fails.patch, failed testing.

yched’s picture

Status: Needs work » Reviewed & tested by the community

Awesome !

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

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

Anonymous’s picture

Issue summary: View changes

typo