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
fix: Registration occurs with a password that does not meet the requirements to Quince #1182
fix: Registration occurs with a password that does not meet the requirements to Quince #1182
Conversation
Thanks for the pull request, @DmytroAlipov! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## open-release/quince.master #1182 +/- ##
=============================================================
Coverage ? 87.14%
=============================================================
Files ? 124
Lines ? 2295
Branches ? 635
=============================================================
Hits ? 2000
Misses ? 286
Partials ? 9 ☔ View full report in Codecov by Sentry. |
db88f42
to
2b63a3d
Compare
@DmytroAlipov, nice catch, thanks! Can you submit a PR to master first, if it doesn't already exist? If it does, can you point to it here? |
Hi @arbrandes I noticed that experiments were being conducted in the master branch of this MFE. I wanted to ask the contributors whether changes should be made to the master branch. |
done |
2b63a3d
to
0a42eba
Compare
f30d62e
to
ad8d780
Compare
@arbrandes @syedsajjadkazmii Everything is done for both Master and Quince. |
988fead
to
868dc63
Compare
Great! Let's wait to merge this until the master PR lands, though. |
Hi @DmytroAlipov, master PR have been merged. Please look at this comment and make that change in this branch too. |
Hi @syedsajjadkazmii
|
src/register/data/utils.js
Outdated
import validateName from '../RegistrationFields/NameField/validator'; | ||
import validateUsername from '../RegistrationFields/UsernameField/validator'; | ||
|
||
export const emailRegex = new RegExp(VALID_EMAIL_REGEX, 'i'); |
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.
this can be removed
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.
Hi @syedsajjadkazmii
Why remove validateUsername
and validateName
? Or was this noted by chance?
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.
Hi @syedsajjadkazmii Why remove
validateUsername
andvalidateName
? Or was this noted by chance?
I only meant for emailRegex. Github picked those lines as well. :)
868dc63
to
eff7964
Compare
9f503eb
into
openedx:open-release/quince.master
@DmytroAlipov 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
This is a backport from the master branch.
Description
The behavior of the registration form is very different from the behavior of this form for the Palm release.
Even with incorrectly filled fields, the form can still be submitted. In certain instances, registration may succeed, such as when the password consists of merely two characters.
There’s an issue where the error message doesn’t disappear even after the error is corrected and the focus is shifted to another field. Here’s how to reproduce it:
My proposals are aimed at returning the behavior as happened on Palm.
PS: I’ve noticed that you’ve been conducting numerous experiments on the master branch. To avoid any conflicts, I’ve refrained from making these changes directly to the master. If there are any upcoming plans for the registration page, please consider these issues before the release of Redwood.
Screenshots/sandbox (optional):
after clicking on "Create an account for free" button (the request to the backend is still sent even if there is a space):
prevent a request to the backend if the password does not meet the conditions:
Post-merge Checklist