Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support both Text & HTML in the same email #14715

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -178,15 +178,11 @@ private EmailMessage FromMailMessage(MailMessage message, Dictionary<string, ILi
bccRecipients = [.. recipients.Bcc.Select(r => new EmailAddress(r))];
}

var content = new EmailContent(message.Subject);
if (message.IsHtmlBody)
var content = new EmailContent(message.Subject)
{
content.Html = message.Body;
}
else
{
content.PlainText = message.Body;
}
PlainText = message.Body?.PlainText,
Html = message.Body?.Html
};

var emailMessage = new EmailMessage(
message.From,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,16 +113,11 @@ private MimeMessage GetMimeMessage(MailMessage message)

mimeMessage.Subject = message.Subject;

var body = new BodyBuilder();

if (message.IsHtmlBody)
{
body.HtmlBody = message.Body;
}
else
var body = new BodyBuilder
{
body.TextBody = message.Body;
}
TextBody = message.Body?.PlainText,
HtmlBody = message.Body?.Html
};

foreach (var attachment in message.Attachments)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ private static MailMessage GetMessage(EmailTestViewModel testSettings)

if (!string.IsNullOrWhiteSpace(testSettings.Body))
{
message.Body = testSettings.Body;
message.Body = new MailMessageBody { PlainText = testSettings.Body };
}

return message;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
@using OrchardCore.Email.Workflows.ViewModels
@model EmailTaskViewModel
@functions
{
private enum MailMessageFormat
{
Text,
Html,
All
}
}

<div class="mb-3" asp-validation-class-for="AuthorExpression">
<label asp-for="AuthorExpression" class="form-label">@T["From"]</label>
Expand Down Expand Up @@ -51,17 +60,25 @@
</div>

<div class="mb-3">
<div class="form-check">
<input type="checkbox" class="form-check-input" asp-for="IsHtmlBody" />
<label class="form-check-label" asp-for="IsHtmlBody">@T["Does the Body contain HTML?"]</label>
<span class="hint dashed">@T["If checked, indicates the body of the email message will be sent as HTML."]</span>
</div>
<label>@T["Format"]</label>
<select class="form-select">
<option value="@MailMessageFormat.Text">@nameof(MailMessageFormat.Text)</option>
<option value="@MailMessageFormat.Html">@nameof(MailMessageFormat.Html)</option>
<option value="@MailMessageFormat.All">@nameof(MailMessageFormat.All)</option>
</select>
<span class="hint">@T["The format of the email message."]</span>
</div>

<div class="mb-3" id="body">
<label asp-for="Body" class="form-label">@T["Body"]</label>
<textarea asp-for="Body" rows="5" class="form-control"></textarea>
<span class="hint">@T["The body of the email message. With Liquid support."]</span>
<div class="mb-3" id="textBody">
<label asp-for="TextBody">@T["Text Body"]</label>
<textarea asp-for="TextBody" rows="5" class="form-control"></textarea>
<span class="hint">@T["The plain text body of the email message. With Liquid support."]</span>
</div>

<div class="mb-3 d-none" id="htmlBody">
<label asp-for="HtmlBody">@T["HTML Body"]</label>
<textarea asp-for="HtmlBody" rows="5" class="form-control"></textarea>
<span class="hint">@T["The HTML body of the email message. With Liquid support."]</span>
</div>

<style asp-name="codemirror"></style>
Expand All @@ -73,12 +90,36 @@
<script asp-src="~/OrchardCore.Liquid/codemirror/liquid.js" at="Foot"></script>

<script at="Foot">
$(function () {
var bodyEditor = CodeMirror.fromTextArea(document.getElementById('@Html.IdFor(x => x.Body)'), {
lineNumbers: true,
styleActiveLine: true,
matchBrackets: true,
mode: { name: "liquid" },
$(function () {
var textBodyEditor = CodeMirror.fromTextArea(document.getElementById('@Html.IdFor(x => x.TextBody)'), {
lineNumbers: true,
styleActiveLine: true,
matchBrackets: true,
mode: { name: "liquid" },
});
var htmlBodyEditor = CodeMirror.fromTextArea(document.getElementById('@Html.IdFor(x => x.HtmlBody)'), {
lineNumbers: true,
styleActiveLine: true,
matchBrackets: true,
mode: { name: "liquid" },
});

$('select').onchange(function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get this when opening a previously existing Email Task:

image

var selectedValue = $(this).val();
switch (selectedValue) {
case '@MailMessageFormat.Text':
$('#textBody').removeClass('d-none');
$('#htmlBody').addClass('d-none');
break;
case '@MailMessageFormat.Html':
$('#textBody').removeClass('d-none');
$('#htmlBody').addClass('d-none');
break;
case '@MailMessageFormat.All':
$('#textBody').removeClass('d-none');
$('#htmlBody').removeClass('d-none');
break;
}
});
});
});
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,15 @@ public WorkflowExpression<string> Subject
set => SetProperty(value);
}

public WorkflowExpression<string> Body
public WorkflowExpression<string> TextBody
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since GetProperty() below uses this as the name by default, this will break existing Email Tasks, since they'll be empty. Add a migration that reads out the bodies of all Email Tasks, and depending on IsHtmlBody, updates one of the fields with the old body value.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to generate both bodies, so isHtmlBody is not make sense here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the consumers of the old API, nothing will change. For the consumers of the new API, Obsolete will tell that they shouldn't use this.

{
get => GetProperty(() => new WorkflowExpression<string>());
set => SetProperty(value);
}

public bool IsHtmlBody
public WorkflowExpression<string> HtmlBody
{
get => GetProperty(() => true);
get => GetProperty(() => new WorkflowExpression<string>());
set => SetProperty(value);
}

Expand All @@ -102,7 +102,8 @@ public override async Task<ActivityExecutionResult> ExecuteAsync(WorkflowExecuti
var cc = await _expressionEvaluator.EvaluateAsync(Cc, workflowContext, null);
var bcc = await _expressionEvaluator.EvaluateAsync(Bcc, workflowContext, null);
var subject = await _expressionEvaluator.EvaluateAsync(Subject, workflowContext, null);
var body = await _expressionEvaluator.EvaluateAsync(Body, workflowContext, IsHtmlBody ? _htmlEncoder : null);
var textBody = await _expressionEvaluator.EvaluateAsync(TextBody, workflowContext, null);
var htmlBody = await _expressionEvaluator.EvaluateAsync(HtmlBody, workflowContext, _htmlEncoder);

var message = new MailMessage
{
Expand All @@ -114,8 +115,11 @@ public override async Task<ActivityExecutionResult> ExecuteAsync(WorkflowExecuti
// Email reply-to header https://tools.ietf.org/html/rfc4021#section-2.1.4
ReplyTo = replyTo?.Trim(),
Subject = subject?.Trim(),
Body = body?.Trim(),
IsHtmlBody = IsHtmlBody
Body = new MailMessageBody
{
PlainText = textBody?.Trim(),
Html = htmlBody?.Trim()
}
};

if (!string.IsNullOrWhiteSpace(sender))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ protected override void EditActivity(EmailTask activity, EmailTaskViewModel mode
model.RecipientsExpression = activity.Recipients.Expression;
model.ReplyToExpression = activity.ReplyTo.Expression;
model.SubjectExpression = activity.Subject.Expression;
model.Body = activity.Body.Expression;
model.IsHtmlBody = activity.IsHtmlBody;
model.TextBody = activity.TextBody.Expression;
model.HtmlBody = activity.HtmlBody.Expression;
model.BccExpression = activity.Bcc.Expression;
model.CcExpression = activity.Cc.Expression;
}
Expand All @@ -27,8 +27,8 @@ protected override void UpdateActivity(EmailTaskViewModel model, EmailTask activ
activity.Recipients = new WorkflowExpression<string>(model.RecipientsExpression);
activity.ReplyTo = new WorkflowExpression<string>(model.ReplyToExpression);
activity.Subject = new WorkflowExpression<string>(model.SubjectExpression);
activity.Body = new WorkflowExpression<string>(model.Body);
activity.IsHtmlBody = model.IsHtmlBody;
activity.TextBody = new WorkflowExpression<string>(model.TextBody);
activity.HtmlBody = new WorkflowExpression<string>(model.HtmlBody);
Comment on lines +30 to +31
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this logic, if you switch from Text to HTML (not All), for example, then the old Text value will still exist, and will be used, even though you won't see it and won't be able to edit it on the UI.

activity.Bcc = new WorkflowExpression<string>(model.BccExpression);
activity.Cc = new WorkflowExpression<string>(model.CcExpression);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ public class EmailTaskViewModel

public string SubjectExpression { get; set; }

public string Body { get; set; }
public string TextBody { get; set; }

public bool IsHtmlBody { get; set; }
public string HtmlBody { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,47 @@ public static class EmailServiceExtensions
/// <param name="emailService">The <see cref="IEmailService"/>.</param>
/// <param name="to">The email recipients.</param>
/// <param name="subject">The email subject.</param>
/// <param name="body">The email body.</param>
/// <param name="isHtmlBody">Whether the <paramref name="body"/> is in HTML format or not. Defaults to <c>true</c>.</param>
/// <returns></returns>
/// <param name="htmlBody">The email body in HTML format.</param>
/// <param name="textBody">The email body in Text format.</param>
/// <exception cref="System.ArgumentException"></exception>
public static Task<EmailResult> SendAsync(this IEmailService emailService, string to, string subject, string body, bool isHtmlBody = true)
public static Task<EmailResult> SendAsync(this IEmailService emailService, string to, string subject, string htmlBody, string textBody)
{
var message = new MailMessage
{
To = to,
Subject = subject,
Body = body,
IsHtmlBody = isHtmlBody
Body = new MailMessageBody
{
Html = htmlBody,
PlainText = textBody
}
};

return emailService.SendAsync(message);
}

/// <summary>
/// Sends the specified message to an SMTP server for delivery.
/// </summary>
/// <param name="emailService">The <see cref="IEmailService"/>.</param>
/// <param name="to">The email recipients.</param>
/// <param name="subject">The email subject.</param>
/// <param name="body">The email body.</param>
/// <param name="isHtmlBody">Whether the <paramref name="body"/> is in HTML format or not. Defaults to <c>true</c>.</param>
/// <exception cref="System.ArgumentException"></exception>
public static async Task<EmailResult> SendAsync(this IEmailService emailService, string to, string subject, string body, bool isHtmlBody = true)
{
string htmlBody = default;
string textBody = default;
if (isHtmlBody)
{
htmlBody = body;
}
else
{
textBody = body;
}

return await emailService.SendAsync(to, subject, htmlBody, textBody);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,7 @@ public class MailMessage
/// <summary>
/// Gets or sets the message content aka body.
/// </summary>
/// <remarks>This property is work in conjunction with <see cref="IsHtmlBody"/> to determine the body type..</remarks>
public string Body { get; set; }

/// <summary>
/// Gets or sets whether the message body is an HTML or not. Default is <c>false</c> which is plain text.
/// </summary>
public bool IsHtmlBody { get; set; }
public MailMessageBody Body { get; set; }

/// <summary>
/// The collection of message attachments.
Expand Down
23 changes: 23 additions & 0 deletions src/OrchardCore/OrchardCore.Email.Abstractions/MailMessageBody.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
namespace OrchardCore.Email;

/// <summary>
/// Represents a body for <see cref="MailMessage"/>.
/// </summary>
public class MailMessageBody
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add convenience methods for HasText() and HasHtml() that use string.IsNullOrEmpty().

{
/// <summary>
/// Gets or sets the body in plain text format.
/// </summary>
public string PlainText { get; set; }

/// <summary>
/// Gets or sets the body in HTML format.
/// </summary>
public string Html { get; set; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe IHtmlContent instead to be better typed? With a convenience conversion method to/from string.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For simplicity, I prefer string instead, like other email messaging APIs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah but string is really overused. IHtmlContent is for when you want to store a string that's actually HTML.


public static implicit operator MailMessageBody(string body) => new()
{
Html = body,
PlainText = body
};
}
4 changes: 4 additions & 0 deletions src/docs/releases/1.8.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ The interface `OrchardCore.Documents.IDocumentSerialiser` has been renamed to `O

This release removes support for `net6.0` and `net7.0`. Only `net8.0` is supported.

### Email

The `MailMessage.Body` now returns `MailMessageBody` instead of `string` to support both plain text & HTML formats.
Comment on lines +15 to +17
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't needed in the 1.8 release notes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was planned for 1.8 :)


### TheAdmin Theme

The `TheAdmin` theme was upgraded to Bootstrap 5.3.2. Here is a list of the breaking changes
Expand Down
8 changes: 8 additions & 0 deletions src/docs/releases/2.0.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ services.AddJsonDerivedTypeInfo<SqlQuery, Query>();
}
```

### Email Module

Previously, emails sent from Orchard Core could have either a plain text body, or an HTML body, but not both. Now, they can have both. This also brings some code-level API changes, see below.

When interacting with email-related services from code, `MailMessage`, the class representing an e-mail, exposed a `string` `Body` property. This could contain either plain text or HTML, which was indicated by `IsHtmlBody`.

While this API is still available, it's deprecated in favor of the new `Body` property that uses the new `MailMessageBody` type. This can contain a plain text and/or HTML body.

### Queries

Previously, any query type had to inherit from `Query` and required its own distinct type (e.g., `SqlQuery`, `LuceneQuery`, `ElasticQuery`). However, with [pull request #16402](https://github.com/OrchardCMS/OrchardCore/pull/16402), creating a custom type for each query type is no longer necessary. This update involved changes to the `IQueryManager` and `IQuerySource` interfaces, as well as the `Query` class. Additionally, a new project, `OrchardCore.Queries.Core`, was introduced.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public async Task ExecuteTask_WhenToAndCcAndBccAreNotSet_ShouldFail()
HtmlEncoder.Default)
{
Subject = new WorkflowExpression<string>("Test"),
Body = new WorkflowExpression<string>("Test message!!")
HtmlBody = new WorkflowExpression<string>("Test message!!")
};

var executionContext = new WorkflowExecutionContext(
Expand Down