Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#16819 closed Bug (duplicate)

list_editable breaks multipage changelists

Reported by: anossov@… Owned by: nobody
Component: contrib.admin Version: dev
Severity: Release blocker Keywords: admin list_editable
Cc: Maniac@…, makovick, jann@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

AssertionError is raised when I try to view a changelist with more than 100 items and some list_editable fields.

Django version: SVN-16714

Environment:

Request Method: GET
Request URL: ...

Django Version: 1.4 pre-alpha
Python Version: 2.6.5
Installed Applications:
['django.contrib.auth',
 'django.contrib.admin',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.sites',
 'django.contrib.markup',
 'django.contrib.redirects',
 '...',
 'south']
Installed Middleware:
['django.middleware.common.CommonMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django_replicated.middleware.ReplicationMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware']


Traceback:
File "/usr/lib/python2.6/dist-packages/django/core/handlers/base.py" in get_response
  111.                         response = callback(request, *callback_args, **callback_kwargs)
File "/usr/lib/python2.6/dist-packages/django/contrib/admin/options.py" in wrapper
  326.                 return self.admin_site.admin_view(view)(*args, **kwargs)
File "/usr/lib/python2.6/dist-packages/django/utils/decorators.py" in _wrapped_view
  91.                     response = view_func(request, *args, **kwargs)
File "/usr/lib/python2.6/dist-packages/django/views/decorators/cache.py" in _wrapped_view_func
  88.         response = view_func(request, *args, **kwargs)
File "/usr/lib/python2.6/dist-packages/django/contrib/admin/sites.py" in inner
  192.             return view(request, *args, **kwargs)
File "/usr/lib/python2.6/dist-packages/django/utils/decorators.py" in _wrapper
  25.             return bound_func(*args, **kwargs)
File "/usr/lib/python2.6/dist-packages/django/utils/decorators.py" in _wrapped_view
  91.                     response = view_func(request, *args, **kwargs)
File "/usr/lib/python2.6/dist-packages/django/utils/decorators.py" in bound_func
  21.                 return func(self, *args2, **kwargs2)
File "/usr/lib/python2.6/dist-packages/django/contrib/admin/options.py" in changelist_view
  1175.             formset = cl.formset = FormSet(queryset=cl.result_list)
File "/usr/lib/python2.6/dist-packages/django/forms/models.py" in __init__
  422.         super(BaseModelFormSet, self).__init__(**defaults)
File "/usr/lib/python2.6/dist-packages/django/forms/formsets.py" in __init__
  47.         self._construct_forms()
File "/usr/lib/python2.6/dist-packages/django/forms/formsets.py" in _construct_forms
  107.         for i in xrange(self.total_form_count()):
File "/usr/lib/python2.6/dist-packages/django/forms/formsets.py" in total_form_count
  83.             initial_forms = self.initial_form_count()
File "/usr/lib/python2.6/dist-packages/django/forms/models.py" in initial_form_count
  427.             return len(self.get_queryset())
File "/usr/lib/python2.6/dist-packages/django/forms/models.py" in get_queryset
  463.                 qs = qs.order_by(self.model._meta.pk.name)
File "/usr/lib/python2.6/dist-packages/django/db/models/query.py" in order_by
  657.                 "Cannot reorder a query once a slice has been taken."

Exception Type: AssertionError at /admin/...
Exception Value: Cannot reorder a query once a slice has been taken.

Attachments (1)

list_editable_fail.tar.gz (1.4 KB ) - added by anossov@… 13 years ago.
minimal project with a test failing with the AssertionError. run ./manage.py test testapp

Download all attachments as: .zip

Change History (14)

comment:1 by anossov@…, 13 years ago

Keywords: admin list_editable added

comment:2 by Alex Gaynor, 13 years ago

Triage Stage: UnreviewedAccepted

by anossov@…, 13 years ago

Attachment: list_editable_fail.tar.gz added

minimal project with a test failing with the AssertionError. run ./manage.py test testapp

comment:3 by anonymous, 13 years ago

Possible solution: fallback to ordering on PK is no ordering is specified anywhere:

*** patched_main.py	2011-09-12 14:00:33.000000000 +0400
--- django/contrib/admin/views/main.py	2011-09-12 14:00:41.000000000 +0400
***************
*** 211,216 ****
--- 211,218 ----
                  except (IndexError, ValueError):
                      continue # Invalid ordering specified, skip it.
  
+         if not ordering:
+             ordering = ('pk',)
          return ordering
  
      def get_ordering_field_columns(self):

comment:4 by Ivan Sagalaev, 13 years ago

Cc: Maniac@… added

comment:5 by Julien Phalip, 13 years ago

The issue occurs when no ordering is not explicitly set by Model.Meta.ordering or ModelAdmin.ordering. In that case the FormSet enforces default ordering by primary key (source:django/trunk/django/forms/models.py?rev=16659#L459) (see the reason in #10163), yet the queryset has already been paginated by the changelist (source:django/trunk/django/contrib/admin/views/main.py?rev=16659#L156).

I'm wondering if the default ordering shouldn't be taken out of the FormSet and moved to ChangeList.get_ordering() as in the suggested patch in comment:3.

comment:6 by makovick, 13 years ago

Cc: makovick added

comment:7 by Jann Kleen, 13 years ago

Cc: jann@… added

comment:8 by Julien Phalip, 13 years ago

See also #17198.

comment:9 by anonymous, 13 years ago

Severity: NormalRelease blocker

#17729 was closed as a duplicate. Marking as release blocker since it actually is a regression in 1.4.

However, I tend to think that this ticket should also be closed as a duplicate of #17198, which tackles the ordering issue more broadly (i.e. not just where list_editable is involved).

comment:10 by Julien Phalip, 13 years ago

Oops, that was me.

comment:11 by Julien Phalip, 13 years ago

A patch added to #17198 fixes this issue.

comment:12 by Aymeric Augustin, 13 years ago

Resolution: duplicate
Status: newclosed

A consensus was reached on #17198. I'm closing this ticket as a duplicate, and making #17198 the release blocker.

comment:13 by Julien Phalip, 13 years ago

In [17635]:

Fixed #17198 -- Ensured that a deterministic order is used across all database backends for displaying the admin change list's results. Many thanks to Luke Plant for the report and general approach, to everyone involved in the design discussions, and to Carl Meyer for the patch review. Refs #16819.

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