#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 Changed 16 months ago by arne

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

comment:2 Changed 16 months ago by bmispelon

  • Has patch set
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

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 Changed 16 months ago by arne

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 Changed 16 months ago by bmispelon

  • Triage Stage changed from Accepted to Ready 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 Changed 16 months ago by arne

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

comment:6 Changed 16 months ago by Baptiste Mispelon <bmispelon@…>

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

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