Opened 3 years ago

Closed 3 years ago

Last modified 3 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 Changed 3 years ago by ramiro

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to worksforme
  • Status changed from new to closed

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 3 years ago by ramiro (previous) (diff)

comment:2 Changed 3 years ago by anonymous

  • Resolution worksforme deleted
  • Status changed from closed to reopened

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 Changed 3 years ago by ramiro

  • Resolution set to worksforme
  • Status changed from reopened to closed

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 3 years ago by ramiro (previous) (diff)

comment:4 Changed 3 years ago by carljm

  • Component changed from Uncategorized to Forms
  • Resolution worksforme deleted
  • Status changed from closed to reopened

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 Changed 3 years ago by carljm

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 Changed 3 years ago by carljm

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 follow-up: Changed 3 years ago by 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.

comment:8 in reply to: ↑ 7 Changed 3 years ago by carljm

  • Resolution set to duplicate
  • Status changed from reopened to closed

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 Changed 3 years ago by ramiro

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