[Django] #33321: Django admin doesn't render "add another / modify" icons next to ForeignKey fields that are declared in the ModelForm
Django
fields that are declared in the ModelForm
-----------------------------------------+------------------------
Reporter: James Pic | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 3.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 1 |
-----------------------------------------+------------------------
Django admin renders very convenient "add another" and "modify" buttons
next to ForeignKey fields when the admin uses a form like this:
{{{
class TForm(forms.ModelForm):
class Meta:
model = TModel
fields = ('name', 'test')
widgets = {
'test': MyWidget(),
}
class TAdmin(admin.ModelAdmin):
form = TForm
}}}
BUT, it won't if my field is declared in the ModelForm as such:
{{{
class TForm(forms.ModelForm):
test = forms.ModelChoiceField(
widget=MyWidget(),
queryset=Some.objects.all(),
)
class Meta:
model = TModel
fields = ('name', 'test')
class TAdmin(admin.ModelAdmin):
form = TForm
}}}
This is confusing behavior is not documented AFAIK, and users have been
opening issues on github about this, I just debugged it out by chance
because it was in my way.
This is because Django admin:
- Does the formfield.widget = RelatedFieldWidgetWrapper(formfield.widget)
decoration on generated fields
- And then overrides generated fields with the declared fields
As you can see in this trace session:
{{{
> /home/jpic/.local/lib/python3.9/site-
packages/django/forms/models.py(279)__new__()
-> fields.update(new_class.declared_fields)
(Pdb) l
274 message = message % (', '.join(missing_fields),
275 opts.model.__name__)
276 raise FieldError(message)
277 # Override default model fields with any custom
declared ones
278 # (plus, include all the other declared fields).
279 -> fields.update(new_class.declared_fields)
280 else:
281 fields = new_class.declared_fields
282
283 new_class.base_fields = fields
284
(Pdb) fields
{'name': <django.forms.fields.CharField object at 0x7f0a251cf0d0>, 'test':
<django.forms.models.ModelChoiceField object at 0x7f0a251cf9a0>}
(Pdb) fields.declared_fields
*** AttributeError: 'dict' object has no attribute 'declared_fields'
(Pdb) new_class.declared_fields
{'test': <dal_alight.fields.ModelAlight object at 0x7f0a283b6460>}
(Pdb) new_class.declared_fields['test'].widget
<dal_alight.fields.ModelAlightWidget object at 0x7f0a283b6520>
(Pdb) fields['test'].widget
<django.contrib.admin.widgets.RelatedFieldWidgetWrapper object at
0x7f0a251cfdf0>
}}}
I believe that Django should always decorate related field widgets with
RelatedFieldWidgetWrapper, unless the widget is explicitely incompatible
with it. The meta code for this proposal is: if not
getattr(formfield.widget,
'incompatible_with_related_field_widget_wrapper', False): formfield.widget
= RelatedFieldWidgetWrapper(formfield.widget)
--
Ticket URL: <https://code.djangoproject.com/ticket/33321>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
fields that are declared in the ModelForm
Reporter: James Pic | Owner: nobody
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => wontfix
Comment:
Hi James.
> BUT, it won't if my field is declared in the ModelForm...
Yes. So, this is the standard (and expected) behaviour for this kind of
generation: if you declare the field yourself, then that's what you get.
The same applies to ModelForms, DRF serialisers, django-filter FilterSets,
and I would expect everything else similar: it comes up on DRF as ''Why
was my `read_only_field` not applied to my explicitly defined serialiser
field?''
The relevant docs are the extended note in the
[https://docs.djangoproject.com/en/3.2/topics/forms/modelforms
/#overriding-the-default-fields Overriding the default fields section of
the ModelForms topic docs]. It says more but:
> The fields that are automatically generated depend on the content of the
Meta class and on which fields have already been defined declaratively.
Basically, ModelForm will only generate fields that are missing from the
form, or in other words, fields that weren’t defined declaratively.
To me, it would be deeply surprising if we were to change the behaviour
for the admin here: ''You get the field you declare except in this case''
is going to be more difficult to reason about in aggregate.
If I wanted to address this in my project, I'd probably look overriding
`ModelAdmin.get_form()`, rather than pulling knowledge of
`RelatedFieldWidgetWrapper ` into `forms/models.py`.
--
Ticket URL: <https://code.djangoproject.com/ticket/33321#comment:1>
Django
fields that are declared in the ModelForm
Reporter: James Pic | Owner: nobody
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by James Pic):
Hello Carlton!
> Basically, ModelForm will only generate fields that are missing from
the form, or in other words, fields that weren’t defined declaratively.
Actually my report shows it's generating all fields and then overriding
them with the declared fields ... Not sure if this distinction is
important enough to be reflected in the documentation though.
It's fine for me though, in our apps we don't advertise to declare
formfields anymore, but rather to [https://yourlabs.io/oss/djhacker monkey
patch Django formfield()] method which actually lets Django think it's
generating the foreign key fields and the decorated it with
RelatedFieldWidgetWrapper, which has also [https://github.com/jonashaag
/django-addanother been copy/pasted into an external app] so it could be
reusable outside the admin, by me.
The most proper solution IMHO is to extract RelatedFieldWidgetWrapper
outside from django.contrib.admin and put in in django.forms but that's
not going to happen neither.
--
Ticket URL: <https://code.djangoproject.com/ticket/33321#comment:2>