Django

Code

Ticket #7553 (closed: fixed)

Opened 5 months ago

Last modified 5 months ago

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

Reported by: Mnewman Assigned to: brosner
Milestone: 1.0 alpha Component: django.contrib.admin
Version: newforms-admin Keywords: admin nfa-blocker
Cc: newmaniese@gmail.com, cmawebsite@gmail.com Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

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

7553.diff (0.7 kB) - added by Mnewman on 06/26/08 21:50:55.
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 Mnewman on 07/04/08 21:57:48.
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 Mnewman on 07/04/08 23:33:28.
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 Mnewman on 07/15/08 21:40:22.
Updated based on brosner's comments

Change History

06/26/08 21:50:55 changed by Mnewman

  • attachment 7553.diff added.

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

06/27/08 10:11:44 changed by garcia_marc

  • needs_better_patch changed.
  • needs_docs changed.
  • needs_tests changed.
  • milestone changed from 1.0 alpha to 1.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.

06/27/08 10:20:59 changed by brosner

  • keywords changed from admin to admin nfa-blocker.
  • owner changed from Mnewman to brosner.
  • stage changed from Unreviewed to Accepted.
  • status changed from new to assigned.
  • milestone changed from 1.0 to 1.0 alpha.

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.

06/29/08 23:52:08 changed by Alex

  • component changed from Contrib apps to Admin interface.

07/02/08 07:28:18 changed by Simon Willison

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

07/04/08 21:57:48 changed by Mnewman

  • attachment 7553-sites-decorators-excpt.diff added.

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

07/04/08 23:33:28 changed by Mnewman

  • attachment 7553-sites-decorators-ref.diff added.

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

07/04/08 23:39:41 changed by Mnewman

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.

07/04/08 23:40:27 changed by Mnewman

  • cc set to newmaniese@gmail.com.

07/05/08 08:03:16 changed by CollinAnderson

  • cc changed from newmaniese@gmail.com to newmaniese@gmail.com, cmawebsite@gmail.com.

07/15/08 21:11:14 changed by brosner

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

07/15/08 21:40:22 changed by Mnewman

  • attachment 7553-sites-decorators-ref.2.diff added.

Updated based on brosner's comments

07/15/08 22:42:43 changed by brosner

  • status changed from assigned to closed.
  • resolution set to fixed.

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


Add/Change #7553 (return None in sites.py doesn't work with decorators)




Change Properties
Action