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: Tim Graham <timograham@…>
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 Tim Graham, 9 years ago

Triage Stage: UnreviewedAccepted
Version: 1.8

comment:2 by James Pic, 9 years ago

This is fixed by : https://github.com/django/django/pull/4747/

Ready for review and probably for merge, since it's really trivial.

comment:3 by David Gouldin, 9 years ago

Owner: changed from nobody to David Gouldin
Status: newassigned

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 David Gouldin, 9 years ago

Owner: David Gouldin removed
Status: assignednew

Oops, just realized there's already a PR for this. Ignore me.

comment:5 by James Pic, 9 years ago

Probably because I forgot to assign the task, my bad, lesson learned: do not sprint too much during a sprint !

This new PR fixes the bug inside ModelAdmin: https://github.com/django/django/pull/4822

Last edited 9 years ago by James Pic (previous) (diff)

comment:6 by James Pic, 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.

Version 2, edited 9 years ago by James Pic (previous) (next) (diff)

comment:7 by Tim Graham, 9 years ago

Has patch: set

comment:8 by Tim Graham, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:9 by Tim Graham <timograham@…>, 9 years ago

Owner: set to Tim Graham <timograham@…>
Resolution: fixed
Status: newclosed

In fedef7b2:

Fixed #24908 -- Fixed duplicate readonly field rendering.

ModelAdmin added readonly_fields to exclude, but would not undeclare
them if they were overridden.

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