Opened 9 years ago
Closed 9 years ago
#24908 closed Bug (fixed)
Overridden field in modelform used in modeladmin which name is returned by get_readonly_fields
Reported by: | James Pic | Owned by: | |
---|---|---|---|
Component: | contrib.admin | Version: | 1.8 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
A Model ForeignKey field will appear twice in the admin if the following conditions are met:
- its ModelAdmin uses a custom ModelForm,
- the ModelForm overrides that field,
- its ModelAdmin.get_readonly_fields returns that field in edit mode.
Example:
https://github.com/jpic/double_readonly_field/raw/master/demo.png
Test project reproducing this issue:
Related:
Change History (9)
comment:1 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Version: | → 1.8 |
comment:2 by , 9 years ago
comment:3 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Looks like this is happening because ModelFormMetaclass.__new__
doesn't expect the form class's declared fields to also be in its exclude opts. Normally, that would be a reasonable assumption. (Why would you exclude a field you've bothered to explicitly deeclared?) But in this case, it's the admin which is excluding the field, so the assumption breaks. Patch coming.
comment:4 by , 9 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
Oops, just realized there's already a PR for this. Ignore me.
comment:5 by , 9 years ago
Probably because I forgot to assign the task, my bad, lesson learned: do not sprint so much during a sprint !
This new PR fixes the bug inside ModelAdmin: https://github.com/django/django/pull/4822
comment:6 by , 9 years ago
Closed PR 4822 in favor of 4824: https://github.com/django/django/pull/4824
It's the same but rebased on master. It seems like the right implementation indeed, which is "encapsulated in the class that provides the readonly feature" (and with all the awesomeness that's in type())
comment:7 by , 9 years ago
Has patch: | set |
---|
comment:8 by , 9 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
This is fixed by : https://github.com/django/django/pull/4747/
Ready for review and probably for merge, since it's really trivial.