diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb index 6fad84f7c7df8..11eed74e5e8a3 100644 --- a/lib/email/message_builder.rb +++ b/lib/email/message_builder.rb @@ -100,35 +100,21 @@ def initialize(to, opts = nil) end def subject - if @opts[:template] && - TranslationOverride.exists?( - locale: I18n.locale, - translation_key: "#{@opts[:template]}.subject_template", - ) + has_override = + TranslationOverride.exists?( + locale: I18n.locale, + translation_key: "#{@opts[:template]}.subject_template", + ) + + if @opts[:template] && has_override augmented_template_args = @template_args.merge( - { - site_name: @template_args[:email_prefix], - optional_re: @opts[:add_re_to_subject] ? I18n.t("subject_re") : "", - optional_pm: @opts[:private_reply] ? @template_args[:subject_pm] : "", - optional_cat: - ( - if @template_args[:show_category_in_subject] - "[#{@template_args[:show_category_in_subject]}] " - else - "" - end - ), - optional_tags: - ( - if @template_args[:show_tags_in_subject] - "#{@template_args[:show_tags_in_subject]} " - else - "" - end - ), - topic_title: @template_args[:topic_title] ? @template_args[:topic_title] : "", - }, + site_name: @template_args[:email_prefix], + optional_re: @opts[:add_re_to_subject] ? I18n.t("subject_re") : "", + optional_pm: @opts[:private_reply] ? @template_args[:subject_pm] : "", + optional_cat: format_category, + optional_tags: format_tags, + topic_title: @template_args[:topic_title] ? @template_args[:topic_title] : "", ) subject = I18n.t("#{@opts[:template]}.subject_template", augmented_template_args) elsif @opts[:use_site_subject] @@ -136,23 +122,11 @@ def subject subject.gsub!("%{site_name}", @template_args[:email_prefix]) subject.gsub!("%{optional_re}", @opts[:add_re_to_subject] ? I18n.t("subject_re") : "") subject.gsub!("%{optional_pm}", @opts[:private_reply] ? @template_args[:subject_pm] : "") - subject.gsub!( - "%{optional_cat}", - ( - if @template_args[:show_category_in_subject] - "[#{@template_args[:show_category_in_subject]}] " - else - "" - end - ), - ) - subject.gsub!( - "%{optional_tags}", - @template_args[:show_tags_in_subject] ? "#{@template_args[:show_tags_in_subject]} " : "", - ) + subject.gsub!("%{optional_cat}", format_category) + subject.gsub!("%{optional_tags}", format_tags) if @template_args[:topic_title] subject.gsub!("%{topic_title}", @template_args[:topic_title]) - end # must be last for safety + end elsif @opts[:use_topic_title_subject] subject = @opts[:add_re_to_subject] ? I18n.t("subject_re") : "" subject = "#{subject}#{@template_args[:topic_title]}" @@ -161,6 +135,7 @@ def subject else subject = @opts[:subject] end + DiscoursePluginRegistry.apply_modifier(:message_builder_subject, subject, @opts, @to) end @@ -211,15 +186,19 @@ def body body = nil if @opts[:template] - template_args_to_escape = %i[topic_title inviter_name] - - template_args_to_escape.each do |key| - next if !@template_args.key?(key) - - @template_args[key] = escaped_template_arg(key) + %i[topic_title inviter_name].each do |key| + @template_args[key] = escaped_template_arg(key) if @template_args.key?(key) end - body = I18n.t("#{@opts[:template]}.text_body_template", template_args).dup + augmented_template_args = + @template_args.merge( + optional_re: "", + optional_pm: "", + optional_cat: format_category, + optional_tags: format_tags, + ) + + body = I18n.t("#{@opts[:template]}.text_body_template", augmented_template_args).dup else body = @opts[:body].dup end @@ -384,5 +363,21 @@ def escaped_template_arg(key) once_escaped = String.new(ERB::Util.html_escape(value)) ERB::Util.html_escape(once_escaped) end + + def format_category + if @template_args[:show_category_in_subject] + "[#{@template_args[:show_category_in_subject]}] " + else + "" + end + end + + def format_tags + if @template_args[:show_tags_in_subject] + "#{@template_args[:show_tags_in_subject]} " + else + "" + end + end end end diff --git a/spec/lib/email/message_builder_spec.rb b/spec/lib/email/message_builder_spec.rb index 36218b88b8272..581cc3821c14f 100644 --- a/spec/lib/email/message_builder_spec.rb +++ b/spec/lib/email/message_builder_spec.rb @@ -540,7 +540,15 @@ it "has the body rendered from a template" do I18n .expects(:t) - .with("mystery.text_body_template", templated_builder.template_args) + .with( + "mystery.text_body_template", + templated_builder.template_args.merge( + optional_re: "", + optional_pm: "", + optional_cat: "", + optional_tags: "", + ), + ) .returns(rendered_template) expect(templated_builder.body).to eq(rendered_template) end diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index 6e380ea4d81a1..a76fa0f4ce26e 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -645,6 +645,52 @@ end end + describe "optional placeholders in email body" do + it "should render optional_tags, optional_cat, optional_pm, and optional_re in body templates" do + custom_body = <<~BODY + You got a reply! + + Category: %{optional_cat} + Tags: %{optional_tags} + PM marker: %{optional_pm} + Re marker: %{optional_re} + + %{message} + BODY + + TranslationOverride.upsert!( + I18n.locale, + "user_notifications.user_replied.text_body_template", + custom_body, + ) + + mail = + UserNotifications.user_replied( + user, + post: response, + notification_type: notification.notification_type, + notification_data_hash: notification.data_hash, + ) + + body = mail.body.to_s + + expect(body).to include(tag2.name) + expect(body).to include(tag3.name) + expect(body).to include(category.name) + + expect(body).not_to include("translation missing") + expect(body).not_to include("%{optional_tags}") + expect(body).not_to include("%{optional_cat}") + expect(body).not_to include("%{optional_pm}") + expect(body).not_to include("%{optional_re}") + + TranslationOverride.revert!( + I18n.locale, + ["user_notifications.user_replied.text_body_template"], + ) + end + end + it "doesn't include details when private_email is enabled" do SiteSetting.private_email = true mail =