Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Form][Validator] Enhance FormError #9250

Closed
WedgeSama opened this issue Oct 9, 2013 · 8 comments
Closed

[Form][Validator] Enhance FormError #9250

WedgeSama opened this issue Oct 9, 2013 · 8 comments

Comments

@WedgeSama
Copy link
Contributor

I currently work on a client side validator base on server side rules and I think somethink can be enhance in FormError.

At the moment, if a FormError is throw from the validator, there is not way to know from which field/form the error come from (to use it on the client side for example).

I think it is a good idea to add the id (or the path) of the field/form where come from the FormError.

What do you think about it?

@stof
Copy link
Member

stof commented Oct 9, 2013

Form errors are retrieved from the form instance. So you know where it is from: it is from the place where you retrieved it.

If your client-side code does not have this information, I would blame the way you serialize the errors to send them to the client (the serialization implemented in JMSSerializerBundle preserves it for instance)

@WedgeSama
Copy link
Contributor Author

I think I missed some explanations.

I serialize all information about constraints for each field/form and I can access to them in the client side, I know where I have to display error messages, this is not the problem.

The problem is if the form is previously submit with errors, some FormError are displayed in template. But if you use error_mapping or error_bubbling to move those errors in another place, you cant be sure where they come from, especially in the case of moving multiple errors in the same place.

This example is a very simple case, just to show the problem:

/** @ORM\Entity */
class Entity {
    /** @Assert\Type(type="string", message="entity.error.field") */
    private $field1;

    /** @Assert\Type(type="string", message="entity.error.field") */
    private $field2;
}

// AbstractType
class EntityType extends AbstractType {
    public function buildForm(FormBuilderInterface $builder, array $options) {
        $builder
            ->add('field1', null, array('error_bubbling' => true))
            ->add('field2', null, array('error_bubbling' => true));
    }
}

In this example, if field1&2 are invalid, the same error message are displayed twice in the root form, one for field1 and a second for field2. In this case, it is not possible to be sure which error message is for which field.

@tyomo4ka
Copy link
Contributor

@WedgeSama

Your case has sense for me. If we will collect all errors in one place and use default messages it will not be clear which error relates to which field.

BTW It is not so hard to implement for first look. We can add field originField to FormError class and fill it in addError method if it is not empty.

@Burgov
Copy link
Contributor

Burgov commented Dec 18, 2013

I'm not sure why you would want to enable error_bubbling if you wish to know where the error came from. However, this problem you describe is an issue when validation fails on a property in your entity which isn't part of your form.

I think this PR should fix your issue: #9582

@tyomo4ka
Copy link
Contributor

@Burgov

I'm not sure why you would want to enable error_bubbling if you wish to know where the error came from.

For example if you want to show all errors at one place (not for each field), but you don't want to create custom error messages. Does it sensible?

@apfelbox
Copy link
Contributor

@tyomo4ka this seems like a view-related issue. I wouldn't change the internal data structure for this, but just iterate over all form fields and collect the errors in the view.

@tyomo4ka
Copy link
Contributor

@apfelbox

Mate, it is not very simple :) It is need to iterate over all the FormView recursively. 1st it requires CPU resources, 2nd it requires some code to be written.

IMO link to field where error was generated might be useful in some cases. But it is possible to live without it ;)

@webmozart
Copy link
Contributor

This was fixed in #9993.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants