[Django] #20999: Cannot specify form_class that isn't subclass of TypedChoiceField for field with choices
Django
with choices
---------------------------------------+------------------------
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------+------------------------
In `Field.formfield()`, if `self.choices` is set, Django overrides any
supplied `form_class` with `TypedChoiceField` unless the given
`form_class` is a subclass of `TypedChoiceField`. This is unnecessarily
restrictive (as explicit type checks usually are), and is particularly
problematic when working with a field that takes multiple values (such as
a Postgres array field), where `TypedMultipleChoiceField` would be the
appropriate form field class, because `TypedMultipleChoiceField` is not a
subclass of `TypedChoiceField`.
Prior to the fix for #18162, Django was even more restrictive; a custom
`form_class` was not respected at all when choices were set.
I think the wrong fix for #18162 was chosen. The original patch provided
by rafallo there would have respected a new keyword argument to
`formfield()`, `form_class_for_choices`. This provides full flexibility,
and is conceptually more correct; since a totally different form field is
generally needed for the same db field when there are choices, a separate
keyword argument to control this form field is appropriate. I propose that
we switch to rafallo's fix (though I prefer the slightly shorter name
`choices_form_class`).
Unfortunately since the subclass check was committed and released in the
1.6 betas, we probably still have to maintain compatibility with people
passing a subclass of `TypedChoiceField` as `form_class` and expecting it
to be used when choices are set.
--
Ticket URL: <https://code.djangoproject.com/ticket/20999>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
with choices
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by carljm):
* has_patch: 0 => 1
Comment:
Pull request at https://github.com/django/django/pull/1530
--
Ticket URL: <https://code.djangoproject.com/ticket/20999#comment:1>
Django
with choices
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by mjtamlyn):
* stage: Unreviewed => Ready for checkin
Comment:
I think this is a better solution to the problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/20999#comment:2>
Django
with choices
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
I wouldn't consider beta releases as stable APIs, so I would not have
considered the change as backwards incompatible. So +1 for the new
keyword, but I would have removed at least the `issubclass` check. There
is a missing `versionchanged` in the docs.
--
Ticket URL: <https://code.djangoproject.com/ticket/20999#comment:3>
Django
with choices
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
If this patch is not backported to 1.6, then the subclass check will be
released in 1.6 final, and not supporting it in 1.7 would be a backwards-
compatibility issue.
If others are ok with it, I'm happy to backport this fix to 1.6 now and
remove the subclass check entirely; that could potentially cause an issue
for someone upgrading from a 1.6 beta to 1.6 rc/final, but maybe that's
not a concern.
--
Ticket URL: <https://code.djangoproject.com/ticket/20999#comment:4>
Django
with choices
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mjtamlyn):
I'm in favour of backporting. It's not exactly a new feature, rather a fix
to an existing one. Last chance before the RC!
--
Ticket URL: <https://code.djangoproject.com/ticket/20999#comment:5>
Django
with choices
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
Ok cool, I will remove the subclass check and backport to 1.6.
--
Ticket URL: <https://code.djangoproject.com/ticket/20999#comment:6>
Django
with choices
Reporter: carljm | Owner: nobody
Type: New feature | Status: closed
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carl Meyer <carl@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"7211741fc5d50425133ab942181cc095c56d7387"]:
{{{
#!CommitTicketReference repository=""
revision="7211741fc5d50425133ab942181cc095c56d7387"
Fixed #20999 - Allow overriding formfield class with choices, without
subclass restrictions.
Refs #18162. Thanks claudep and mjtamlyn for review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20999#comment:7>
Django
with choices
Reporter: carljm | Owner: nobody
Type: New feature | Status: closed
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carl Meyer <carl@…>):
In [changeset:"21a3efcf48559b3336ca1743d94c741a82feffd6"]:
{{{
#!CommitTicketReference repository=""
revision="21a3efcf48559b3336ca1743d94c741a82feffd6"
[1.6.x] Fixed #20999 - Allow overriding formfield class with choices,
without subclass restrictions.
Refs #18162. Thanks claudep and mjtamlyn for review.
Backport of 7211741fc5d50425 from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20999#comment:8>