eca_content: Provide plugins for entity validation

Created on 28 May 2023, over 1 year ago
Updated 21 August 2024, 3 months ago

Problem/Motivation

Currently we are able to validate form input in the eca_form module, by reacting upon the "Validate form" event. However, in many cases, the validation itself belongs to the level of the entity, and not just the form.

Since entities can have a validation handler, it would be great being able to add validation there using ECA.

Steps to reproduce

Proposed resolution

Add events, conditions and action plugins for being able to perform entity validation on entity level.

Remaining tasks

User interface changes

API changes

Data model changes

✨ Feature request
Status

Fixed

Version

2.1

Component

Miscellaneous

Created by

πŸ‡©πŸ‡ͺGermany mxh Offenburg

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @mxh
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen
  • πŸ‡ΊπŸ‡Έ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):

    1. 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")
    2. Upon that event, do whatever you want in ECA, e.g. check for certain combination of fields values that must be correct
    3. 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 called Set 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
  • πŸ‡ΊπŸ‡Έ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
  • πŸ‡©πŸ‡ͺGermany mxh Offenburg
  • Merge request !437Provide plugins for entity validation β†’ (Merged) created by mlncn
  • Status changed to Needs review 4 months ago
  • πŸ‡ΊπŸ‡ΈUnited States mlncn Minneapolis, MN, USA

    Was all right there in the issue branch; now it is a merge request too.

  • Pipeline finished with Failed
    4 months ago
    Total: 485s
    #229470
  • πŸ‡©πŸ‡ͺ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
  • πŸ‡©πŸ‡ͺ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.

  • Pipeline finished with Failed
    4 months ago
    Total: 434s
    #238628
  • Pipeline finished with Failed
    4 months ago
    Total: 425s
    #238679
  • Pipeline finished with Failed
    4 months ago
    #238689
  • Pipeline finished with Success
    4 months ago
    Total: 401s
    #238690
  • Issue was unassigned.
  • Status changed to Needs review 4 months ago
  • πŸ‡©πŸ‡ͺ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
  • πŸ‡©πŸ‡ͺ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
  • πŸ‡©πŸ‡ͺGermany danielspeicher Steisslingen
  • Status changed to Needs review 4 months ago
  • πŸ‡©πŸ‡ͺ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
  • πŸ‡©πŸ‡ͺ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

  • Pipeline finished with Skipped
    4 months ago
    #246754
  • Status changed to Fixed 4 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024