Constraints on paragraph field highlight all fields in paragraph

Created on 26 December 2018, over 5 years ago
Updated 30 October 2023, 8 months ago

Problem/Motivation

I recently attempted to add a constraint on a paragraph field so that a user could not select the same dropdown value twice. The constraint works but highlights the whole paragraph item rather than the field the error is on. This is misleading and possibly confusing for the end user.

Here is the content setup:

Content Type

  • Entity Reference field to paragraph

Paragraph

  • text list
  • Entity reference to content

Actual:

Expected:

I found that field error handling is passed up to the widget and that InlineParagraphsWidget and ParagraphsWidget handle errorElement differently. For my use case, I want to drill down to the actual field causing the error so I started using "Paragraphs EXPERIMENTAL". While testing, I came to the understanding that FormState::setError expects the #parents key of the element to contain the correct property path to the element. Quick tangent, what constraints consider property paths and formState considers $name are identical except property paths expect "." to separate the field names and $name expects "][".

property path: field_quote_detail_page_componen.1.subform.field_component_name_wrapper
name: field_quote_detail_page_componen][1][subform][field_component_name_wrapper

Here is where the problem exists, (WidgetBase::flagErrors()):

foreach ($delta_violations as $violation) {
            // @todo: Pass $violation->arrayPropertyPath as property path.
            $error_element = $this->errorElement($delta_element, $violation, $form, $form_state);
            if ($error_element !== FALSE) {
              $form_state->setError($error_element, $violation->getMessage());
            }
          }

$this->errorElement will call ParagraphsWidget::errorElement which in terns calls NestedArray::getValue($element, $error->arrayPropertyPath). The returned element, $error_element, will have a parent key as follows:

result = {array} [4]
 0 = "field_quote_detail_page_componen"
 1 = 1
 2 = "subform"
 3 = "field_component_name_wrapper"

When FormState::setError is called it uses the above array to build at the $name arg for FormState::setErrorByName, field_quote_detail_page_componen][1][subform][field_component_name_wrapper. While we cannot assign an error to a wrapper element the error message will show but there is no error styling to alert the user where the error is.

Proposed resolution

Escalate the todo in WidgetBase::flagErrors():

// @todo: Pass $violation->arrayPropertyPath as property path.

Come up with workaround

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Needs review

Version

1.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States robpowell Boston

Live updates comments and jobs are added and updated live.
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.

  • πŸ‡¨πŸ‡­Switzerland tcrawford

    ParagraphsWidget::errorElement() attempts to get the sub element when the path is valid. It currently checks for empty($this->arrayPropertyPath) where arrayPropertyPath is got via magic on Drupal\Core\Field\InternalViolation, but InternalViolation does not implement __isset(). Therefore the code on errorElement never returns $sub_element, even if there is a valid sub element found by NestedArray::getValue(). We could open an issue in core for InternalViolation to implement __isset(), but likely that won't move forward as accessing arrayPropertyPath via magic is deprecated. Therefore, I would propose the following solution (patch to follow):

    ?>
    /**
       * {@inheritdoc}
       */
      public function errorElement(array $element, ConstraintViolationInterface $error, array $form, FormStateInterface $form_state) {
        // Validation errors might be a about a specific (behavior) form element
        // attempt to find a matching element.
        // A temporary variable needed as arrayPropertyPath is accessed via magic
        // and Drupal\Core\Field\InternalViolation does not implement __isset.
        // Therefore, empty($error->arrayPropertyPath) returns true even when the
        // magic getter returns a value.
        $arrayPropertyPath = $error->arrayPropertyPath;
        if (!empty($arrayPropertyPath) && $sub_element = NestedArray::getValue($element, $error->arrayPropertyPath)) {
          return $sub_element;
        }
        return $element;
      }
    
  • πŸ‡¨πŸ‡­Switzerland tcrawford

    Patch attached.

  • Status changed to Needs review 12 months ago
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update 12 months ago
    180 pass
  • πŸ‡¨πŸ‡­Switzerland tcrawford
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MariaDB 10.3.22
    last update 12 months ago
    159 pass, 12 fail
  • πŸ‡§πŸ‡ͺBelgium herved

    Hi @robpowell, @tcrawford,

    It would be great if you could check out the patch in πŸ“Œ Stop using $error->arrayPropertyPath Needs review which should fix both issues (this one + the deprecation of arrayPropertyPath), so we could close this as duplicate.

    Thanks

Production build 0.69.0 2024