Entity reference does not validate auto-created entities

Created on 25 February 2015, almost 10 years ago
Updated 10 June 2024, 7 months ago

Problem

At #2429037: Allow adding entity level constraints β†’ we figured entity references do not validate auto-created entities for being valid. If there is custom validation logic e.g. on their label this should be run though?

By allowing for validating new entities a bug in the recursive validation was discovered, which didn't allow for the validation errors of the referenced entities to propagate. The problem is that if a constraint validator triggers the validation of a referenced entity and that referenced entity has the same constraint then currently we'll reuse the same instance of the validator and exchange its execution context while validating the referenced entities. This will lead to loosing the validation errors of the referenced entities.

Proposed resolution

  1. Let the current ValidReferenceConstraint also validate new entities.
  2. Remove the static cache for initialized constraint validator instances.

Remaining tasks

None.

User interface changes

Validation errors of newly created referenced entities will be shown.

API changes

New methods:

  1. FieldableEntityInterface::isValidationRunning()
  2. FieldableEntityInterface::isValidated()
  3. FieldableEntityInterface::setValidated()

FieldableEntityInterface::validate() now states that whoever is validating an entity is also responsible for showing its validation errors, otherwise other validation logic might decide to skip the validation of the entity based on the output of FieldableEntityInterface::isValidated(), which could be disabled by calling FieldableEntityInterface::setValidated(FALSE) on the validated entity in order to allow other further code to validate the entity. The purpose of this is to prevent showing the same errors multiple times.

πŸ› Bug report
Status

Needs work

Version

11.0 πŸ”₯

Component
EntityΒ  β†’

Last updated about 17 hours ago

Created by

πŸ‡¦πŸ‡ΉAustria fago Vienna

Live updates comments and jobs are added and updated live.
  • Needs change record

    A change record needs to be drafted before an issue is committed. Note: Change records used to be called change notifications.

  • Needs usability review

    Used to alert the usability topic maintainer(s) that an issue significantly affects (or has the potential to affect) the usability of Drupal, and their signoff is needed. When adding this tag, make it easy to review the issue. Make sure the issue summary describes the problem and the proposed solution. Screenshots usually help a lot! To get sign-off on issues with the "Needs usability review" tag, post about them in the #ux channel on Drupal Slack, and/or attend a UX meeting to demo the patch and get direct feedback from designers/UX folks/product management on next steps. If an issue represents a significant new feature, UI change, or change to the general "user experience" of Drupal, use Needs product manager review instead. See the scope of responsibilities for product managers.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request β†’ as a guide.

    +  public function isValidationRunning() {
    +    return $this->validationRunning;
    +  }
    +
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public function isValidated() {
    +    return $this->validated;
    +  }
    +
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public function setValidated($validated) {
    +    $this->validated = $validated;
    +    return $this;
       }
    

    1. these functions should be typehinted

    2. Will need a change record for the new functions on the interface (as they may be used by others)

    Since this is adding a new error message tagging for usability review. Can bring up in the #ux channel in slack to bring up early.

    Curious though if there's a good way to test this one?

  • πŸ‡ΊπŸ‡ΈUnited States caesius

    The latest posted patch fails to apply on Drupal 10.2.x. It seems to just be a merge conflict on the tests though, so the underlying code should still be good.

    Posting reroll for 10.2. I did not take the above suggestion re: typehinting into account as I was focused on just making a working patch.

    However, I'm having trouble verifying that this still does what I thought it was supposed to do. I'm certain I originally applied this patch so that custom validators I implemented on taxonomy terms run when migrating content via CSV. This would ideally have two effects:

    1. Nodes that try to create taxonomy terms with invalid names fail to import.
    2. Taxonomy terms with invalid names are never created in the first place.

    The first point always happens with or without the patch on both 10.1 and 10.2. The second point, however, does not -- the invalid taxonomy terms always get created and pollute the term list without getting referenced. Surely when I applied this patch it resolved one point or the other?... unfortunately I can't really revert the site back to 9.x to check.

Production build 0.71.5 2024