FormState::setErrorByName does not set multiple errors on the same element

Created on 14 October 2016, almost 9 years ago
Updated 10 October 2023, almost 2 years ago

Problem/Motivation

This is a follow-up from the alexpott's comment #2767125-22: EntityFormDisplay::validateFormValues sets form state errors by empty name string instead using the provided form element โ†’ .

The problem with FormState::setErrorByName is that it will bet only one error per element and ignore all following calls with the same name.
What currently would happen is that if there are 3 errors for an element only the first one will be shown to the user, after the users clears the first problem and submits again now the second error will be shown ...
This would be the case e.g. when having entity level validation and having to set multiple errors on a whole nested entity form. This would happen in EntityFormDisplay::validateFormValues where all the entity level validation errors are set on the given form element.

Proposed resolution

FormState::setErrorByName should support multiple errors per name, but changing how the function sets errors and what FormState::getErrors returns will be an API break, so a non invasive solution is needed.

Remaining tasks

Find a proper solution.

User interface changes

See all the errors at once for a given element instead of having to resubmit the form to see the next error after the first one is fixed.

API changes

none at moment.

๐Ÿ› Bug report
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Formย  โ†’

Last updated about 4 hours ago

Created by

๐Ÿ‡ฉ๐Ÿ‡ชGermany hchonov ๐Ÿ‡ช๐Ÿ‡บ๐Ÿ‡ฉ๐Ÿ‡ช๐Ÿ‡ง๐Ÿ‡ฌ

Live updates comments and jobs are added and updated live.
  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

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

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Anybody Porta Westfalica

    @smustgrave: Pinging here, as this issue for example blocks contrib issues like ๐Ÿ› Use of $form_state->setErrorByName() within email_registration_user_login_validate prevents other core errors Needs work . Also see #39

    To me, it looks like a misconception in multiplicity here, what can we do for "Needs subsystem maintainer review" as of #37?

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Grevil

    I know this is probably far-fetched and might need too much refactoring, but in some cases it would be super helpful if each element error could have their own ID. This way, it would be possible to override specific element errors by ID.

    I just came to this idea, while taking a look at ๐Ÿ› Use of $form_state->setErrorByName() within email_registration_user_login_validate prevents other core errors Needs work . Basically what we want there is to only overwrite the error message that gets thrown, when using incorrect credentials (password / username) and leave all other errors thrown (e.g. the "Too many failed login attempts" error message). An optional "$eid" parameter would be enough:

      /**
       * @param string $name
       *   The name of the form element. If the #parents property of your form
       *   element is array('foo', 'bar', 'baz') then you may set an error on 'foo'
       *   or 'foo][bar][baz'. Setting an error on 'foo' sets an error for every
       *   element where the #parents array starts with 'foo'.
       * @param string $eid
       *   (optional) The error id to use for your error
       * @param string $message
       *   (optional) The error message to present to the user.
       *
       * @return $this
       */
      public function setErrorByName($name, $eid, $message = '');
    

    The only problem is, that core would need to adjust all its "setErrorByName()" calls. Otherwise, this wouldn't have any real benefit.

    What are everybody's thoughts on this? Would you agree with this feature request? Should I create a follow up issue for that?

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Anybody Porta Westfalica

    Agree with additional benefits from #42!
    That shouldn't be a blocker and might be a follow-up, but it has been a good idea to allow a key for such, to allow altering!

    Like other areas do better or worse since Drupal 7, e.g. https://api.drupal.org/api/drupal/includes%21common.inc/function/drupal_... ($key) or #attached in Drupal 8+! It's a lovely and helpful general concept.

    Good point @Grevil!

    Let's hear what the maintainers think.

  • ๐Ÿ‡ฉ๐Ÿ‡ฐDenmark uv516 Denmark

    Drupal 11.2:
    When I call FormState::setError multible times it only show the first message.
    I call: $form_state->setError($form,$message);

  • First commit to issue fork.
  • Pipeline finished with Failed
    12 days ago
    Total: 226s
    #574800
  • Pipeline finished with Failed
    12 days ago
    Total: 144s
    #574805
  • Pipeline finished with Failed
    12 days ago
    Total: 148s
    #574815
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany rgpublic Dรผsseldorf ๐Ÿ‡ฉ๐Ÿ‡ช ๐Ÿ‡ช๐Ÿ‡บ

    I really, really like #42 as well. I often thought about this. This would be so useful. There is no way to overwrite specific error messages with more specific or different ones at the moment. With page attachments / headers we already have the concept of specific keys so that's a very "Drupal-ish" idea as well. This could indeed be introduced in a followup. I think the best way to do that would be a new addErrorByName (instead of setErrorByName). That would also be a more fitting name now that we actually append stuff. This way we could add the new $eid signature. Those two could exist in parallel for quite a while (to avoid any compatibility problems) and then later by deprecated. Incrementally more and more errors would get the $eid. And finally we make the switch. After that we could introduce more stuff to actually modify/overwrite things like removeErrorsById($eid) etc.

Production build 0.71.5 2024