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

Clarify 'to_internal_value()' validation behavior #5466

Merged
merged 1 commit into from Oct 2, 2017

Conversation

rpkilby
Copy link
Member

@rpkilby rpkilby commented Sep 29, 2017

This should resolve #3864 and #5438.

@rpkilby
Copy link
Member Author

rpkilby commented Sep 29, 2017

cc @tomchristie, @carltongibson

Copy link
Collaborator

@carltongibson carltongibson left a comment

Yep. That looks great.

@tomchristie
Copy link
Member

tomchristie commented Sep 29, 2017

I'm not totally sure this is the tack that the docs should take.

Perhaps we should instead just be calling out more clearly (at L997) that to_internal_value and to_representation are generally not the override that developers should be using, and that validate_<field> and validate are.

@@ -1011,7 +1011,7 @@ Takes the object instance that requires serialization, and should return a primi

Takes the unvalidated incoming data as input and should return the validated data that will be made available as `serializer.validated_data`. The return value will also be passed to the `.create()` or `.update()` methods if `.save()` is called on the serializer class.

If any of the validation fails, then the method should raise a `serializers.ValidationError(errors)`. Typically the `errors` argument here will be a dictionary mapping field names to error messages.
If any of the validation fails, then the method should raise a `serializers.ValidationError(errors)`. The `errors` argument should be a dictionary mapping field names (or `settings.NON_FIELD_ERRORS_KEY`) to a list of error messages. If you don't need to alter deserialization behavior and instead want to provide object-level validation, it's recommended that you intead override the [`.validate()`](#object-level-validation) method.
Copy link
Member

@tomchristie tomchristie Sep 29, 2017

Choose a reason for hiding this comment

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

If we did accept this docs change, I think we ought to at least split this into two paragraphs.

@MadWombat
Copy link

MadWombat commented Sep 29, 2017

Just to leave my 2 cents here. I am not sure how this is supposed to work, but in my project we use to_internal_value and to_representation a lot because the data sent to the API and the data received from the API are different for most of our APIs and a lot of fields do not map directly to the underlying models. We still get a lot of mileage from ModelSerializer, since about 90% of fields do map directly, but it is the other 10% that makes us use custom serialization heavily.

@rpkilby
Copy link
Member Author

rpkilby commented Sep 29, 2017

@MadWombat, it's difficult to give advice without having a practical/concrete example to work from. Open a discussion on the support group and I'll see if I can help.

@carltongibson
Copy link
Collaborator

carltongibson commented Sep 29, 2017

@tomchristie I take your point about steering people to validate_field, but does this change on its own not help that? i.e. removing the mention of validation from the opening paragraph.

Perhaps an additional note saying “make sure you read all the stuff higher up before you take on this” might be enough... it is in the Advance Use section.

(Slight aside, I think my first conversations on using DRF, moons ago, were around using validate_field functions to transform values.)

@tomchristie tomchristie merged commit e0a6c4b into encode:master Oct 2, 2017
1 check passed
@rpkilby rpkilby deleted the object-validation-docs branch Nov 20, 2017
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.

Raising ValidationError from Serializer.to_internal_value() with error message(s) as list causes ValueError
4 participants