What to do about formfield_callback?

[James Bennett]

Currently, the ability to set 'formfield_callback' on a ModelForm is
undocumented, and a ticket to document it[1] was closed wontfix by Tim
with the comment:

> The attribute is described as an "internal implementation detail" in #12915 and the possibility of moving it to Meta is discussed. Therefore, I don't think it's a good idea to document it as currently implemented.

I currently have an issue with a codebase at work which uses
formfield_callback (to impose uniform custom widget attributes and
error messages across a bunch of ModelForms) which is not quite
working as intended because of a somewhat-complex chain of issues:

1. The forms define formfield_callback
2. When ModelFormMetaclass runs it *pops* the 'formfield_callback'
attribute out of the form class, meaning it's no longer present
3. Some of those forms later get passed to modelformset_factory, and
since they no longer have formfield_callback set on them the forms
constructed by the factory do not get the effect of the
formfield_callback.

Which in turn is going to be solved, for now, by a bit of a hack which
will effectively triplicate the declaration of formfield_callback for
these forms:

1. Declare the "real" formfield_callback inside the form's Meta
(necessary to preserve it from being popped away by
ModelFormMetaclass)
2. Redeclare it on the form class as 'formfield_callback =
Meta.formfield_callback' (necessary for normal construction of the
form to work properly)
3. In view code which constructs formsets, look for
Meta.formfield_callback, grab it when present and pass it as the
formfield_callback argument to modelformset_factory (necessary for
formset construction to work properly)

One reason why this gets complicated is that Django has weird and
inconsistent support for declaring formfield_callback on Meta (the
reason appears to be that it's only allowed in order to support the
argument to modelform_factory). Suppose you have a ModelForm class
called "MyForm" which declares formfield_callback in its Meta. Now, as
far as I can tell, this will happen:

* Constructing instances of MyForm directly (i.e., via "my_form =
MyForm()") will *not* invoke the callback
* Constructing instances of MyForm via modelform_factory or
modelformset_factory *will* invoke the callback
* Defining a subclass of MyForm *will* pass on the formfield_callback
inherited from the parent Meta and use it when directly constructing
instances of the *subclass*

This seems to me to be sub-optimal.

So I'd like to propose the following:

1. Make formfield_callback officially supported and documented as an
attribute of ModelForm.Meta.
2. Have modelform_factory pick up a formfield_callback declared on
Meta and use it.
3. Put the current undocumented support for setting it directly on the
form class through deprecation.

If this started immediately we could have it supported for Django 4.0,
and the deprecation could complete in Django 4.2.

[1] https://code.djangoproject.com/ticket/26456

[Carlton Gibson]

Hey James.

Thanks for this. Good explanation. 

I'm sympathetic to the suggestion here, but wary of expanding the Forms API, which already has a number of different ways of holding it. 

> ...to impose uniform custom widget attributes and error messages across a bunch of ModelForms...

My initial query is, would not defining `field_classes`[1] on, say, a base form class for your project be a/the way to go here?

Thanks. 

Carlton 

[1]: https://docs.djangoproject.com/en/3.2/topics/forms/modelforms/#overriding-the-default-fields

[James Bennett]

On Fri, Jun 4, 2021 at 11:54 PM Carlton Gibson <carlton...@gmail.com> wrote:
> I'm sympathetic to the suggestion here, but wary of expanding the Forms API, which already has a number of different ways of holding it.
>
> > ...to impose uniform custom widget attributes and error messages across a bunch of ModelForms...
>
> My initial query is, would not defining `field_classes`[1] on, say, a base form class for your project be a/the way to go here?

Without getting too far off into details, the basic issue is I have an
app where there are a lot of ModelForms, and for certain model field
types we want the form field to always get set up in a way that
differs from Django's defaults (think of, say, always wanting a
particular custom widget setup, or custom error messages, for a
particular model field type).

It's a thing that you *can* do by writing out overrides or
re-declarations of the relevant fields on each form. But when you have
dozens of forms and there's no consistent pattern to the naming of the
fields you want to override, it gets really tedious to write out and
maintain all those overrides.

Which is why the code uses formfield_callback -- that is Django's
customization hook for this type of "every instance of model field of
this type, output a form field of this type" stuff. And it's already
crept into documented API, to some extent, because the model
form/formset factories document it as a parameter.

So I'd just like for it to become proper fully-documented API with
consistent behavior (since right now its behavior and effect varies
according to how you specify it and how you construct your forms).

(the only other real alternative right now is to go write a custom
model field subclass and implement formfield() on it, then go migrate
a bunch of models to use the new custom field instead of the default
Django one. But it feels odd and wrong to me that form-only behavior
would need to be overridden in the model rather than in the form)

[Carlton Gibson]

Hey James,

Yes, I’ll top-post because I basically agree with all of that. Thanks.
+1

C.

> --
> You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAL13Cg8thHw7Sj-8KoAzU4J0QgWo9EXMNXDw1jkt2QtZBvDf5g%40mail.gmail.com.