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

Check the email against the email address, not the code #205

Merged
merged 3 commits into from
Nov 24, 2021
Merged

Check the email against the email address, not the code #205

merged 3 commits into from
Nov 24, 2021

Conversation

larubbio
Copy link
Contributor

@larubbio larubbio commented Jan 5, 2016

When checking if a signup code exists the existing code was comparing the email address against the code

@jtauber
Copy link
Member

jtauber commented Jan 5, 2016

We should write a regression test for this (although needn't block merging)

@larubbio
Copy link
Contributor Author

larubbio commented Jan 5, 2016

Just an FYI, I'm working on adding REST apis for this module with django rest framework. I'm not sure if you want that submitted as a PR. I do have tests for this there and that is how I found the issue.

@larubbio
Copy link
Contributor Author

larubbio commented Jan 6, 2016

I've added tests for the SignupCode exists method.

@ossanna16
Copy link
Contributor

@larubbio Thank you for adding the tests! @jtauber Can this be merged?

@jtauber
Copy link
Member

jtauber commented Jan 13, 2016

I'd like @brosner to approve and do the merge.

@ossanna16
Copy link
Contributor

👍

@brosner
Copy link
Member

brosner commented Jul 21, 2016

Can the tests be updated to reflect the Pinax coding style guide? Thanks!

@larubbio
Copy link
Contributor Author

If someone else has the time to update them feel free. I'm not actually using this package anymore and don't have time to research the pinax style guide and update the tests.

@paltman paltman merged commit 121d6af into pinax:master Nov 24, 2021
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.

5 participants