diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 5269d3d4164..e3b9b255ed7 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -4016,10 +4016,15 @@ en: header_instructions: "" reply_by_email: "[Visit Topic](%{base_url}%{url}) or reply to this email to respond." reply_by_email_pm: "[Visit Message](%{base_url}%{url}) or reply to this email to respond to %{participants}." + reply_by_email_button_only: "[Visit Topic](%{base_url}%{url})" + reply_by_email_pm_button_only: "[Visit Message](%{base_url}%{url})" only_reply_by_email: "Reply to this email to respond." only_reply_by_email_pm: "Reply to this email to respond to %{participants}." + only_reply_by_email_pm_button_only: "[Visit Message](%{base_url}%{url})" visit_link_to_respond: "[Visit Topic](%{base_url}%{url}) to respond." visit_link_to_respond_pm: "[Visit Message](%{base_url}%{url}) to respond to %{participants}." + visit_link_to_respond_button_only: "[Visit Topic](%{base_url}%{url})" + visit_link_to_respond_pm_button_only: "[Visit Message](%{base_url}%{url})" reply_above_line: "## Please type your reply above this line. ##" posted_by: "Posted by %{username} on %{post_date}" diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb index 062d0757bb0..c1141720ebf 100644 --- a/lib/email/message_builder.rb +++ b/lib/email/message_builder.rb @@ -7,6 +7,7 @@ module Email attr_reader :template_args, :reply_by_email_key ALLOW_REPLY_BY_EMAIL_HEADER = "X-Discourse-Allow-Reply-By-Email" + INSTRUCTIONS_SEPARATOR = "---\n" def initialize(to, opts = nil) @to = to @@ -18,65 +19,83 @@ module Email user_preferences_url: "#{Discourse.base_url}/my/preferences", hostname: Discourse.current_hostname, }.merge!(@opts) - if @template_args[:url].present? - @template_args[:header_instructions] ||= I18n.t( - "user_notifications.header_instructions", - @template_args, - ) - @visit_link_to_respond_key = - DiscoursePluginRegistry.apply_modifier( - :message_builder_visit_link_to_respond, - "user_notifications.visit_link_to_respond", - @opts, - @to, - ) - @reply_by_email_key = - DiscoursePluginRegistry.apply_modifier( - :message_builder_reply_by_email, - "user_notifications.reply_by_email", - @opts, - @to, - ) - if @opts[:include_respond_instructions] == false - @template_args[:respond_instructions] = "" + return if @template_args[:url].blank? + + @template_args[:header_instructions] ||= I18n.t( + "user_notifications.header_instructions", + @template_args, + ) + @visit_link_to_respond_key = + DiscoursePluginRegistry.apply_modifier( + :message_builder_visit_link_to_respond, + "user_notifications.visit_link_to_respond", + @opts, + @to, + ) + @reply_by_email_key = + DiscoursePluginRegistry.apply_modifier( + :message_builder_reply_by_email, + "user_notifications.reply_by_email", + @opts, + @to, + ) + + if @opts[:include_respond_instructions] == false + if @opts[:private_reply] @template_args[:respond_instructions] = I18n.t( "user_notifications.pm_participants", @template_args, - ) if @opts[:private_reply] + ) else - if @opts[:only_reply_by_email] - string = +"user_notifications.only_reply_by_email" - if @opts[:private_reply] && @opts[:username] != Discourse.system_user.username - string << "_pm" - end - else - string = - ( - if allow_reply_by_email? - +@reply_by_email_key - else - +@visit_link_to_respond_key - end - ) - if @opts[:private_reply] && @opts[:username] != Discourse.system_user.username - string << "_pm" + @template_args[:respond_instructions] = "" + end + else + if @opts[:only_reply_by_email] + respond_instructions_key = +"user_notifications.only_reply_by_email" + if @opts[:private_reply] + if @opts[:username] == Discourse.system_user.username + respond_instructions_key << "_pm_button_only" + else + respond_instructions_key << "_pm" end end - @template_args[:respond_instructions] = "---\n" + I18n.t(string, @template_args) - end - - if @opts[:add_unsubscribe_link] - unsubscribe_string = - if @opts[:mailing_list_mode] - "unsubscribe_mailing_list" - elsif SiteSetting.unsubscribe_via_email_footer - "unsubscribe_link_and_mail" + else + respond_instructions_key = + ( + if allow_reply_by_email? + +@reply_by_email_key + else + +@visit_link_to_respond_key + end + ) + if @opts[:private_reply] + if @opts[:username] == Discourse.system_user.username + respond_instructions_key << "_pm_button_only" else - "unsubscribe_link" + respond_instructions_key << "_pm" end - @template_args[:unsubscribe_instructions] = I18n.t(unsubscribe_string, @template_args) + end end + @template_args[:respond_instructions] = ( + if respond_instructions_key != "" + INSTRUCTIONS_SEPARATOR + I18n.t(respond_instructions_key, @template_args) + else + "" + end + ) + end + + if @opts[:add_unsubscribe_link] + unsubscribe_string = + if @opts[:mailing_list_mode] + "unsubscribe_mailing_list" + elsif SiteSetting.unsubscribe_via_email_footer + "unsubscribe_link_and_mail" + else + "unsubscribe_link" + end + @template_args[:unsubscribe_instructions] = I18n.t(unsubscribe_string, @template_args) end end diff --git a/spec/lib/email/message_builder_spec.rb b/spec/lib/email/message_builder_spec.rb index cf3e0ee93f5..7cfaf0e0669 100644 --- a/spec/lib/email/message_builder_spec.rb +++ b/spec/lib/email/message_builder_spec.rb @@ -111,6 +111,143 @@ RSpec.describe Email::MessageBuilder do expect(header_args["X-Auto-Response-Suppress"]).to eq("All") end + describe "include_respond_instructions" do + context "when include_respond_instructions is false" do + let(:private_reply) { false } + let(:builder) do + Email::MessageBuilder.new( + "to@to.com", + subject: "test", + body: "test", + include_respond_instructions: false, + url: "/t/123", + participants: %w[moe joe], + private_reply: private_reply, + ) + end + + it "does not include any instructions" do + expect(builder.template_args[:respond_instructions]).to eq("") + end + + context "for a private_reply" do + let(:private_reply) { true } + + it "includes the pm_participants instruction" do + expect(builder.template_args[:respond_instructions]).to eq( + I18n.t("user_notifications.pm_participants", builder.template_args), + ) + end + end + end + + context "when include_respond_instructions is true" do + let(:other_opts) { {} } + let(:builder) do + Email::MessageBuilder.new( + "to@to.com", + { + subject: "test", + body: "test", + include_respond_instructions: true, + allow_reply_by_email: true, + participants: %w[moe joe], + url: "/t/123", + }.merge(other_opts), + ) + end + + context "when only_reply_by_email" do + let(:other_opts) { { only_reply_by_email: true } } + + it "includes the correct instructions" do + expect(builder.template_args[:respond_instructions]).to eq( + Email::MessageBuilder::INSTRUCTIONS_SEPARATOR + + I18n.t("user_notifications.only_reply_by_email", builder.template_args), + ) + end + + context "for private_reply to regular users" do + let(:other_opts) do + { private_reply: true, username: "someguy", only_reply_by_email: true } + end + + it "includes the correct instructions" do + expect(builder.template_args[:respond_instructions]).to eq( + Email::MessageBuilder::INSTRUCTIONS_SEPARATOR + + I18n.t("user_notifications.only_reply_by_email_pm", builder.template_args), + ) + end + end + + context "for private_reply to system users" do + let(:other_opts) do + { + private_reply: true, + username: Discourse.system_user.username, + only_reply_by_email: true, + } + end + + it "only includes a button for respond_instructions" do + expect(builder.template_args[:respond_instructions]).to eq( + Email::MessageBuilder::INSTRUCTIONS_SEPARATOR + + I18n.t( + "user_notifications.only_reply_by_email_pm_button_only", + builder.template_args, + ), + ) + end + end + end + + context "when not only_reply_by_email" do + it "includes the correct instructions when allowing reply by email" do + SiteSetting.manual_polling_enabled = true + SiteSetting.reply_by_email_address = "test+%{reply_key}@test.com" + SiteSetting.reply_by_email_enabled = true + + expect(builder.template_args[:respond_instructions]).to eq( + Email::MessageBuilder::INSTRUCTIONS_SEPARATOR + + I18n.t("user_notifications.reply_by_email", builder.template_args), + ) + end + + it "includes the correct instructions when not allowing reply by email" do + expect(builder.template_args[:respond_instructions]).to eq( + Email::MessageBuilder::INSTRUCTIONS_SEPARATOR + + I18n.t("user_notifications.visit_link_to_respond", builder.template_args), + ) + end + + context "for private_reply to regular users" do + let(:other_opts) { { private_reply: true, username: "someguy" } } + + it "includes the correct instructions" do + expect(builder.template_args[:respond_instructions]).to eq( + Email::MessageBuilder::INSTRUCTIONS_SEPARATOR + + I18n.t("user_notifications.visit_link_to_respond_pm", builder.template_args), + ) + end + end + + context "for private_reply to system users" do + let(:other_opts) { { private_reply: true, username: Discourse.system_user.username } } + + it "only includes a button for respond_instructions" do + expect(builder.template_args[:respond_instructions]).to eq( + Email::MessageBuilder::INSTRUCTIONS_SEPARATOR + + I18n.t( + "user_notifications.visit_link_to_respond_pm_button_only", + builder.template_args, + ), + ) + end + end + end + end + end + describe "reply by email" do context "without allow_reply_by_email" do it "does not have a X-Discourse-Reply-Key" do @@ -165,7 +302,7 @@ RSpec.describe Email::MessageBuilder do end end - context "with allow_reply_by_email" do + context "with allow_reply_by_email and private_reply" do let(:reply_by_email_builder) do Email::MessageBuilder.new( to_address,