#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 , 9 years ago
comment:2 by , 9 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 , 9 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 , 9 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 , 9 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 , 9 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 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':
#
# ...
#
comment:7 by , 9 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 , 9 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
| Type: | Uncategorized → New feature |
comment:9 by , 9 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 , 9 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."