smsohan / MvcMailer

A Mailer for ASP.Net MVC that forms the Email Body using MVC Views (Razor etc.) following Ruby on Rails ActionMailer style
MIT License
582 stars 178 forks source link

MailMessage.Body vs. MailMessage.AlternateViews #5

Closed frankvaneykelen closed 13 years ago

frankvaneykelen commented 13 years ago

Thanks for this great project!

It now works great, but I had some problems getting it to work because the MailMessage returned by PopulateBody actually doesn't set the value MailMessage.Body, but instead adds one or two (in the case of multipart) MailMessage.AlternateViews.

That apparently isn't a problem for sending mail using the standard SmtpClient, but in our project we are using Postmark (http://developer.postmarkapp.com/) and the Postmark .NET library by Daniel Crenna to send e-mail.

For this I have to convert a MailMessage to a PostmarkDotNet.PostmarkMessage. That's where I had a problem: the MailMessage.Body was always empty!

It wasn't until I went into your source code that I discovered that the value of MailMessage.Body is actually never set. I wasn't expecting that. I think it make more sense to always set the Body, and provide an AlternateView when needed.

I case anybody else has this problem - here's the code to retrieve strings from the AlternateViews:

var stream = mailMessage.AlternateViews[0].ContentStream; using (StreamReader reader = new StreamReader(stream)) { message.TextBody = reader.ReadToEnd(); } stream = mailMessage.AlternateViews[1].ContentStream; using (StreamReader reader = new StreamReader(stream)) { message.HtmlBody = reader.ReadToEnd(); }

smsohan commented 13 years ago

Thanks for your feedback. It was a deliberate decision to use alternate views, as it simplifies multi-part emailing with embedded images. However, I didn't envision the issue that you posted - it looks like the same can happen to others as well. So, I will fix this in the coming release of MvcMailer, within a day or two from now. Till then, you can use the EmailBody() method to get the string version of your views and assign it directly into the body. So, it will be like the following:

mailMessage.Body = EmailBody(viewName, masterName)
frankvaneykelen commented 13 years ago

Thanks for replying so quickly!

smsohan commented 13 years ago

I looked into the Rails ActionMailer behavior on this issue. Here is a pseudo code of how it works:

if multipart populate mail_message alternate views, body will be empty else if text_only or html_only without any linked resource populate mail_message body, set the appropriate content type else populate html alternate view with linked resources end

I will porbably conform to this standard, since the Rails ActionMailer library has been there for a long time and I suppose this standard is respected by the SMTP servers as well as Email readers. Any thoughts on this?

frankvaneykelen commented 13 years ago

In my case I tried sending HTML-only mail, so that's the "if text_only or html_only without any linked resource" case. Right now the body is empty, and you suggest to change that to "populate mail_message body", which of course would fix the problem I had. So that's OK!

About creating a multipart mail. You suggest to keep it the way it is: "populate mail_message alternate views, body will be empty". That's fine by me, but it is against the recommendation for creating a multipart mail in the MSDN Library:

"Use the AlternateViews property to specify copies of an e-mail message in different formats. For example, if you send a message in HTML, you might also want to provide a plain text version in case some of the recipients use e-mail readers that cannot display HTML content.

To add an alternate view to a MailMessage object, create an Attachment for the view, and then add it to the collection returned by AlternateViews. Use the Body property to specify the text version and use the AlternateViews collection to specify views with other MIME types."

http://msdn.microsoft.com/en-us/library/system.net.mail.mailmessage.alternateviews.aspx

smsohan commented 13 years ago

Thanks for pointing this out. I might go with this recommendation. Stay tuned.

smsohan commented 13 years ago

I am doing the following, please let me know your feedback on this:

        //if Text exists, it always goes to the body
        if (textExists)
        {
            PopulateTextBody(mailMessage, viewName, masterName);
        }

        // if html exists
        if (HtmlViewExists(viewName, masterName))
        {
            if (textExists || linkedResourcesPresent)
            {
                PopulateHtmlPart(mailMessage, viewName, masterName, linkedResources);
            }
            else
            {
                PopulateHtmlBody(mailMessage, viewName, masterName);
            }
        }
smsohan commented 13 years ago

Closing this issue, as a new release V1.1 solves this. Thanks for your feedback.

frankvaneykelen commented 13 years ago

Great! Thank you very much. Cool to see the update on NuGet already!

smsohan commented 13 years ago

You're welcome.