Opened 13 years ago

Closed 12 years ago

#16655 closed Bug (fixed)

Silent error in ModelAdmin.list_display callables...

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

Description

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!

Attachments (1)

silent_error_in_list_display_callables1.diff (1.1 KB ) - added by jedie 13 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 by jedie, 13 years ago

Has patch: set

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.

comment:2 by Aymeric Augustin, 13 years ago

Triage Stage: UnreviewedDesign decision needed

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?

comment:3 by Aymeric Augustin, 13 years ago

Needs tests: set
Triage Stage: Design decision neededAccepted

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.

in reply to:  2 comment:4 by jedie, 13 years ago

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?

comment:5 by frank.harper@…, 13 years ago

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.

comment:6 by Pi Delport, 12 years ago

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().

comment:7 by Aymeric Augustin, 12 years ago

Needs tests: unset

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

comment:8 by Aymeric Augustin, 12 years ago

Resolution: fixed
Status: newclosed

Fixed in [617d077f].

Note: See TracTickets for help on using tickets.
Back to Top