Silent error in ModelAdmin.list_display callables...

[jedie]

Errors in ​ModelAdmin.list_display callables would be ignored.

I suspect not only a AttributeError.

Example:

class FooAdmin(admin.ModelAdmin):
    list_display = ('foobar',)

    def foobar(self, obj):
      raise AttributeError # ignored!

[jedie]

Is probably only AttributeError, catch here: https://code.djangoproject.com/browser/django/trunk/django/contrib/admin/templatetags/admin_list.py#L168

Seems that one unittests is wrong, isn't it? I correct this in the patch.

[Aymeric Augustin]

This code was introduced as part of r11965 (read-only fields in the admin). It catches AttributeError explicitly, and it certainly does so for a reason (or at least it did at the time it was written). Unfortunately, after some investigation, I can't figure out that reason.

Note that if someone has a callable in list_display that sometimes raises AttributeError, currently this is silently ignored, but with your patch it will crash the admin. It is possible to break backwards compatibility to fix bugs, but we should just be extra careful and make sure your proposal is really the right thing to do. I don't feel we have enough information right now.

Finally, regarding the test that seems wrong to you, could you open a separate ticket?

[Aymeric Augustin]

I reached Alex on IRC, he doesn't have a strong opinion on this ticket, besides the fact that it "needs more comprehensive tests for starters".

We've done a decent effort and found no reason to catch AttributeError. Let's consider it a bug.

[jedie]

Replying to aaugustin:

Finally, regarding the test that seems wrong to you, could you open a separate ticket?

IMHO the two things are related. Both should be changed together, isn't it?

[frank.harper@…]

I just started learning Django using version 1.3.1, and got bitten by this bug.

I think the fix really should be a high priority because the tutorial (very well done BTW), starts out with models and Admin mode. By definition beginners will be making a lot of mistakes that can cause AttributeError, and because of this bug they will have no feedback whatsoever about what is wrong. This makes for a very bad learning experience.

[Pi Delport]

I found this ticket after independently running into and submitting a fix for this bug: see #18593.

The patch is almost the same, but includes a test, and fixes callable_year() to be more in line with model_year() and modeladmin_year().

[Aymeric Augustin]

Now there's a pull request with tests (from #18593): ​https://github.com/django/django/pull/193

[Aymeric Augustin]

Fixed in [617d077f].