Opened 5 years ago
Closed 5 years ago
#32336 closed New feature (wontfix)
Allow functions in ModelAdmin.readonly_fields.
| Reported by: | Tim McCurrach | Owned by: | Tim McCurrach |
|---|---|---|---|
| Component: | contrib.admin | Version: | 3.1 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Currently if you have a get_readonly_fields that defines a function within its scope, and then uses it such as the following:
def get_readonly_fields(self, request, obj=None):
def my_field(obj):
return "This is readonly"
return self.readonly_fields + (my_field, )
This will lead to an error:
File "...lib/python3.8/site-packages/django/forms/models.py", line 265, in __new__
message = message % (', '.join(missing_fields),
TypeError: sequence item 0: expected str instance, function found
Motivation
Whilst the above may seem a bit of an edge case, the reason I came across it was wanting to use the request object for a readonly field. Readonly fields can be callables that accept only one parameter (obj). If you want a readonly field that behaves differently based on the request object, you need to do something like this:
def my_field(request, obj):
# something here
class MyAdmin(admin.ModelAdmin):
def get_readonly_fields(self, request, obj=None):
readonly_field = functools.partial(my_field, request)
return (readonly_field,)
But this obviously leads to the same error. Changing the behaviour of a specific readonly field based on the user seems like a reasonable use-case to cater for.
Cause of the error
get_readonly_fieldsis called several times per change view request.
- Taking the example above, the value
my_fieldstored infieldsandexcluded(local variables inModelAdmin.get_form) refer to different objects since they come from separate calls toget_readonly_fields. - This is a problem further down the line when the ModelForm is being generated. At the end of
forms.models.fields_for_modelwe return values infieldsthat are not inexcludebut since there are different versions ofmy_fieldin both, it ends up not being filtered out when it should be.
Proposed Solution
Cache the return value of get_readonly_fields in some instance variable self._readonly_fields, and reset this value at the beginning of every request. This way the various variables that store readonly fields will all be referring to the same objects. For InlineModelAdmins the value of self._readonly_fields would need to be set to None on instantiation (which happens for every new change view request).
Tests
I'm not sure if this needs a separate test. I have adapted PostAdmin (used in lots of admin tests, in particular the ones that test readonly_fields) to cater for this case. So although no tests are added, there are plenty of failing tests before the fix.
Change History (4)
comment:1 by , 5 years ago
| Owner: | changed from to |
|---|
comment:2 by , 5 years ago
comment:3 by , 5 years ago
| Description: | modified (diff) |
|---|---|
| Has patch: | set |
comment:4 by , 5 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | assigned → closed |
| Summary: | Allow readonly callables defined in get_readonly_fields → Allow functions in ModelAdmin.readonly_fields. |
| Type: | Bug → New feature |
Functions are not a supported in ModelAdmin.readonly_fields, see docs:
"A read-only field can not only display data from a model’s field, it can also display the output of a model’s method or a method of the ModelAdmin class itself."
I agree with Tim that it's not worth complexity.
PR is on the way soon