Code

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#10792 closed (fixed)

documentation error on ModelForm ModelChoiceField empty option

Reported by: carljm Owned by: carljm
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 carljm 5 years ago.
patch with test and doc update

Download all attachments as: .zip

Change History (7)

comment:1 Changed 5 years ago by mtredinnick

  • milestone set to 1.1
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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

patch with test and doc update

comment:2 Changed 5 years ago by carljm

  • Owner changed from nobody to carljm
  • Status changed from new to assigned

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 5 years ago by mattmcc

  • Has patch set

comment:4 Changed 5 years ago by jacob

  • Triage Stage changed from Accepted to Ready for checkin

comment:5 Changed 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from assigned to closed

(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 3 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.