Email.Azure: Deconstructing email addresses that include a display name (Lombiq Technologies: OCORE-204) #16889
Conversation
| @@ -224,4 +226,33 @@ private EmailMessage FromMailMessage(MailMessage message, Dictionary<string, ILi | |||
|
|
|||
| return emailMessage; | |||
| } | |||
|
|
|||
| private static EmailAddress ToAzureEmailAddress(string emailWithDisplayName) | |||
There was a problem hiding this comment.
The method name should be a verb, why didn't you add this code to ParseEmailAddressWithDisplayName?
There was a problem hiding this comment.
The method name should be a verb
Yeah, I initially intended it to be an extension method, I'll fix that.
why didn't you add this code to
ParseEmailAddressWithDisplayName?
ParseEmailAddressWithDisplayName is used separately too.
There was a problem hiding this comment.
One more thing can we create a virtual method so subclasses can implement how to get the display name
There was a problem hiding this comment.
The logic here is laser-focused on this scenario, so I don't see any reason to override it without overriding the whole email sending logic. But then you can handle email addresses however you want.
BenedekFarkas
changed the title
src/OrchardCore.Modules/OrchardCore.Email.Azure/Services/AzureEmailProviderBase.cs
Outdated
Show resolved
Hide resolved
|
|
||
| private static (string DisplayName, string EmailAddress) TryCreateMailAddressOrFail(string email) | ||
| { | ||
| if (MailAddress.TryCreate(email, out var mailAddress)) |
There was a problem hiding this comment.
You can use IEmailAddressValidator instead
There was a problem hiding this comment.
I need the deconstructed email address (i.e. the display name and the address itself separately), not just validation.
| @@ -187,13 +188,14 @@ private EmailMessage FromMailMessage(MailMessage message, Dictionary<string, ILi | |||
| } | |||
|
|
|||
| var emailMessage = new EmailMessage( | |||
| message.From, | |||
| // For compatibility with configuration for other providers that allow a sender with display name. | |||
| CreateMailAddressOrFail(message.From).EmailAddress, | |||
There was a problem hiding this comment.
This can throw an exception. If there is a way it can throw then we should catch it and return the error message since this code here supports error messages.
There was a problem hiding this comment.
Hmm, right. It doesn't need to be an exception here, just need to add an error message that will be surfaced.
There was a problem hiding this comment.
Updated error handling.
Fixes #16888