#26369: default formfield callback override

[James Pic]

Hi all,

I'm looking for a way to override the default form field widget for some fields of some model classes, at the project level.

Currently, we have to override all the model classes for that. That's considerable as a hack, because we don't exactly want to "override the default in every form class" but we want to "override the default".

The reason I need this at the project level is that really it's that unique combination in INSTALLED_APPS that provides whatever widget I'd like to override the default widget with, and how my project forms will look like.

For example, I'd like to have a RadioSelect for all relations to Foo model and a django-redactor for all text fields named "html", my logic looks like:

        if getattr(db_field, 'rel', None) and db_field.rel.to == Foo:

            defaults['widget'] = forms.RadioSelect

        if db_field.model == Bar and 'html' in db_field.name:

            defaults['widget'] = redactor.RedactorWidget

A simple possibility is to add a setting which value would be a dotted path to a callback, and basically change Field.formfield() so that it would use this callback if it's defined. Here's an example patch: https://github.com/django/django/pull/7723

Thanks for your feedback.

[Tim Graham]

Hi,

It's not clear to me whether or not this proposal considers the concerns from the last time you raised the idea some months ago. Perhaps you could summarize that discussion so we don't rehash it.

Thanks.

https://groups.google.com/d/topic/django-developers/zG-JvS_opi4/discussion

[James Pic]

Wow, nice memory Tim ! 

Yes it's a problem I've been trying to find a solution for during the last years. We've had a solution in DAL v2 by providing a custom model form which would make relation fields to a model that has an autocomplete registered use an autocomplete field by default. This solution was really hard to maintain, and not even portable.

I couldn't reuse the system to just FlatPage use a wysiwyg field by default, and had to go the usual, and I recon a bit messy road: https://gist.github.com/Kub-AT/3676137 This is when I realized it had to be part of the features that should be dropped from DAL v2 to v3, in favor of a more generic and supported way.

The older discussion proposed another angle, in the spirit of #22609 [0], that was not convincing because it would increase coupling between forms and models because it requires to add even more form stuff in models, because of the nature of the solution that was per-modelfield.

This is why this new proposal is based on a project-level callback setting instead:

- does not require to add any additional form stuff to models.py,

- allows to override default form fields or form field options without touching their code.

Note that there was also a suggestion to do something with fields_for_model [1] to solve the problem, I don't think we're done studying this road on the other thread so there might be more implementations to suggest here, even though I feel like just being able to tweak defaults with a global callback as proposed would just do the trick. 

[0] https://code.djangoproject.com/ticket/22609

[1] https://github.com/django/django/blob/master/django/forms/models.py#L111

[Adam Johnson]

I've only skimmed the original thread so idk if the following was suggested there. What I'd suggest for your project for the first condition is:

1. Subclass the model field you want to change and make it default to having widget=RadioSelect:

class RadioSelectForeignKey(ForeignKey):

    def formfield(self, **kwargs):

        kwargs.setdefault('widget', RadioSelect)

        return super().formfield(**kwargs)

2. Use RadioSelectForeignKey on all the models that have a FK pointing to Foo

3. Add a system check to your project that errors if a FK is added pointing to Foo that isn't a RadioSelectForeignKey (scan all fields on all models, if they're FK's and point to Foo, check their class with isinstance)

And similar for your Bar html fields. This one can be done with a check() method on the model itself.

--
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+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CALC3Kad7%2BPrkJC0Cz2SsoB374%2BNxuJcN7z_2pANOQzpdBneBFg%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam

[James Pic]

Thanks for your suggestion Adam ! However, I have a feeling that to apply that technique on the flatpages use case we'd also have to:

4. Monkey patch django.contrib.models.FlatPage.content,

5. Override and deal with flatpages migrations from now on.

The proposal removes the cost of steps 1., 3., 4. and 5 and cuts the cost of step 2. replacing the change in multiple model files to a single change at one place.

I definitely agree that a signal would be a lot better than a callback setting ;)

======= ERRATA

There was an error in my previous email, this was actually the second feature of the list:

- allows to override default form fields or form field options without touching their code.

Should have been:

- allows to override external app model default form fields or form field options without touching their code.

[Adam Johnson]

Oh, I misunderstood, I thought you controlled both the models and forms but wanted to enforce consistency. If you don't control the models (like FlatPage), I presume you control the forms then?You can do it then with a custom ModelForm subclass that overrides some of the building behaviour.

You'd have to subclass ModelFormMetaclass but it seems doable to me - you can customize the form class after ModelFormMetaclass has built it, by replacing any form fields that have been built with the ones you like.

[James Pic]

Absolutely ! If we don't want to monkey patch, we can use the other step:

4. get control on the flatpage form in the admin: https://gist.github.com/Kub-AT/3676137

Then, there's the fatpages use case. Fatpage is an imaginary fork of flatpages that adds create and edit views. In this case, we also need the following steps:

5. Subclass Fatpage views somewhere in our project so that it uses our forms

6. Override the create/edit urls to use our new views.

In the fatpages case, the proposal removes the cost of 1. 3. 4. 5. 6. and cuts the cost of 2. by making it DRY-friendly.

About ModelFormMetaclass: are you suggesting to move the signal in ModelFormMetaclass or do you have a lot more ambition ie. add new APIs ?

[0] https://github.com/yourlabs/django-autocomplete-light/blob/v2/autocomplete_light/forms.py 

[1] https://github.com/jpic/xmodelform/blob/master/README.rst

[Adam Johnson]

I was suggesting in your project you subclass the ModelFormMetaclass, use it in your own subclass of ModelForm, and use that everywhere in your project.

Fatpage is an imaginary fork of flatpages that adds create and edit views

There are a lot of problems out there with imaginary third party apps that you want to modify the internals of :) Often it's not hard to get third party apps to add some hook for extension (e.g. their own setting, or signal, ...) or to fork it yourself - easier than modifying Django core.

[James Pic]

Thanks a lot for your time and quick answer Adam ! The ModelFormMetaclass usage you suggest is not supported by Django at this point. Suggesting that doing it is easy and supported seems incorrect in my experience, starting with the fact that there is no documentation.

There are a lot of problems out there with imaginary third party apps that you want to modify the internals of :)

I have to disagree here, it's perfectly supported to override the apps views, forms and admins to change a widget: when it's done right it works. It's just a lot of error-prone and manual work and I wish Django proposed a way to reduce the cost of this.

In my experience, it works fine to enhance one app with the widget provided by another. As demonstrated, it's already possible, but the cost of 6 steps is something I'd like to reduce to one, DRY step.

 

Often it's not hard to get third party apps to add some hook for extension (e.g. their own setting, or signal, ...) or to fork it yourself - easier than modifying Django core.

I don't understand why the community would be against reducing the cost of using a form field or widget of an app to enhance another: given the number of apps which provide just a form field or widget, I can hardly believe I'm the only one to do this kind of things. I mean, are we formfield/widget app maintainers so ahead time for maintaining such apps ?

I'm really sorry that I couldn't make it clear that we need a way to "overriding the default form field generator ***mechanism***", and that answers that were given to me are about just "overriding a form field", similarly to the hundreds of StackOverflow answers and countless number of blog posts I've published on this subject myself.

[Adam Johnson]

The ModelFormMetaclass usage you suggest is not supported by Django at this point

Sorry if I was unclear, I meant something like this (untested):

class MyModelFormMetaclass(ModelFormMetaclass):

    """

    Customize the way our forms get built

    """

    def __new__(*args, **kwargs):

        new_class = super(MyModelFormMetaclass, self).__new__(*args, **kwargs)

        for name in list(new_class.base_fields):

            field = new_class.base_fields[name]

            if should_fixup(field):

                new_class.base_fields[name] = fixup_field(field)

        return new_class

class MyModelForm(six.with_metaclass(ModelFormMetaclass, BaseModelForm)):

    pass

I understand it's not so general purpose as to allow you to edit the forms in third party apps, but it can be done today with current Django, and you could fork the third party app so it imports MyModelForm.

--

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+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CALC3KacjBh8XO09cStzAvRbSCdcuJn8Wd-CWZp26LuejMFMrZw%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam

[James Pic]

Thanks for your reply Adam ! To make it general purpose, perhaps we could make such a patch in Django and replace should_fixup by a signal ?

[Adam Johnson]

I've thought a bit more about this, and have had three ideas:

1. The signal idea doesn't seem likely to work in all cases due to import order considerations. Forms can easily be imported during Django startup whilst models, views, or urls, are imported, before AppConfig.ready() is fired, which is the recommended point to register signals (docs). It would be surprising if this one particular signal needed registering at some other point in time to work. Django currently has one signal which has this property, class_prepared , but it's also noted as not generally being used outside of Django's internals.

2. It is probably possible to use class_prepared at current to replace the model fields on third party models with subclasses that define formfield() to point at custom form fields.

3. It's definitely possible right now to modify form classes in existing apps by modifying the fields on them after their construction with something similar to the metaclass suggestion, like:

from otherapp.forms import FooForm

# We need to enforce our widgets on that form

FooForm.base_fields['html'] = fixup_field(FooForm.base_fields['html'])

On 23 December 2016 at 16:14, James Pic <jame...@gmail.com> wrote:

Thanks for your reply Adam ! To make it general purpose, perhaps we could make such a patch in Django and replace should_fixup by a signal ?

--

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+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CALC3Kad0Y9ibX2Z9%2Bpj9mWCbc%2B63zEBqAGC8K1eD47mgMKe5Vg%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam

[jp...@yourlabs.org]

Hi Adam !

About 1, if it's already working with a non-signal, how could it not work with a signal ? It is, after all, called from within formfield() in this implementation, so anything that's working today when formfield() is called, should work when formfield() emits a signal.

For 2 & 3, yes, I know a lot of hacks exist. I just feel like I've been using them for the last decade, to every time replace the default field everywhere (forms, admin, drf, etc ...), it really looks like I'm fighting that I can't change the default myself.

And this is systematically happening when I have a relation to another model, because the default widget does not work well when there is actual data in the relations, outside of the testing world where we have only a few instances.

Am I really the only witness of this phenomenon ? 

<3

[Adam Johnson]

About 1, if it's already working with a non-signal, how could it not work with a signal ? It is, after all, called from within formfield() in this implementation, so anything that's working today when formfield() is called, should work when formfield() emits a signal.

Signal handlers need to be registered before they get called. ModelForms can be imported and thus instantiate their form fields by calling the model fields' formfield() methods before your custom handler gets registered, so they wouldn't be changed.

Btw another point for customization of forms would be the post_init form signal as suggested here: https://groups.google.com/forum/#!topic/django-developers/SviNiWy3Bjc

--

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+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/ee180045-fd08-45bd-aced-4a674da80912%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam

[Jamesie Pic]

Ooops yes this is correct, it doesn't work in the advised file, apps.py

Thank you so much for your feedback, I need to ditch this patch just
find a way to make Django default usable date, time, image, relations
form fields useable ootb by marrying a JS framework.

Best
<3

[David Seddon]

Hi Jamesie,

I recently proposed adding a post_init signal for forms (https://groups.google.com/forum/#!topic/django-developers/SviNiWy3Bjc).

Here's an example of how you might adjust the forms: https://github.com/seddonym/formsignals/blob/master/formsignals/priority/receivers.py

If such a signal existed, I think you could use that to adjust widgets, fields etc. from a different app. You could even create a third party module which implemented a way of overriding this stuff using the settings you propose.

If you think it might be useful, then it might be worth saying so on the forms signals feature thread?

David

[Jamesie Pic]

Hi David,

Yep, I'm with you on this, another good reason to close this PR. I'm
building DAL's next feature on your fork, so you'll have my feedback
for sure at some point ;)

Best
<3