fix: use built-in Angular email validator for eperson form#4999
fix: use built-in Angular email validator for eperson form#4999pzlakowski wants to merge 1 commit intoDSpace:mainfrom
Conversation
|
Thanks @pzlakowski. I agree that we should not be maintaining our own validation logic for something common like emails when Angular provides one that is likely better tested. Having said that, I tested this on DSpace 8 and noticed that the email |
|
@alanorth this is desired behaviour https://angular.dev/api/forms/Validators#email given WHATWG spec. Angular recommends to use custom regex pattern if business needs are different. There should be no difference. I think if we wanted to support to require top-level domain we could use regex like this: ^(?=.{1,254}$)(?=.{1,64}@)[-!#$%&'*+/0-9=?A-Z^_`a-z{|}~]+(\.[-!#$%&'*+/0-9=?A-Z^_`a-z{|}~]+)*@[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])?)+$This is derived from Angular regex, simply swapped from |
|
Thanks @pzlakowski. I don't understand the upstream use case of allowing +1 by me by review and testing. |
|
Which email address validation is performed by the backend? Both validation rules (of frontend and backend) should be identical. Do we need to adapt the backend validation rule? |
References
Fixes Not able to create an account with an email containing uppercase letters from eperson form #4338
Description
I have replaced regex pattern to use built-in one from Angular when we create e-person via form.
I think we should refrain from using our own regex when Angular provides one similar. Unless there is a reason which I do not know about yet.
e.g. issue shows to use pattern from
src/app/register-email-form/register-email-form.component.tswhere Angular tries to adhere to WHATWG spec too https://github.com/angular/angular/blob/4755bbd94964f27921ec203fc1a9233a01c84792/packages/forms/src/validators.ts#L151
Angular adds 3 different constraints compared to WHATWG:
local-partto begin or end with a period (.).local-partlength to exceed 64 characters.Instructions for Reviewers
List of changes in this PR:
Steps test this PR:
TEST@example.comChecklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
mainbranch of code (unless it is a backport or is fixing an issue specific to an older branch).npm run lintnpm run check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.