Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#16819 closed Bug (duplicate)

list_editable breaks multipage changelists

Reported by: anossov@… Owned by: nobody
Component: contrib.admin Version: master
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@… 4 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 Changed 4 years ago by anossov@…

  • Keywords admin list_editable added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 4 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

Changed 4 years ago by anossov@…

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

comment:3 Changed 4 years ago by anonymous

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 Changed 4 years ago by isagalaev

  • Cc Maniac@… added

comment:5 Changed 4 years ago by julien

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 Changed 4 years ago by makovick

  • Cc makovick added

comment:7 Changed 4 years ago by JannKleen

  • Cc jann@… added

comment:8 Changed 4 years ago by julien

See also #17198.

comment:9 Changed 3 years ago by anonymous

  • Severity changed from Normal to Release 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 Changed 3 years ago by julien

Oops, that was me.

comment:11 Changed 3 years ago by julien

A patch added to #17198 fixes this issue.

comment:12 Changed 3 years ago by aaugustin

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

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

comment:13 Changed 3 years ago by julien

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