Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#17747 closed Bug (duplicate)

BooleanField on ModelForms in ModelAdmin is always True

Reported by: Cochise Ruhulessin Owned by: nobody
Component: Forms Version: 1.4-beta-1
Severity: Release blocker Keywords: booleanfield, modelform, modeladmin
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When viewing an existing object in a ModelAdmin, it's BooleanFields are always True (checked), even if values in the database/on the model instance are not. This problem occured on 17575, and re-installing 1.3.1 solved the problem.

Change History (9)

comment:1 by Ramiro Morales, 13 years ago

Resolution: worksforme
Status: newclosed

I can't reproduce what's reported by this ticket using this setup:

# models.py
from django.db import models


class ModelOne(models.Model):
    boolean_field = models.BooleanField()

class ModelTwo(models.Model):
    boolean_field = models.BooleanField()

class ModelThree(models.Model):
    boolean_field = models.BooleanField()
# admin.py
from django.contrib import admin
from django import forms
from models import ModelOne, ModelTwo, ModelThree


class ModelTwoAdmin(admin.ModelAdmin):
    pass


class ModelThreeForm(forms.ModelForm):
    class Meta:
        model = ModelThree


class ModelThreeAdmin(admin.ModelAdmin):
    form = ModelThreeForm


admin.site.register(ModelOne)
admin.site.register(ModelTwo, ModelTwoAdmin)
admin.site.register(ModelThree, ModelTwoAdmin)
Last edited 13 years ago by Ramiro Morales (previous) (diff)

comment:2 by anonymous, 13 years ago

Resolution: worksforme
Status: closedreopened

Consider the following:

class DirectoryForm(forms.ModelForm)
    def __init__(self, *args, **kwargs):
        instance = kwargs.get('instance', None)
        if instance:
            is_enabled = instance.enabled
            model2dict = forms.models.model_to_dict(instance)
        super(DirectoryForm, self).__init__(*args, **kwargs)
        enabled_field_value = self.fields['enabled'].initial
        raise Exception
    
    class Meta:
        model = Directory

The traceback prints out this:

▼ Local vars

is_enabled = 0
model2dict = {
    'directory_id': 23L,
    'directory_name': u'video',
    'directory_path': u'video',
    'enabled': 0,
    'public': 1,
    'remote': 0,
    'storage': u'local',
    'temporary': 0,
    'virtual': 0
}
self = <django.forms.models.DirectoryForm object at 0x35cef90>
args = ()
instance = <Directory: Directory object>
enabled_field_value = True
kwargs = {'instance': <Directory: Directory object>}

comment:3 by Ramiro Morales, 13 years ago

Resolution: worksforme
Status: reopenedclosed

Can't reproduce that either with r17575 and this ModelAdmin when trying to edit a already existing instance of ModelThree that has its boolean field set to False:

# admin.py
class ModelThreeForm(forms.ModelForm):
    def __init__(self, *args, **kwargs):
        instance = kwargs.get('instance', None)
        if instance:
            boolean_field_value1 = instance.boolean_field
            model2dict = forms.models.model_to_dict(instance)
        super(ModelThreeForm, self).__init__(*args, **kwargs)
        boolean_field_value2 = self.fields['boolean_field'].initial
        raise Exception

    class Meta:
        model = ModelThree


class ModelThreeAdmin(admin.ModelAdmin):
    form = ModelThreeForm

admin.site.register(ModelThree, ModelThreeAdmin)
model2dict           {'boolean_field': False, 'id': 1}

self                 <django.forms.models.ModelThreeForm object at 0x159e890>

args                 ()

instance             <ModelThree: ModelThree object>

kwargs               {'instance': <ModelThree: ModelThree object>}

boolean_field_value1  False

boolean_field_value2  False

Please don't reopen this ticket as this doesn't appear to be an error in Django code. Try to seek help in the django-users mailing list or the #django IRC channel @ Freenode.

Also, for future reports, please provide a simple test case (reducing your model(s) to its minimal expression showing the buggy behavior).

Last edited 13 years ago by Ramiro Morales (previous) (diff)

comment:4 by Carl Meyer, 13 years ago

Component: UncategorizedForms
Resolution: worksforme
Status: closedreopened

There's a thread on the django-developers list indicating that perhaps this bug exists only on MySQL, and perhaps only on the MySQL GIS backend. I don't have time to verify at this moment (don't have MySQL GIS set up), but to be on the safe side I'm going to reopen this (and mark it as a release blocker, since apparently the symptom in form checkboxes is a regression from 1.3) until I have time to reproduce.

Setting the component to "forms", although if the mailing list thread is correct, it sounds like the correct fix here may in fact be in the MySQL GIS backend, so that it returns True/False rather than 1/0 from the database.

We'd also need to consider whether the checkbox change in 1.4 is itself a problematic backwards incompatibility, since in effect it now requires third-party DB backends to return True/False from boolean fields rather than 1/0. I don't know if in practice there are any third-party db backends that return 1/0.

comment:5 by Carl Meyer, 13 years ago

For reference from that thread, r17132 was the relevant change in checkbox handling in 1.4. r12939 may be the earlier fix that needs to be ported to the MySQL GIS backend (though I'm not sure if that's right, it claims to only be related to use of .extra()).

comment:6 by Carl Meyer, 13 years ago

It looks like #15169 is already tracking the root problem in the db backend. So the only remaining question for this ticket is whether it's acceptable to now require that db backends return True/False from boolean fields in order for checkboxes in a ModelForm to work correctly.

comment:7 by Aymeric Augustin, 13 years ago

IMO the database backends should return the most appropriate type under all circumstances — here, booleans.

Loose typing in the ORM leads to backwards-compatibility headaches and problems with no good solution, like the datetime/date can of worms.

in reply to:  7 comment:8 by Carl Meyer, 13 years ago

Resolution: duplicate
Status: reopenedclosed

Replying to aaugustin:

IMO the database backends should return the most appropriate type under all circumstances — here, booleans.

Loose typing in the ORM leads to backwards-compatibility headaches and problems with no good solution, like the datetime/date can of worms.

Agreed. And given that returning 0/1 for boolean values in built-in DB backends has long been considered a bug in that backend, I no longer even think this justifies a note in the release notes. Closing this as a duplicate of #15169.

comment:9 by Ramiro Morales, 13 years ago

FWIW I can confirm the problem reported here happens with when the active DB backend is the MySQL spatial one.

Note: See TracTickets for help on using tickets.
Back to Top