Using get_readonly_fields and StackedInline/TabularInline admin objects doesn't allow creating new objects, immutible existing objects

[bradwhittington]

I am using a pattern described ​here, on a StackedInline admin class, AKA:

    def get_readonly_fields(self, request, obj = None):
        if obj:
            return self.readonly_fields+('text','author','private')
        return self.readonly_fields

But it seems to currently be broken (in 1.2.5), I get all fields repeated twice per object, and the add new object fields are all marked read-only, as if obj is always a non-None value. If I throw logging into that method (log.debug(obj==None)), I am seeing (in an example with one inline object already created):

2011-03-14 00:23:52 SAST+0000 - DEBUG - task.admin - False
2011-03-14 00:23:52 SAST+0000 - DEBUG - task.admin - True
2011-03-14 00:23:52 SAST+0000 - DEBUG - task.admin - False
2011-03-14 00:23:52 SAST+0000 - DEBUG - task.admin - False

[Karen Tracey]

The obj passed to get_readonly_fields defined in an inline ModelAdmin looks to be the parent object, not the object for the inline itself. This looks to be similar to #15424.

[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

[Julien Phalip]

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.

[David Isaacson]

#24185 is a duplicate of this.

[David Isaacson]

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 render the formset (including the readonly fields), they also get passed into the formset 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 get_readonly_fields.

• 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?

[Mikhail]

5 years old ticket and no decision...
Suppose to change version to master. Has the same issue with django 1.9.

[Jamie Bliss]

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.

[Jimmy Merrild Krag]

Replying to 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.

[direx]

Replying to 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).

[Prateem Shrestha]

This is impacting a project I am working on as well.

[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  # extra must be 1 to make the magic work
	max_num = 0  # somehow this disable the "add" because max_num > extra
	fields = (contains only a handful of fields)
	can_delete = False
	show_change_link = True  # allow viewing the full entry in another page 
	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()  # no existing records will appears

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.

I found queryset.none() from django's code, when users has no permission on the object. So, I suppose this is "okay" when I do not want to display existing records for my own reason.

[Scott Stevens]

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.).

[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 ​InlineModelAdmin ref.

Similar differences are already documented for ​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?

[Gualberto Escalante]

Nice solution, this worked in my scenario.

Replying to 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?

[Wojciech Bartosiak]

Replying to 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

[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.

[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