Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15424 closed (fixed)

readonly_fields in InlineModelAdmin looks up wrong callable

Reported by: kmike Owned by: nobody
Component: contrib.admin Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

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__'


Attachments (3)

FooAdmin.png (58.2 KB) - added by kmike 3 years ago.
FooAdmin2.png (169.2 KB) - added by kmike 3 years ago.
15424.diff (2.1 KB) - added by kmtracey 3 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 3 years ago by ramiro

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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.

Changed 3 years ago by kmike

Changed 3 years ago by kmike

comment:2 Changed 3 years ago by kmike

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.

comment:3 Changed 3 years ago by kmike

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

comment:4 Changed 3 years ago by ramiro

  • Triage Stage changed from Accepted to Design decision needed

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.

comment:5 Changed 3 years ago by kmike

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.

comment:6 Changed 3 years ago by kmike

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

Changed 3 years ago by kmtracey

comment:7 Changed 3 years ago by kmtracey

  • Triage Stage changed from Design decision needed to Accepted

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.

comment:8 Changed 3 years ago by ramiro

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.

comment:9 Changed 3 years ago by ramiro

  • Resolution set to fixed
  • Status changed from new to closed

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.

comment:10 Changed 3 years ago by ramiro

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.

comment:11 Changed 3 years ago by kmike

Ramiro, Karen, many thanks!

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.