Follow-up to #2405009-11: DER needs its own FieldItemList class.2

Problem/Motivation

We are using ValidReferenceConstraint in DynamicEntityReferenceItem which uses ValidReferenceConstraintValidator and ValidReferenceConstraintValidator::validate is specific to ER field.

and maybe

Do we depend on ReferenceAccessConstraintValidator if yes then it is also specific to ER.

Proposed resolution

Fix it by creating ValidDynamicReferenceConstraint.

Remaining tasks

Addes tests.
Complete the patch.
Review.

User interface changes

None

API changes

None

CommentFileSizeAuthor
#7 2405607-DER-constraint-validation-7.patch6.85 KBjibran
#7 interdiff.txt1.43 KBjibran
#5 2405607-DER-constraint-validation-5.patch6.6 KBjibran
#5 interdiff.txt1.11 KBjibran
#5 2405607-DER-constraint-validation-4.patch6.2 KBjibran
#5 interdiff.txt4.63 KBjibran
#4 der-core-2407587-43.patch52.31 KBjibran
#4 interdiff.txt11.46 KBjibran
der-voilation.patch2.19 KBjibran

Comments

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Assigned: Unassigned » larowlan
Status: Postponed » Needs review

@larowlan can you please have a look at it?

Status: Needs review » Needs work

The last submitted patch, der-voilation.patch, failed testing.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Related issues: +#2142997: Test for ValidReferenceConstraintValidator
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
StatusFileSize
new interdiff.txt11.46 KB
new der-core-2407587-43.patch52.31 KB

This fixes the bug.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt4.63 KB
new 2405607-DER-constraint-validation-4.patch6.2 KB
new interdiff.txt1.11 KB
new 2405607-DER-constraint-validation-5.patch6.6 KB

When we assign and non-reference-able entity the validation passes. So I am conflicted here. Is it the validaters duty to create a violation for this or DynamicEntityReference::setValue() shouldn't load the $item->enitity?

Status: Needs review » Needs work

The last submitted patch, 5: 2405607-DER-constraint-validation-5.patch, failed testing.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.43 KB
new 2405607-DER-constraint-validation-7.patch6.85 KB

When we assign and non-reference-able it is the validaters duty to create a violation.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Reviewed & tested by the community 
+++ b/src/Plugin/DataType/DynamicEntityReference.php
@@ -9,6 +9,7 @@ namespace Drupal\dynamic_entity_reference\Plugin\DataType;
+use Drupal\dynamic_entity_reference\Plugin\Field\FieldType\DynamicEntityReferenceItem;

Not needed? but can be fixed on commit.

Great work, see you on the core criticals for validation :-P

  • jibran committed 4d6fc71 on 8.x-1.x 
    Issue #2405607 by jibran: Fix DER constraint validation
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Reviewed & tested by the community » Fixed

Thanks for the review. Fixed #8 on commit.

Status: Fixed » Closed (fixed)

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

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Assigned: larowlan » Unassigned