Opened 13 years ago

Closed 9 years ago

#15497 closed New feature (wontfix)

BooleanField should work for all PostgreSQL expressions

Reported by: Luc Saffre Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: julie@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When using ExtJS as user interface for a Django application, one problem is that ExtJS Checkboxes submit a string "on" when they are checked. Django's BooleanField.to_python understands "t", "1", "true", but not "on". Here is a patch. I'd be glad to see this merged into Django.

Attachments (2)

extjs_checkboxes.diff (887 bytes ) - added by Luc Saffre 13 years ago.
Patch against rev. 14995
15497.diff (2.8 KB ) - added by Julie Pichon 13 years ago.

Download all attachments as: .zip

Change History (14)

by Luc Saffre, 13 years ago

Attachment: extjs_checkboxes.diff added

Patch against rev. 14995

comment:1 by Russell Keith-Magee, 13 years ago

Component: UncategorizedDatabase layer (models, ORM)
Needs tests: set
Triage Stage: UnreviewedAccepted

comment:2 by Mark Lavin, 13 years ago

This patch is against the models.BooleanField but it seems like this problem should be dealt with at the form level. I seems like this issue could be resolved by simply using a TypedChoiceField in the form or a custom subclass of forms.BooleanField. I can't see how using a particular JS framework would necessitate a change to the Django ORM.

comment:3 by Luc Saffre, 13 years ago

In my Lino project (http://code.google.com/p/lino/) I don't use Django forms.
Lino's server-side code that handles ExtJS form submits is here:
http://code.google.com/p/lino/source/browse/lino/ui/extjs/ext_ui.py#740

Or maybe you can imagine a customized import process for csv files. Wouldn't such a process also use the to_python method?

comment:4 by Mark Lavin, 13 years ago

I'm going to go ahead and change my mind on this issue. I still don't agree with your use cases. I think that custom data cleaning can either by handled at the form level or by subclassing BooleanField if you want to use model validation. The special cases in the BooleanField.to_python should be motivated by the boolean types as defined by the supported database backends. However PostgreSQL 8.4 supports the following True values:

TRUE
't'
'true'
'y'
'yes'
'on'
'1'

and False values:

FALSE
'f'
'false'
'n'
'no'
'off'
'0'

so this perhaps this should be included.

comment:5 by Luc Saffre, 13 years ago

Summary: BooleanField should work for ExtJS CheckboxesBooleanField should work for all PostgreSQL expressions

Aha, to_python is only for parsing SQL expressions, not as a general-purpose parser.
Yes that sounds good. Maybe change the sentence

"Converts the input value into the expected Python data type"
of the docstring into

"Converts the input value (an SQL expression) into the expected Python data type".

And I change the summary of this ticket to "BooleanField should work for all PostgreSQL expressions".

comment:6 by Łukasz Rekucki, 13 years ago

Severity: Normal
Type: New feature

by Julie Pichon, 13 years ago

Attachment: 15497.diff added

comment:7 by Julie Pichon, 13 years ago

Cc: julie@… added
Easy pickings: unset
Needs tests: unset
UI/UX: unset

I attached a patch that includes Isaffre's diff, makes the same change for NullBooleanField and adds a couple of tests.

comment:8 by Malcolm Tredinnick, 13 years ago

Whilst PostgreSQL supports various literals for true and false, it returns a much more constrained set, which is what Django needs to understand (and they are typically handled long before getting to this level). Most of the slightly odd "not really boolean" values that are currently accepted in model fields are historical legacy stuff and that isn't an excuse to add yet more. Anything populating a model field can reasonably be expected to normalise it's true/false input to Python True/False values, I feel.

As noted earlier, altering model fields to handle particular form (HTTP) submission raw data is the wrong level. If you're not using Django's forms, then your own form handling code should be doing various pieces of normalisation (or writing a custom model field to do it for you).

Will leave open for a second opinion, but I'm -1 on this.

comment:9 by Luc Saffre, 13 years ago

As reporter I also meanwhile think it's better to leave BooleanField
unchanged. I'd just change the docstring of Field.to_python to make
clear that it parses only SQL statements (but for all supported db backends).

comment:10 by Kamu, 11 years ago

Needs documentation: set

It seems that the conclusion was to not add any more boolean values and update Field.to_python docs to state it is used for the parsing of SQL statements. If that is the case, then this requires a document patch. Otherwise this can be resolved as wontfix.

comment:11 by Omer Katz, 9 years ago

I think it's safe to close this as won't fix. It's pretty obvious that this should be done at the form level or view level.

comment:12 by Tim Graham, 9 years ago

Resolution: wontfix
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top