Missing code in django.contrib.auth.views.LogoutView documentation on Django < 2.1
jacinator opened this issue · 11 comments
Hello gentlemen,
Since finding this site I spend a lot of time here and find it very helpful. However, whenever I was looking at the Django 1.11 django.contrib.auth.views.LogoutView I noticed that there didn't seem to be anything on http://ccbv.co.uk/projects/Django/1.11/django.contrib.auth.views/LogoutView/ that would indicate the user is actually being logged out.
I went digging into the Django source code and found that there is a discrepancy between http://ccbv.co.uk/projects/Django/1.11/django.contrib.auth.views/LogoutView/#dispatch and https://github.com/django/django/blob/master/django/contrib/auth/views.py#LC118
I have no idea how you would want to categorize this, but I thought that at least I should alert you to this issue.
What a frustrating bug! Well found!
I wonder if it has anything to do with that @method_decorator(never_cache)...
Wow great spot!
I think that decorator is definitely involved in this! Interestingly, I checked the output of the new static site builder version of classy that's in development and that inspector successfully found the dispatch() defined by LogoutView in addition to the one inherited from View, but the code is that of method_decorator._wrapper not the wrapped function we want.
My guess would be that the current version ignores the function because the module it's defined in is not what we're interested in, whereas the new inspector doesn't, but neither is any closer than the other to getting at the wrapped function.
Oh and I like to think our community is not all gentlemen! Something like "folks", "everyone" or "y'all" is more inclusive 😃
re the static builder version: IIRC we use a dispatch flow to pick how to handle various members (methods, attributes, etc) so my guess is we're not handling decorators correctly because we see them as "just" methods. I'm somewhat rusty on the flow but this looks like the place where we want to be fixing that: ideally we'd filter out decorators from the methods, delve "into" them to get the actual methods and then chain those back onto methods.
This also brings up the need to show that methods are decoratored which was not something I ever tackled in classify :/
Turns out this could be surprisingly straightforward 😮
So I've basically done exactly what @ghickman described, albeit very quickly and dirtily for now to test the concept. This is on the static builder, as it's easier for me to iterate on right now.
It's just an extra bit of work when getting methods: look for im_func on the function, if it's present then replace func with the last function from func_closure; rinse and repeat on that function until we reach an empty func_closure. This handles multiple @method_decorators (such as on LoginView.dispatch) and the existing code inspection seems to work fine once pointed at the correct function like this.
I don't really understand my code so it could be super wrong in the general case, but for what I've got it working on right now it seems to be doing the trick.
(the line numbers being wrong is unrelated)
Now tweaked to work with @cached_property! (slightly different pattern for not being decorated with @method_decorator, and a more refined check for what constitutes a method on a class). Although I'd like some way to tell it's a property without expanding it first, I can't think right now what that would look like.
So, this is sorted in the static site version, but once I'm back from holiday in a couple of weeks I might see how easy it is to shove into the original inspector so that we can get it up on the site before the static version is finished.
Looks as though the fix works for the original inspector, too :)
But it'll require a whole bunch of re-inspecting and generating fixtures, which I'm just not in the mood for right now ;)