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

Resetting a CharFiled to an empty string after fix for #3130 #3318

Closed
mkemmerling opened this issue Aug 22, 2015 · 6 comments
Closed

Resetting a CharFiled to an empty string after fix for #3130 #3318

mkemmerling opened this issue Aug 22, 2015 · 6 comments
Labels
Milestone

Comments

@mkemmerling
Copy link

Commit d231f36 to fix #3130 added the following check to fields.Field.get_value():

        elif ret == '' and self.default:
            return empty

Maybe I am wrong, but it looks like with this change it is no longer possible to reset a CharField with allow_blank=True but no default argument which currently has a non-empty value back to an empty string: The default value for the default argument is the empty marker. Since the validate_empty_values() method raises a SkipField execption for this marker the field is skipped on update if set to an empty string.

To work around this problem the field may be defined with the empty string as default argument, but to have both allow_blank=True and default='' in the field's argument list seems counter-intuitive.

@xordoquy
Copy link
Collaborator

I'm sorry it's a bit abstract to me. Do you have a failing test case or snippet we could look at ?

@mkemmerling
Copy link
Author

Hi Xavier,

thanks for your prompt reply. You can reproduce the problem with the tutorial.

The Snippet class has a title field:

class Snippet(models.Model):
    ...
    title = models.CharField(max_length=100, blank=True, default='')
    ...

(btw I wonder why the default='' argument is added, since that's what blank=True is saying already).

To reproduce the problem:

  1. Go to the browsable API and add a snippet with a title.
  2. Update the snippet and reset the title to an empty string. This has no effect, the original title is still displayed.

Adding

title = serializers.CharField(required=False, allow_blank=True,
                              max_length=100, default='')

to the SnippetSerializer fixes this. Note the default='' argument (actually any value evaluating to False will do).

I forget to mention that the problem occurs with HTML requests only, not with JSON requests (as can also be reproduced in the browsable API).

The reason for the described behaviour is that with the change introduced in commit d231f36 the empty string is skipped on validation of the serializer, therefore not included in the validated serializer data, and therefore not updated. See https://gist.github.com/mkemmerling/c6f532b31c8c41e9b5c8 .

Does this help to clarify the problem?

@xordoquy
Copy link
Collaborator

Actually, I gave this a try and my title was set to an empty string.
What Django REST framework version are you using ?

Note that default is enforced at the DB level which means that if you're creating an instance with the Django shell without giving a title it'll be an empty string.

@xordoquy
Copy link
Collaborator

I forget to mention that the problem occurs with HTML requests only, not with JSON requests (as can also be reproduced in the browsable API).

This is a limitation of the html forms that would send an empty string where JSON data makes a difference between empty and null.

Will have a look at your test case. Thanks for the details.

@mkemmerling
Copy link
Author

I'm using version 3.2.3 (with Django 1.8.,4). The problem arises with the changes made for 3.2.0. And yes it occurs only with html forms where the empty marker is used.

Thanks again for having a look into this.

@xordoquy
Copy link
Collaborator

ok, I had an issue in my DRF version and confirme I could reproduce your issue with the tutorial.

@tomchristie tomchristie added this to the 3.2.3 Release milestone Aug 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants