#8306 closed (fixed)
BaseModelAdmin.formfield_for_dbfield() in desperate need of refactor
Reported by: | James Bennett | Owned by: | nobody |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
See attached patch, which moves all of the model-field-specific overrides out into a dictionary. This has the side effect of increasing customizability :)
Attachments (2)
Change History (10)
by , 16 years ago
Attachment: | admin_field_overrides.diff added |
---|
comment:1 by , 16 years ago
Has patch: | set |
---|---|
Triage Stage: | Unreviewed → Design decision needed |
comment:2 by , 16 years ago
Triage Stage: | Design decision needed → Accepted |
---|
comment:3 by , 16 years ago
milestone: | 1.0 beta → 1.0 |
---|
comment:4 by , 16 years ago
This fixes #8326 but breaks admin widgets for subclasses of model fields.
I suggest the dictionary keys are not model.Field
s but forms.Widget
s. It is the widgets that we are replacing, so as long as the admin widgets are drop-in replacements, we can just call .formfield()
and check for replacement widgets for the widget class in the forms.Field
returned.
comment:5 by , 16 years ago
milestone: | 1.0 → post-1.0 |
---|
This is an obvious win, but I see no reason to let the code churn around at this point in the 1.0 process. Let's do this later.
comment:6 by , 16 years ago
I've attached a more complete patch with tests/docs; it's also on my github. Since James' version, I've done a couple things:
- Broke what was left of
formfield_for_dbfield
into a handful of smaller methods to make those easier to screw around with. - Separated the defaults and the overrides so that it's easier for subclasses to override the defaults without accidentally emptying out the dictionary (as I did while testing).
- Made sure that the ordering is correct (
**kwargs
overridesformfield_overrides
which overrides the defaults) to fix #9148 while we at it.
by , 16 years ago
Attachment: | admin-field-overrides.patch added |
---|
comment:7 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [9760]) Cleaned up and refactored ModelAdmin.formfield_for_dbfield
:
- The new method uses an admin configuration option (
formfield_overrides
); this makes custom admin widgets especially easy. - Refactored what was left of
formfield_for_dbfield
into a handful of smaller methods so that it's easier to hook in and return custom fields where needed. - These
formfield_for_*
methods now pass aroundrequest
so that you can easily modify fields based on request (as in #3987).
Thanks to James Bennet for the original patch; Alex Gaynor and Brian Rosner also contributed.
I'd like to hear if brosner has any particular qualms about this, but it looks like a win to me. That function is a little scary and small children might accidentally look at it.
(Brian: wontfix if you have some really good reason why this is a bad idea.)