- Issue created by @mxh
- πΊπΈUnited States mlncn Minneapolis, MN, USA
Very much in favor of this direction and would like to help! If there are any top-of-your-head thoughts to start with i'd appreciate any and all pointers. I have made a couple validation plugins and i would presume the approach will be to create "the last validation plugin you'll ever need" that hands the entity off to ECA and returns any message ECA wants to.
- π©πͺGermany jurgenhaas Gottmadingen
@mlncn that sounds great, let me try getting you going.
The OP talks about the validation handler of an entity type, while you're now mentioning validation
plugins
. Those are two different things, but we could probably leverage both.With the validation handler, I could imagine that we implement an ECA condition which takes a content entity as an argument and runs it through its validation handler. It returns TRUE or FALSE, depending on the result of the handler. If it doesn't have one, it falls back to TRUE. Note, there is also always the negation flag for those result, so that should also apply to this result.
With validation plugins, they are applied to distinct values, e.g. field values. So, to utilize a validation plugin, one needs to select the plugin and pass it a value that should be validated. Please correct me if I'm wrong. So, if that is not too far off, then this would qualify for another ECA condition plugin that provides a drop-down list of available validation plugins and a value field. It would then process that combination like described for the handler before.
The entity handler validation plugin should be going into the eca_content sub-module. The other one with validation plugins would be better placed in eca_base, as it could be useful with arbitrary values, not necessarily related to content entities.
- π©πͺGermany mxh Offenburg
I'm currently not sure whether my IS was understood correctly. Being able to use single validation plugins or running the whole validation handler in ECA itself could be an option, but hast not been the main suggestion of this issue. I think #4 is already addressing the right direction for getting to my suggestion.
My suggestion is being able to do the following (step-by-step example):
- A new event is being dispatched - whenever an entity is being validated - that can be react upon within ECA (could be called "Entity being validated")
- Upon that event, do whatever you want in ECA, e.g. check for certain combination of fields values that must be correct
- When something is wrong with the entity, execute an action to set the validation error (could be called "Entity: set validation error")
What's basically needed is a new event plugin and one new action plugin. The event plugin must be aware of the entity so that we are able to set a validation error for it. Probably it also needs to pass along the validation handler itself or something where the validation errors are being hold.
A similar mechanic for having an event and action can be found in the eca_access module (EntityAccess event and SetAccessResult action). Maybe it's worth considering only supporting content entities for that, so the right place may be within the eca_content module.
- π©πͺGermany mxh Offenburg
Be most difficult part is probably how the event can be dispatched. Don't know whether there is already a hook or system event available, but probably not. A way could be to replace all handlers by existing content entity definitions with a handler using the decorator pattern, but I wish there was an easier way.
- π©πͺGermany jurgenhaas Gottmadingen
Thanks for the clarification @mxh I was really off-track here.
In order to hook into the validation of TypedData and Entities, we should be able to provide validation constraint plugins for each data type. They should be discovered automatically according to https://www.drupal.org/docs/drupal-apis/entity-api/entity-validation-api... β , and we could dispatch an ECA event, from which a model can take over and do whatever is needed.
- πΊπΈUnited States mlncn Minneapolis, MN, USA
I think i may have the event dispatch part figured out, but how do we get back from the action "into" the validator so as to return the custom message?
- π©πͺGermany jurgenhaas Gottmadingen
The event should implement a
setValidationErrorMessage
method which can be called by an action plugin calledSet validation error
or something like that.The validation plugin, which dispatched the event, should afterwards receive that error message from the event and signal that upstream, if there is any.
- Status changed to Needs review
4 months ago 4:55pm 18 July 2024 - πΊπΈUnited States mlncn Minneapolis, MN, USA
I have the branch working for my purpose.* Needs to switch to using "{entity_type}__{entity_id}" as a composite key with the action ID as keys as the nested array for ensuring that validation errors can be stashed to our hearts content without any risk of collision.
But i am sure there is a lot more maintainers will want to change before bringing this in.
*Validation works when an entity reference field is filled out, even when using Inline Entity Form to reference the existing entityβ it does not work for inline entity form creating a new entity. Process form on the other hand did not pick up even the existing entity reference on initial save; i am sure that is possible without this validation layer integration but it was enough of a hiccup to get me moving on this.
Attached is a model with this working; requires a vocabulary 'sources' and a term reference 'field_source' to it on article nodes.
- π©πͺGermany mxh Offenburg
Please create an according merge request if you have any code for reviewing.
- Status changed to Active
4 months ago 6:21pm 19 July 2024 - Status changed to Needs review
4 months ago 1:59am 20 July 2024 - πΊπΈUnited States mlncn Minneapolis, MN, USA
Was all right there in the issue branch; now it is a merge request too.
- π©πͺGermany mxh Offenburg
Current approach is a good start, however we need to check whether it's possible to set validation errors on any entity field.
As mentioned in the code review we should try to accomplish what was mentioned in #10.
As mentioned in #6 orientation on how eca_access does it may be a good guideline to realize this new feature.
- Status changed to Needs work
4 months ago 7:42am 21 July 2024 - π©πͺGermany mxh Offenburg
Many thanks for providing the MR and kicking this off. Took a first look and left some notes.
- π©πͺGermany jurgenhaas Gottmadingen
I've added a couple comments to the MR. Hope that helps.
Please note, before you set this to NR again, make sure that all tests need to be green first.
- π©πͺGermany mxh Offenburg
Did a bit of research and indeed we may be already able to set validation errors on multiple fields using the added
EcaConstraintValidator
within the merge request. To accomplish that we may need to adjust the implementation so that it will build a validation error similar to this:<?php class EcaConstraintValidator extends ConstraintValidator implements ContainerInjectionInterface { /** * {@inheritdoc} */ public static function create(ContainerInterface $container) { return new static(); } /** * {@inheritdoc} */ public function validate(mixed $value, Constraint $constraint) { // ... // Dispatch event, collect validation errors. Then loop through and do something like that: $this->context ->buildViolation($message) // Those parameters may not be needed when the message text is completely coming from ECA config. ->setParameter('@entity_type', $entity_type_label) ->setParameter('@field_name', $field_label) // This is the most interesting part: It allows us to set a certain field and even a property from it. // Example: $property_path = 'body.0.value'; ->atPath($property_path) ->addViolation(); } } ?>
So good news is that we are not in the need of decorating any handler, we can proceed with the added constraint and validator.
- Assigned to mxh
- π©πͺGermany mxh Offenburg
Will continue on this for a bit. Feel free to tell me here when someone else wants to take over instead. I'll just continue on the provided branch, if that's not good feel free to revert it.
- Issue was unassigned.
- Status changed to Needs review
4 months ago 7:39pm 30 July 2024 - π©πͺGermany mxh Offenburg
Refactored and implemented approach from #19. Please try it out whether it works for you.
This new feature should be covered with at least one test, thus adding tag.
- Status changed to Needs work
4 months ago 8:48am 1 August 2024 - π©πͺGermany jurgenhaas Gottmadingen
The code looks great and could be RTBC'd, but setting to NW as it requires a test first.
- Assigned to danielspeicher
- Status changed to Needs review
4 months ago 4:05pm 5 August 2024 - π©πͺGermany danielspeicher Steisslingen
I have added the test for the event to the existing test class, which tests all the events being fired.
For the new action, I have created a new class.Feel free to enhance by yourself, if I missed something.
All tests are running. - π©πͺGermany mxh Offenburg
Tests are looking good, thank you a lot for adding them! +1 for RTBC.
With the validation handler, I could imagine that we implement an ECA condition which takes a content entity as an argument and runs it through its validation handler. It returns TRUE or FALSE, depending on the result of the handler.
Taken from #5, I think it really makes sense to also have such a condition plugin available. It may be especially useful for example when creating new entities on the fly coming from external APIs. We could address it in a separate issue.
- Issue was unassigned.
- Status changed to RTBC
4 months ago 11:35am 7 August 2024 - π©πͺGermany jurgenhaas Gottmadingen
Thanks for the tests, they're looking good to me too. I've also created the follow-up issue β¨ Add condition to determine if a content entity is valid Active as suggested by @mxh
-
jurgenhaas β
committed 4df9ac9c on 2.1.x authored by
mlncn β
Issue #3363212 by mlncn, mxh, danielspeicher, jurgenhaas: eca_content:...
-
jurgenhaas β
committed 4df9ac9c on 2.1.x authored by
mlncn β
- Status changed to Fixed
4 months ago 11:37am 7 August 2024 Automatically closed - issue fixed for 2 weeks with no activity.