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 10 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
@@ -1,4 +1,3 @@
using System;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
Expand Down Expand Up @@ -93,7 +92,7 @@ private static MailMessage CreateMessageFromViewModel(SmtpSettingsViewModel test

if (!string.IsNullOrWhiteSpace(testSettings.Body))
{
message.Body = testSettings.Body;
message.Body = new MailMessageBody { Text = 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,19 +76,18 @@ 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);
}


public override IEnumerable<Outcome> GetPossibleOutcomes(WorkflowExecutionContext workflowContext, ActivityContext activityContext)
{
return Outcomes(S["Done"], S["Failed"]);
Expand All @@ -103,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 @@ -115,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
{
Text = textBody?.Trim(),
Html = htmlBody?.Trim()
Copy link
Member

Choose a reason for hiding this comment

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

Have you checked that actually both are sent if both are provided?

}
};

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 @@ -39,8 +39,7 @@ internal static async Task<bool> SendEmailAsync(this Controller controller, stri
{
To = email,
Subject = subject,
Body = body,
IsHtmlBody = true
Body = new MailMessageBody { Html = body }
};

var result = await smtpService.SendAsync(message);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Text.Encodings.Web;
Expand Down Expand Up @@ -97,13 +96,12 @@ public async Task<IActionResult> RequestCode()

var code = await UserManager.GenerateEmailConfirmationTokenAsync(user);

var setings = (await SiteService.GetSiteSettingsAsync()).As<EmailAuthenticatorLoginSettings>();
var settings = (await SiteService.GetSiteSettingsAsync()).As<EmailAuthenticatorLoginSettings>();
var message = new MailMessage()
{
To = await UserManager.GetEmailAsync(user),
Subject = await GetSubjectAsync(setings, user, code),
Body = await GetBodyAsync(setings, user, code),
IsHtmlBody = true,
Subject = await GetSubjectAsync(settings, user, code),
Body = new MailMessageBody { Html = await GetBodyAsync(settings, user, code) }
};

var result = await _smtpService.SendAsync(message);
Expand Down Expand Up @@ -174,8 +172,7 @@ public async Task<IActionResult> SendCode()
{
To = await UserManager.GetEmailAsync(user),
Subject = await GetSubjectAsync(settings, user, code),
Body = await GetBodyAsync(settings, user, code),
IsHtmlBody = true,
Body = new MailMessageBody { Html = await GetBodyAsync(settings, user, code) }
};

var result = await _smtpService.SendAsync(message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,11 @@ public override async Task<ActivityExecutionResult> ExecuteAsync(WorkflowExecuti

var body = await _expressionEvaluator.EvaluateAsync(ConfirmationEmailTemplate, workflowContext, _htmlEncoder);

var message = new MailMessage()
var message = new MailMessage
{
To = email,
Subject = subject,
Body = body,
IsHtmlBody = true
Body = new MailMessageBody { Html = body }
};
var smtpService = _httpContextAccessor.HttpContext.RequestServices.GetService<ISmtpService>();

Expand Down
45 changes: 2 additions & 43 deletions src/OrchardCore/OrchardCore.Email.Abstractions/MailMessage.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System;
using System.Collections.Generic;

namespace OrchardCore.Email
Expand Down Expand Up @@ -49,51 +48,11 @@ 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 the message content as plain text.
/// </summary>
[Obsolete("This property is deprecated, please use Body instead.")]
public string BodyText
{
get => Body;
set
{
Body = value;
IsHtmlBody = false;
}
}

/// <summary>
/// Gets or sets whether the message body is an HTML.
/// </summary>
[Obsolete("This property is deprecated, please use IsHtmlBody instead.")]
public bool IsBodyHtml
{
get => IsHtmlBody;
set => IsHtmlBody = value;
}

/// <summary>
/// Gets or sets whether the message body is plain text.
/// </summary>
[Obsolete("This property is deprecated, please use IsHtmlBody instead.")]
public bool IsBodyText
{
get => !IsHtmlBody;
set => IsHtmlBody = !value;
}

/// <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; }
Copy link
Member

Choose a reason for hiding this comment

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

Since this is a published interface, keep the old properties with Obsolete, and make them use the new Body behind the scenes.

public MailMessageBody Body { get; set; }

/// <summary>
/// The collection of message attachments.
/// </summary>
public List<MailMessageAttachment> Attachments { get; } = new List<MailMessageAttachment>();
public List<MailMessageAttachment> Attachments { get; } = [];
}
}
17 changes: 17 additions & 0 deletions src/OrchardCore/OrchardCore.Email.Abstractions/MailMessageBody.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
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 Text { 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.

Suggested change
public string Text { get; set; }
public string PlainText { get; set; }

Copy link
Member Author

Choose a reason for hiding this comment

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

I used Text something similar to BodyBuilder.Text

Copy link
Member

Choose a reason for hiding this comment

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

I just think that in contrast to "HTML" it's clearer to have "plain text". It's also a term commonly used for this when talking about e-mail.

Copy link
Member Author

@hishamco hishamco Jun 17, 2024

Choose a reason for hiding this comment

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

No problem we can rename it, but I usually "PlainText" in security :)


/// <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.

}
13 changes: 4 additions & 9 deletions src/OrchardCore/OrchardCore.Email.Core/Services/SmtpService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,16 +214,11 @@ private MimeMessage FromMailMessage(MailMessage message, IList<LocalizedString>

mimeMessage.Subject = message.Subject;

var body = new BodyBuilder();

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

foreach (var attachment in message.Attachments)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System;
using System.Threading.Tasks;
using Microsoft.Extensions.Localization;
using OrchardCore.Email;
Expand Down Expand Up @@ -36,17 +35,15 @@ public async Task<bool> TrySendAsync(object notify, INotificationMessage message
{
To = user.Email,
Subject = message.Summary,
Body = new MailMessageBody()
};

if (message.IsHtmlPreferred && !string.IsNullOrWhiteSpace(message.HtmlBody))
{
mailMessage.Body = message.HtmlBody;
mailMessage.IsHtmlBody = true;
mailMessage.Body.Html = message.HtmlBody;
}
else
{
mailMessage.Body = message.TextBody;
mailMessage.IsHtmlBody = false;
mailMessage.Body.Text = message.TextBody;
}

var result = await _smtpService.SendAsync(mailMessage);
Expand Down
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
4 changes: 4 additions & 0 deletions src/docs/releases/1.9.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ Previously, the `yessql.db` file was used as a default SQLite database for newly
!!! warning
For backward compatibility, you should use `yessql.db` as the database name in the `OrchardCore_Data_Sqlite` configuration. For more info read the [Data (`OrchardCore.Data`) documentation](../reference/core/Data/README.md).

### Email Module

In the past, we use `Body` and `BodyText` to distinguish between the `MailMessage` body format, in this release `MailMessageBody` has been used instead.
hishamco marked this conversation as resolved.
Show resolved Hide resolved

### SMS Module

In the past, we utilized the injection of `ISmsProvider`for sending SMS messages. However, in this release, it is now necessary to inject `ISmsService` instead.
Expand Down
Loading