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

Improve check for existing email addresses #3662

Conversation

k3KAW8Pnf7mkmdSMPHz27
Copy link
Contributor

Background
When a user attempts to register a new account, the provided e-mail address is checked against the existing e-mails in the database to detect if there already exists an account with that e-mail address.

The proposed changes

  1. Perform the e-mail address comparison in lower-case to make it case-insensitive
  2. If there are two or more users with the same e-mail address, return a 400 error (user already exists) instead of a 500 error (internal server error)

@Aadesh-Baral Aadesh-Baral merged commit 24bb1d0 into hotosm:develop Jun 15, 2023
@k3KAW8Pnf7mkmdSMPHz27
Copy link
Contributor Author

@Aadesh-Baral not sure what requirements you have, but if I'd redo this code today (i.e., three years later), I'd either use casefold or normalized Unicode (see https://docs.python.org/3/howto/unicode.html#comparing-strings for background).

On the other hand, since this has been fine for three years, it is unlikely it needs fixing.

Motivation for the original PR:
I was looking at mobile usability. Mobile phones often force an upper-case letter for the first character in an e-mail.
It is, fortunately, less of a problem these days when password managers are omnipresent.

@Aadesh-Baral
Copy link
Contributor

@k3KAW8Pnf7mkmdSMPHz27 Thank you for your comment! I understand your suggestion to use casefold or normalized Unicode for string comparison. While those approaches could potentially fine-tune the code, as you mentioned, it's not a high priority since the function in question isn't used frequently.

However, if you would like to create another pull request to implement the changes you suggested, I would be more than happy to review it. It's always good to consider improvements, especially if they can enhance the code's compatibility or robustness. Feel free to submit the PR.

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

Successfully merging this pull request may close these issues.

None yet

2 participants