[Django] #28585: Set `has_file_field` context attribute for admin change forms conditionally on form `is_multipart` method
Django
on form `is_multipart` method
-----------------------------------------+--------------------------
Reporter: Mark Rogaski | Owner: nobody
Type: New feature | Status: assigned
Component: contrib.admin | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+--------------------------
In django.contrib.admin.options.ModelAdmin:
{{{
def render_change_form(self, request, context, add=False,
change=False, form_url='', obj=None):
...
context.update({
...
'has_file_field': True, # FIXME - this should check if form
or formsets have a FileField,
...
}}}
This attribute is used to determine whether a `multipart/form-data` is
used for the form content type instead of `application/x-www-form-
urlencoded`.
I'd like to implement this behavior as it appears to have been intended
(and I have a situation where multipart forms should only be used if
necessary for file uploads). However, it's likely that some applications
might be adversely impacted if the behavior changes. Perhaps adding an
`AVOID_MULTIPART_ADMIN_FORMS` setting or an `avoid_multipart ModelAdmin`
attribute that defaults to the current behavior is appropriate (better
suggestions for the naming are welcome).
In any case, I'd like to be able to have non-multipart forms returned when
multipart isn't needed without having to subclass ModelAdmin.
I am also happy to submit a PR for this based upon the preferred approach
to backward compatibility.
--
Ticket URL: <https://code.djangoproject.com/ticket/28585>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
--------------------------------------+------------------------------------
Reporter: Mark Rogaski | Owner: nobody
Component: contrib.admin | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
* type: New feature => Cleanup/optimization
Comment:
I think the check can reasonably use `Form.is_multipart()` and expected
users to provide a custom form with that method overridden if needed. Do
you have a case in mind where this isn't sufficient?
--
Ticket URL: <https://code.djangoproject.com/ticket/28585#comment:1>
Django
--------------------------------------+------------------------------------
Reporter: Mark Rogaski | Owner: nobody
Component: contrib.admin | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mark Rogaski):
I used `Form.is_multipart()` on an instantiated form in a mixin and that
worked fine, but it doesn't appear to recognize file fields in any
inlines.
--
Ticket URL: <https://code.djangoproject.com/ticket/28585#comment:2>
Django
--------------------------------------+------------------------------------
Reporter: Mark Rogaski | Owner: nobody
Component: contrib.admin | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
Perhaps my comment implied that checking `'has_file_field':
form.is_multipart()` is sufficient. I think the check for `has_file_field`
will also have to check `is_multipart()` for the forms on the inline
formset.
--
Ticket URL: <https://code.djangoproject.com/ticket/28585#comment:3>
Django
--------------------------------------+------------------------------------
Reporter: Mark Rogaski | Owner: nobody
Component: contrib.admin | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mark Rogaski):
I'll put together a patch. I'd looked at it earlier and got stuck in the
weeds a bit with the form introspection, but I'll look at it again over
the weekend.
--
Ticket URL: <https://code.djangoproject.com/ticket/28585#comment:4>
Django
--------------------------------------+------------------------------------
Reporter: Mark Rogaski | Owner: nobody
Component: contrib.admin | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/9298 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28585#comment:5>
Django
--------------------------------------+------------------------------------
Reporter: Mark Rogaski | Owner: nobody
Component: contrib.admin | Version: 1.11
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"0cf00769ad11fa5ff0dff585d7f491a80f3e45ef" 0cf0076]:
{{{
#!CommitTicketReference repository=""
revision="0cf00769ad11fa5ff0dff585d7f491a80f3e45ef"
Fixed #28585 -- Calculated admin's change form multipart context variable
from forms and formsets
Thanks Tim Graham for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28585#comment:6>