Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Class-based Login, Logout and Callback views, plus successful_login overridable method #179

Merged
merged 7 commits into from Oct 29, 2018

Conversation

duoi
Copy link
Contributor

@duoi duoi commented Oct 27, 2018

As per #175.

These changes seem to make it fully Django 2.1 compatible, and running the tests locally also show no issues with Python 3.7. May be worth updating tox or the docs to reflect this if the tests are passing on your end.

The only major change that might affect users is that cas_ng_login now has to be the name of the login URL, in line with what is already in the documentation. Previously we were just calling reverse() on the function itself. But since we have class-based views we can't do that anymore (as far as I'm aware).

I'm also passing the entire request object to successful_login now instead of just user. I imagine this will give the most possible flexibility if people are passing in additional flags from the CAS server or have some middleware or something.

@mingchen
Copy link
Collaborator

Could you document this change in README.md and its impact?

@duoi
Copy link
Contributor Author

duoi commented Oct 27, 2018

@mingchen done

If you want to add something to release notes:

The `name` value given in the documentation for the `login` view ("`cas_ng_login`") is now required. Historically we were using function-based views and were able to pass that to the `reverse()` function, but we can no longer do that. This change only affects users of the `middleware` at the moment, so if you are not using it then it might not affect you. This is a breaking change if you have given the `login` view a different name than what is specified in the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants