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

Model defaults ignored on empty text field or empty boolean field. #1101

Closed
ZeaQQ4Ug opened this issue Sep 12, 2013 · 33 comments · Fixed by #2311
Closed

Model defaults ignored on empty text field or empty boolean field. #1101

ZeaQQ4Ug opened this issue Sep 12, 2013 · 33 comments · Fixed by #2311
Labels
Milestone

Comments

@ZeaQQ4Ug
Copy link

ZeaQQ4Ug commented Sep 12, 2013

My tables has boolean fields like below.

enabled = models.BooleanField(default=True)

and save it using a serializer in a view class without passing the enabled field in the request.

serializer = SomeSerializer(data=request.DATA)
        if serializer.is_valid():
            object = serializer.save()

the result is object.enabled == False.

I've updated the framework to version 2.3.8 then the issue started occurring. Everything works fine in 2.3.7.

Affected versions >= 2.3.8

@lonli078
Copy link

lonli078 commented Sep 12, 2013

this problem have in tests and when open auto generic api web, in section raw data
Content: {
"is_active": false,
"username": "",
"email": "",
"name": "" }
but in models is_active default = True

but if send post request via curl or another...everything is working properly, is_active set TRUE

@ZeaQQ4Ug
Copy link
Author

ZeaQQ4Ug commented Nov 17, 2013

This problem hasn't fixed even in 2.3.9.
I can't update it from 2.3.7 because of this, breaking numerous test cases.

@tomchristie
Copy link
Member

tomchristie commented Nov 17, 2013

Is there any open pull request associated with this issue?
First starting point would be coding a failing test case.

@tomwalker
Copy link

tomwalker commented Nov 19, 2013

I have never submitted a patch or anything before, but would this be a suitable failing test case? Would it work with /tests/test_serializer.py?

class DefaultTrueBooleanModel(models.Model):                                                                                                                                                                                              
    enabled = models.BooleanField(default=True)                                                                                                                                                                                           


class SerializerDefaultTrueBoolean(TestCase):                                                                                                                                                                                             

    def setUp(self):                                                                                                                                                                                                                      
        super(SerializerDefaultTrueBoolean, self).setUp()                                                                                                                                                                                 

        class DefaultTrueBooleanSerializer(serializers.ModelSerializer):                                                                                                                                                                  
            class Meta:                                                                                                                                                                                                                   
                model = DefaultTrueBooleanModel                                                                                                                                                                                           
                fields = ('enabled')                                                                                                                                                                                                      

        self.default_true_boolean_serializer = DefaultTrueBooleanSerializer                                                                                                                                                               

    def test_blank_input(self):                                                                                                                                                                                                           
        serializer = self.default_true_boolean_serializer()                                                                                                                                                                               
        self.assertEqual(serializer.data['enabled'], True)                                                                                                                                                                                

    def test_enabled_as_false(self):                                                                                                                                                                                                      
        serializer = self.default_true_boolean_serializer({'enabled': False})                                                                                                                                                             
        self.assertEqual(serializer.data['enabled'], False)                                                                                                                                                                               

    def test_enabled_as_true(self):                                                                                                                                                                                                       
        serializer = self.default_true_boolean_serializer({'enabled': True})                                                                                                                                                              
        self.assertEqual(serializer.data['enabled'], True)      

@akenney
Copy link

akenney commented Feb 5, 2014

Hello,

I encountered this bug when upgrading from version 2.3.9 to 2.3.12 of django-rest-framework, and eventually narrowed it down to this commit (90edcbf). I think this fix doesn't quite address what the actual issue is: the issue is that if you create an object using the HTML form and leave a checkbox unchecked, it will leave out that value from the form, and that should be interpreted as indicating that the value should be false, regardless of its default; whereas if you create an object using some other format (JSON or whatever) and leave out a value, that means the value should be its default. In other words, the same data needs to be treated differently depending on whether it came from an HTML form or not.

So I think this means that either the HTML form needs to be changed to explicitly represent an unchecked checkbox as meaning false (which I'm not sure whether is possible), or the serializer needs to be changed to check whether its data came from an HTML form or not (which I'm also not sure whether is possible, but maybe it could be done in some pre-serializer step?).

Is this a correct assessment? I'm going to put in a fix on my end for the specific case I have, and if I have time I'll try to figure out how to fix it within django-rest-framework. But if someone who's familiar with the code knows of any obvious fixes, that would be great.

Avril

@tomchristie
Copy link
Member

tomchristie commented Feb 6, 2014

if you create an object using the HTML form and leave a checkbox unchecked, it will leave out that value from the form, and that should be interpreted as indicating that the value should be false, regardless of its default; whereas if you create an object using some other format (JSON or whatever) and leave out a value, that means the value should be its default.

That all sounds sensible, yeah.

Is this a correct assessment?

Not sure. The acid test is - can this be translated into a test case that clearly demonstrates non-expected behaviour?

@akenney
Copy link

akenney commented Feb 6, 2014

So, I'm not sure exactly what behavior is expected/correct, because I don't know all the use cases. It sounds like the person who originally opened this issue was expecting that unspecified values will always get their default value, but from my point of view that behavior is wrong, because then there is no way to indicate in an HTML form request that you want a value to be false when its default is true. (Does django-rest-framework allow partial-updates to be submitted using HTML forms? That would have the same problem of not being able to specify that a boolean should be false.)

I found this solution on the internet to the problem of forms leaving out unchecked checkboxes: http://planetozh.com/blog/2008/09/posting-unchecked-checkboxes-in-html-forms/. But it's a frontend solution, which means it doesn't solve the problem if someone submits form data from some other frontend.

In 90edcbf#diff-af402f515db75c7c6754453cf15455d9L431, does the data's being a QueryDict indicate that it definitely came from a form? Because if that is the case, then I think it should be the way that it was before that commit, where unspecified means false.

@ZeaQQ4Ug
Copy link
Author

ZeaQQ4Ug commented Feb 7, 2014

The main problem of this issue was not honouring default values specified in models when we run Django's test runner for partial update/create, which was apparently a bug so that had to be fixed.
The behaviour of HTML form of Browsable API akenney mentioning seems to me that should be dealt in HTML form in some way instead of reverting the fix and bringing back the bug.

@justnick21
Copy link

justnick21 commented Feb 7, 2014

I think im seeing another instance of this. The django test client and testing by hand using runserver (or any web interface) yields different results.

EG.

class MyModel(models.Model):
    name = models.CharField(max_length=100)
    is_person = models.BooleanField(default=True)


class MyModelViewSet(ModelViewSet):
    queryset = MyModel.objects.all()


class MyModelSerializer(ModelSerializer):
    is_person = models.BooleanField(default=True)

    class Meta:
        model = MyModel


class TestCase(TastCase):
    def test_example_problem(self):
        self.client.post(
            viewset_url,
            {
               'name': 'nick'
            }
        )
        model = MyModel.objects.all()[0]
        # This will fail.
        self.assertTrue(model.is_person)

That final test shouldn't fail, and doesn't testing using a web client. The response from the post has a 200 status.

@caogecym
Copy link

caogecym commented Apr 23, 2014

#1291 doesn't solve the problem, actually @tomwalker 's solution is targeting for the root cause (horouring model default value). I would suggest reopen PR #1248, get the test pass, and merge the fix. Anybody know how to run the rest_framework test?

@xordoquy
Copy link
Collaborator

xordoquy commented Apr 23, 2014

@caogecym it's explained in the documentation. See http://www.django-rest-framework.org/topics/contributing#testing

@ZeaQQ4Ug
Copy link
Author

ZeaQQ4Ug commented Dec 17, 2014

This bug is happening again in version 3.0.1... :(

@tomchristie
Copy link
Member

tomchristie commented Dec 17, 2014

Assuming same as #2280?

@ZeaQQ4Ug
Copy link
Author

ZeaQQ4Ug commented Dec 18, 2014

Unfortunately it was not.
I've updated DRF to version 3.0.2 today and tried it again but to no avail. default values defined in models such as default=True, default="text" are still ignored and tuned into False or ''.
It can be reproduced with the test client and JSONRenderer.

@tomchristie tomchristie changed the title Default Boolean value always becomes False even default=True is specified in model class. Model defaults ignored on empty text field or empty boolean field. Dec 18, 2014
@tomchristie
Copy link
Member

tomchristie commented Dec 18, 2014

Retitling with what I think may be a more specific wording for the behavior you're seeing.

@tomchristie tomchristie reopened this Dec 18, 2014
@tomchristie
Copy link
Member

tomchristie commented Dec 18, 2014

Apologies for the slightly higher than usual churn rate, due to 3.0 just being released - pretty much all settled down now, but this is one of the few remaining outstanding issues.

@tomchristie tomchristie added this to the 3.0.3 Release milestone Dec 18, 2014
@tomchristie
Copy link
Member

tomchristie commented Dec 18, 2014

Fix in the meantime, add the following to the serializer to forcibly set the default values...

class Meta:
    extra_kwargs = {'my_field': {'default': True}}

@ZeaQQ4Ug
Copy link
Author

ZeaQQ4Ug commented Dec 18, 2014

@tomchristie Thanks for the swift reply. Yes, the temporal workaround did the trick.

@tomchristie
Copy link
Member

tomchristie commented Dec 18, 2014

Issue here: When a model has a default we simply set required=False on the serializer field, and allow the model instance to use it's default when setting the value. Because empty text and boolean fields are coerced to an empty value by the serializer field that doesn't work.

Answer is probably that required=False serializer fields always have default_empty_html = empty.

@ghost
Copy link

ghost commented Oct 27, 2017

djangorestframework==3.7.1

    response = client.post(
        '/api/v1/items/',
        {'title': 'blah_title',
                  'description': 'blah_description'})

Notice, not sending the field 'open'

Had to this to get my test to pass - Item.open = True by default

class Meta:
extra_kwargs = {'open': {'default': True}}

tonioo pushed a commit to modoboa/modoboa that referenced this issue Oct 27, 2017
@Diaga
Copy link

Diaga commented Jan 26, 2020

djangorestframework>=3.10.1,<3.11.0

Encountered the same problem.
The boolean field was set to default to true in models.py, however, when the request was sent to create the object, the object was created with boolean field set to false.

@Sarctiann
Copy link

Sarctiann commented Apr 22, 2020

I have the same problem on version 3.11.0

@xordoquy
Copy link
Collaborator

xordoquy commented Apr 23, 2020

that's the way boolean field send by HTML work. Specification says that if it's not set it should be considered as False and therefore it won't use the default.

@felixpk
Copy link

felixpk commented Jun 19, 2020

Is there any workaround?
is_active is not part of the request, but I want it to be active by default.

Why can you define the default value of models.BooleanField in the first place?

Even when setting is_active = serializers.BooleanField(default=True) on the serializer does not help

@xordoquy
Copy link
Collaborator

xordoquy commented Jun 19, 2020

Why can you define the default value of models.BooleanField in the first place?

Because it works with anything except html forms.

Is there any workaround?

Sure and the cool thing is it's already documented

@amikrop
Copy link
Contributor

amikrop commented Oct 28, 2020

This still happens to me (3.12.1). httpie sending no data results in a True value, self.client.post sending no data results in False.

@xordoquy
Copy link
Collaborator

xordoquy commented Oct 28, 2020

@amikrop which is consistent with previous comments. httpie sending json by default as opposed to self.client.post.

@amikrop
Copy link
Contributor

amikrop commented Oct 28, 2020

That's right, adding data=json.dumps(my_dict), content_type='application/json' to self.client.post made it work.
Btw, if I don't explicitly add the json.dumps part myself I get a JSON parse error. If I understand correctly, https://docs.djangoproject.com/en/3.1/topics/testing/tools/#django.test.Client.post says if it sees a json content type it serializes the data automatically, but I guess that's a difference between django test client and drf test client, and could be a separate issue (feature request).

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 a pull request may close this issue.