Opened 17 years ago

Closed 17 years ago

#3268 closed defect (fixed)

[patch] form_for_model() should use ChoiceField for any DB field with "choices" set

Reported by: brooks.travis@… Owned by: Adrian Holovaty
Component: Forms Version: dev
Severity: normal Keywords:
Cc: kilian.cavalotti@…, larlet@…, brosner@…, real.human@…, espen@…, jm.bugtracking@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a model with a models.CharField(choices=GENDER_CHOICES), but when I run newforms.models.form_for_model() on the model containing that field, and do a print on the resulting form, I get a plain <input type="text">.

Attachments (4)

__init__.diff (6.4 KB ) - added by ryan.moe@… 17 years ago.
models.diff (1.4 KB ) - added by ryan.moe@… 17 years ago.
select_widget_for_fields_with_choices.diff (8.0 KB ) - added by Tai Lee 17 years ago.
updated *Field.formfield() methods, with tests.
choices.diff (837 bytes ) - added by Baptiste <baptiste.goupil@…> 17 years ago.
Patch of the #3991, so much simpler (but maybe less powerful, I don't know)

Download all attachments as: .zip

Change History (23)

comment:1 by Adrian Holovaty, 17 years ago

priority: highnormal
Severity: majornormal

comment:2 by Adrian Holovaty, 17 years ago

Summary: newforms.models.form_for_model() does not print CharField with field option choices=CHOICES_LIST as a <select> itemform_for_model() should use ChoiceField for any DB field with "choices" set

Agreed -- good catch.

by ryan.moe@…, 17 years ago

Attachment: __init__.diff added

by ryan.moe@…, 17 years ago

Attachment: models.diff added

comment:3 by ryan.moe@…, 17 years ago

Summary: form_for_model() should use ChoiceField for any DB field with "choices" set[patch] form_for_model() should use ChoiceField for any DB field with "choices" set

I wasn't sure which fix would be best so I included both. One fixes this in django.db.models.fields by changing all the *Field.formfield() calls, while the other only changes the form_for_model and for_for_instance calls. Both ways worked for the forms I have where this problem was occurring.

comment:4 by Adrian Holovaty, 17 years ago

Triage Stage: UnreviewedAccepted

Thanks for the patches. I'll have a look.

comment:5 by anonymous, 17 years ago

Cc: kilian.cavalotti@… added

comment:6 by Gary Wilson <gary.wilson@…>, 17 years ago

Has patch: set
Needs tests: set

There is also a competing patch in #3401 that alters CharField.formfield() to use ChoiceField for a CharField that has choices.

comment:7 by anonymous, 17 years ago

Cc: larlet@… added

comment:8 by brosner <brosner@…>, 17 years ago

Cc: brosner@… added

comment:9 by mrmachine <real dot human at mrmachine dot net>, 17 years ago

Cc: real.human@… added

by Tai Lee, 17 years ago

updated *Field.formfield() methods, with tests.

comment:10 by Tai Lee, 17 years ago

Needs tests: unset

the last two patches were made out of date by recent changes to django/db/models/fields/init.py, so i've re-implemented this functionality by creating an appropriate Select widget (which can still be overridden if a custom widget is passed in directly) for any form fields that have choices.

i left the implementation in django/db/models/fields/init.py so that it would be useful for anyone wanting to return a form field from a model field, not only when generating a form object with the form_for_* methods.

i also updated tests/modeltests/model_forms/models.py by adding an integer field with choices to the models, updating the expected output, and a comment explaining that any fields with choices defined in the model are represented by a select list (same as foreignkeys).

comment:11 by Chris Beaven, 17 years ago

Triage Stage: AcceptedReady for checkin

Looks good

by Baptiste <baptiste.goupil@…>, 17 years ago

Attachment: choices.diff added

Patch of the #3991, so much simpler (but maybe less powerful, I don't know)

comment:12 by brosner <brosner@…>, 17 years ago

Baptiste,

Your patch only effects admin display of the widgets. This bug you discovered is an underlying newforms problem that affects form_for_instance and form_for_model. Which is used by newforms-admin (go figure, right) ;)

comment:13 by Baptiste <baptiste.goupil@…>, 17 years ago

So does the patch of mrmachine since the modified functions are the ones called by form_for_instance & form_for_model, no ?
I think that my patch just factorizes what mrmachine does in its... but I can be wrong !

comment:14 by anonymous, 17 years ago

Cc: espen@… added

comment:15 by anonymous, 17 years ago

Cc: jm.bugtracking@… added

comment:16 by Jonas von Poser, 17 years ago

just a heads-up that both attached patches don't work for me anymore at least as of rev. 5063. The widgets in newforms-admin remain simple text input fields with either one.

comment:17 by Tai Lee, 17 years ago

i've not used the newforms-admin branch. does it use form_for_model? is this a problem or conflict with that branch, or a problem with this patch? does it still work for your own form_for_models forms?

comment:18 by Jonas von Poser, 17 years ago

Oh, I'm sorry. Both patches work with newforms-admin, they're just off by a few lines each, so basically it was me being unable to use patch correctly. I should learn to double-check before updating any bugs. :-/ Apologies.

comment:19 by Malcolm Tredinnick, 17 years ago

Resolution: fixed
Status: newclosed

(In [5119]) Fixed #3268 -- Changed default model formfields to use a select widget when the
field has a choices attribute. Based on a patch from mrmachine.

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