Opened 22 months ago

Closed 21 months ago

Last modified 15 months ago

#27231 closed New feature (wontfix)

Initialize forms in ModelAdmin like View (i.e. add get_form_kwargs to contrib.admin)

Reported by: Thomas Hauk Owned by: nobody
Component: contrib.admin Version: 1.10
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: no

Description

There's no way to control initialization of a form instance with ModelAdmin like you can with, say, a View, by providing an override for get_form_kwargs. This is something I need to do.

Is there a architectural reason why ModelAdmin doesn't have get_form_kwargs? Or if I provided a PR, would that be welcome?

The closest ticket I can find related to this is #10305.

This is kind of how I'm working around it for now... to get one particular kind of value into my form constructor call, I'm kind of "injecting" it in:

class MyModelAdmin(ModelAdmin):
    model = MyModel
    form = MyForm
 
    def get_form(self, request, obj=None, **kwargs):
        form_class = super(MyModelAdmin, self).get_form(request, obj=obj, **kwargs)
 
        some_parameter_for_one_thing = self.get_some_parameter_for_one_thing()
 
        from django.forms.models import ModelFormMetaclass
        class FormMetaclass(ModelFormMetaclass):
            def __new__(mcs, name, bases, attrs):
                attrs["some_parameter_for_one_thing"] = some_parameter_for_one_thing
                return super(FormMetaclass, mcs).__new__(mcs, name, bases, attrs)
 
        another_parameter_for_another_thing= obj.another_parameter_for_another_thing
 
        import six
        class FormWrapper(six.with_metaclass(FormMetaclass, form_class)):
            def __init__(self, *args, **kwargs):
                data = {"another_parameter_for_another_thing": six.text_type(another_parameter_for_another_thing)}
                super(FormWrapper, self).__init__(data, *args, **kwargs)
 
        return FormWrapper

Change History (10)

comment:1 Changed 22 months ago by Tim Graham

What implementation do you have in mind? How would it look like in combination with #10305? I'm not sure if we need both methods. By the way, it's helpful to describe your use case in a bit more detail than "This is something I need to do."

comment:2 Changed 22 months ago by Thomas Hauk

Hi Tim, sorry for not being specific enough.

My form has an autocomplete menu (django-autocomplete-light), which visualizes a field in the model, but whose choices need to be configured based on some properties of the model being loaded (this is the some_parameter_for_one_thing part in my example).

I also need to set the default for this menu, if a value is chosen already, when I show the detail page (that is the another_parameter_for_another_thing part). To get the form/widget to get the default correctly (via data/cleaned_data), I have to "inject" it into the constructor call for the form.

Seeing as there's already an existing pattern in FormMixin with get_form and get_form_kwargs, I'm thinking the implementation would follow a similar pattern, to be consistent and as least surprising as possible, but I haven't started working on it yet. I was hoping to first find out if what I was proposing was a bad idea and/or could already be done.

comment:3 Changed 22 months ago by Thomas Hauk

As for your question about how it would look in combination with #10305, well, there's nothing there, so there's no conflict or integration. That is an open ticket that is 8 years old with a small patch that wasn't integrated. The patch does not actually do any kwargs injection, which is what I need, and is the existing pattern.

comment:4 Changed 22 months ago by Tim Graham

What I mean is that if we add a hook for form initialization, couldn't that include specifying some custom kwargs? It might be overengineering to add two hooks -- ModelAdmin is already very complex with a large number of hooks.

Can you make customizations in Form.__init__() using self.instance?

comment:5 Changed 22 months ago by Thomas Hauk

Yes, I can make customizations there, at least, some of them; e.g. there was potential for a requirement where the value of request.user would modify what choices I would display, and although the request object is generally available at the ModelAdmin level (e.g. get_form), it's not generally not a parameter at the Form level. I'll have a think this afternoon.

comment:6 Changed 22 months ago by Thomas Hauk

In django.contrib.admin.ModelAdmin, the form class is obtained by get_form (1) and then instaniated in one of three different ways, depending on if the request was a POST (2) or not (3 and 4).

    @csrf_protect_m
    @transaction.atomic
    def changeform_view(self, request, object_id=None, form_url='', extra_context=None):
        #
        # ...
        #
        add = object_id is None
        #
        # ...
        #
        ModelForm = self.get_form(request, obj)  # (1)
        if request.method == 'POST':
            form = ModelForm(request.POST, request.FILES, instance=obj)  # (2)
            #
            # ...
            #
        else:
            if add:
                initial = self.get_changeform_initial_data(request)
                form = ModelForm(initial=initial)  # (3)
                formsets, inline_instances = self._create_formsets(request, form.instance, change=False)
            else:
                form = ModelForm(instance=obj)  # (4)
                formsets, inline_instances = self._create_formsets(request, obj, change=True)

In django.views.generic.edit.FormMixin, get_form returns an instantiated form; it gets the form class via get_form_class() (1), and instantiates it with the returned value from get_form_kwargs() (2). get_form_kwargs() sets the "initial" value based on the returned value from get_initial() (3), and sets "data" and "files" based on if the request was a POST (4)

    def get_form(self, form_class=None):
        """
        Returns an instance of the form to be used in this view.
        """
        if form_class is None:
            form_class = self.get_form_class()  # (1)
        return form_class(**self.get_form_kwargs())  # (2)

    def get_form_kwargs(self):
        """
        Returns the keyword arguments for instantiating the form.
        """
        kwargs = {
            'initial': self.get_initial(),  # (3)
            'prefix': self.get_prefix(),
        }

        if self.request.method in ('POST', 'PUT'):  # (4)
            kwargs.update({
                'data': self.request.POST,
                'files': self.request.FILES,
            })
        return kwargs

One discrepancy is that ModelForm.get_form() returns a form class, while FormMixin.get_form() returns a form instance with FormMixin.get_form_class() returns a form class.

API alignment might look something like this:

    def get_form_class(self, request, obj=None, **kwargs):
        #
        # ... Old contents of get_form()
        #


    def get_form(self, request, obj=None, form_class=None, **kwargs):
        if form_class is None:
            form_class = self.get_form_class(request, obj=obj)
        return form_class(**self.get_form_kwargs(request, obj=obj))


    def get_form_kwargs(self, request, obj=None, obj=None):
        if request.method == 'POST':
            kwargs = {
                'data': request.POST,
                'files': request.FILES,
                'instance': obj,
            }
        else if not obj:
            kwargs = {
                'initial': self.get_changeform_initial_data(request),
            }
        else:
            kwargs = {
                'instance': obj,
            }

        return kwargs


    @csrf_protect_m
    @transaction.atomic
    def changeform_view(self, request, object_id=None, form_url='', extra_context=None):
        #
        # ...
        #
        form = self.get_form(request, obj)
        if request.method == 'POST':
            #
            # ...
            #
Last edited 22 months ago by Thomas Hauk (previous) (diff)

comment:7 Changed 22 months ago by Tim Graham

Perhaps a more fruitful effort would be to try converting the admin to class-based views as described in #17208.

comment:8 Changed 21 months ago by Tim Graham

Resolution: wontfix
Status: newclosed
Type: UncategorizedNew feature

comment:9 Changed 20 months ago by Thomas Hauk

Hi Tim -- I think this might be a case of "the perfect is the enemy of the good" here.

I agree that the admin should be completely refactored to be view-based, but that could be years of effort.

Providing a get_form_kwargs() API here wouldn't be perfect but it would fix this particular leak in the abstraction...

comment:10 Changed 15 months ago by Cody Hiar

Hey Thomas - I had a very similar situation I ended up using django's curry for clean solution:

from django.utils.functional import curry

    def get_form(self, form_class=None):
        """
        Returns an instance of the form to be used in this view.
        """
        if form_class is None:
            form_class = self.get_form_class() 
        return curry(form_class, **self.get_form_kwargs())
Note: See TracTickets for help on using tickets.
Back to Top