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

[ZeaQQ4Ug]

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]

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]

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]

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

[tomwalker]

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]

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]

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]

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]

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]

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]

#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]

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

[ZeaQQ4Ug]

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

[tomchristie]

Assuming same as #2280?