Opened 10 years ago

Closed 10 years ago

#21870 closed Bug (fixed)

Admin Checks fail for list_editable

Reported by: arne Owned by: nobody
Component: contrib.admin Version: 1.7-alpha-1
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

A valid list_editable configuration results in the following error during "manage.py validate":

Traceback (most recent call last):
  [...]
  File "django/django/contrib/admin/options.py", line 145, in check
    return cls.checks_class().check(cls, model, **kwargs)
  File "django/django/contrib/admin/checks.py", line 499, in check
    errors.extend(self._check_list_editable(cls, model))
  File "django/django/contrib/admin/checks.py", line 749, in _check_list_editable
    for index, item in enumerate(cls.list_editable)
TypeError: 'NoneType' object is not iterable

I will post a link to the GitHub Pull Request with Tests and possible Fix shortly.

Change History (6)

comment:1 by arne, 10 years ago

I've added a Pull Request on GitHub with Test and Fix:
https://github.com/django/django/pull/2209

comment:2 by Baptiste Mispelon, 10 years ago

Has patch: set
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Hi,

There does appear to be a problem, thanks for catching that.

The patch looks good overall but I'm wondering why you removed the original test_readonly_and_editable.

Thanks.

comment:3 by arne, 10 years ago

Hi,

I've not removed the "test_readonly_and_editable" test. I've added a new test "test_editable" to show the original problem and changed the "test_readonly_and_editable" test to test the case, that a readonly field ist listed in list_editable. Because the name implies that it does that. Everything else regarding readonly fields is tested further down the file in for example "test_readonly", etc.

Here is the link to the commit (part of the pull request) which changes the testcase: https://github.com/arneb/django/commit/ade488933946a33ffb6d09918331069ed9397d55

Without my change "test_readonly" and "test_readonly_and_editable" are doing basically the same thing.

comment:4 by Baptiste Mispelon, 10 years ago

Triage Stage: AcceptedReady for checkin

You're right, the current test_readonly_and_editable seems like it does the same as test_readonly so your change makes sense.

This looks merge-ready.

Could you rebase the PR on master (if that's not already the case) and squash the commits into one, following the commit guidelines (the typo fix could actually go in a separate commit if possible, but I'm OK either way).

Thanks.

comment:5 by arne, 10 years ago

I've tried my best to update the Pull Request according to your instructions.

comment:6 by Baptiste Mispelon <bmispelon@…>, 10 years ago

Resolution: fixed
Status: newclosed

In 38be3cf5e4c4980b484e1013c2a4047725ba8d3e:

Fixed #21870 -- Admin check for list_editable_item

During the admin check for list_editable _check_list_editable_item
should return an empty list if all checks pass. Additionally the
Testcase test_readonly_and_editable was changed to test what the
name implies instead of duplicating the logic of test_readonly.

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