Re: [Django] #15602: Using get_readonly_fields and StackedInline/TabularInline admin objects doesn't allow creating new objects, immutible existing objects
Django
doesn't allow creating new objects, immutible existing objects
-------------------------------------------+-------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Milestone: | Component: contrib.admin
Version: 1.2 | 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 Solvik):
* cc: Solvik (added)
* ui_ux: => 0
* easy: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
doesn't allow creating new objects, immutible existing objects
-------------------------------------------+-------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Milestone: | Component: contrib.admin
Version: 1.2 | 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 |
-------------------------------------------+-------------------------------
Comment (by nivhaa@…):
Happens the same to me, but I'm working with 1.3... can't use
get_readonly_fields on an inline models when I'm having two inlines in the
same change_view page. "Please correct the errors below" is shown above,
without any error shown below. when trying to debug, somewhere along the
road, the formset gets this error for each inline object "This field is
required" - but as I said, it's not shown in the page
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:6>
Django
doesn't allow creating new objects, immutible existing objects
Type: Bug | Status: new
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 wilburb):
* cc: wilburb (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:7>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.2
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
---------------------------------+------------------------------------
Comment (by julien):
The core issue here, as pointed out by Karen, is that `obj` is the parent
object, not a child object from the inline. Note that the same issue
affects `get_readonly_fields()`, `get_fieldsets()` and
`get_prepopulated_fields()`:
https://code.djangoproject.com/browser/django/trunk/django/contrib/admin/options.py?rev=17065#L1080
Those methods are called as each inline **formset** is instantiated, not
as each **individual form** within the formsets is instantiated. The
inline formsets are instantiated in the admin view, whereas the individual
inline forms are instantiated later during the template-rendering stage,
at the same time as the related objects are fetched from the database via
the corresponding inline's queryset:
https://code.djangoproject.com/browser/django/trunk/django/contrib/admin/templates/admin/change_form.html?rev=17065#L60
https://code.djangoproject.com/browser/django/trunk/django/contrib/admin/helpers.py?rev=17065#L209
The three methods `get_readonly_fields()`, `get_fieldsets()` and
`get_prepopulated_fields()` all need to be passed the `HttpRequest`
object. Yet, by the stage at which the inline forms are instantiated the
request object isn't available.
To sum up, if we want to actually pass the inline's individual object to
those methods, then some pretty significant refactoring will be necessary.
Maybe this is a good time to rethink the way inlines are managed in the
admin.
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:8>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.2
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
---------------------------------+------------------------------------
* cc: net147 (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:9>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.2
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
---------------------------------+------------------------------------
* cc: zachborboa (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:10>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.2
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
---------------------------------+------------------------------------
Comment (by dji):
#24185 is a duplicate of this.
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:11>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.2
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
---------------------------------+------------------------------------
Comment (by dji):
I've done some additional research, and the problem actually goes much
deeper than this. Not only do the results of `get_readonly_fields` get
passed to the `InlineAdminFormSet` object and others in
`django.contrib.admin.helpers` that actually mark the individual field
objects as readonly and render them differently, they also get passed into
the regular form validation logic.
The way formsets work is to take a single form class and instantiate it
multiple times; an inline model formset uses a constructed `ModelForm`
subclass. (see `django.forms.models.modelformset_factory`)
Now, the results of `get_readonly_fields` are added to this constructed
form class as the `Meta.exclude` list, the list of fields that are
excluded from validation. Because this same class is instantiated
multiple times by the formset, every instance of the form must have the
same fields excluded. So, as it stands now, every instance must have the
same readonly fields - because you want to exclude at least these of
fields from validation. Decisions can't be made on a per-form/per-inline-
model-instance basis.
The only real "solutions" that come to mind both involve _underscore
methods, which is exactly what you're not supposed to do:
- The simplest is to allow the custom model form class to be created with
`exclude=()`, and let the individual forms in the formset be instantiated.
Then we'd tweak each form's `_meta.exclude` as we wanted to include the
results of its exclude.
- Another possibility is to use a custom `formset` that overrides
`BaseModelFormSet._construct_form`, the method that model formsets use to
get each model instance in the queryset and to instantiate each of their
individual forms. Instead of instantiating the formset's normal form,
we'd have to check the readonly fields on the instance, create a new model
form class for that particular exclude list, and instantiate a single
instance of that class. Whew.
I suppose the ideal solution would be to allow a model form's `exclude` to
be callable with the form's instance, but that's outside the scope of this
ticket I imagine.
Of course none of this solves the earlier problem of what to do with the
request, but in comparison that seems like a fairly simple problem to have
- worst case you send it along when you instantiate the `InlineAdminForm`
wrapper or bind up a `functools.partial` version of `get_readonly_fields`
with the request in it. But perhaps there's even a cleaner way, I didn't
look into this too much as it depends on the larger problem.
So, I guess in conclusion - how bad is it considered to mess with a form's
`_meta` after it's been instantiated? Would that be a deal-breaker even
if it fixed this?
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:12>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
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
---------------------------------+------------------------------------
* cc: vmspike@… (added)
* version: 1.2 => master
Comment:
5 years old ticket and no decision...
Suppose to change version to master. Has the same issue with django 1.9.
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:13>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
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
---------------------------------+------------------------------------
Comment (by astronouth7303):
I can confirm this for 1.10.1.
My two cents, I consider the current behavior (passing the top-level
object as the `obj` parameter) to be The Wrong Thing. An Inline Admin
should get its own model.
This bug significantly hinders the ability of `get_readonly_fields()` to
be useful in inline admins because you can only mark a field as read-only
for every instance of the admin. Which I could just do with
`readonly_fields`.
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:14>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
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
---------------------------------+------------------------------------
Comment (by Jimmy Merrild Krag):
Replying to [comment:14 Jamie Bliss]:
> My two cents, I consider the current behavior (passing the top-level
object as the `obj` parameter) to be The Wrong Thing. An Inline Admin
should get its own model.
>
> This bug significantly hinders the ability of `get_readonly_fields()` to
be useful in inline admins because you can only mark a field as read-only
for every instance of the admin. Which I could just do with
`readonly_fields`.
I agree completely. Actually my current work is hindered by it.
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:15>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
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
---------------------------------+------------------------------------
* cc: direx (added)
Comment:
Replying to [comment:14 Jamie Bliss]:
> My two cents, I consider the current behavior (passing the top-level
object as the `obj` parameter) to be The Wrong Thing. An Inline Admin
should get its own model.
Full ACK. And this is still the current behavior in Django 2.0 (and
master).
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:16>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
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
---------------------------------+------------------------------------
* cc: Prateem Shrestha (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:17>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
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
---------------------------------+------------------------------------
* cc: Paolo Dina (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:18>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
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
---------------------------------+------------------------------------
Comment (by John):
I'm using Django 2.0.7 and have the same issue. However, I've a dumb
solution to this (untested).
I'm building an account-transaction admin page. As one would expected,
the AccountAdmin will have TransactionStackedInlines. Undoubtably, we use
the inline form to add new transaction with most fields editable. And for
existing transaction, we want them as read-only.
Since, existing transactions look better in tabular inline (not all fields
displayed), while new transaction is better in stacked form, I created two
inlines. Here is my codes:
{{{
@admin.register(models.Account)
class AccountAdmin(admin.ModelAdmin):
inlines = [
inlines.TransactionTabularInline,
inlines.TransactionStackedInline,
]
}}}
And for the two inlines, I want that the top, tabular inline has no "add
new" action, while the bottom stacked inline shows nothing but a new
transaction:
{{{
class TransactionTabularInline(admin.TabularInline):
model = models.Transaction
extra = 1
max_num = 0
fields = (contains only a handful of fields)
can_delete = False
show_change_link = True
readonly_fields = [
the list of 'fields'
]
class TransactionStackedInline(admin.StackedInline):
model = models.Transaction
extra = 0
def get_queryset(self, request):
queryset = super().get_queryset(request)
return queryset.none()
}}}
There will be an arrow on each row in tabular inline, which points to
TransactionAdmin, in which `get_readonly_fields` will work as every one
expected.
I think you can use two (stacked) inlines, one for existing records with
more `readonly_fields`, and one without readonly but return an empty
query_set.
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:19>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
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
---------------------------------+------------------------------------
* cc: Scott Stevens (added)
Comment:
This also affects `has_<value>_permission` calls, and actually causes the
wrong object to be checked for permissions.
I feel as though this increases the severity, since it no longer just
affects what fields are read-only, but is now a failure to enforce
permissions correctly (and could allow object permissions where it's
expected to be prohibited).
I'm unsure if we need a new bug for this, or to expand the scope of this
existing ticket (rename/etc.).
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:20>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
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
---------------------------------+------------------------------------
Comment (by Carlton Gibson):
Given the change it would involve, I think this will probably need to be
resolved as a documentation issue.
That they're passed the parent object means that `get_readonly_fields()`
&co as they appear on an inline have the same signature but a different
semantics from when they appear on the normal ModelAdmin. As such, they
should be documented separately in the
[https://docs.djangoproject.com/en/2.1/ref/contrib/admin
/#inlinemodeladmin-objects InlineModelAdmin ref].
Similar differences are already documented for
[https://docs.djangoproject.com/en/2.1/ref/contrib/admin/#django.contrib.admin.InlineModelAdmin.has_add_permission
the various InlineModelAdmin versions of the `has_<value>_permission()`
methods].
On the actual issue, could it be resolved with permissions? If you have
`add` but not `change` on the inline model (possibly via custom
`has_add_permission()` and `has_change_permission()` methods) do you not
get the `allow creating new objects, immutible existing objects` from the
title?
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:21>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
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
---------------------------------+------------------------------------
Comment (by gual):
Nice solution, this worked in my scenario.
Replying to [comment:21 Carlton Gibson]:
> On the actual issue, could it be resolved with permissions? If you have
`add` but not `change` on the inline model (possibly via custom
`has_add_permission()` and `has_change_permission()` methods) do you not
get the `allow creating new objects, immutible existing objects` from the
title?
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:22>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
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
---------------------------------+------------------------------------
Comment (by Wojciech Bartosiak):
Replying to [comment:14 Jamie Bliss]:
> My two cents, I consider the current behavior (passing the top-level
object as the `obj` parameter) to be The Wrong Thing. An Inline Admin
should get its own model.
I need to agree with you and inform that this still exists in Django 3.0.2
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:23>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
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
---------------------------------+------------------------------------
Comment (by John Baker):
This issue is also affecting my current project. I expected `obj` within
`get_readonly_fields()` to be the inline instance, and not the parent.
I have a simplified permissions system that does not check per object
permissions, and cannot implement the workaround Carlton suggested.
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:24>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
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
---------------------------------+------------------------------------
Comment (by WhiteSage):
One can selectively disable certain fields on the add_fields method of the
associated formset as a workaround:
{{{
class MyFormSet(BaseInlineFormSet):
def add_fields(self, form, index):
super().add_fields(form, index)
if form.initial:
form.fields['my_field'].disabled = True
class My_Inline(admin.TabularInline):
model = MyModel
formset = MyFormSet
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:25>
Django
doesn't allow creating new objects, immutible existing objects
---------------------------------+------------------------------------
Reporter: bradwhittington | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: dev
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
---------------------------------+------------------------------------
* cc: Ülgen Sarıkavak (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15602#comment:26>