Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#10792 closed (fixed)

documentation error on ModelForm ModelChoiceField empty option

Reported by: Carl Meyer Owned by: Carl Meyer
Component: Forms Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The ModelForms documentation at the bottom of the "field types" list states:

The blank choice will not be included if the model field has blank=False and an explicit default value (the default value will be initially selected instead).

It is true that the default value is initially selected if given, but it is not true that the "blank choice will not be included". It is included regardless, unless empty_label is explicitly set to None.

I would consider the documented behavior here to be better than the actual behavior, but given backwards-compatibility it's probably the docs that need to change.

Attachments (1)

10792_r10524.diff (2.7 KB) - added by Carl Meyer 7 years ago.
patch with test and doc update

Download all attachments as: .zip

Change History (7)

comment:1 Changed 7 years ago by Malcolm Tredinnick

milestone: 1.1
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

The current behaviour sounds like a bug. We fix bugs, rather than documenting around them. There's no real backwards-compat problem here, since there's no good reason to include the blank option in those cases. Would be nice to fix this for 1.1 if you feel like knocking up a patch.

Changed 7 years ago by Carl Meyer

Attachment: 10792_r10524.diff added

patch with test and doc update

comment:2 Changed 7 years ago by Carl Meyer

Owner: changed from nobody to Carl Meyer
Status: newassigned

Great, I was hoping that would be the answer. Patch attached.

The current patch means that if someone has a ModelChoiceField with required=True and an initial value, they cannot use empty_label to force the creation of an empty option. I can't think of a use case for this, but if we want to preserve that possibility it will mean complicating the empty_label argument a bit: giving it some default sentinel value (normally would be None, but since that has meaning in this case we can't use it) so we know when it's been set explicitly.

comment:3 Changed 7 years ago by Matt McClanahan

Has patch: set

comment:4 Changed 7 years ago by Jacob

Triage Stage: AcceptedReady for checkin

comment:5 Changed 7 years ago by Russell Keith-Magee

Resolution: fixed
Status: assignedclosed

(In [10729]) Fixed #10792 -- Ensured that ModelChoiceFields don't provide an empty option when the underlying field has blank=False and there is a default value available. Thanks to carljm for the report and patch.

comment:6 Changed 5 years ago by Jacob

milestone: 1.1

Milestone 1.1 deleted

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