Opened 16 years ago

Closed 16 years ago

Last modified 13 years ago

#7553 closed (fixed)

return None in sites.py doesn't work with decorators

Reported by: Michael Newman Owned by: Brian Rosner
Component: contrib.admin Version: newforms-admin
Severity: Keywords: admin nfa-blocker
Cc: newmaniese@…, cmawebsite@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Simply, adding the cache to the login view in the new admin broke the way it handled persistent data. http://buildbot.djangoproject.com/newforms-admin%202.4%20sqlite/builds/237/step-test/0

Attachments (4)

7553.diff (696 bytes ) - added by Michael Newman 16 years ago.
Simple patch that uses the request path to get the response from model_page instead
7553-sites-decorators-excpt.diff (3.7 KB ) - added by Michael Newman 16 years ago.
A proof-of-concept patch using an exception to catch data that may fall through.
7553-sites-decorators-ref.diff (3.6 KB ) - added by Michael Newman 16 years ago.
Another way to fix this problem using refactored code, without a try and except. Still not the cleanest.
7553-sites-decorators-ref.2.diff (2.5 KB ) - added by Michael Newman 16 years ago.
Updated based on brosner's comments

Download all attachments as: .zip

Change History (14)

by Michael Newman, 16 years ago

Attachment: 7553.diff added

Simple patch that uses the request path to get the response from model_page instead

comment:1 by Marc Garcia, 16 years ago

milestone: 1.0 alpha1.0

According to ticket organization defined in http://code.djangoproject.com/wiki/VersionOneRoadmap#how-you-can-help 1.0 alpha tickets should be just features in the Must have (http://code.djangoproject.com/wiki/VersionOneRoadmap#must-have-features) list.

As bug, it should be fixed before 1.0 milestone.

comment:2 by Brian Rosner, 16 years ago

Keywords: nfa-blocker added
milestone: 1.01.0 alpha
Owner: changed from Michael Newman to Brian Rosner
Status: newassigned
Triage Stage: UnreviewedAccepted

This is broken functionality since [7737]. It needs to block the merge. Tagging as so and milestone for 1.0 alpha. I will have it fixed shortly.

comment:3 by Alex Gaynor, 16 years ago

Component: Contrib appsAdmin interface

comment:4 by Simon Willison, 16 years ago

I fixed this in [7824] without realising there was a ticket for it. brosner: you should check that my fix is appropriate (the fix in the patch attached to this ticket may be a better option).

by Michael Newman, 16 years ago

A proof-of-concept patch using an exception to catch data that may fall through.

by Michael Newman, 16 years ago

Another way to fix this problem using refactored code, without a try and except. Still not the cleanest.

comment:5 by Michael Newman, 16 years ago

Here are two patches which fix this problem. One is raising an exception from the view, that can be caught when there is Post Data that might be lost and the second reuses the root function with the self.root_url variable that is available to just return the root function. I prefer the later (Catching a manufactured exception is a little ... well...). Simon, your revision fixes the problem, but Brian and I decided that this needs to be fixed in the admin and that the views shouldn't be changed to handle the admin. Also, login is a view function, except that one time when it returns None and it becomes just another function.

comment:6 by Michael Newman, 16 years ago

Cc: newmaniese@… added

comment:7 by Collin Anderson, 16 years ago

Cc: cmawebsite@… added

comment:8 by Brian Rosner, 16 years ago

What is the reason for moving delete_test_cookie and removing the is_active check?

by Michael Newman, 16 years ago

Updated based on brosner's comments

comment:9 by Brian Rosner, 16 years ago

Resolution: fixed
Status: assignedclosed

(In [7933]) newforms-admin: Fixed #7553 -- Reverted [7824] in favor of a better fix in #7553. The never_cache decorator is no longer special casing None. Thanks Michael Newman for the patch.

comment:10 by Jacob, 13 years ago

milestone: 1.0 alpha

Milestone 1.0 alpha deleted

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