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
set email validator to lowercase #7645
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7645 +/- ##
==========================================
+ Coverage 27.16% 27.20% +0.03%
==========================================
Files 1163 1164 +1
Lines 15518 15519 +1
Branches 2410 2412 +2
==========================================
+ Hits 4216 4222 +6
+ Misses 9534 9528 -6
- Partials 1768 1769 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@bglidwell thanks a lot for your contribution! Before reviewing your PR can you make sure the DCO checks? |
Yup .lowercase() converts the string to lowercase which should be done in all instances of email across the application. Fixes bug where users created inside Strapi admin panel end up with mixed case emails in database. Signed-off-by: bglidwell <sintex+github@gmail.com>
Removed .min(5) from backend validation due to redundancy with .email() check Signed-off-by: bglidwell <sintex+github@gmail.com>
e4044e4
to
6be88a7
Compare
@soupette done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM to me for the front-end part.
Signed-off-by: Bobby Glidwell <sintex+github@gmail.com>
6ec3b5f
to
4565054
Compare
Worth noting, this optimization can be used in a lot of different areas. I scoped this particular PR to just the users & permissions plugin for simplicity. I think this route makes schemas easier to test. |
This reverts commit 4565054. Signed-off-by: Bobby Glidwell <sintex+github@gmail.com>
e2a95af
to
f8e2e78
Compare
Interesting that the backend doesn't also use yup for validation. Seems a lot of that could be shared between FE/BE, reducing complexity and improving consistency. It appears the admin panel uses a different controller, so that needed to be updated with toLowerCase() as well. I feel the plugin itself could use some more work. |
Signed-off-by: Bobby Glidwell <sintex+github@gmail.com>
Signed-off-by: Bobby Glidwell <sintex+github@gmail.com>
Signed-off-by: Bobby Glidwell <sintex+github@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description of what you did:
Yup .lowercase() converts the string to lowercase which should be done in all instances of email across the application.
Fixes bug where users created inside Strapi admin panel end up with mixed case emails in database.