diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb index ec0a8c651..aba01b83c 100644 --- a/app/jobs/regular/user_email.rb +++ b/app/jobs/regular/user_email.rb @@ -175,11 +175,11 @@ module Jobs # If this email has a related post, don't send an email if it's been deleted or seen recently. def skip_email_for_post(post, user) if post - return I18n.t('email_log.topic_nil') if post.topic.blank? - return I18n.t('email_log.post_user_deleted') if post.user.blank? - return I18n.t('email_log.post_deleted') if post.user_deleted? - return I18n.t('email_log.user_suspended') if (user.suspended? && !post.user.try(:staff?)) - return I18n.t('email_log.already_read') if PostTiming.where(topic_id: post.topic_id, post_number: post.post_number, user_id: user.id).present? + return I18n.t('email_log.topic_nil') if post.topic.blank? + return I18n.t('email_log.post_user_deleted') if post.user.blank? + return I18n.t('email_log.post_deleted') if post.user_deleted? + return I18n.t('email_log.user_suspended') if (user.suspended? && !post.user.try(:staff?)) + return I18n.t('email_log.already_read') if PostTiming.where(topic_id: post.topic_id, post_number: post.post_number, user_id: user.id).present? else false end diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index a1b5361f0..284cd00ef 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -381,7 +381,7 @@ class UserNotifications < ActionMailer::Base context: context, username: username, add_unsubscribe_link: !user.staged, - add_unsubscribe_via_email_link: user.user_option.mailing_list_mode, + mailing_list_mode: user.user_option.mailing_list_mode, unsubscribe_url: post.topic.unsubscribe_url, allow_reply_by_email: allow_reply_by_email, only_reply_by_email: allow_reply_by_email && user.staged, diff --git a/app/views/email/notification.html.erb b/app/views/email/notification.html.erb index 4fcf2c512..5e33b4625 100644 --- a/app/views/email/notification.html.erb +++ b/app/views/email/notification.html.erb @@ -27,7 +27,7 @@ <% end %> - + diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index b202b9a99..348f831cd 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2105,10 +2105,19 @@ en: [Please review and fix them](%{base_url}/admin). unsubscribe_link: | - To stop receiving notifications for this particular topic, [click here](%{unsubscribe_url}). To unsubscribe from these emails, change your [user preferences](%{user_preferences_url}) + To stop receiving notifications for this particular topic, [click here](%{unsubscribe_url}). - unsubscribe_via_email_link: | - or, [click here](mailto:reply@%{hostname}?subject=unsubscribe) to unsubscribe via email. + To unsubscribe from these emails, change your [user preferences](%{user_preferences_url}). + + unsubscribe_link_and_mail: | + To stop receiving notifications for this particular topic, [click here](%{unsubscribe_url}). + + To unsubscribe from these emails, change your [user preferences](%{user_preferences_url}) or [click here](mailto:reply@%{hostname}?subject=unsubscribe) to unsubscribe via email. + + unsubscribe_mailing_list: | + You have mailing list mode enabled. To stop receiving notifications for this particular topic, [click here](%{unsubscribe_url}). + + To unsubscribe from these emails, change your [user preferences](%{user_preferences_url}) or [click here](mailto:reply@%{hostname}?subject=unsubscribe) to unsubscribe via email. subject_re: "Re: " subject_pm: "[PM] " @@ -2124,11 +2133,11 @@ en: description: "Not interested in getting these emails? No problem! Click below to unsubscribe instantly:" 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" - only_reply_by_email: "Reply to this email to respond" - visit_link_to_respond: "[Visit Topic](%{base_url}%{url}) to respond" - visit_link_to_respond_pm: "[Visit Message](%{base_url}%{url}) to respond" + 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." + only_reply_by_email: "Reply to this email to respond." + visit_link_to_respond: "[Visit Topic](%{base_url}%{url}) to respond." + visit_link_to_respond_pm: "[Visit Message](%{base_url}%{url}) to respond." posted_by: "Posted by %{username} on %{post_date}" diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb index 6f1210bb9..f099b11f1 100644 --- a/lib/email/message_builder.rb +++ b/lib/email/message_builder.rb @@ -28,10 +28,11 @@ module Email site_name: SiteSetting.email_prefix.presence || SiteSetting.title, base_url: Discourse.base_url, 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', locale: @opts[:locale]) + @template_args[:header_instructions] = I18n.t('user_notifications.header_instructions', @template_args) if @opts[:include_respond_instructions] == false @template_args[:respond_instructions] = '' @@ -44,6 +45,17 @@ module Email 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 + "unsubscribe_link" + end + @template_args[:unsubscribe_instructions] = I18n.t(unsubscribe_string, @template_args) + end end end @@ -51,13 +63,13 @@ module Email if @opts[:use_site_subject] subject = String.new(SiteSetting.email_subject) subject.gsub!("%{site_name}", @template_args[:site_name]) - subject.gsub!("%{optional_re}", @opts[:add_re_to_subject] ? I18n.t('subject_re', template_args) : '') - subject.gsub!("%{optional_pm}", @opts[:private_reply] ? I18n.t('subject_pm', template_args) : '') + subject.gsub!("%{optional_re}", @opts[:add_re_to_subject] ? I18n.t('subject_re', @template_args) : '') + subject.gsub!("%{optional_pm}", @opts[:private_reply] ? I18n.t('subject_pm', @template_args) : '') subject.gsub!("%{optional_cat}", @template_args[:show_category_in_subject] ? "[#{@template_args[:show_category_in_subject]}] " : '') subject.gsub!("%{topic_title}", @template_args[:topic_title]) if @template_args[:topic_title] # must be last for safety else subject = @opts[:subject] - subject = I18n.t("#{@opts[:template]}.subject_template", template_args) if @opts[:template] + subject = I18n.t("#{@opts[:template]}.subject_template", @template_args) if @opts[:template] end subject end @@ -65,42 +77,31 @@ module Email def html_part return unless html_override = @opts[:html_override] - if @opts[:add_unsubscribe_link] - unsubscribe_link = PrettyText.cook(I18n.t('unsubscribe_link', template_args), sanitize: false).html_safe - html_override.gsub!("%{unsubscribe_link}", unsubscribe_link) - - if SiteSetting.unsubscribe_via_email_footer && @opts[:add_unsubscribe_via_email_link] - unsubscribe_via_email_link = PrettyText.cook(I18n.t('unsubscribe_via_email_link', hostname: Discourse.current_hostname, locale: @opts[:locale]), sanitize: false).html_safe - html_override.gsub!("%{unsubscribe_via_email_link}", unsubscribe_via_email_link) - else - html_override.gsub!("%{unsubscribe_via_email_link}", "") - end + if @template_args[:unsubscribe_instructions].present? + unsubscribe_instructions = PrettyText.cook(@template_args[:unsubscribe_instructions], sanitize: false).html_safe + html_override.gsub!("%{unsubscribe_instructions}", unsubscribe_instructions) else - html_override.gsub!("%{unsubscribe_link}", "") - html_override.gsub!("%{unsubscribe_via_email_link}", "") + html_override.gsub!("%{unsubscribe_instructions}", "") end - header_instructions = @template_args[:header_instructions] - if header_instructions.present? - header_instructions = PrettyText.cook(header_instructions, sanitize: false).html_safe + if @template_args[:header_instructions].present? + header_instructions = PrettyText.cook(@template_args[:header_instructions], sanitize: false).html_safe html_override.gsub!("%{header_instructions}", header_instructions) else html_override.gsub!("%{header_instructions}", "") end - if response_instructions = @template_args[:respond_instructions] - respond_instructions = PrettyText.cook(response_instructions, sanitize: false).html_safe + if @template_args[:respond_instructions].present? + respond_instructions = PrettyText.cook(@template_args[:respond_instructions], sanitize: false).html_safe html_override.gsub!("%{respond_instructions}", respond_instructions) else html_override.gsub!("%{respond_instructions}", "") end - styled = Email::Styles.new(html_override, @opts) styled.format_basic - if style = @opts[:style] - styled.send "format_#{style}" + styled.send("format_#{style}") end Mail::Part.new do @@ -113,23 +114,22 @@ module Email body = @opts[:body] body = I18n.t("#{@opts[:template]}.text_body_template", template_args).dup if @opts[:template] - if @opts[:add_unsubscribe_link] + if @template_args[:unsubscribe_instructions].present? body << "\n" - body << I18n.t('unsubscribe_link', template_args) - if SiteSetting.unsubscribe_via_email_footer && @opts[:add_unsubscribe_via_email_link] - body << I18n.t('unsubscribe_via_email_link', hostname: Discourse.current_hostname, locale: @opts[:locale]) - end + body << @template_args[:unsubscribe_instructions] end body end def build_args - { to: @to, + { + to: @to, subject: subject, body: body, charset: 'UTF-8', - from: from_value } + from: from_value + } end def header_args diff --git a/spec/components/email/message_builder_spec.rb b/spec/components/email/message_builder_spec.rb index 15a0a8e79..9e0e7990f 100644 --- a/spec/components/email/message_builder_spec.rb +++ b/spec/components/email/message_builder_spec.rb @@ -180,6 +180,7 @@ describe Email::MessageBuilder do let(:message_with_unsubscribe) { Email::MessageBuilder.new(to_address, body: 'hello world', add_unsubscribe_link: true, + url: "/t/1234", unsubscribe_url: "/t/1234/unsubscribe") } it "has an List-Unsubscribe header" do @@ -190,23 +191,29 @@ describe Email::MessageBuilder do expect(message_with_unsubscribe.body).to match(builder.template_args[:user_preferences_url]) end - end - - context "with unsubscribe_via_email_link true" do - let(:message_with_unsubscribe_via_email) { Email::MessageBuilder.new(to_address, - body: 'hello world', - add_unsubscribe_link: true, - add_unsubscribe_via_email_link: true, - unsubscribe_url: "/t/1234/unsubscribe") } - it "can add an unsubscribe via email link" do SiteSetting.stubs(:unsubscribe_via_email_footer).returns(true) - expect(message_with_unsubscribe_via_email.body).to match(/mailto:reply@#{Discourse.current_hostname}\?subject=unsubscribe/) + expect(message_with_unsubscribe.body).to match(/mailto:reply@#{Discourse.current_hostname}\?subject=unsubscribe/) end it "does not add unsubscribe via email link without site setting set" do - expect(message_with_unsubscribe_via_email.body).to_not match(/mailto:reply@#{Discourse.current_hostname}\?subject=unsubscribe/) + expect(message_with_unsubscribe.body).to_not match(/mailto:reply@#{Discourse.current_hostname}\?subject=unsubscribe/) end + + end + + context "with mailing_list_mode enabled" do + let(:message_with_unsubscribe_via_email) { Email::MessageBuilder.new(to_address, + body: 'hello world', + add_unsubscribe_link: true, + mailing_list_mode: true, + url: "/t/1234", + unsubscribe_url: "/t/1234/unsubscribe") } + + it "add an unsubscribe via email link" do + expect(message_with_unsubscribe_via_email.body).to match(/mailto:reply@#{Discourse.current_hostname}\?subject=unsubscribe/) + end + end end