Code

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#10059 closed (fixed)

New admin formfield_for_dbfield doesn't play nice with subclassing

Reported by: Alex Owned by: Alex
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: UI/UX:

Description

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 5 years ago.
use the mro
admin-widgets.2.diff (3.0 KB) - added by Alex 5 years ago.
10059-formfield_for_dbfield-r10365.diff (2.8 KB) - added by mrmachine 5 years ago.
Patch created with svn diff with correct root.

Download all attachments as: .zip

Change History (12)

Changed 5 years ago by Alex

use the mro

comment:1 Changed 5 years ago by jacob

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

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

comment:2 Changed 5 years ago by Alex

  • Needs tests set

Changed 5 years ago by Alex

comment:3 Changed 5 years ago by Alex

  • Triage Stage changed from Accepted to Ready for checkin

comment:4 Changed 5 years ago by jacob

  • Triage Stage changed from Ready for checkin to Accepted

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

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

  • 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.

Changed 5 years ago by mrmachine

Patch created with svn diff with correct root.

comment:7 Changed 5 years ago by julien

I applied the patch and I confirm that it fixes the bug I described in: http://groups.google.com/group/django-users/browse_thread/thread/ffd3d498724fd0a1

+1 for checking this in.

comment:8 Changed 5 years ago by jacob

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

Fixed in r10454.

comment:9 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.