Skip to content

If a field triggers two validation errors, they are not listed separately #20

@simonrjones

Description

@simonrjones

Spotted an issue - if a field triggers two validation errors, they are not listed separately in the error bag, and are just appended to a single item.

Example:

There are 2 problems with this request
ERROR: The new password field must be at least 8 characters. ERROR: The new password field confirmation does not match.
ERROR: The current password is incorrect.

This means the "2 problems with this request" message is incorrect (it should be 3), and it also looks strange and the second error isn't correctly semantically defined as a separate list item.

Did a bit of snooping and found the culprit (brain dump ahead):

src/Twig/AccessibleFormsExtension.php:60

foreach ($form->children as $child) {
    if (isset($child->vars['errors']) && count($child->vars['errors']) > 0) {
        $errors[] = [
            'id' => $child->vars['id'],
            'name' => $child->vars['full_name'],
            'message' => (string) $child->vars['errors']
        ];
    }
}

This iterates over fields, not errors. And:

(string) $child->vars['errors']

invokes: vendor/symfony/form/FormErrorIterator.php:71

public function __toString(): string
{
    $string = '';

    foreach ($this->errors as $error) {
        if ($error instanceof FormError) {
            $string .= 'ERROR: '.$error->getMessage()."\n";
        } else {
            /** @var self $error */
            $string .= $error->getForm()->getName().":\n";
            $string .= self::indent((string) $error);
        }
    }

    return $string;
}

Meaning each field will get all its associated errors dumped into a single string, rather than listed separately.

We'll need to refactor this a bit.

One solution is exploding on the word ERROR:, and processing the fragments as separate errors. Works, but feels a bit hacky.

if (isset($child->vars['errors']) && count($child->vars['errors']) > 0) {

$fieldErrors = explode('ERROR: ', (string) $child->vars['errors']);

foreach ($fieldErrors as $fieldError) {
    if (!$fieldError) {
        continue;
    }

    $errors[] = [
        'id' => $child->vars['id'],
        'name' => $child->vars['full_name'],
        'message' => 'ERROR: ' . $fieldError
    ];
}

}

As a side note, the issue with the radio/checkbox button validation failures would probably also involve this piece of code. I haven't been able to detect whether a field has options at this stage, so can't tell if it's a radio/checkbox group and add the _0 to the ID as appropriate... needs more investigation.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions