#33321 closed Bug (wontfix)
Django admin doesn't render "add another / modify" icons next to ForeignKey fields that are declared in the ModelForm
Reported by: | James Pic | Owned by: | nobody |
---|---|---|---|
Component: | contrib.admin | Version: | 3.2 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | yes |
Description
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)
Change History (2)
comment:1 by , 3 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:2 by , 3 years ago
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 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 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. We'd have to at least begin integrating ESM, and change jQuery for custom HTMLElements to deal with element lifecycle, if we want to be able provide native fields with solid JS integration.
Hi James.
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 Overriding the default fields section of the ModelForms topic docs. It says more but:
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 ofRelatedFieldWidgetWrapper
intoforms/models.py
.