mjtamlyn/django — pull request #4 by benoitbryon (2012-11-07 10:17:26+00:00)
Title: Minor improvements proposal

Hi,

I started to work on https://github.com/django/django/pull/502, but then I discovered the auth-cbv branch... So, let's contribute to it!

Here is a proposal for some minor improvements in the auth-cbv branch:

* moved some implementation logic into methods (such as LoginView.set_test_cookie) so that, if one wants to inherit from LoginView (that's my use case), it's easier to perform fine-grained overrides.

* converted is_valid_redirect() to a validator.

* used success_url in LoginView too.

Then, here are some notes about the branch status...

* We currently pass (i.e. fail silently) when LoginView or LogoutView have empty success_url. Is that ok? I mean, shouldn't we raise ImproperlyConfigured instead?

* We don't validate settings.LOGIN_REDIRECT_URL or views' success_url. Is that ok? I know it's safer than input from the request, but shouldn't we use at least some URLValidator?

* [contrib/auth/views.py changed] (https://github.com/django/django/commits/master/django/contrib/auth/views.py) since last ["rebase" (actually a merge)] (https://github.com/mjtamlyn/django/commit/bcb46fe31acf4c24b4eb0d7cd21c422e30418515). Could be useful to update.