Convert to internal value before validation by johtso · Pull Request #2712 · encode/django-rest-framework (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation5 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Fixes #2710

How does this look?

Choose a reason for hiding this comment

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

I would consider re-jigging how this works - as it is this kinda confuses me.

Could we just alter the if data == '': check so that it also checks for whitespace only. Eg: if data == '' or self.trim_whitespace and six.text_type(data).strip() == '':

Choose a reason for hiding this comment

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

Why shouldn't we be relying upon validate_empty_values ?
Seems sensible to me.

Choose a reason for hiding this comment

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

Forget about the previous comment. Actually, it's the call to to_internal_value that does the trimming.

Choose a reason for hiding this comment

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

Can't we go with this instead:

value = super(CharField, self).run_validation(value)
if value == '':
    if not self.allow_blank:
        self.fail('blank')
return value

This was referenced

Mar 9, 2017

This was referenced

Oct 6, 2017

This was referenced

Oct 16, 2017

This was referenced

Nov 6, 2017

This was referenced

Nov 14, 2017

This was referenced

Dec 10, 2017