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

Add support for validation error codes #702

Closed
wants to merge 4 commits into from

Conversation

rpkilby
Copy link
Collaborator

@rpkilby rpkilby commented May 2, 2017

This is for both the base FilterSet and the DRF variant.

  • The base FilterSet is slightly broken, since it simply raises ValidationError(self.form.errors). This loses the individual validation error codes.
  • The raw_validation approach needs to be modified to return an actual serializers.ValidationError, since the plain data structure lacks the error codes.

(This is WIP)

@codecov-io
Copy link

Codecov Report

Merging #702 into develop will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #702   +/-   ##
========================================
  Coverage    98.13%   98.13%           
========================================
  Files           15       15           
  Lines         1123     1123           
========================================
  Hits          1102     1102           
  Misses          21       21
Impacted Files Coverage Δ
django_filters/filterset.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd0af3c...e14d2e0. Read the comment docs.

@carltongibson carltongibson added this to the Version 1.0.3 milestone May 2, 2017
@rpkilby
Copy link
Collaborator Author

rpkilby commented May 2, 2017

I'm currently stuck on this. I'm 95% sure there's a bug preventing DRF ValidationErrors from having custom error codes. In essence, there's this section which interacts with how ValidationErrors coerce their string messages to a list here. In short, instead of getting an ErrorDetail with a string and code, you just get a list of strings.

I need to look into this further.

@rpkilby
Copy link
Collaborator Author

rpkilby commented Aug 30, 2017

Note to self... https://github.com/encode/django-rest-framework/issues/4787#issuecomment-326062971

return ValidationError(OrderedDict([
(key, [e.message for e in error_list])
for key, error_list in error.error_dict.items()
]))
Copy link
Contributor

Choose a reason for hiding this comment

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

You might want to keep codes here:

        error_dict = OrderedDict([
            (key, [exceptions.ErrorDetail(e.message,
                                          e.code if e.code else default_code)
                   for e in error_list])
            for key, error_list in error_dict.items()])
        drf_validation_error = exceptions.ValidationError(error_dict)

@blueyed
Copy link
Contributor

blueyed commented Oct 18, 2017

See encode/django-rest-framework#4787 (comment) for an updated version that also handles params.

@rpkilby
Copy link
Collaborator Author

rpkilby commented Oct 23, 2017

I'm closing this off in favor of #788, which is generally reworking filterset validation.

@rpkilby rpkilby closed this Oct 23, 2017
@rpkilby rpkilby deleted the validation-error-code branch October 23, 2017 23:22
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

4 participants