From 9656a21fdb0da85a16bff74120a521046867fe42 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 30 Jul 2019 15:05:08 -0400 Subject: [PATCH] FEATURE: customization of html emails (#7934) This feature adds the ability to customize the HTML part of all emails using a custom HTML template and optionally some CSS to style it. The CSS will be parsed and converted into inline styles because CSS is poorly supported by email clients. When writing the custom HTML and CSS, be aware of what email clients support. Keep customizations very simple. Customizations can be added and edited in Admin > Customize > Email Style. Since the summary email is already heavily styled, there is a setting to disable custom styles for summary emails called "apply custom styles to digest" found in Admin > Settings > Email. As part of this work, RTL locales are now rendered correctly for all emails. --- Gemfile | 1 + Gemfile.lock | 3 + .../admin/adapters/email-style.js.es6 | 7 + .../components/email-styles-editor.js.es6 | 45 +++++++ .../admin-customize-email-style-edit.js.es6 | 33 +++++ .../admin/models/email-style.js.es6 | 10 ++ .../admin-customize-email-style-edit.js.es6 | 39 ++++++ .../routes/admin-customize-email-style.js.es6 | 9 ++ .../admin/routes/admin-route-map.js.es6 | 7 + .../components/email-styles-editor.hbs | 20 +++ .../templates/customize-email-style-edit.hbs | 9 ++ .../admin/templates/customize-email-style.hbs | 7 + .../javascripts/admin/templates/customize.hbs | 1 + .../stylesheets/common/admin/customize.scss | 15 +++ .../admin/email_styles_controller.rb | 16 +++ app/helpers/email_helper.rb | 10 ++ app/mailers/invite_mailer.rb | 5 +- app/mailers/user_notifications.rb | 27 +--- app/models/email_style.rb | 37 ++++++ app/serializers/email_style_serializer.rb | 5 + app/services/email_style_updater.rb | 38 ++++++ app/services/user_notification_renderer.rb | 7 + app/views/email/default_template.html | 24 ++++ app/views/email/invite.html.erb | 11 -- app/views/email/template.html.erb | 13 -- app/views/email/unsubscribe.html.erb | 14 +- app/views/layouts/email_template.html.erb | 6 + app/views/user_notifications/digest.html.erb | 25 +--- config/locales/client.en.yml | 10 ++ config/locales/server.en.yml | 4 + config/routes.rb | 3 + config/site_settings.yml | 7 + lib/email/message_builder.rb | 15 ++- lib/email/renderer.rb | 18 ++- lib/email/styles.rb | 84 ++++++++---- spec/integration/email_style_spec.rb | 122 ++++++++++++++++++ spec/mailers/user_notifications_spec.rb | 30 ++--- .../admin/email_styles_controller_spec.rb | 71 ++++++++++ spec/services/email_style_updater_spec.rb | 46 +++++++ 39 files changed, 720 insertions(+), 134 deletions(-) create mode 100644 app/assets/javascripts/admin/adapters/email-style.js.es6 create mode 100644 app/assets/javascripts/admin/components/email-styles-editor.js.es6 create mode 100644 app/assets/javascripts/admin/controllers/admin-customize-email-style-edit.js.es6 create mode 100644 app/assets/javascripts/admin/models/email-style.js.es6 create mode 100644 app/assets/javascripts/admin/routes/admin-customize-email-style-edit.js.es6 create mode 100644 app/assets/javascripts/admin/routes/admin-customize-email-style.js.es6 create mode 100644 app/assets/javascripts/admin/templates/components/email-styles-editor.hbs create mode 100644 app/assets/javascripts/admin/templates/customize-email-style-edit.hbs create mode 100644 app/assets/javascripts/admin/templates/customize-email-style.hbs create mode 100644 app/controllers/admin/email_styles_controller.rb create mode 100644 app/models/email_style.rb create mode 100644 app/serializers/email_style_serializer.rb create mode 100644 app/services/email_style_updater.rb create mode 100644 app/services/user_notification_renderer.rb create mode 100644 app/views/email/default_template.html delete mode 100644 app/views/email/invite.html.erb delete mode 100644 app/views/email/template.html.erb create mode 100644 app/views/layouts/email_template.html.erb create mode 100644 spec/integration/email_style_spec.rb create mode 100644 spec/requests/admin/email_styles_controller_spec.rb create mode 100644 spec/services/email_style_updater_spec.rb diff --git a/Gemfile b/Gemfile index 2db7fffbcec..dde65dcd2aa 100644 --- a/Gemfile +++ b/Gemfile @@ -78,6 +78,7 @@ gem 'discourse_image_optim', require: 'image_optim' gem 'multi_json' gem 'mustache' gem 'nokogiri' +gem 'css_parser', require: false gem 'omniauth' gem 'omniauth-openid' diff --git a/Gemfile.lock b/Gemfile.lock index 3e50a65a278..c0438f2fdfa 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -88,6 +88,8 @@ GEM crack (0.4.3) safe_yaml (~> 1.0.0) crass (1.0.4) + css_parser (1.7.0) + addressable debug_inspector (0.0.3) diff-lcs (1.3) diffy (3.3.0) @@ -438,6 +440,7 @@ DEPENDENCIES certified colored2 cppjieba_rb + css_parser diffy discourse-ember-source (~> 3.10.0) discourse_image_optim diff --git a/app/assets/javascripts/admin/adapters/email-style.js.es6 b/app/assets/javascripts/admin/adapters/email-style.js.es6 new file mode 100644 index 00000000000..c9f3865d4c3 --- /dev/null +++ b/app/assets/javascripts/admin/adapters/email-style.js.es6 @@ -0,0 +1,7 @@ +import RestAdapter from "discourse/adapters/rest"; + +export default RestAdapter.extend({ + pathFor() { + return "/admin/customize/email_style"; + } +}); diff --git a/app/assets/javascripts/admin/components/email-styles-editor.js.es6 b/app/assets/javascripts/admin/components/email-styles-editor.js.es6 new file mode 100644 index 00000000000..d0e569421f6 --- /dev/null +++ b/app/assets/javascripts/admin/components/email-styles-editor.js.es6 @@ -0,0 +1,45 @@ +import computed from "ember-addons/ember-computed-decorators"; + +export default Ember.Component.extend({ + editorId: Ember.computed.reads("fieldName"), + + @computed("fieldName", "styles.html", "styles.css") + resetDisabled(fieldName) { + return ( + this.get(`styles.${fieldName}`) === + this.get(`styles.default_${fieldName}`) + ); + }, + + @computed("styles", "fieldName") + editorContents: { + get(styles, fieldName) { + return styles[fieldName]; + }, + set(value, styles, fieldName) { + styles.setField(fieldName, value); + return value; + } + }, + + actions: { + reset() { + bootbox.confirm( + I18n.t("admin.customize.email_style.reset_confirm", { + fieldName: I18n.t(`admin.customize.email_style.${this.fieldName}`) + }), + I18n.t("no_value"), + I18n.t("yes_value"), + result => { + if (result) { + this.styles.setField( + this.fieldName, + this.styles.get(`default_${this.fieldName}`) + ); + this.notifyPropertyChange("editorContents"); + } + } + ); + } + } +}); diff --git a/app/assets/javascripts/admin/controllers/admin-customize-email-style-edit.js.es6 b/app/assets/javascripts/admin/controllers/admin-customize-email-style-edit.js.es6 new file mode 100644 index 00000000000..b8054c8bc3f --- /dev/null +++ b/app/assets/javascripts/admin/controllers/admin-customize-email-style-edit.js.es6 @@ -0,0 +1,33 @@ +import computed from "ember-addons/ember-computed-decorators"; + +export default Ember.Controller.extend({ + @computed("model.isSaving") + saveButtonText(isSaving) { + return isSaving ? I18n.t("saving") : I18n.t("admin.customize.save"); + }, + + @computed("model.changed", "model.isSaving") + saveDisabled(changed, isSaving) { + return !changed || isSaving; + }, + + actions: { + save() { + if (!this.model.saving) { + this.set("saving", true); + this.model + .update(this.model.getProperties("html", "css")) + .catch(e => { + const msg = + e.jqXHR.responseJSON && e.jqXHR.responseJSON.errors + ? I18n.t("admin.customize.email_style.save_error_with_reason", { + error: e.jqXHR.responseJSON.errors.join(". ") + }) + : I18n.t("generic_error"); + bootbox.alert(msg); + }) + .finally(() => this.set("model.changed", false)); + } + } + } +}); diff --git a/app/assets/javascripts/admin/models/email-style.js.es6 b/app/assets/javascripts/admin/models/email-style.js.es6 new file mode 100644 index 00000000000..29d7568aba0 --- /dev/null +++ b/app/assets/javascripts/admin/models/email-style.js.es6 @@ -0,0 +1,10 @@ +import RestModel from "discourse/models/rest"; + +export default RestModel.extend({ + changed: false, + + setField(fieldName, value) { + this.set(`${fieldName}`, value); + this.set("changed", true); + } +}); diff --git a/app/assets/javascripts/admin/routes/admin-customize-email-style-edit.js.es6 b/app/assets/javascripts/admin/routes/admin-customize-email-style-edit.js.es6 new file mode 100644 index 00000000000..74649c1a004 --- /dev/null +++ b/app/assets/javascripts/admin/routes/admin-customize-email-style-edit.js.es6 @@ -0,0 +1,39 @@ +export default Ember.Route.extend({ + model(params) { + return { + model: this.modelFor("adminCustomizeEmailStyle"), + fieldName: params.field_name + }; + }, + + setupController(controller, model) { + controller.setProperties({ + fieldName: model.fieldName, + model: model.model + }); + this._shouldAlertUnsavedChanges = true; + }, + + actions: { + willTransition(transition) { + if ( + this.get("controller.model.changed") && + this._shouldAlertUnsavedChanges && + transition.intent.name !== this.routeName + ) { + transition.abort(); + bootbox.confirm( + I18n.t("admin.customize.theme.unsaved_changes_alert"), + I18n.t("admin.customize.theme.discard"), + I18n.t("admin.customize.theme.stay"), + result => { + if (!result) { + this._shouldAlertUnsavedChanges = false; + transition.retry(); + } + } + ); + } + } + } +}); diff --git a/app/assets/javascripts/admin/routes/admin-customize-email-style.js.es6 b/app/assets/javascripts/admin/routes/admin-customize-email-style.js.es6 new file mode 100644 index 00000000000..8e202e62bd8 --- /dev/null +++ b/app/assets/javascripts/admin/routes/admin-customize-email-style.js.es6 @@ -0,0 +1,9 @@ +export default Ember.Route.extend({ + model() { + return this.store.find("email-style"); + }, + + redirect() { + this.transitionTo("adminCustomizeEmailStyle.edit", "html"); + } +}); diff --git a/app/assets/javascripts/admin/routes/admin-route-map.js.es6 b/app/assets/javascripts/admin/routes/admin-route-map.js.es6 index a20165db02e..478a851d864 100644 --- a/app/assets/javascripts/admin/routes/admin-route-map.js.es6 +++ b/app/assets/javascripts/admin/routes/admin-route-map.js.es6 @@ -90,6 +90,13 @@ export default function() { path: "/robots", resetNamespace: true }); + this.route( + "adminCustomizeEmailStyle", + { path: "/email_style", resetNamespace: true }, + function() { + this.route("edit", { path: "/:field_name" }); + } + ); } ); diff --git a/app/assets/javascripts/admin/templates/components/email-styles-editor.hbs b/app/assets/javascripts/admin/templates/components/email-styles-editor.hbs new file mode 100644 index 00000000000..883149644d2 --- /dev/null +++ b/app/assets/javascripts/admin/templates/components/email-styles-editor.hbs @@ -0,0 +1,20 @@ +
+
+ +
+
+ +{{ace-editor content=editorContents mode=fieldName editorId=editorId}} + +
+
+ {{#d-button action=(action "reset") disabled=resetDisabled class='btn-default'}} + {{i18n 'admin.customize.email_style.reset'}} + {{/d-button}} +
+
diff --git a/app/assets/javascripts/admin/templates/customize-email-style-edit.hbs b/app/assets/javascripts/admin/templates/customize-email-style-edit.hbs new file mode 100644 index 00000000000..1e68fd0c1a2 --- /dev/null +++ b/app/assets/javascripts/admin/templates/customize-email-style-edit.hbs @@ -0,0 +1,9 @@ +{{email-styles-editor styles=model fieldName=fieldName}} + +
+
+ {{#d-button action=(action "save") disabled=saveDisabled class='btn-primary'}} + {{saveButtonText}} + {{/d-button}} +
+
diff --git a/app/assets/javascripts/admin/templates/customize-email-style.hbs b/app/assets/javascripts/admin/templates/customize-email-style.hbs new file mode 100644 index 00000000000..d46e7326039 --- /dev/null +++ b/app/assets/javascripts/admin/templates/customize-email-style.hbs @@ -0,0 +1,7 @@ +
+

{{i18n 'admin.customize.email_style.heading'}}

+ +

{{i18n 'admin.customize.email_style.instructions'}}

+
+ +{{outlet}} diff --git a/app/assets/javascripts/admin/templates/customize.hbs b/app/assets/javascripts/admin/templates/customize.hbs index d5d38163108..768eaf29a26 100644 --- a/app/assets/javascripts/admin/templates/customize.hbs +++ b/app/assets/javascripts/admin/templates/customize.hbs @@ -3,6 +3,7 @@ {{nav-item route='adminCustomize.colors' label='admin.customize.colors.title'}} {{nav-item route='adminSiteText' label='admin.site_text.title'}} {{nav-item route='adminCustomizeEmailTemplates' label='admin.customize.email_templates.title'}} + {{nav-item route='adminCustomizeEmailStyle' label='admin.customize.email_style.title'}} {{nav-item route='adminUserFields' label='admin.user_fields.title'}} {{nav-item route='adminEmojis' label='admin.emoji.title'}} {{nav-item route='adminPermalinks' label='admin.permalink.title'}} diff --git a/app/assets/stylesheets/common/admin/customize.scss b/app/assets/stylesheets/common/admin/customize.scss index e9f8fc23e2a..ce30f493126 100644 --- a/app/assets/stylesheets/common/admin/customize.scss +++ b/app/assets/stylesheets/common/admin/customize.scss @@ -790,3 +790,18 @@ height: 55vh; } } + +.admin-customize-email-style { + .ace-wrapper { + position: relative; + width: 100%; + height: 400px; + .ace_editor { + position: absolute; + left: 0; + right: 0; + top: 0; + bottom: 0; + } + } +} diff --git a/app/controllers/admin/email_styles_controller.rb b/app/controllers/admin/email_styles_controller.rb new file mode 100644 index 00000000000..5649708bfe4 --- /dev/null +++ b/app/controllers/admin/email_styles_controller.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class Admin::EmailStylesController < Admin::AdminController + def show + render_serialized(EmailStyle.new, EmailStyleSerializer) + end + + def update + updater = EmailStyleUpdater.new(current_user) + if updater.update(params.require(:email_style).permit(:html, :css)) + render_serialized(EmailStyle.new, EmailStyleSerializer) + else + render_json_error(updater.errors, status: 422) + end + end +end diff --git a/app/helpers/email_helper.rb b/app/helpers/email_helper.rb index 9687138ecaa..c579473a4d2 100644 --- a/app/helpers/email_helper.rb +++ b/app/helpers/email_helper.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'erb' + module EmailHelper def mailing_list_topic(topic, post_count) @@ -23,6 +25,14 @@ module EmailHelper raw "#{title}" end + def email_html_template(binding_arg) + template = EmailStyle.new.html.sub( + '%{email_content}', + '<%= yield %><% if defined?(html_body) %><%= html_body %><% end %>' + ) + ERB.new(template).result(binding_arg) + end + protected def extract_details(topic) diff --git a/app/mailers/invite_mailer.rb b/app/mailers/invite_mailer.rb index 4c12e697a9f..469a3804158 100644 --- a/app/mailers/invite_mailer.rb +++ b/app/mailers/invite_mailer.rb @@ -5,10 +5,7 @@ require_dependency 'email/message_builder' class InviteMailer < ActionMailer::Base include Email::BuildEmailHelper - class UserNotificationRenderer < ActionView::Base - include UserNotificationsHelper - include EmailHelper - end + layout 'email_template' def send_invite(invite) # Find the first topic they were invited to diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index cf73aa5abe8..0ffc6f77ba0 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -12,6 +12,7 @@ class UserNotifications < ActionMailer::Base include ApplicationHelper helper :application, :email default charset: 'UTF-8' + layout 'email_template' include Email::BuildEmailHelper @@ -362,11 +363,6 @@ class UserNotifications < ActionMailer::Base result end - class UserNotificationRenderer < ActionView::Base - include UserNotificationsHelper - include EmailHelper - end - def self.get_context_posts(post, topic_user, user) if (user.user_option.email_previous_replies == UserOption.previous_replies_type[:never]) || SiteSetting.private_email? @@ -580,15 +576,7 @@ class UserNotifications < ActionMailer::Base site_description: SiteSetting.site_description ) - unless translation_override_exists - html = UserNotificationRenderer.with_view_paths(Rails.configuration.paths["app/views"]).render( - template: 'email/invite', - format: :html, - locals: { message: PrettyText.cook(message, sanitize: false).html_safe, - classes: Rtl.new(user).css_class - } - ) - end + html = PrettyText.cook(message, sanitize: false).html_safe else reached_limit = SiteSetting.max_emails_per_day_per_user > 0 reached_limit &&= (EmailLog.where(user_id: user.id) @@ -608,7 +596,6 @@ class UserNotifications < ActionMailer::Base end unless translation_override_exists - html = UserNotificationRenderer.with_view_paths(Rails.configuration.paths["app/views"]).render( template: 'email/notification', format: :html, @@ -651,7 +638,6 @@ class UserNotifications < ActionMailer::Base site_description: SiteSetting.site_description, site_title: SiteSetting.title, site_title_url_encoded: URI.encode(SiteSetting.title), - style: :notification, locale: locale } @@ -689,13 +675,6 @@ class UserNotifications < ActionMailer::Base @anchor_color = ColorScheme.hex_for_name('tertiary') @markdown_linker = MarkdownLinker.new(@base_url) @unsubscribe_key = UnsubscribeKey.create_key_for(@user, "digest") - end - - def apply_notification_styles(email) - email.html_part.body = Email::Styles.new(email.html_part.body.to_s).tap do |styles| - styles.format_basic - styles.format_notification - end.to_html - email + @disable_email_custom_styles = !SiteSetting.apply_custom_styles_to_digest end end diff --git a/app/models/email_style.rb b/app/models/email_style.rb new file mode 100644 index 00000000000..6d460ccbd19 --- /dev/null +++ b/app/models/email_style.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class EmailStyle + include ActiveModel::Serialization + + attr_accessor :html, :css, :default_html, :default_css + + def id + 'email-style' + end + + def html + SiteSetting.email_custom_template.presence || default_html + end + + def css + SiteSetting.email_custom_css || default_css + end + + def default_html + self.class.default_template + end + + def default_css + self.class.default_css + end + + def self.default_template + @_default_template ||= File.read( + File.join(Rails.root, 'app', 'views', 'email', 'default_template.html') + ) + end + + def self.default_css + '' + end +end diff --git a/app/serializers/email_style_serializer.rb b/app/serializers/email_style_serializer.rb new file mode 100644 index 00000000000..16225f7f533 --- /dev/null +++ b/app/serializers/email_style_serializer.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class EmailStyleSerializer < ApplicationSerializer + attributes :id, :html, :css, :default_html, :default_css +end diff --git a/app/services/email_style_updater.rb b/app/services/email_style_updater.rb new file mode 100644 index 00000000000..d0fb9186365 --- /dev/null +++ b/app/services/email_style_updater.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +class EmailStyleUpdater + + attr_reader :errors + + def initialize(user) + @user = user + @errors = [] + end + + def update(attrs) + if attrs.has_key?(:html) + if attrs[:html] == EmailStyle.default_template + SiteSetting.remove_override!(:email_custom_template) + else + if !attrs[:html].include?('%{email_content}') + @errors << I18n.t( + 'email_style.html_missing_placeholder', + placeholder: '%{email_content}' + ) + else + SiteSetting.email_custom_template = attrs[:html] + end + end + end + + if attrs.has_key?(:css) + if attrs[:css] == EmailStyle.default_css + SiteSetting.remove_override!(:email_custom_css) + else + SiteSetting.email_custom_css = attrs[:css] + end + end + + @errors.empty? + end +end diff --git a/app/services/user_notification_renderer.rb b/app/services/user_notification_renderer.rb new file mode 100644 index 00000000000..8ad7d209145 --- /dev/null +++ b/app/services/user_notification_renderer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class UserNotificationRenderer < ActionView::Base + include ApplicationHelper + include UserNotificationsHelper + include EmailHelper +end diff --git a/app/views/email/default_template.html b/app/views/email/default_template.html new file mode 100644 index 00000000000..382f939273b --- /dev/null +++ b/app/views/email/default_template.html @@ -0,0 +1,24 @@ + + + + + + + + + + + + +%{email_content} + + +
                                    +                        
+ + + diff --git a/app/views/email/invite.html.erb b/app/views/email/invite.html.erb deleted file mode 100644 index 9903735c528..00000000000 --- a/app/views/email/invite.html.erb +++ /dev/null @@ -1,11 +0,0 @@ -
> - -
%{header_instructions}
- - <% if message.present? %> -
<%= message %>
- <% end %> - -
%{respond_instructions}
- -
diff --git a/app/views/email/template.html.erb b/app/views/email/template.html.erb deleted file mode 100644 index 052a9b561f0..00000000000 --- a/app/views/email/template.html.erb +++ /dev/null @@ -1,13 +0,0 @@ - - - - - - - -
- - -
- <%= raw(html_body) %> -
diff --git a/app/views/email/unsubscribe.html.erb b/app/views/email/unsubscribe.html.erb index aee145e5ba3..4bc4a3d0dc0 100644 --- a/app/views/email/unsubscribe.html.erb +++ b/app/views/email/unsubscribe.html.erb @@ -54,22 +54,22 @@ <% if @digest_unsubscribe %>

- + <% if @digest_frequencies[:current] %>

<%= t( - 'unsubscribe.digest_frequency.title', + 'unsubscribe.digest_frequency.title', frequency: t("unsubscribe.digest_frequency.#{@digest_frequencies[:current]}") ) %>


<% end %> - + - <%= - select_tag :digest_after_minutes, - options_for_select(@digest_frequencies[:frequencies], @digest_frequencies[:selected]), - class: 'combobox' + <%= + select_tag :digest_after_minutes, + options_for_select(@digest_frequencies[:frequencies], @digest_frequencies[:selected]), + class: 'combobox' %>

<% end %> diff --git a/app/views/layouts/email_template.html.erb b/app/views/layouts/email_template.html.erb new file mode 100644 index 00000000000..80da92efc20 --- /dev/null +++ b/app/views/layouts/email_template.html.erb @@ -0,0 +1,6 @@ +<% if @disable_email_custom_styles %> + <%= yield %> + <% if defined?(html_body) %><%= html_body %><% end %> +<% else %> + <%= email_html_template(binding).html_safe %> +<% end %> diff --git a/app/views/user_notifications/digest.html.erb b/app/views/user_notifications/digest.html.erb index ec36e142ba2..dc237d15f00 100644 --- a/app/views/user_notifications/digest.html.erb +++ b/app/views/user_notifications/digest.html.erb @@ -1,19 +1,4 @@ - - - - - - - - - - - - +
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d00ba1c3d7d..4bafe972516 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3644,6 +3644,16 @@ en: title: "Override your site's robots.txt file:" warning: "This will permanently override any related site settings." overridden: Your site's default robots.txt file is overridden. + email_style: + title: "Email Style" + heading: "Customize Email Style" + html: "HTML Template" + css: "CSS" + reset: "Reset to default" + reset_confirm: "Are you sure you want to reset to the default %{fieldName} and lose all your changes?" + save_error_with_reason: "Your changes were not saved. %{error}" + instructions: "Customize the template in which all html emails are rendered, and style using CSS." + email: title: "Emails" settings: "Settings" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 6dc6bd93c9c..6290dcaf63e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1854,6 +1854,7 @@ en: suppress_digest_email_after_days: "Suppress summary emails for users not seen on the site for more than (n) days." digest_suppress_categories: "Suppress these categories from summary emails." disable_digest_emails: "Disable summary emails for all users." + apply_custom_styles_to_digest: "Custom email template and css are applied to summary emails." email_accent_bg_color: "The accent color to be used as the background of some elements in HTML emails. Enter a color name ('red') or hex value ('#FF0000')." email_accent_fg_color: "The color of text rendered on the email bg color in HTML emails. Enter a color name ('white') or hex value ('#FFFFFF')." email_link_color: "The color of links in HTML emails. Enter a color name ('blue') or hex value ('#0000FF')." @@ -4554,3 +4555,6 @@ en: title: "Delete User" confirm: "Are you sure you want to delete that user? This will remove all of their posts and block their email and IP address." reason: "Deleted via review queue" + + email_style: + html_missing_placeholder: "The html template must include %{placeholder}" diff --git a/config/routes.rb b/config/routes.rb index d1e4bf0bb2a..68b1b00fe93 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -244,6 +244,9 @@ Discourse::Application.routes.draw do get 'robots' => 'robots_txt#show' put 'robots.json' => 'robots_txt#update' delete 'robots.json' => 'robots_txt#reset' + + resource :email_style, only: [:show, :update] + get 'email_style/:field' => 'email_styles#show', constraints: { field: /html|css/ } end resources :embeddable_hosts, constraints: AdminConstraint.new diff --git a/config/site_settings.yml b/config/site_settings.yml index 5535315930d..29dc73a1dd5 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -911,6 +911,7 @@ email: disable_digest_emails: default: false client: true + apply_custom_styles_to_digest: true email_accent_bg_color: "#2F70AC" email_accent_fg_color: "#FFFFFF" email_link_color: "#006699" @@ -1024,6 +1025,12 @@ email: enable_forwarded_emails: false always_show_trimmed_content: false private_email: false + email_custom_template: + default: "" + hidden: true + email_custom_css: + default: "" + hidden: true email_total_attachment_size_limit_kb: default: 0 max: 51200 diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb index 92f4a2bab89..88669eb729d 100644 --- a/lib/email/message_builder.rb +++ b/lib/email/message_builder.rb @@ -107,16 +107,17 @@ module Email html_override.gsub!("%{respond_instructions}", "") end - styled = Email::Styles.new(html_override, @opts) - styled.format_basic - - if style = @opts[:style] - styled.public_send("format_#{style}") - end + html = UserNotificationRenderer.with_view_paths( + Rails.configuration.paths["app/views"] + ).render( + template: 'layouts/email_template', + format: :html, + locals: { html_body: html_override.html_safe } + ) Mail::Part.new do content_type 'text/html; charset=UTF-8' - body styled.to_html + body html end end diff --git a/lib/email/renderer.rb b/lib/email/renderer.rb index f8d0328dc40..0c616be0436 100644 --- a/lib/email/renderer.rb +++ b/lib/email/renderer.rb @@ -17,15 +17,21 @@ module Email end def html - if @message.html_part - style = Email::Styles.new(@message.html_part.body.to_s, @opts) - style.format_basic - style.format_html + style = if @message.html_part + Email::Styles.new(@message.html_part.body.to_s, @opts) else - style = Email::Styles.new(PrettyText.cook(text), @opts) - style.format_basic + unstyled = UserNotificationRenderer.with_view_paths( + Rails.configuration.paths["app/views"] + ).render( + template: 'layouts/email_template', + format: :html, + locals: { html_body: PrettyText.cook(text).html_safe } + ) + Email::Styles.new(unstyled, @opts) end + style.format_basic + style.format_html style.to_html end diff --git a/lib/email/styles.rb b/lib/email/styles.rb index 7dc7f14b3ae..1b6373878d0 100644 --- a/lib/email/styles.rb +++ b/lib/email/styles.rb @@ -16,6 +16,7 @@ module Email @html = html @opts = opts || {} @fragment = Nokogiri::HTML.fragment(@html) + @custom_styles = nil end def self.register_plugin_style(&block) @@ -32,6 +33,26 @@ module Email end end + def custom_styles + return @custom_styles unless @custom_styles.nil? + + css = EmailStyle.new.css + @custom_styles = {} + + if !css.blank? + require 'css_parser' unless defined?(CssParser) + + parser = CssParser::Parser.new(import: false) + parser.load_string!(css) + parser.each_selector do |selector, value| + @custom_styles[selector] ||= +'' + @custom_styles[selector] << value + end + end + + @custom_styles + end + def format_basic uri = URI(Discourse.base_url) @@ -83,29 +104,6 @@ module Email end end - def format_notification - style('.previous-discussion', 'font-size: 17px; color: #444; margin-bottom:10px;') - style('.notification-date', "text-align:right;color:#999999;padding-right:5px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;font-size:11px") - style('.username', "font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;color:#{SiteSetting.email_link_color};text-decoration:none;font-weight:bold") - style('.user-title', "font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;text-decoration:none;margin-left:7px;color: #999;") - style('.user-name', "font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;text-decoration:none;margin-left:7px;color: #{SiteSetting.email_link_color};font-weight:normal;") - style('.post-wrapper', "margin-bottom:25px;") - style('.user-avatar', 'vertical-align:top;width:55px;') - style('.user-avatar img', nil, width: '45', height: '45') - style('hr', 'background-color: #ddd; height: 1px; border: 1px;') - style('.rtl', 'direction: rtl;') - style('div.body', 'padding-top:5px;') - style('.whisper div.body', 'font-style: italic; color: #9c9c9c;') - style('.lightbox-wrapper .meta', 'display: none') - correct_first_body_margin - correct_footer_style - style('div.undecorated-link-footer a', "font-weight: normal;") - correct_footer_style_hilight_first - reset_tables - onebox_styles - plugin_styles - end - def onebox_styles # Links to other topics style('aside.quote', 'padding: 12px 25px 2px 12px; margin-bottom: 10px;') @@ -164,6 +162,16 @@ module Email end def format_html + html_lang = SiteSetting.default_locale.sub("_", "-") + style('html', nil, lang: html_lang, 'xml:lang' => html_lang) + style('body', "text-align:#{ Rtl.new(nil).enabled? ? 'right' : 'left' };") + style('body', nil, dir: Rtl.new(nil).enabled? ? 'rtl' : 'ltr') + + style('.with-dir', + "text-align:#{ Rtl.new(nil).enabled? ? 'right' : 'left' };", + dir: Rtl.new(nil).enabled? ? 'rtl' : 'ltr' + ) + style('.with-accent-colors', "background-color: #{SiteSetting.email_accent_bg_color}; color: #{SiteSetting.email_accent_fg_color};") style('h4', 'color: #222;') style('h3', 'margin: 15px 0 20px 0;') @@ -177,11 +185,39 @@ module Email style('code', 'background-color: #f1f1ff; padding: 2px 5px;') style('pre code', 'display: block; background-color: #f1f1ff; padding: 5px;') style('.featured-topic a', "text-decoration: none; font-weight: bold; color: #{SiteSetting.email_link_color}; line-height:1.5em;") + style('.summary-email', "-moz-box-sizing:border-box;-ms-text-size-adjust:100%;-webkit-box-sizing:border-box;-webkit-text-size-adjust:100%;box-sizing:border-box;color:#0a0a0a;font-family:Helvetica,Arial,sans-serif;font-size:14px;font-weight:400;line-height:1.3;margin:0;min-width:100%;padding:0;width:100%") + + style('.previous-discussion', 'font-size: 17px; color: #444; margin-bottom:10px;') + style('.notification-date', "text-align:right;color:#999999;padding-right:5px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;font-size:11px") + style('.username', "font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;color:#{SiteSetting.email_link_color};text-decoration:none;font-weight:bold") + style('.user-title', "font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;text-decoration:none;margin-left:7px;color: #999;") + style('.user-name', "font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;text-decoration:none;margin-left:7px;color: #{SiteSetting.email_link_color};font-weight:normal;") + style('.post-wrapper', "margin-bottom:25px;") + style('.user-avatar', 'vertical-align:top;width:55px;') + style('.user-avatar img', nil, width: '45', height: '45') + style('hr', 'background-color: #ddd; height: 1px; border: 1px;') + style('.rtl', 'direction: rtl;') + style('div.body', 'padding-top:5px;') + style('.whisper div.body', 'font-style: italic; color: #9c9c9c;') + style('.lightbox-wrapper .meta', 'display: none') + correct_first_body_margin + correct_footer_style + style('div.undecorated-link-footer a', "font-weight: normal;") + correct_footer_style_hilight_first + reset_tables onebox_styles plugin_styles style('.post-excerpt img', "max-width: 50%; max-height: 400px;") + + format_custom + end + + def format_custom + custom_styles.each do |selector, value| + style(selector, value) + end end # this method is reserved for styles specific to plugin @@ -240,7 +276,7 @@ module Email end def correct_first_body_margin - @fragment.css('.body p').each do |element| + @fragment.css('div.body p').each do |element| element['style'] = "margin-top:0; border: 0;" end end diff --git a/spec/integration/email_style_spec.rb b/spec/integration/email_style_spec.rb new file mode 100644 index 00000000000..e035aec1028 --- /dev/null +++ b/spec/integration/email_style_spec.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe EmailStyle do + before do + SiteSetting.email_custom_template = "

FOR YOU

%{email_content}
" + SiteSetting.email_custom_css = 'h1 { color: red; } div.body { color: #FAB; }' + end + + after do + SiteSetting.remove_override!(:email_custom_template) + SiteSetting.remove_override!(:email_custom_css) + end + + context 'invite' do + fab!(:invite) { Fabricate(:invite) } + let(:invite_mail) { InviteMailer.send_invite(invite) } + + subject(:mail_html) { Email::Renderer.new(invite_mail).html } + + it 'applies customizations' do + expect(mail_html.scan('

FOR YOU

').count).to eq(1) + expect(mail_html).to match("#{Discourse.base_url}/invites/#{invite.invite_key}") + end + + it 'can apply RTL attrs' do + SiteSetting.default_locale = 'he' + body_attrs = mail_html.match(/])+/) + expect(body_attrs[0]&.downcase).to match(/text-align:\s*right/) + expect(body_attrs[0]&.downcase).to include('dir="rtl"') + end + end + + context 'user_replied' do + let(:response_by_user) { Fabricate(:user, name: "John Doe") } + let(:category) { Fabricate(:category, name: 'India') } + let(:topic) { Fabricate(:topic, category: category, title: "Super cool topic") } + let(:post) { Fabricate(:post, topic: topic, raw: 'This is My super duper cool topic') } + let(:response) { Fabricate(:basic_reply, topic: post.topic, user: response_by_user) } + let(:user) { Fabricate(:user) } + let(:notification) { Fabricate(:replied_notification, user: user, post: response) } + + let(:mail) do + UserNotifications.user_replied( + user, + post: response, + notification_type: notification.notification_type, + notification_data_hash: notification.data_hash + ) + end + + subject(:mail_html) { Email::Renderer.new(mail).html } + + it "customizations are applied to html part of emails" do + expect(mail_html.scan('

FOR YOU

').count).to eq(1) + matches = mail_html.match(/
#{post.raw}/) + expect(matches[1]).to include('color: #FAB;') # custom + expect(matches[1]).to include('padding-top:5px;') # div.body + end + + # TODO: translation override + end + + context 'signup' do + let(:signup_mail) { UserNotifications.signup(Fabricate(:user)) } + subject(:mail_html) { Email::Renderer.new(signup_mail).html } + + it "customizations are applied to html part of emails" do + expect(mail_html.scan('

FOR YOU

').count).to eq(1) + expect(mail_html).to include('activate-account') + end + + context 'translation override' do + before do + TranslationOverride.upsert!( + 'en', + 'user_notifications.signup.text_body_template', + "CLICK THAT LINK: %{base_url}/u/activate-account/%{email_token}" + ) + end + + after do + TranslationOverride.revert!('en', ['user_notifications.signup.text_body_template']) + end + + it "applies customizations when translation override exists" do + expect(mail_html.scan('

FOR YOU

').count).to eq(1) + expect(mail_html.scan('CLICK THAT LINK').count).to eq(1) + end + end + + context 'with some bad css' do + before do + SiteSetting.email_custom_css = '@import "nope.css"; h1 {{{ size: really big; ' + end + + it "can render the html" do + expect(mail_html.scan(/FOR YOU<\/h1>/).count).to eq(1) + expect(mail_html).to include('activate-account') + end + end + end + + context 'digest' do + fab!(:popular_topic) { Fabricate(:topic, user: Fabricate(:coding_horror), created_at: 1.hour.ago) } + let(:summary_email) { UserNotifications.digest(Fabricate(:user)) } + subject(:mail_html) { Email::Renderer.new(summary_email).html } + + it "customizations are applied to html part of emails" do + expect(mail_html.scan('

FOR YOU

').count).to eq(1) + expect(mail_html).to include(popular_topic.title) + end + + it "doesn't apply customizations if apply_custom_styles_to_digest is disabled" do + SiteSetting.apply_custom_styles_to_digest = false + expect(mail_html).to_not include('

FOR YOU

') + expect(mail_html).to_not include('FOR YOU') + expect(mail_html).to include(popular_topic.title) + end + end +end diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index 7106d4641f2..f86892b3281 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -260,7 +260,7 @@ describe UserNotifications do expect(mail.subject).to match(/Taggo/) expect(mail.subject).to match(/Taggie/) - mail_html = mail.html_part.to_s + mail_html = mail.html_part.body.to_s expect(mail_html.scan(/My super duper cool topic/).count).to eq(1) expect(mail_html.scan(/In Reply To/).count).to eq(1) @@ -287,7 +287,7 @@ describe UserNotifications do notification_data_hash: notification.data_hash ) - expect(mail.html_part.to_s.scan(/In Reply To/).count).to eq(0) + expect(mail.html_part.body.to_s.scan(/In Reply To/).count).to eq(0) SiteSetting.enable_names = true SiteSetting.display_name_on_posts = true @@ -304,7 +304,7 @@ describe UserNotifications do notification_data_hash: notification.data_hash ) - mail_html = mail.html_part.to_s + mail_html = mail.html_part.body.to_s expect(mail_html.scan(/>Bob Marley/).count).to eq(1) expect(mail_html.scan(/>bobmarley/).count).to eq(0) @@ -317,7 +317,7 @@ describe UserNotifications do notification_data_hash: notification.data_hash ) - mail_html = mail.html_part.to_s + mail_html = mail.html_part.body.to_s expect(mail_html.scan(/>Bob Marley/).count).to eq(0) expect(mail_html.scan(/>bobmarley/).count).to eq(1) end @@ -331,8 +331,8 @@ describe UserNotifications do notification_data_hash: notification.data_hash ) - expect(mail.html_part.to_s).to_not include(response.raw) - expect(mail.html_part.to_s).to_not include(topic.url) + expect(mail.html_part.body.to_s).to_not include(response.raw) + expect(mail.html_part.body.to_s).to_not include(topic.url) expect(mail.text_part.to_s).to_not include(response.raw) expect(mail.text_part.to_s).to_not include(topic.url) end @@ -365,10 +365,10 @@ describe UserNotifications do expect(mail.subject).not_to match(/Uncategorized/) # 1 respond to links as no context by default - expect(mail.html_part.to_s.scan(/to respond/).count).to eq(1) + expect(mail.html_part.body.to_s.scan(/to respond/).count).to eq(1) # 1 unsubscribe link - expect(mail.html_part.to_s.scan(/To unsubscribe/).count).to eq(1) + expect(mail.html_part.body.to_s.scan(/To unsubscribe/).count).to eq(1) # side effect, topic user is updated with post number tu = TopicUser.get(post.topic_id, user) @@ -384,7 +384,7 @@ describe UserNotifications do notification_data_hash: notification.data_hash ) - expect(mail.html_part.to_s).to_not include(response.raw) + expect(mail.html_part.body.to_s).to_not include(response.raw) expect(mail.text_part.to_s).to_not include(response.raw) end @@ -451,13 +451,13 @@ describe UserNotifications do expect(mail.subject).to include("[PM] ") # 1 "visit message" link - expect(mail.html_part.to_s.scan(/Visit Message/).count).to eq(1) + expect(mail.html_part.body.to_s.scan(/Visit Message/).count).to eq(1) # 1 respond to link - expect(mail.html_part.to_s.scan(/to respond/).count).to eq(1) + expect(mail.html_part.body.to_s.scan(/to respond/).count).to eq(1) # 1 unsubscribe link - expect(mail.html_part.to_s.scan(/To unsubscribe/).count).to eq(1) + expect(mail.html_part.body.to_s.scan(/To unsubscribe/).count).to eq(1) # side effect, topic user is updated with post number tu = TopicUser.get(topic.id, user) @@ -473,8 +473,8 @@ describe UserNotifications do notification_data_hash: notification.data_hash ) - expect(mail.html_part.to_s).to_not include(response.raw) - expect(mail.html_part.to_s).to_not include(topic.url) + expect(mail.html_part.body.to_s).to_not include(response.raw) + expect(mail.html_part.body.to_s).to_not include(topic.url) expect(mail.text_part.to_s).to_not include(response.raw) expect(mail.text_part.to_s).to_not include(topic.url) end @@ -635,7 +635,7 @@ describe UserNotifications do # WARNING: you reached the limit of 100 email notifications per day. Further emails will be suppressed. # Consider watching less topics or disabling mailing list mode. - expect(mail.html_part.to_s).to match(I18n.t("user_notifications.reached_limit", count: 2)) + expect(mail.html_part.body.to_s).to match(I18n.t("user_notifications.reached_limit", count: 2)) expect(mail.body.to_s).to match(I18n.t("user_notifications.reached_limit", count: 2)) end diff --git a/spec/requests/admin/email_styles_controller_spec.rb b/spec/requests/admin/email_styles_controller_spec.rb new file mode 100644 index 00000000000..9d1f297e7eb --- /dev/null +++ b/spec/requests/admin/email_styles_controller_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Admin::EmailStylesController do + fab!(:admin) { Fabricate(:admin) } + let(:default_html) { File.read("#{Rails.root}/app/views/email/default_template.html") } + let(:default_css) { "" } + + before do + sign_in(admin) + end + + after do + SiteSetting.remove_override!(:email_custom_template) + SiteSetting.remove_override!(:email_custom_css) + end + + describe 'show' do + it 'returns default values' do + get '/admin/customize/email_style.json' + expect(response.status).to eq(200) + + json = ::JSON.parse(response.body)['email_style'] + expect(json['html']).to eq(default_html) + expect(json['css']).to eq(default_css) + end + + it 'returns customized values' do + SiteSetting.email_custom_template = "For you: %{email_content}" + SiteSetting.email_custom_css = ".user-name { font-size: 24px; }" + get '/admin/customize/email_style.json' + expect(response.status).to eq(200) + + json = ::JSON.parse(response.body)['email_style'] + expect(json['html']).to eq("For you: %{email_content}") + expect(json['css']).to eq(".user-name { font-size: 24px; }") + end + end + + describe 'update' do + let(:valid_params) do + { + html: 'For you: %{email_content}', + css: '.user-name { color: purple; }' + } + end + + it 'changes the settings' do + SiteSetting.email_custom_css = ".user-name { font-size: 24px; }" + put '/admin/customize/email_style.json', params: { email_style: valid_params } + expect(response.status).to eq(200) + expect(SiteSetting.email_custom_template).to eq(valid_params[:html]) + expect(SiteSetting.email_custom_css).to eq(valid_params[:css]) + end + + it 'reports errors' do + put '/admin/customize/email_style.json', params: { + email_style: valid_params.merge(html: 'No email content') + } + expect(response.status).to eq(422) + json = JSON.parse(response.body) + expect(json['errors']).to include( + I18n.t( + 'email_style.html_missing_placeholder', + placeholder: '%{email_content}' + ) + ) + end + end +end diff --git a/spec/services/email_style_updater_spec.rb b/spec/services/email_style_updater_spec.rb new file mode 100644 index 00000000000..3ef6f1c7ea9 --- /dev/null +++ b/spec/services/email_style_updater_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe EmailStyleUpdater do + fab!(:admin) { Fabricate(:admin) } + let(:default_html) { File.read("#{Rails.root}/app/views/email/default_template.html") } + let(:updater) { EmailStyleUpdater.new(admin) } + + describe 'update' do + it 'can change the settings' do + expect( + updater.update( + html: 'For you: %{email_content}', + css: 'h1 { color: blue; }' + ) + ).to eq(true) + expect(SiteSetting.email_custom_template).to eq('For you: %{email_content}') + expect(SiteSetting.email_custom_css).to eq('h1 { color: blue; }') + end + + it 'will not store defaults' do + updater.update(html: default_html, css: '') + expect(SiteSetting.email_custom_template).to_not be_present + expect(SiteSetting.email_custom_css).to_not be_present + end + + it 'can clear settings if defaults given' do + SiteSetting.email_custom_template = 'For you: %{email_content}' + SiteSetting.email_custom_css = 'h1 { color: blue; }' + updater.update(html: default_html, css: '') + expect(SiteSetting.email_custom_template).to_not be_present + expect(SiteSetting.email_custom_css).to_not be_present + end + + it 'fails if html is missing email_content' do + expect(updater.update(html: 'No email content', css: '')).to eq(false) + expect(updater.errors).to include( + I18n.t( + 'email_style.html_missing_placeholder', + placeholder: '%{email_content}' + ) + ) + end + end +end