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 On Saturday, 5 June 2021 at 02:10:31 UTC+2 James Bennett wrote: > 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 > -- 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-developers+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/ed102665-33ec-404a-90f1-a15eb304d7edn%40googlegroups.com.
