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

Modified form validation #527

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

davidebianchi03
Copy link

Hi, I was trying to use unicorn form validation with a model that has the unique_together model's Meta option.
Unicorn validation didn't validate this option. I saw your validation code and i noted two things:

  1. In the validation of a model with unique_together option or other types of validations that work on more than one fields of model error messages are indexed by Django with key called __all__ and with existing validation this field was never read
  2. I saw that you validate only the field that has been modified by the user and not the entire form like Django does with his forms

I fixed this two problems with the code that i modified in this pull request. I tried to run your tests and they didn't pass, i saw that this happens because the tests didn't enter in old code but now the tests enter and they don't pass. I haven't changed the tests.

Probably you need to change the documentation and tests if you merge this pull request

@adamghill
Copy link
Owner

Thanks for this PR!

with key called all and with existing validation this field was never read

Nice catch, I think it makes sense to support this.

I saw that you validate only the field that has been modified by the user and not the entire form like Django does with his forms

Yeah, that was by design to enable more iterative workflows that use forms. For example, consider a form with 2 fields. If the form is validated on blur then the second field will be considered invalid after the first one is filled in. I am concerned this functionality is confusing and/or differs too much from Django semantics, though.

I wonder if there could be a way support both approaches? Maybe with a new magic keyword like $validateAll or something similar. What do you think?

@davidebianchi03
Copy link
Author

I add the possibility to declare in UnicornView a special attribute called 'validate_all', if it is set to true value it validates the entire form, otherwise it validates only the fields that have been changed by the user.

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

Successfully merging this pull request may close these issues.

None yet

2 participants