Bug6451 email without top domain

[Dan-Karlsson]

Summary

Validate email addresses to ensure they contain a top domain and that the top domain is at least 2 characters long

Work Item(s)

Fixes #6451

[duichwer]

Could you add some tests with error assertions?

[Dan-Karlsson]

Could you add some tests with error assertions?

I can't find any tests of those functions, so I'm not sure where to put them. And I'm also not sure how to write them to not risk failure with .Net-calls in different environments and similar.

[duichwer]

Could you add some tests with error assertions?

I can't find any tests of those functions, so I'm not sure where to put them. And I'm also not sure how to write them to not risk failure with .Net-calls in different environments and similar.

@Dan-Karlsson I think you're correct.
I couldn't find any tests for the mail validation in the system application tests either.
That doesn't mean that we shouldn't have there some tests.

There are some tests in the Base Application for the procedure CheckValidEmailAddress of codeunit "Mail Management"
https://github.com/microsoft/BusinessCentralApps/blob/main/App/Layers/W1/Tests/Integration-Internal/MailManagementTest.Codeunit.al
The procedure CheckValidEmailAddress internall calls the procedure of the email account codeunit

    [TryFunction]
    procedure CheckValidEmailAddress(EmailAddress: Text)
    var
        EmailAccount: Codeunit "Email Account";
        IsHandled: Boolean;
    begin
        IsHandled := false;
        OnBeforeCheckValidEmailAddr(EmailAddress, IsHandled);
        if IsHandled then
            exit;

        EmailAddress := DelChr(EmailAddress, '<>');

        // Check that only one address is validated.
        if EmailAddress.Split('@').Count() <> 2 then
            Error(InvalidEmailAddressErr, EmailAddress);

        if not EmailAccount.ValidateEmailAddress(EmailAddress) then
            Error(InvalidEmailAddressErr, EmailAddress);
    end;

Therefore I would suggest to some tests to https://github.com/microsoft/BCApps/blob/main/src/System%20Application/Test/Email/src/EmailAccountsTest.Codeunit.al.

[Dan-Karlsson]

@Dan-Karlsson I think you're correct. I couldn't find any tests for the mail validation in the system application tests either. That doesn't mean that we shouldn't have there some tests.

There are some tests in the Base Application for the procedure CheckValidEmailAddress of codeunit "Mail Management" https://github.com/microsoft/BusinessCentralApps/blob/main/App/Layers/W1/Tests/Integration-Internal/MailManagementTest.Codeunit.al The procedure CheckValidEmailAddress internall calls the procedure of the email account codeunit

Therefore I would suggest to some tests to https://github.com/microsoft/BCApps/blob/main/src/System%20Application/Test/Email/src/EmailAccountsTest.Codeunit.al.

Thank you! I didn't think to check in Base Application.

But I think the tests should be in Base Application, with added tests in the procedure TestInvalidEmailAddress. It's bad design to test some cases there and some other cases in a completely different part of the code.

    procedure TestInvalidEmailAddress()
    var
        MailManagement: Codeunit "Mail Management";
    begin
        asserterror MailManagement.CheckValidEmailAddress('@a');
        asserterror MailManagement.CheckValidEmailAddress('b@');
        asserterror MailManagement.CheckValidEmailAddress('ab)@c.d');
        asserterror MailManagement.CheckValidEmailAddress('a@@b');
    end;

But I don't know how to add it there (without creating a new bug report in Base and a separate pull request for the tests, that won't work until this is approved).