Fixed email pattern to allow emails TLDs with more than 4 characters #2958
Conversation
tdonohue
added
bug
1 APPROVAL
oscar-escire
force-pushed
the
dspace-7_x
branch
from
94f6bdd to
679702b
Compare
|
Hi @oscar-escire, |
|
Somehow this PR got "out of sync" in GitHub and wasn't showing the commit that was added. So, I've briefly changed its base branch to main & then back to |
|
We have tested and worked for us. Before this PR it indeed does not allow for an LTD longer than 4 chars. Using this PR:
|
There was a problem hiding this comment.
@oscar-escire : I gave this a look today. The overall concept is correct, but I think we need to move this email pattern to a single location & reuse it everywhere it is needed.
For instance, I notice in this PR that you *modified the email pattern in the registration form. But I believe that email pattern already works (at least the registration form is working for longer TLDs on both demo & sandbox)
| @@ -99,7 +99,7 @@ export class RegisterEmailFormComponent implements OnDestroy, OnInit { | |||
| Validators.email, | |||
| // Regex pattern borrowed from HTML5 specs for a valid email address: | |||
| // https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address | |||
| Validators.pattern('^[a-zA-Z0-9.!#$%&\'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$') | |||
| Validators.pattern('^[a-zA-Z0-9.!#$%&\'*+\/=?^_`{|}~-]+@(?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])\\.(?:[a-zA-Z0-9](?:\\.[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$') | |||
There was a problem hiding this comment.
Why has this been modified? The pattern here is the same one noted in the HTML5 docs here: https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address
When I test this at https://demo.dspace.org/register, I've found that I already can add an email with a longer TLD. So, I think this pattern was already correct, and just needed to be copied elsewhere (or moved into a constant to be reused elsewhere)
Change patterns for form with email inputs
|
Hi @tdonohue, I updated the branch, based on the comments, I changed the current form pattern to the old one and moved it to a file with the patterns prefix for the forms. I hope these changes are what you expected |
There was a problem hiding this comment.
@VictorDuranEscire and @oscar-escire : The new changes look better to me. However, this is now failing specs because the updated pattern is saying that test@test is a valid email address.
Here's the test that fails: https://github.com/DSpace/dspace-angular/blob/main/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts#L358-L361
Additional notes on the pattern inline below
| @@ -0,0 +1,2 @@ | |||
| //Patterns for form validations | |||
| export const VALID_EMAIL_PATTERN = "^[a-zA-Z0-9.!#$%&\\' * +\\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$"; | |||
There was a problem hiding this comment.
This doesn't appear to be the same pattern we were using previously in the register-email-form.component.ts. Is there a reason you've changed it?
I think you need to update this pattern to be:
"^[a-zA-Z0-9.!#$%&\'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$"
That value is copied directly from the register-email-form.component.ts and is the same as the HTML5 spec recommended JavaScript pattern at https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address
|
H! @tdonohue, I submitted a fix for the email pattern, the previously pattern fails on validate ("^[a-zA-Z0-9.!#$%&'+/=?^_`{|}~-]+@a-zA-Z0-9?(?:.a-zA-Z0-9?)$") emails as 'test@test' i verified this on demo page, i don't know why the pattern pass the unit tests. |
|
Hi @oscar-escire, |
Hi @tdonohue, I'm @jtimal partner. We have been working on this issue and we will send you the PR.
References
Description
I changed the email validation pattern on 4 components. This validation pattern is a version of HTML5 pattern that request user to use a email "[email protected]" style and to add at least one LTD.
Instructions for Reviewers
List of changes in this PR:
Checklist
yarn lintyarn check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.