Allow readonly callables defined in get_readonly_fields

[Tim McCurrach]

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 in fields and excluded (local variables in ModelAdmin.get_form) refer to different objects since they come from separate calls to get_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 in fields that are not in exclude but since there are different versions of my_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 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.

[Tim McCurrach]

PR is on the way soon