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

Fixed redirect_to after successful login #204

Merged
merged 2 commits into from Apr 11, 2017
Merged

Fixed redirect_to after successful login #204

merged 2 commits into from Apr 11, 2017

Conversation

FabioFleitas
Copy link
Contributor

The redirect_to was not working correctly on a successful POST request login. I updated the logic to be consistent with how Django does it. See here as example to how Django handles getting redirect_to from params.

@coveralls
Copy link

coveralls commented Apr 10, 2017

Coverage Status

Coverage remained the same at 95.879% when pulling 5ac5581 on Tesorio:fix-redirect into 5b671df on Bouke:master.

Copy link
Collaborator

@Bouke Bouke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in itself look good (as they are taken from upstream), however as these changes are adding functionality we'd also need some tests to make sure it keeps working as intended. See also the existing tests.

@FabioFleitas
Copy link
Contributor Author

@Bouke I went ahead and added a test_valid_login_with_custom_post_redirect test case. Let me know if there's any other changes you'd like.

@coveralls
Copy link

coveralls commented Apr 11, 2017

Coverage Status

Coverage remained the same at 95.879% when pulling 0c65922 on Tesorio:fix-redirect into 5b671df on Bouke:master.

@Bouke Bouke merged commit 4e02f16 into jazzband:master Apr 11, 2017
@Bouke Bouke removed the in progress label Apr 11, 2017
@Bouke
Copy link
Collaborator

Bouke commented Apr 11, 2017

Nice work, thank you!

@FabioFleitas
Copy link
Contributor Author

No problem! When will you deploy this to PyPI?

@FabioFleitas FabioFleitas deleted the fix-redirect branch April 11, 2017 20:23
@Bouke
Copy link
Collaborator

Bouke commented Apr 11, 2017

I have not date for it; as a deploy takes quite a bit of my free time.

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

3 participants