Add support for inline attachments in SmtpMailer #413
Conversation
Enhanced the `Attachment` class by adding a `ContentId` property to support inline content. Updated `SmtpMailer` to handle attachments with non-empty `ContentId` as linked resources, generating RFC-compliant `ContentId` using `MimeKit.Utils.MimeUtils.GenerateMessageId()`. Modified the HTML body to reference the correct `ContentId`. Attachments with empty or whitespace `ContentId` are added as regular attachments.
|
@jamesmh do you have time to review this so we can get this update released? |
|
Hey, thanks for this! I'm super busy FYI but I'll try to check things out asap :) |
There was a problem hiding this comment.
Thanks again for this! Have a few minor things that I think make sense. We'll also need tests + documentation that I can add after this PR is merged.
I think we should add a method on Attachment:
public Attachment WithContentId()
{
this.ContentId = MimeKit.Utils.MimeUtils.GenerateMessageId();
return this;
} Then, when adding an attachment the user can generate the id once, and we don't need to do the regex search + generate another id, etc.
So the usage would be something similar to this:
var attachment = new Attachment
{
Bytes = new byte[] { },
Name = "test_image.png"
}
.WithContentId();
UserModel user = new UserModel()
{
Email = "[email protected]",
Name = "Coravel Test Person",
ImgContentId = attachment.ContentId
};
string message = await this._mailer.RenderAsync(
new NewUserViewMail(user)
.Attach(attachment));Or using an HTML template, same idea. The content id is generated once - and we'll add this to the docs as the way to to generate the content id.
So then the SmtpMailer method becomes simpler:
private static void SetMailBody(MessageBody message, IEnumerable<Attachment> attachments, MimeMessage mail)
{
var bodyBuilder = new BodyBuilder { HtmlBody = message.Html, TextBody = message.Text };
if (attachments != null)
{
foreach (var attachment in attachments)
{
if (string.IsNullOrWhiteSpace(attachment.ContentId))
{
bodyBuilder.Attachments.Add(attachment.Name, attachment.Bytes);
}
else
{
bodyBuilder.LinkedResources.Add(attachment.Name, attachment.Bytes);
}
}
}
mail.Body = bodyBuilder.ToMessageBody();
}Finally, feel free to add a really basic test in UnitTests.Mail.SmtpMailerTests something like:
[Fact]
public async Task SmtpMailerRenderCidAttachments()
{
var renderer = RazorRendererFactory.MakeInstance(new ConfigurationBuilder().Build());
var mailer = new SmtpMailer(renderer, "dummy", 1, "dummy", "dummy");
var attachments = new[]{
new Attachment
{
Bytes = new byte[] { },
Name = "Attachment 1",
}.WithContentId(),
new Attachment
{
Bytes = new byte[] { },
Name = "Attachment 2",
}.WithContentId()
};
string message = await mailer.RenderAsync(
new GenericHtmlMailable()
.Subject("test")
.Attach(attachments[0])
.Attach(attachments[1])
.From("[email protected]")
.To("[email protected]")
.Html($@"
<html>
<img src=""cid:{attachments[0].ContentId}"" />
<img src=""cid:{attachments[1].ContentId}"" />
</html>")
);
Assert.Equal($@"
<html>
<img src=""cid:{attachments[0].ContentId}"" />
<img src=""cid:{attachments[1].ContentId}"" />
</html>", message);
}Other than these minor adjustments - love it! I can add documentation after these changes are made + merged 👍
This works ok when there a few embedded images, but in some of our razor templates we have 10 or 15 embedded images, maybe more. I would loath having to use an array everywhere we have images embedded. I like the idea of the fluid method that generates the ContentId, in the cases where it can be used like your example. There's also the idea of making a razor helper/extension that can be called in the template that returns the content id of the image based on the name they gave when adding the image. This way, we don't have to worry about if the array of images changed at any time, and developers later looking at the code know exactly what it is referring to in the html. @model My.Namespace.PageModel
@using Coravel.Mailer.Mail.Razor
<html>
<h3>Hello, @Model.FirstName</h3>
<img src="cid:@Html.AttachmentContentId("mobile")" /> Cell: @Model.Cell
<img src="cid:@Html.AttachmentContentId("office")" /> Office: @Model.Office
<img src="cid:@Html.AttachmentContentId("email")" /> Cell: @Model.Email
</html> |
|
Curious how you are embedding images? The cid that your attachments have need to be unique. Where / when are you setting that if not via the attachment? |
The content id is unique for each attachment. I don't follow your confusion you have with it. Can you elaborate more? |
|
You said
I'm not sure what an alternative approach to using an array would be? Wouldn't you have to still set a unique value for each cid anyways? You said "I like the idea of the fluid method that generates the ContentId, in the cases where it can be used like your example" -what are the cases not like my example where the original approach you had would seem better? Thanks! |
When using embedded images in an email template that has several images throughout the template. (Think icons scattered throughout the email. Email, phone, office, house icon, external link icon, etc...) Having to keep track of what array index goes with what attachment, that would be a really big pain. As well as another developer coming in months later to update the template, inserts an image at the beginning(or wherever they want) of the array, now causing all the embedded images to be inaccurate. The new developer not realizing what they did, now eats up time trying to figure out how they broke it. Whereas if there was a razor helper function, like in the example I gave; we would look up the attachment based on the attachment's name and return the content id for it Also, don't want to have to be forced to have 40 additional property fields in the model to hold every image's content id. |
|
Hey, just looking over this again. After considering your approach and reasons around it I think the approach is worth moving forward with 👍 |
|
I'll merge this in and fix the GH actions issue separately - thanks! |
Enhanced the
Attachmentclass by adding aContentIdproperty to support inline content. UpdatedSmtpMailerto handle attachments with non-emptyContentIdas linked resources, generating RFC-compliantContentIdusingMimeKit.Utils.MimeUtils.GenerateMessageId(). Modified the HTML body to reference the correctContentId. Attachments with empty or whitespaceContentIdare added as regular attachments.Thank you for your contribution!
Before submitting this PR, please consider:
Based on feature request #376