trenz-gmbh / TRENZ.Lib.RazorMail

Templated transactional e-mail using Razor
MIT License
2 stars 0 forks source link

Revise how mail sending works #14

Closed ricardoboss closed 4 months ago

ricardoboss commented 5 months ago

I think the API for MailSender is a bit strange. Sure, it works, but let's look at a "normal" use case for sending an e-mail at the moment:

  1. Create model with information
  2. Pass model and view name to the IRazorEmailRenderer
  3. Pass the rendered mail, the sender and the recipient mail to a MailSender constructor
  4. Call SendAsync on it

I think step 3 can be simplified (or rather generalized), because the sender doesn't really change between mails.

My thinking is that a different API might look like this:

IRazorEmailRenderer renderer = ...;
SmtpAccount account = ...;

var sender = new MailKitMailSender(account);
sender.DefaultFrom = new MailAddress("no-reply@mycompany.com", "My Comany");
sender.DefaultReplyTo.Add(new MailAddress("support@mycompany.com", "My Company Support"));
sender.DefaultBcc.Add(new MailAddress("shadow-copy@mycompany.com"));

var model = new ExampleModel();
var body = await renderer.RenderAsync("MyMail", model);
var recipient = new MailAddress("you@example.com", "You");

var mail = new Mail
{
    Recipient = recipient,
    Body = body,
};

await sender.SendAsync(mail);

Things to note here are:

The details will need to be discussed, but I think this API is better as the sender can be easily reused and the Mail class would provide an extension point for future development.

ricardoboss commented 5 months ago

Another aspect: with .NET 8 named instances, you could register multiple MailSender services in your DI container and choose your instance directly before sending the mail.

chucker commented 5 months ago

I think the API for MailSender is a bit strange.

It is, but if I understand your proposal correctly, it only takes into account the body. RenderedMail can also contain a Subject and Attachments. That is, you can set those from within the cshtml (the "view").

While we can quibble over whether Subject is a good design (the thinking was that you may want to alter the subject depending on the model, much like, on an HTML page, you would alter the <title> depending on context as well), and could drop that capability, Attachments currently must be set from within the view. Therefore, this assumption isn't quite right:

var body = await renderer.RenderAsync("MyMail", model);
ricardoboss commented 5 months ago

Well okay, I just called it body in my example, but I actually like the benefits you described.

Maybe we should move more metadata for the mail in the RenderedMail class (like Recipient, From, Cc, Bcc and Reply-To). That way, the additional Mail class I included above isn't needed at all.

chucker commented 5 months ago

I think the basic idea is that MailTemplateBase<T> and the resulting RenderedMail (which should perhaps be RenderedMailMessage? Or does that add to the confusion?) should be things the user sees as part of the message "content", which is perhaps just the subject and body, and then a separate model contains the metadata that's unrelated to that.

chucker commented 5 months ago

RenderedMailRenderedMailMessageContent and this new one MailMessageHeaders? A bit verbose, perhaps.

ricardoboss commented 5 months ago

Yes, I think that's exactly what we need.

One model for the visual stuff and one model for the metadata.

Perhaps MailBody and MailHeaders/MailHeaderCollection? Whereas MailBody would include the rendered HTML, the subject and any attached files and the MailHeaders would be a class with properties you can set for the mail or MailHeaderCollection a dictionary of key-value-pairs to be interpreted by the implementation of MailSender?

chucker commented 5 months ago

I would avoid Body because it's not just the body, but I like the idea of a MailHeadersCollection that behaves a bit like for HTTP, with properties to set a bunch of well-known headers like Importance (which sets both Importance and X-Priority) and Priority (which sets, well, Priority), and the ability to set additional custom headers.

(Additionally, it's currently called HtmlBody because, although not implemented, we could in theory support a plain text body.)