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

IntegerField.to_internal_value coerces decimal to integer; should fail #2835

Closed
bradford281 opened this issue Apr 17, 2015 · 3 comments
Closed

Comments

@bradford281
Copy link

bradford281 commented Apr 17, 2015

The to_internal_value method coerces data to an int, so when a float/decimal is passed, the int value is saved, rather than raising an exception.

ryangallen pushed a commit to ryangallen/django-rest-framework that referenced this issue Apr 17, 2015
ryangallen pushed a commit to ryangallen/django-rest-framework that referenced this issue Apr 17, 2015
@tomchristie
Copy link
Member

tomchristie commented Apr 17, 2015

I'm afraid you'll need to customize the field if you want this behavior.
An integer field that coerces values to an integer wherever possible isn't unreasonable.

While I absolutely understand see why you might want/expect this change, other folks probably wouldn't.

@bradford281
Copy link
Author

bradford281 commented Apr 19, 2015

It just seems inconsistent with Django, as entering floats in a Django form for IntegerField will raise a ValidationError, whereas in DRF 3.x it does not (I believe it did in DRF 2.x).

Strangely enough, the expected ValidationError will be raised if the float is wrapped in a string (the attempt to coerce it to int fails "ValueError: invalid literal...") so it may even be inconsistent with itself. To clarify:
data = 1 # success
data = "1" # success; equivalent to data = 1
data = 0.1 # success; unexpectedly equivalent to data = 0
data = "0.1" # fail; expected ValidationError, in line with Django form validation

This commit seems to return the field to the expected behavior: ryangallen@3e6a149

ryangallen pushed a commit to ryangallen/django-rest-framework that referenced this issue Apr 20, 2015
…s being stored as ints, fixes encode#2835.

Match IntegerField validation with Django IntegerField, prevents decimal values being stored as ints, fixes encode#2835
 On branch master
@ryangallen
Copy link
Contributor

ryangallen commented Apr 20, 2015

@tomchristie This pull handles cases where values such as 1.0 or "1.0" pass through as integers but values like 1.5 or "1.5" do not. It matches Django's IntegerField validation and I believe is the behavior most developers would expect. Changing a clear decimal value, say 0.4 to 0, is not expected and could have negative consequences in an application.

kevin-brown added a commit that referenced this issue Apr 20, 2015
only allow integer field to accept integers and strings, fixes #2835
tinchodipalma added a commit to tinchodipalma/django-rest-framework that referenced this issue May 25, 2015
The following items had a wrong href value:

- Dont require pk strictly for related fields. (encode#2745, encode#2754)
- Restrict integer field to integers and strings. (encode#2835, encode#2836)
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

No branches or pull requests

3 participants