Admin view redirection failures

[Vinay Sajip <vinay_sajip@…>]

Environment
===========
This is against a checkout of SVN rev 9800.
OS: Ubuntu Dapper 2.6.15-53-386 #1 PREEMPT Mon Nov 24 17:50:35 UTC 2008 i686 GNU/Linux
Python: Python 2.4.3 (#2, Jul 31 2008, 21:56:52) [GCC 4.0.3 (Ubuntu 4.0.3-1ubuntu5)] on linux2
Database: postgresql (in settings.py), PostgreSQL 8.3.5 on Windows Server 2003

Test Output
===========
FAIL: testBasicEditPost (regressiontests.admin_views.tests.AdminViewBasicTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vinay/projects/django/upstream/tests/regressiontests/admin_views/tests.py", line 115, in testBasicEditPost
    self.failUnlessEqual(response.status_code, 302) # redirect somewhere
AssertionError: 200 != 302

======================================================================
FAIL: testUnicodeEdit (regressiontests.admin_views.tests.AdminViewUnicodeTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vinay/projects/django/upstream/tests/regressiontests/admin_views/tests.py", line 713, in testUnicodeEdit
    self.failUnlessEqual(response.status_code, 302) # redirect somewhere
AssertionError: 200 != 302

======================================================================
FAIL: testBasicEditPost (regressiontests.admin_views.tests.CustomModelAdminTest)----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vinay/projects/django/upstream/tests/regressiontests/admin_views/tests.py", line 115, in testBasicEditPost
    self.failUnlessEqual(response.status_code, 302) # redirect somewhere
AssertionError: 200 != 302

----------------------------------------------------------------------
Ran 43 tests in 10.249s

FAILED (failures=3)

[Karen Tracey]

This only seems to happen on some PostgreSQL setups (I see it on mine, which is also on a Windows box, though I'm not sure of the version and that machine is not turned on at the moment to check). I believe it's caused by #9758 and/or #9006, though perhaps it could use some more independent investigation. Crux of the problem is that some admin views tests that are attempting to update a page with multiple inline-edited objects are coming back with an error page (re-display of the form with errors, status code 200) instead of succeeding and redirecting (status code 302).

[(none)]

Milestone post-1.0 deleted

[Jacob]

Duplicate of #9758

[Karen Tracey]

#9758 was actually a user error, not passing in the queryset during both GET and POST processing. Admin doesn't suffer from that, but it does suffer from expecting consistent ordering in a queryset even if no ordering has been specified. That is, it expects the order of the queryset used during POST to be the same as the order of the queryset used during GET, even if no ordering has been applied to the queryset. That assumption is what is causing these errors in the test suite on some PostgreSQL installs. It's more logically fixed under this ticket than #9758 so I closed that one and am re-opening this one to deal with it.

[Karen Tracey]

I'll attach a patch that has the virtue of fixing the observed problem, but little else to recommend it. It forces the queryset (if it's actually a queryset..turns out sometimes lists are passed in for the queryset arg) returned by BaseModelFormSet get_queryset to have some ordering. But I couldn't see any way to easily determine if an arbitrary queryset already has an ordering specified, so the patch goes and digs into various attributes of the queryset's query to try to figure that out. There must be an easier way to do this??

[jkocherhans]

I think we ought to just suck it up and do this. Yes, it smells a bit, but the other option I can think of would be to use the primary key in the prefix somehow, which makes other parts of formset handling a lot harder.

Malclom, would it make sense to add something like .has_ordering() or .is_ordered() to a query(set) to abstract some of that code away? Does something like that already exist?

[Jacob]

(In [10623]) Added a QuerySet.ordered property to check if a queryset is already ordered. Refs #10163.

[Jacob]

(In [10624]) Fixed a needless list() coercion in in ChangeList. Refs #10163.

[Jacob]

(In [10625]) Fixed #10163: add an artificial ordering to querysets used by formsets, thus ensuring that POSTed data "lines up" correctly every time. Thanks to Karen Tracey for pointing in the right direction here.

[Jacob]

(In [10630]) [1.0.X] Fixed #10163: add an artificial ordering to querysets used by formsets, thus ensuring that POSTed data "lines up" correctly every time. Thanks to Karen Tracey for pointing in the right direction here.

This is a backport of [10625] from trunk, in a sense. In 1.1 I added a QuerySet.ordered property to deal with the logic of determining whether a queryset has ordering, but we can't add new features on a bugfix branch. So here in 1.0-land, the logic has to live in the formset. This smells, but it's better than having a bug.

[Jacob]

Milestone 1.1 deleted