Opened 15 years ago

Closed 15 years ago

Last modified 13 years ago

#10059 closed (fixed)

New admin formfield_for_dbfield doesn't play nice with subclassing

Reported by: Alex Gaynor Owned by: Alex Gaynor
Component: contrib.admin Version: 1.0
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


If you subclass a model field you don't get to keep the admin widgets unless you readd them to the formfield overides.

Attachments (3)

admin-widgets.diff (966 bytes ) - added by Alex Gaynor 15 years ago.
use the mro
admin-widgets.2.diff (3.0 KB ) - added by Alex Gaynor 15 years ago.
10059-formfield_for_dbfield-r10365.diff (2.8 KB ) - added by Tai Lee 15 years ago.
Patch created with svn diff with correct root.

Download all attachments as: .zip

Change History (12)

by Alex Gaynor, 15 years ago

Attachment: admin-widgets.diff added

use the mro

comment:1 by Jacob, 15 years ago

milestone: 1.1
Triage Stage: UnreviewedAccepted

Wow, I never knew mro existed. Learn something new every day.

comment:2 by Alex Gaynor, 15 years ago

Needs tests: set

by Alex Gaynor, 15 years ago

Attachment: admin-widgets.2.diff added

comment:3 by Alex Gaynor, 15 years ago

Triage Stage: AcceptedReady for checkin

comment:4 by Jacob, 15 years ago

Triage Stage: Ready for checkinAccepted

I'm really not sure this patch is needed. ModelAdmin.init already applies self.formfield_overrides *over* the defaults, so I don't quite see what the point is here. Plus, the description of the ticket implies something about custom model fields, which I don't quite get either.

I'm kicking this back to accepted; we need more details from the original poster about what he's trying to do and what's broken here.

comment:5 by Tai Lee, 15 years ago

I hit this problem as well. The ModelAdmin.formfield_for_dbfield refactor in [9760] changed behaviour by using a dict key lookup instead of isinstance() to map model fields to form fields and widgets.

Previously if you had subclassed DateTimeField (for example as AUDateTimeField to use Australian format date input formats by default), formfield_for_dbfield() would still see it as a DateTimeField and use the SplitDateTimeField form field and AdminSplitDateTime form widget.

Now you need to explicitly add an item to ModelAdmin.formfield_overrides for every model that uses your custom AUDateTimeField, which seems to defeat the purpose of using a custom model field with a different default input format and violates DRY.

This patch should restore the existing behaviour from before the refactor and will still allow users to add their own mapping if in fact they do want to use their own form field or widget instead of the admin default.

comment:6 by Tai Lee, 15 years ago

Needs tests: unset

I couldn't apply this patch easily with the patch app. I'm not sure if that is because the diff was created with the wrong tool (diff --git as opposed to svn diff), or with the wrong base path ("a"), or is just not understood by patch because it includes the diff statement in the first line.

Whatever the reason, I applied it manually and ran the tests (before and after) and they do demonstrate the problem and show that it is fixed, so I'm changing the "needs tests" flag.

by Tai Lee, 15 years ago

Patch created with svn diff with correct root.

comment:7 by Julien Phalip, 15 years ago

I applied the patch and I confirm that it fixes the bug I described in:

+1 for checking this in.

comment:8 by Jacob, 15 years ago

Resolution: fixed
Status: newclosed

Fixed in r10454.

comment:9 by Jacob, 13 years ago

milestone: 1.1

Milestone 1.1 deleted

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