Skip to content

Commit 5789d7e

Browse files
ZogStriPZogStriP
authored
FIX: translation missing when using optional_* placeholder in body (#36023)
All the "optional_*" placeholders are meant to be used in the subject but if you were to use them in the body template you would get a "translation missing" error. This commit ensures these optional placeholders are also provided when i18ning the body even though they don't make a lot of sense. Internal ref - t/150916
1 parent 1868839 commit 5789d7e

File tree

3 files changed

+99
-50
lines changed

3 files changed

+99
-50
lines changed

‎lib/email/message_builder.rb‎

Lines changed: 44 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -100,59 +100,33 @@ def initialize(to, opts = nil)
100100
end
101101

102102
def subject
103-
if @opts[:template] &&
104-
TranslationOverride.exists?(
105-
locale: I18n.locale,
106-
translation_key: "#{@opts[:template]}.subject_template",
107-
)
103+
has_override =
104+
TranslationOverride.exists?(
105+
locale: I18n.locale,
106+
translation_key: "#{@opts[:template]}.subject_template",
107+
)
108+
109+
if @opts[:template] && has_override
108110
augmented_template_args =
109111
@template_args.merge(
110-
{
111-
site_name: @template_args[:email_prefix],
112-
optional_re: @opts[:add_re_to_subject] ? I18n.t("subject_re") : "",
113-
optional_pm: @opts[:private_reply] ? @template_args[:subject_pm] : "",
114-
optional_cat:
115-
(
116-
if @template_args[:show_category_in_subject]
117-
"[#{@template_args[:show_category_in_subject]}] "
118-
else
119-
""
120-
end
121-
),
122-
optional_tags:
123-
(
124-
if @template_args[:show_tags_in_subject]
125-
"#{@template_args[:show_tags_in_subject]} "
126-
else
127-
""
128-
end
129-
),
130-
topic_title: @template_args[:topic_title] ? @template_args[:topic_title] : "",
131-
},
112+
site_name: @template_args[:email_prefix],
113+
optional_re: @opts[:add_re_to_subject] ? I18n.t("subject_re") : "",
114+
optional_pm: @opts[:private_reply] ? @template_args[:subject_pm] : "",
115+
optional_cat: format_category,
116+
optional_tags: format_tags,
117+
topic_title: @template_args[:topic_title] ? @template_args[:topic_title] : "",
132118
)
133119
subject = I18n.t("#{@opts[:template]}.subject_template", augmented_template_args)
134120
elsif @opts[:use_site_subject]
135121
subject = String.new(SiteSetting.email_subject)
136122
subject.gsub!("%{site_name}", @template_args[:email_prefix])
137123
subject.gsub!("%{optional_re}", @opts[:add_re_to_subject] ? I18n.t("subject_re") : "")
138124
subject.gsub!("%{optional_pm}", @opts[:private_reply] ? @template_args[:subject_pm] : "")
139-
subject.gsub!(
140-
"%{optional_cat}",
141-
(
142-
if @template_args[:show_category_in_subject]
143-
"[#{@template_args[:show_category_in_subject]}] "
144-
else
145-
""
146-
end
147-
),
148-
)
149-
subject.gsub!(
150-
"%{optional_tags}",
151-
@template_args[:show_tags_in_subject] ? "#{@template_args[:show_tags_in_subject]} " : "",
152-
)
125+
subject.gsub!("%{optional_cat}", format_category)
126+
subject.gsub!("%{optional_tags}", format_tags)
153127
if @template_args[:topic_title]
154128
subject.gsub!("%{topic_title}", @template_args[:topic_title])
155-
end # must be last for safety
129+
end
156130
elsif @opts[:use_topic_title_subject]
157131
subject = @opts[:add_re_to_subject] ? I18n.t("subject_re") : ""
158132
subject = "#{subject}#{@template_args[:topic_title]}"
@@ -161,6 +135,7 @@ def subject
161135
else
162136
subject = @opts[:subject]
163137
end
138+
164139
DiscoursePluginRegistry.apply_modifier(:message_builder_subject, subject, @opts, @to)
165140
end
166141

@@ -211,15 +186,19 @@ def body
211186
body = nil
212187

213188
if @opts[:template]
214-
template_args_to_escape = %i[topic_title inviter_name]
215-
216-
template_args_to_escape.each do |key|
217-
next if !@template_args.key?(key)
218-
219-
@template_args[key] = escaped_template_arg(key)
189+
%i[topic_title inviter_name].each do |key|
190+
@template_args[key] = escaped_template_arg(key) if @template_args.key?(key)
220191
end
221192

222-
body = I18n.t("#{@opts[:template]}.text_body_template", template_args).dup
193+
augmented_template_args =
194+
@template_args.merge(
195+
optional_re: "",
196+
optional_pm: "",
197+
optional_cat: format_category,
198+
optional_tags: format_tags,
199+
)
200+
201+
body = I18n.t("#{@opts[:template]}.text_body_template", augmented_template_args).dup
223202
else
224203
body = @opts[:body].dup
225204
end
@@ -384,5 +363,21 @@ def escaped_template_arg(key)
384363
once_escaped = String.new(ERB::Util.html_escape(value))
385364
ERB::Util.html_escape(once_escaped)
386365
end
366+
367+
def format_category
368+
if @template_args[:show_category_in_subject]
369+
"[#{@template_args[:show_category_in_subject]}] "
370+
else
371+
""
372+
end
373+
end
374+
375+
def format_tags
376+
if @template_args[:show_tags_in_subject]
377+
"#{@template_args[:show_tags_in_subject]} "
378+
else
379+
""
380+
end
381+
end
387382
end
388383
end

‎spec/lib/email/message_builder_spec.rb‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,15 @@
540540
it "has the body rendered from a template" do
541541
I18n
542542
.expects(:t)
543-
.with("mystery.text_body_template", templated_builder.template_args)
543+
.with(
544+
"mystery.text_body_template",
545+
templated_builder.template_args.merge(
546+
optional_re: "",
547+
optional_pm: "",
548+
optional_cat: "",
549+
optional_tags: "",
550+
),
551+
)
544552
.returns(rendered_template)
545553
expect(templated_builder.body).to eq(rendered_template)
546554
end

‎spec/mailers/user_notifications_spec.rb‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,52 @@
645645
end
646646
end
647647

648+
describe "optional placeholders in email body" do
649+
it "should render optional_tags, optional_cat, optional_pm, and optional_re in body templates" do
650+
custom_body = <<~BODY
651+
You got a reply!
652+
653+
Category: %{optional_cat}
654+
Tags: %{optional_tags}
655+
PM marker: %{optional_pm}
656+
Re marker: %{optional_re}
657+
658+
%{message}
659+
BODY
660+
661+
TranslationOverride.upsert!(
662+
I18n.locale,
663+
"user_notifications.user_replied.text_body_template",
664+
custom_body,
665+
)
666+
667+
mail =
668+
UserNotifications.user_replied(
669+
user,
670+
post: response,
671+
notification_type: notification.notification_type,
672+
notification_data_hash: notification.data_hash,
673+
)
674+
675+
body = mail.body.to_s
676+
677+
expect(body).to include(tag2.name)
678+
expect(body).to include(tag3.name)
679+
expect(body).to include(category.name)
680+
681+
expect(body).not_to include("translation missing")
682+
expect(body).not_to include("%{optional_tags}")
683+
expect(body).not_to include("%{optional_cat}")
684+
expect(body).not_to include("%{optional_pm}")
685+
expect(body).not_to include("%{optional_re}")
686+
687+
TranslationOverride.revert!(
688+
I18n.locale,
689+
["user_notifications.user_replied.text_body_template"],
690+
)
691+
end
692+
end
693+
648694
it "doesn't include details when private_email is enabled" do
649695
SiteSetting.private_email = true
650696
mail =

0 commit comments

Comments
 (0)