-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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 validate method back to ListSerializer #2225
Conversation
# Eg. Calling Model.clean() explicitly inside Serializer.validate() | ||
raise ValidationError({ | ||
api_settings.NON_FIELD_ERRORS_KEY: list(exc.messages) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a large copy/paste from Serializer.run_validation
. I would prefer to refactor this to be more DRY, but I'm not sure how best to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we dropped the ValidationError
and DjangoValidationError
handling code into a function that takes an exception and returns the details for ValidationError
to be raised, we could have that just in once place, which would be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also consider pushing this... https://github.com/tomchristie/django-rest-framework/blob/master/rest_framework/fields.py#L307-L320 into a validate_empty_vaues
@@ -462,6 +462,42 @@ def get_value(self, dictionary): | |||
return html.parse_html_list(dictionary, prefix=self.field_name) | |||
return dictionary.get(self.field_name, empty) | |||
|
|||
def run_validation(self, data=empty): | |||
data = super(ListSerializer, self).run_validation(data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably recommend we drop the call to super here, in the same way we do for Serializer
. It gets pretty confusing otherwise. This'd also let us move the if not isinstance(data, list)
check out of to_internal_value
which would be nicer and would mirror the Serializer
classes if not isinstance(data, dict)
check that occurs.
Going to take a look at some of this now based on your input. |
Fixes #2168.