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

Tests: check existing username case-insensitive #167

Conversation

ptamarit
Copy link
Member

@ptamarit ptamarit commented Feb 27, 2024

Fixes https://github.com/zenodo/ops/issues/326

❤️ Thank you for your contribution!

Description

Usernames are enforced to be unique in the DB regardless of their case (see related DB column and constraint definition).

This change applies the same validation at the registration of a user to avoid hitting the DB constraint which is then not well handled by Flask (users end up seeing an nginx 502 error).

This is a test-only pull request related to inveniosoftware/invenio-accounts#476.
Here we only improve the test to make sure that invenio-accounts properly finds by usernames case-insensitively.

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

Third-party code

If you've added third-party code (copy/pasted or new dependencies), please reach out to an architect.

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

  1. You license your contribution under the same terms as the current repository’s license.
  2. You agree that you have the right to license your contribution under the current repository’s license.

@ptamarit ptamarit force-pushed the username-case-insensitive-uniqueness-check branch from 4473cc3 to a454d93 Compare February 27, 2024 14:34
@ptamarit ptamarit closed this Mar 18, 2024
@ptamarit ptamarit force-pushed the username-case-insensitive-uniqueness-check branch from a454d93 to 35845fe Compare March 18, 2024 12:06
@ptamarit ptamarit reopened this Mar 18, 2024
@ptamarit ptamarit changed the title username: use lowercase to compare Tests: check existing username case-insensitive Mar 18, 2024
@ptamarit
Copy link
Member Author

ptamarit commented Mar 18, 2024

This is now a test-only pull request (see updated description).
It will turn green as soon as a new version of invenio-accounts containing inveniosoftware/invenio-accounts#476 is released.

  • One question: should I bump the minimum version of invenio-accounts from >=2.0.0 to >=5.0.x, since test would fail if using a smaller version?
  • And if yes, does it mean that we should also release a version of invenio-userprofiles due to the dependency requirement change?

@ptamarit ptamarit force-pushed the username-case-insensitive-uniqueness-check branch from 0c8dd4d to 8baafd6 Compare March 26, 2024 07:47
@slint slint merged commit 1794934 into inveniosoftware:master Mar 27, 2024
8 checks passed
@ptamarit ptamarit deleted the username-case-insensitive-uniqueness-check branch March 28, 2024 07:35
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.

4 participants