[Django] #27231: Initialize forms in ModelAdmin like View (i.e. add get_form_kwargs to contrib.admin)
Django
contrib.admin)
----------------------------------+--------------------
Reporter: thauk-copperleaf | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------
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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27231>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
contrib.admin)
Reporter: thauk-copperleaf | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
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."
--
Ticket URL: <https://code.djangoproject.com/ticket/27231#comment:1>
Django
contrib.admin)
Reporter: thauk-copperleaf | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by thauk-copperleaf):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27231#comment:2>
Django
contrib.admin)
Reporter: thauk-copperleaf | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by thauk-copperleaf):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27231#comment:3>
Django
contrib.admin)
Reporter: thauk-copperleaf | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
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`?
--
Ticket URL: <https://code.djangoproject.com/ticket/27231#comment:4>
Django
contrib.admin)
Reporter: thauk-copperleaf | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by thauk-copperleaf):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27231#comment:5>
Django
contrib.admin)
Reporter: thauk-copperleaf | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by thauk-copperleaf):
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':
#
# ...
#
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27231#comment:6>
Django
contrib.admin)
Reporter: thauk-copperleaf | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Perhaps a more fruitful effort would be to try converting the admin to
class-based views as described in #17208.
--
Ticket URL: <https://code.djangoproject.com/ticket/27231#comment:7>
Django
contrib.admin)
Reporter: thauk-copperleaf | Owner: nobody
Type: New feature | Status: closed
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* resolution: => wontfix
* type: Uncategorized => New feature
--
Ticket URL: <https://code.djangoproject.com/ticket/27231#comment:8>
Django
contrib.admin)
Reporter: Thomas Hauk | Owner: nobody
Type: New feature | Status: closed
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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...
--
Ticket URL: <https://code.djangoproject.com/ticket/27231#comment:9>
Django
contrib.admin)
Reporter: Thomas Hauk | Owner: nobody
Type: New feature | Status: closed
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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())
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27231#comment:10>