SF-3231 Correctly bubble invalid email errors to the user

[pmachapman]

This PR properly shows an invalid email address notice to the user when they enter an email address that MimeKit flags as invalid.

---

This change is 

[codecov]

Codecov Report

Attention: Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 81.98%. Comparing base (6e96876) to head (26941bc).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...pp/src/app/shared/share/share-control.component.ts	85.71%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3208      +/-   ##
==========================================
+ Coverage   81.92%   81.98%   +0.06%     
==========================================
  Files         604      604              
  Lines       34657    34673      +16     
  Branches     5620     5622       +2     
==========================================
+ Hits        28393    28428      +35     
+ Misses       5438     5417      -21     
- Partials      826      828       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[RaymondLuong3]

Nice work. I think this is the right approach since validating an email is really difficult. I do like that the front end still has email validation, and then if somehow the email passes that, we still handle if the email address was invalid.

Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/share/share-control.component.spec.ts line 120 at r1 (raw file):

  it('Invalid email address message shown if invitee has an invalid email address', fakeAsync(() => {
    const env = new TestEnvironment();
    env.setTextFieldValue(env.emailTextField, 'invalid-email-address@example.com');

Nit: Maybe something more obviously invalid, like invalid@email

Code quote:

'invalid-email-address@example.com'

---

src/SIL.XForge/Services/EmailService.cs line 42 at r1 (raw file):

    }

    public bool ValidateEmail(string? email)

Wouldn't it be better if email was not a nullable type? Since we already know that if the string is null, it is an invalid email?

Code quote:

    public bool ValidateEmail(string? email)

---

src/SIL.XForge.Scripture/Services/SFProjectService.cs line 713 at r1 (raw file):

        // Validate the email address
        if (!_emailService.ValidateEmail(email))
            throw new DataNotFoundException(InvalidEmailAddress);

Nit: I think I understand why a DataNotFound makes sense, but more accurately this would be a InvalidOperation.

Code quote:

            throw new DataNotFoundException(InvalidEmailAddress);

[pmachapman]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @RaymondLuong3)

---

src/SIL.XForge/Services/EmailService.cs line 42 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Wouldn't it be better if email was not a nullable type? Since we already know that if the string is null, it is an invalid email?

I made it nullable I can unit test a null value, maintain correct nullability reference type warnings, and have it trapped correctly, as MailboxAddress throws an ArgumentNullException, which i trap in the catch statement.

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/share/share-control.component.spec.ts line 120 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Nit: Maybe something more obviously invalid, like invalid@email

invalid@email fails in XFValidators.email - I wanted a value that would not fail at the pattern validation stage, and would still pass this validation phase even if we decide in future to have a much more comprehensive email validation regex.

---

src/SIL.XForge.Scripture/Services/SFProjectService.cs line 713 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Nit: I think I understand why a DataNotFound makes sense, but more accurately this would be a InvalidOperation.

Done. Great idea.

[RaymondLuong3]

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @pmachapman)

---

src/SIL.XForge/Services/EmailService.cs line 42 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

I made it nullable I can unit test a null value, maintain correct nullability reference type warnings, and have it trapped correctly, as MailboxAddress throws an ArgumentNullException, which i trap in the catch statement.

Ok, that makes sense.

---

src/SIL.XForge.Scripture/ClientApp/src/app/shared/share/share-control.component.spec.ts line 120 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

invalid@email fails in XFValidators.email - I wanted a value that would not fail at the pattern validation stage, and would still pass this validation phase even if we decide in future to have a much more comprehensive email validation regex.

Good thinking. That makes sense to me.