Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#10163 closed (fixed)

Admin view redirection failures

Reported by: Vinay Sajip <vinay_sajip@…> Owned by: nobody
Component: Forms Version: master
Severity: Keywords: admin view redirection
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

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)

Attachments (1)

10163.diff (903 bytes) - added by kmtracey 6 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 6 years ago by kmtracey

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

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).

comment:2 Changed 6 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:3 Changed 6 years ago by jacob

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

Duplicate of #9758

comment:4 Changed 6 years ago by kmtracey

  • milestone set to 1.1
  • Resolution duplicate deleted
  • Status changed from closed to reopened

#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.

comment:5 Changed 6 years ago by kmtracey

  • Triage Stage changed from Unreviewed to Accepted

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??

Changed 6 years ago by kmtracey

comment:6 Changed 6 years ago by 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?

comment:7 Changed 6 years ago by Alex

  • Component changed from Uncategorized to Forms

comment:8 Changed 6 years ago by Alex

  • Has patch set

comment:9 Changed 6 years ago by jacob

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

comment:10 Changed 6 years ago by jacob

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

comment:11 Changed 6 years ago by jacob

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

(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.

comment:12 Changed 6 years ago by 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.

comment:13 Changed 4 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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