TBD: access mixins: support for custom exceptions

[blueyed]

I wanted to use a custom exception with SuperuserRequiredMixin (Http404) and added support for it.

raise_exception can now be True, False, a custom exception or a callable (that should return a response).

I have then factored the no-permission handling into a separate function, and moved redirect_unauthenticated_users to the base mixin.

This does not include any documentation changes yet, because I wanted to gather feedback on it first.

E.g., I am not sure if handle_no_permission should fall back to raising an exception for any non-HTTP-response (not isinstance(ret, (HttpResponse, StreamingHttpResponse))).

[blueyed]

I would appreciate some feedback on this.

E.g. is redirect_unauthenticated_users still useful / correctly named? Because now it's used to fallback to / call no_permissions_fail.

[kennethlove]

This definitely needs docs. I'm not sure why you deleted the AnonymousUser test, though? That should still pass based on the changes to AccessMixin, shouldn't it?

We also definitely want to make sure the response is a valid HTTPResponse or StreamingHTTPResponse since Django requires a valid responses from views.

As for the naming of redirect_unauthenticated_users, that has been around for awhile, so if we did decide to rename/remove it, we'd need to go through a deprecation process. I'd say it should probably be renamed to redirect_invalid_users if anything.

[chrisjones-brack3t]

Yep, that removed test does pass. All tests pass for me after merging in master. I'd like to get a new release out tomorrow or at the latest early next week. If you can get this together with docs it'll make it into the 1.5 release. Otherwise I'll push it back to 1.6.

[blueyed]

@chrisjones-brack3t
Great. I'll probably not get to it until tomorrow, but will do so in the next days.

[blueyed]

I'm not sure why you deleted the AnonymousUser test, though?

If you mean test_anonymous_redirects, this has been moved (e294f01).

I'll leave redirect_unauthenticated_users as-is for now.

[blueyed]

I've rebased the branch and updated the documentation: only the .rst for now, I am not sure about the class documentation - should this get added to the base class, or to all mixins, like it's currently done, or just both?

I've also changed it to pass the request argument to the raise_exception callback. Good?

[blueyed]

@kennethlove
Can you please provide some feedback to my last comment?
I can (try to) rebase it again, of course.

[blueyed]

Thanks for merging!

However, it is missing docs probably and some questions are unanswered.
Should be followed up in #173 though anyway.