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

Minimal change required to show all validation errors for all required fields #7

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

Conversation

macie-korte
Copy link

Minimal change required to show all validation errors for all required fields instead of only a small subset.

I know you will probably not want this change as-is, I am opening this PR only to demonstrate what change was needed to make the desired behavior work for me.

if any([not self.parent.partial,
self.parent.Meta.failure_mode == FormSerializerFailure.fail,
self.field_name in self.parent.Meta.minimum_required]):
if self.field_name in self.parent.Meta.minimum_required:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does this help? the original conditions are more strict when validating fields

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now it makes sense. you want to bypass the DRF field validations in order to force form validations to kick in. better place for that would be to add an option in the Meta which will force the serializer to not include any DRF fields and will force validation to happen in the form.

places to look in the code:

FormSerializerBase.get_fields
FormSerializerOptions

@coveralls
Copy link

coveralls commented Jul 22, 2016

Coverage Status

Coverage remained the same at 99.793% when pulling 816e547 on macie-korte:full-validation into ab6fec6 on dealertrack:master.

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.

3 participants