#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 by , 8 years ago
comment:2 by , 8 years ago
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 by , 8 years ago
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 by , 8 years ago
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 by , 8 years ago
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 by , 8 years ago
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 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': # # ... #
comment:7 by , 8 years ago
Perhaps a more fruitful effort would be to try converting the admin to class-based views as described in #17208.
comment:8 by , 8 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Type: | Uncategorized → New feature |
comment:9 by , 8 years ago
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 by , 8 years ago
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())
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."