readonly_fields in InlineModelAdmin looks up wrong callable

[Mikhail Korobov]

my_app.models:

from django.db import models

class Foo(models.Model):
    name = models.CharField(max_length=100)

class Bar(models.Model):
    foo = models.ForeignKey(Foo)

my_app.admin:

from django.contrib import admin
from my_app import Foo, Bar

class BarInline(admin.TabularInline):
    model = Bar
    readonly_fields=['call_me']
    
    def call_me(self, obj):
        return 'BarInline'

class FooAdmin(admin.ModelAdmin):
    inlines = [BarInline]

    def call_me(self, obj):
        return 'FooAdmin'

admin.site.register(Foo, FooAdmin)

This will show 'FooAdmin' as a 'call_me' value for each inline. If there is no 'call_me' method in FooAdmin, django ends with exception (example above is a simplified version of my setup, exception is from real project so model names don't match):

Environment:


Request Method: GET
Request URL: http://127.0.0.1:8000/admin/sales/position/2/

Django Version: 1.3 beta 1 SVN-15636
Python Version: 2.6.6
Installed Applications:
['django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.sites',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'django.contrib.admin',
 'south',
 'info',
 'accounts',
 'sales',
 'debug_toolbar']
Installed Middleware:
['django.middleware.common.CommonMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.locale.LocaleMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware']


Template error:
In template /Users/kmike/envs/tenders/src/django/django/contrib/admin/templates/admin/edit_inline/tabular.html, error at line 10
   Caught AttributeError while rendering: 'PositionAdmin' object has no attribute '__name__'
   1 : {% load i18n adminmedia admin_modify %}


   2 : <div class="inline-group" id="{{ inline_admin_formset.formset.prefix }}-group">


   3 :   <div class="tabular inline-related {% if forloop.last %}last-related{% endif %}">


   4 : {{ inline_admin_formset.formset.management_form }}


   5 : <fieldset class="module">


   6 :    <h2>{{ inline_admin_formset.opts.verbose_name_plural|capfirst }}</h2>


   7 :    {{ inline_admin_formset.formset.non_form_errors }}


   8 :    <table>


   9 :      <thead><tr>


   10 :       {% for field in inline_admin_formset.fields %} 


   11 :        {% if not field.widget.is_hidden %}


   12 :          <th{% if forloop.first %} colspan="2"{% endif %}{% if field.required %} class="required"{% endif %}>{{ field.label|capfirst }}</th>


   13 :        {% endif %}


   14 :      {% endfor %}


   15 :      {% if inline_admin_formset.formset.can_delete %}<th>{% trans "Delete?" %}</th>{% endif %}


   16 :      </tr></thead>


   17 : 


   18 :      <tbody>


   19 :      {% for inline_admin_form in inline_admin_formset %}


   20 :         {% if inline_admin_form.form.non_field_errors %}


Traceback:
File "/Users/kmike/envs/tenders/src/django/django/core/handlers/base.py" in get_response
  111.                         response = callback(request, *callback_args, **callback_kwargs)
File "/Users/kmike/envs/tenders/src/django/django/contrib/admin/options.py" in wrapper
  308.                 return self.admin_site.admin_view(view)(*args, **kwargs)
File "/Users/kmike/envs/tenders/src/django/django/utils/decorators.py" in _wrapped_view
  93.                     response = view_func(request, *args, **kwargs)
File "/Users/kmike/envs/tenders/src/django/django/views/decorators/cache.py" in _wrapped_view_func
  79.         response = view_func(request, *args, **kwargs)
File "/Users/kmike/envs/tenders/src/django/django/contrib/admin/sites.py" in inner
  196.             return view(request, *args, **kwargs)
File "/Users/kmike/envs/tenders/src/django/django/utils/decorators.py" in _wrapper
  28.             return bound_func(*args, **kwargs)
File "/Users/kmike/envs/tenders/src/django/django/utils/decorators.py" in _wrapped_view
  93.                     response = view_func(request, *args, **kwargs)
File "/Users/kmike/envs/tenders/src/django/django/utils/decorators.py" in bound_func
  24.                 return func(self, *args2, **kwargs2)
File "/Users/kmike/envs/tenders/src/django/django/db/transaction.py" in inner
  217.                 res = func(*args, **kwargs)
File "/Users/kmike/envs/tenders/src/django/django/contrib/admin/options.py" in change_view
  1031.         return self.render_change_form(request, context, change=True, obj=obj)
File "/Users/kmike/envs/tenders/src/django/django/contrib/admin/options.py" in render_change_form
  709.         ], context, context_instance=context_instance)
File "/Users/kmike/envs/tenders/src/django/django/shortcuts/__init__.py" in render_to_response
  20.     return HttpResponse(loader.render_to_string(*args, **kwargs), **httpresponse_kwargs)
File "/Users/kmike/envs/tenders/src/django/django/template/loader.py" in render_to_string
  188.         return t.render(context_instance)
File "/Users/kmike/envs/tenders/src/django/django/template/base.py" in render
  123.             return self._render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/base.py" in _render
  117.         return self.nodelist.render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/base.py" in render
  744.                 bits.append(self.render_node(node, context))
File "/Users/kmike/envs/tenders/src/django/django/template/debug.py" in render_node
  73.             result = node.render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/loader_tags.py" in render
  127.         return compiled_parent._render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/base.py" in _render
  117.         return self.nodelist.render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/base.py" in render
  744.                 bits.append(self.render_node(node, context))
File "/Users/kmike/envs/tenders/src/django/django/template/debug.py" in render_node
  73.             result = node.render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/loader_tags.py" in render
  127.         return compiled_parent._render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/base.py" in _render
  117.         return self.nodelist.render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/base.py" in render
  744.                 bits.append(self.render_node(node, context))
File "/Users/kmike/envs/tenders/src/django/django/template/debug.py" in render_node
  73.             result = node.render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/loader_tags.py" in render
  64.             result = block.nodelist.render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/base.py" in render
  744.                 bits.append(self.render_node(node, context))
File "/Users/kmike/envs/tenders/src/django/django/template/debug.py" in render_node
  73.             result = node.render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/defaulttags.py" in render
  227.                 nodelist.append(node.render(context))
File "/Users/kmike/envs/tenders/src/django/django/template/loader_tags.py" in render
  170.             return self.render_template(template, context)
File "/Users/kmike/envs/tenders/src/django/django/template/loader_tags.py" in render_template
  141.         output = template.render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/base.py" in render
  123.             return self._render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/base.py" in _render
  117.         return self.nodelist.render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/base.py" in render
  744.                 bits.append(self.render_node(node, context))
File "/Users/kmike/envs/tenders/src/django/django/template/debug.py" in render_node
  73.             result = node.render(context)
File "/Users/kmike/envs/tenders/src/django/django/template/defaulttags.py" in render
  190.             values = list(values)
File "/Users/kmike/envs/tenders/src/django/django/contrib/admin/helpers.py" in fields
  225.                     'label': label_for_field(field, self.opts.model, self.model_admin),
File "/Users/kmike/envs/tenders/src/django/django/contrib/admin/util.py" in label_for_field
  252.                     message += " or %s" % (model_admin.__name__,)

Exception Type: TemplateSyntaxError at /admin/sales/position/2/
Exception Value: Caught AttributeError while rendering: 'PositionAdmin' object has no attribute '__name__'

[Ramiro Morales]

It seems you went too far when stripping your project to obtain the test case because as presented I don't get any Bar tabular inlines in the edit view of a Foo instance. Are you getting tabular inlines with a Bar model that has no other field than the the FK to Foo? Do you have a fields or fieldsets option in your TabularInline along with the readonly_fields?.

I might have some lead about which might be the problem but I'd like to know the exact environment you are using before trying to fix things.

[Mikhail Korobov]

I've just pasted the snippet from stripped-down example ("from my_app import Foo, Bar" was incorrect, should read " from my_app.models import Foo, Bar ") to a new app.
Then went to admin and clicked to 'Foos'. Then clicked 'add Foo' link. The resulting FooAdmin.png screenshot (sorry for Russian locale) contains "FooAdmin" text instead of "BarInline".

Then I removed FooAdmin.call_me method and get an exception (screenshot FooAdmin2.png), it is the same as pasted in ticket.

Disabling django-debug-toolbar and all other apps doesn't change anything, project was almost in clean state.

[Mikhail Korobov]

So I mean the bug is repeatable even with stripped example. And thanks for quick response!

[Ramiro Morales]

I can reproduce what you describe if I also create an identical standalone app (but strangely enough I can't recreate the rendering of the Bar inlines when using test client).

I'm pretty sure sure that although the documntation describes it as behavung nearly identical to the changelist view-related 'list_display' option, 'readonly_fields' doesn't support callables in Modeladmin/{Stacked\Tabular}Inlines, other two options related in the sense that they are relevant to the add/change views like 'fieldsets.fields' and 'fields' don't support it either. If this is the case, what you are getting is behavior that seems to work partially but isn't really supported.

[Mikhail Korobov]

Callable readonly_fields seems to work fine for ModelAdmin. The release notes for django 1.2 ( ​http://docs.djangoproject.com/en/1.2/releases/1.2/#readonly-fields-in-modeladmin ) say explicitly that both field values and calculated values can be displayed using readonly_fields option. So I think this feature is really intended to be supported and ticket describes a real bug, not a new feature or unfortunate wordings in the docs.

[Mikhail Korobov]

Callable readonly_fields for ModelAdmin are in django's test suite: ​http://code.djangoproject.com/browser/django/trunk/tests/regressiontests/admin_views/models.py#L544

[Karen Tracey]

I can recreate if I take a callable readonly_field I have specified on an inline and try to move its definition from the model to the inline model admin. Attached patch fixes that problem, but I don't have time at the moment to check into whether the change might break anything else.

Note the exception being raised is a bug in itself -- the code is raising an exception trying to compose a message about being unable to find the callable on either the model or the model admin, but it's using model_admin.__name__ to try to retrieve the name of the model admin's class where it needs to be model_admin.__class__.__name__. Fix that and you at least get a better message about the real source of the problem.

[Ramiro Morales]

Karen's patch not only fixes the problem but also fixes the inconsistency I was seeing: I could reproduce the issue in a real application but not under the test environment when trying to prepare tests initial conditions.

Only thing I'd add is early detection (during admin validation) of a missing callable specified in an admin inline 'readonly_fields'. Currently the check is done only for ModelAdmin's, this check effectively overlaps a bit with the fixed model_admin.__class__.__name__ check in django/contrib/admin/util.py.

Commit will include this extra modification.

Also, the fact that such kind of errors weren't being caught early might be related to the problem I describe above because including a missing callable in a inline 'readonly_fields' in a test app, with DEBUG=False shows a broken add view when requested from a browser: No CSS styling, missing inlines block, multiple missing 500.html errors on the console, but HTTP status is 200.

[Ramiro Morales]

In [15650]:

Fixed #15424 -- Corrected lookup of callables listed in admin inlines' readonly_fields by passing the right ModelAdmin (sub)class instance when instantiating inline forms admin wrappers. Also, added early validation of its elements. Thanks kmike for the report and Karen for the patch fixing the issue.

[Ramiro Morales]

In [15651]:

[1.2.X] Fixed #15424 -- Corrected lookup of callables listed in admin inlines' readonly_fields by passing the right ModelAdmin (sub)class instance when instantiating inline forms admin wrappers. Also, added early validation of its elements. Thanks kmike for the report and Karen for the patch fixing the issue.

Backport of [15650] from trunk.

[Mikhail Korobov]

Ramiro, Karen, many thanks!