Skip to content

only allow integer field to accept integers and strings, fixes #2835#2836

Merged
kevin-brown merged 1 commit intoencode:masterfrom
ryangallen:master
Apr 20, 2015
Merged

only allow integer field to accept integers and strings, fixes #2835#2836
kevin-brown merged 1 commit intoencode:masterfrom
ryangallen:master

Conversation

@ryangallen
Copy link
Copy Markdown

No description provided.

@kevin-brown
Copy link
Copy Markdown
Contributor

I can't say I'm thrilled at the idea of doing type checking here, but I will point out that in Python 2 you also have to be able to handle long. And you can pass in a tuple as the second argument to isinstance instead of doing or multiple times.

Django itself handles this by calling str before int (and also stripping trailing decimals). This will trigger errors if decimal values are passed in, and it avoids type checking.

@ryangallen
Copy link
Copy Markdown
Author

Thanks Kevin, good call. I adjusted and squashed the change.

…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
@jpadilla
Copy link
Copy Markdown
Contributor

LGTM

kevin-brown added a commit that referenced this pull request Apr 20, 2015
only allow integer field to accept integers and strings, fixes #2835
@kevin-brown kevin-brown merged commit 5cc0a3c into encode:master Apr 20, 2015
@kevin-brown
Copy link
Copy Markdown
Contributor

Looks good, thanks for making the change.

Comment thread rest_framework/fields.py
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  1. We should push this to self.re_decimal as per in Django so that it's only compiled once.
  2. We should comment that this regex substitution is so that we allow eg '1.0' as an int, but not '1.2'

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@lovelydinosaur
Copy link
Copy Markdown
Contributor

Coupla inline comments that would be good to resolve.

@lovelydinosaur lovelydinosaur added this to the 3.1.2 Release milestone Apr 21, 2015
tinchodipalma added a commit to tinchodipalma/django-rest-framework that referenced this pull request 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)
@tinchodipalma tinchodipalma mentioned this pull request May 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants