Opened 4 years ago
Closed 4 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_fields
is called several times per change view request.
- Taking the example above, the value
my_field
stored infields
andexcluded
(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_model
we return values infields
that are not inexclude
but since there are different versions ofmy_field
in 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 InlineModelAdmin
s 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 , 4 years ago
Owner: | changed from | to
---|
comment:2 by , 4 years ago
comment:3 by , 4 years ago
Description: | modified (diff) |
---|---|
Has patch: | set |
comment:4 by , 4 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