From 93bbe190c0b5cef5c33000c3ed61963452440c21 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 10 Jun 2013 15:33:37 -0400 Subject: [PATCH] Moved Email components into a module --- app/controllers/admin/email_controller.rb | 4 +- app/mailers/invite_mailer.rb | 4 +- app/mailers/test_mailer.rb | 4 +- app/mailers/user_notifications.rb | 4 +- lib/email.rb | 4 ++ lib/email/builder.rb | 32 ++++++++++++ lib/email/renderer.rb | 38 ++++++++++++++ lib/email/sender.rb | 49 +++++++++++++++++++ lib/email/styles.rb | 44 +++++++++++++++++ lib/email_builder.rb | 28 ----------- lib/email_renderer.rb | 36 -------------- lib/email_sender.rb | 47 ------------------ lib/email_styles.rb | 42 ---------------- lib/jobs/invite_email.rb | 4 +- lib/jobs/test_email.rb | 4 +- lib/jobs/user_email.rb | 4 +- .../{ => email}/email_sender_spec.rb | 14 +++--- spec/components/{ => email}/email_spec.rb | 0 .../{ => email}/email_styles_spec.rb | 8 +-- spec/components/jobs/invite_email_spec.rb | 2 +- spec/components/jobs/test_email_spec.rb | 2 +- spec/components/jobs/user_email_spec.rb | 18 +++---- 22 files changed, 203 insertions(+), 189 deletions(-) create mode 100644 lib/email/builder.rb create mode 100644 lib/email/renderer.rb create mode 100644 lib/email/sender.rb create mode 100644 lib/email/styles.rb delete mode 100644 lib/email_builder.rb delete mode 100644 lib/email_renderer.rb delete mode 100644 lib/email_sender.rb delete mode 100644 lib/email_styles.rb rename spec/components/{ => email}/email_sender_spec.rb (87%) rename spec/components/{ => email}/email_spec.rb (100%) rename spec/components/{ => email}/email_styles_spec.rb (76%) diff --git a/app/controllers/admin/email_controller.rb b/app/controllers/admin/email_controller.rb index 7353fc2a3..eaa1f853f 100644 --- a/app/controllers/admin/email_controller.rb +++ b/app/controllers/admin/email_controller.rb @@ -1,4 +1,4 @@ -require_dependency 'email_renderer' +require_dependency 'email/renderer' class Admin::EmailController < Admin::AdminController @@ -30,7 +30,7 @@ class Admin::EmailController < Admin::AdminController def preview_digest params.require(:last_seen_at) - renderer = EmailRenderer.new(UserNotifications.digest(current_user, since: params[:last_seen_at]), html_template: true) + renderer = Email::Renderer.new(UserNotifications.digest(current_user, since: params[:last_seen_at]), html_template: true) render json: MultiJson.dump(html_content: renderer.html, text_content: renderer.text) end diff --git a/app/mailers/invite_mailer.rb b/app/mailers/invite_mailer.rb index c6644ddb0..ac6336fe8 100644 --- a/app/mailers/invite_mailer.rb +++ b/app/mailers/invite_mailer.rb @@ -1,8 +1,8 @@ -require_dependency 'email_builder' +require_dependency 'email/builder' class InviteMailer < ActionMailer::Base default charset: 'UTF-8' - include EmailBuilder + include Email::Builder def send_invite(invite) diff --git a/app/mailers/test_mailer.rb b/app/mailers/test_mailer.rb index 132471d1d..2a7f2a871 100644 --- a/app/mailers/test_mailer.rb +++ b/app/mailers/test_mailer.rb @@ -1,8 +1,8 @@ -require_dependency 'email_builder' +require_dependency 'email/builder' class TestMailer < ActionMailer::Base default charset: 'UTF-8' - include EmailBuilder + include Email::Builder def send_test(to_address) build_email to_address, 'test_mailer' diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index ee80633a2..e347e8730 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -1,10 +1,10 @@ require_dependency 'markdown_linker' -require_dependency 'email_builder' +require_dependency 'email/builder' class UserNotifications < ActionMailer::Base default charset: 'UTF-8' - include EmailBuilder + include Email::Builder def signup(user, opts={}) build_email(user.email, "user_notifications.signup", email_token: opts[:email_token]) diff --git a/lib/email.rb b/lib/email.rb index 1f19cb33d..bab4a99c7 100644 --- a/lib/email.rb +++ b/lib/email.rb @@ -1,4 +1,8 @@ require 'mail' +require_dependency 'email/builder' +require_dependency 'email/renderer' +require_dependency 'email/sender' +require_dependency 'email/styles' module Email diff --git a/lib/email/builder.rb b/lib/email/builder.rb new file mode 100644 index 000000000..d68b73742 --- /dev/null +++ b/lib/email/builder.rb @@ -0,0 +1,32 @@ +# Help us build an email +module Email + + module Builder + + def build_email(to, email_key, params={}) + params[:site_name] = SiteSetting.title + params[:base_url] = Discourse.base_url + params[:user_preferences_url] = "#{Discourse.base_url}/user_preferences" + + body = I18n.t("#{email_key}.text_body_template", params) + + # Are we appending an unsubscribe link? + if params[:add_unsubscribe_link] + body << "\n" + body << I18n.t("unsubscribe_link", params) + headers 'List-Unsubscribe' => "<#{params[:user_preferences_url]}>" + end + + mail_args = { + to: to, + subject: I18n.t("#{email_key}.subject_template", params), + body: body + } + mail_args[:from] = params[:from] || SiteSetting.notification_email + mail_args[:charset] = 'UTF-8' + mail(mail_args) + end + + end + +end diff --git a/lib/email/renderer.rb b/lib/email/renderer.rb new file mode 100644 index 000000000..f3bf7575b --- /dev/null +++ b/lib/email/renderer.rb @@ -0,0 +1,38 @@ +require_dependency 'email/styles' + +module Email + class Renderer + + def initialize(message, opts=nil) + @message = message + @opts = opts || {} + end + + def text + @text ||= @message.body.to_s.force_encoding('UTF-8') + end + + def logo_url + logo_url = SiteSetting.logo_url + if logo_url !~ /http(s)?\:\/\// + logo_url = "#{Discourse.base_url}#{logo_url}" + end + logo_url + end + + def html + cooked = PrettyText.cook(text, environment: 'email') + + if @opts[:html_template] + ActionView::Base.new(Rails.configuration.paths["app/views"]).render( + template: 'email/template', + format: :html, + locals: { html_body: Email::Styles.new(cooked).format, logo_url: logo_url } + ) + else + cooked + end + end + + end +end \ No newline at end of file diff --git a/lib/email/sender.rb b/lib/email/sender.rb new file mode 100644 index 000000000..d0ffc31a9 --- /dev/null +++ b/lib/email/sender.rb @@ -0,0 +1,49 @@ +# +# A helper class to send an email. It will also handle a nil message, which it considers +# to be "do nothing". This is because some Mailers will decide not to do work for some +# reason. For example, emailing a user too frequently. A nil to address is also considered +# "do nothing" +# +# It also adds an HTML part for the plain text body +# +require_dependency 'email/renderer' + +module Email + class Sender + + def initialize(message, email_type, user=nil) + @message = message + @email_type = email_type + @user = user + end + + def send + return if @message.blank? + return if @message.to.blank? + return if @message.body.blank? + + @message.charset = 'UTF-8' + + opts = {} + + # Only use the html template on digest emails + opts[:html_template] = true if (@email_type == 'digest') + + renderer = Email::Renderer.new(@message, opts) + + @message.html_part = Mail::Part.new do + content_type 'text/html; charset=UTF-8' + body renderer.html + end + + @message.text_part.content_type = 'text/plain; charset=UTF-8' + @message.deliver + + to_address = @message.to + to_address = to_address.first if to_address.is_a?(Array) + + EmailLog.create!(email_type: @email_type, to_address: to_address, user_id: @user.try(:id)) + end + + end +end \ No newline at end of file diff --git a/lib/email/styles.rb b/lib/email/styles.rb new file mode 100644 index 000000000..b2ee0ddb3 --- /dev/null +++ b/lib/email/styles.rb @@ -0,0 +1,44 @@ +# +# HTML emails don't support CSS, so we can use nokogiri to inline attributes based on +# matchers. +# +module Email + class Styles + + def initialize(html) + @html = html + end + + def format + fragment = Nokogiri::HTML.fragment(@html) + + fragment.css('h3').each do |h3| + h3['style'] = 'margin: 15px 0 20px 0; border-bottom: 1px solid #ddd;' + end + + fragment.css('hr').each do |hr| + hr['style'] = 'background-color: #ddd; height: 1px; border: 1px;' + end + + fragment.css('a').each do |a| + a['style'] = 'text-decoration: none; font-weight: bold; font-size: 15px; color: #006699;' + end + + fragment.css('ul').each do |ul| + ul['style'] = 'margin: 0 0 0 10px; padding: 0 0 0 20px;' + end + + fragment.css('li').each do |li| + li['style'] = 'padding-bottom: 10px' + end + + fragment.css('pre').each do |pre| + pre.replace(pre.text) + end + + fragment.to_html + end + + + end +end \ No newline at end of file diff --git a/lib/email_builder.rb b/lib/email_builder.rb deleted file mode 100644 index ee219e3e4..000000000 --- a/lib/email_builder.rb +++ /dev/null @@ -1,28 +0,0 @@ -# Help us build an email -module EmailBuilder - - def build_email(to, email_key, params={}) - params[:site_name] = SiteSetting.title - params[:base_url] = Discourse.base_url - params[:user_preferences_url] = "#{Discourse.base_url}/user_preferences" - - body = I18n.t("#{email_key}.text_body_template", params) - - # Are we appending an unsubscribe link? - if params[:add_unsubscribe_link] - body << "\n" - body << I18n.t("unsubscribe_link", params) - headers 'List-Unsubscribe' => "<#{params[:user_preferences_url]}>" - end - - mail_args = { - to: to, - subject: I18n.t("#{email_key}.subject_template", params), - body: body - } - mail_args[:from] = params[:from] || SiteSetting.notification_email - mail_args[:charset] = 'UTF-8' - mail(mail_args) - end - -end diff --git a/lib/email_renderer.rb b/lib/email_renderer.rb deleted file mode 100644 index b6951c6e6..000000000 --- a/lib/email_renderer.rb +++ /dev/null @@ -1,36 +0,0 @@ -require_dependency 'email_styles' - -class EmailRenderer - - def initialize(message, opts=nil) - @message = message - @opts = opts || {} - end - - def text - @text ||= @message.body.to_s.force_encoding('UTF-8') - end - - def logo_url - logo_url = SiteSetting.logo_url - if logo_url !~ /http(s)?\:\/\// - logo_url = "#{Discourse.base_url}#{logo_url}" - end - logo_url - end - - def html - cooked = PrettyText.cook(text, environment: 'email') - - if @opts[:html_template] - ActionView::Base.new(Rails.configuration.paths["app/views"]).render( - template: 'email/template', - format: :html, - locals: { html_body: EmailStyles.new(cooked).format, logo_url: logo_url } - ) - else - cooked - end - end - -end diff --git a/lib/email_sender.rb b/lib/email_sender.rb deleted file mode 100644 index 6e6613b02..000000000 --- a/lib/email_sender.rb +++ /dev/null @@ -1,47 +0,0 @@ -# -# A helper class to send an email. It will also handle a nil message, which it considers -# to be "do nothing". This is because some Mailers will decide not to do work for some -# reason. For example, emailing a user too frequently. A nil to address is also considered -# "do nothing" -# -# It also adds an HTML part for the plain text body -# -require_dependency 'email_renderer' - -class EmailSender - - def initialize(message, email_type, user=nil) - @message = message - @email_type = email_type - @user = user - end - - def send - return if @message.blank? - return if @message.to.blank? - return if @message.body.blank? - - @message.charset = 'UTF-8' - - opts = {} - - # Only use the html template on digest emails - opts[:html_template] = true if (@email_type == 'digest') - - renderer = EmailRenderer.new(@message, opts) - - @message.html_part = Mail::Part.new do - content_type 'text/html; charset=UTF-8' - body renderer.html - end - - @message.text_part.content_type = 'text/plain; charset=UTF-8' - @message.deliver - - to_address = @message.to - to_address = to_address.first if to_address.is_a?(Array) - - EmailLog.create!(email_type: @email_type, to_address: to_address, user_id: @user.try(:id)) - end - -end diff --git a/lib/email_styles.rb b/lib/email_styles.rb deleted file mode 100644 index 20aed22b6..000000000 --- a/lib/email_styles.rb +++ /dev/null @@ -1,42 +0,0 @@ -# -# HTML emails don't support CSS, so we can use nokogiri to inline attributes based on -# matchers. -# -class EmailStyles - - def initialize(html) - @html = html - end - - def format - fragment = Nokogiri::HTML.fragment(@html) - - fragment.css('h3').each do |h3| - h3['style'] = 'margin: 15px 0 20px 0; border-bottom: 1px solid #ddd;' - end - - fragment.css('hr').each do |hr| - hr['style'] = 'background-color: #ddd; height: 1px; border: 1px;' - end - - fragment.css('a').each do |a| - a['style'] = 'text-decoration: none; font-weight: bold; font-size: 15px; color: #006699;' - end - - fragment.css('ul').each do |ul| - ul['style'] = 'margin: 0 0 0 10px; padding: 0 0 0 20px;' - end - - fragment.css('li').each do |li| - li['style'] = 'padding-bottom: 10px' - end - - fragment.css('pre').each do |pre| - pre.replace(pre.text) - end - - fragment.to_html - end - - -end diff --git a/lib/jobs/invite_email.rb b/lib/jobs/invite_email.rb index 22b9f0fb4..efaca7af4 100644 --- a/lib/jobs/invite_email.rb +++ b/lib/jobs/invite_email.rb @@ -1,4 +1,4 @@ -require_dependency 'email_sender' +require_dependency 'email/sender' module Jobs @@ -10,7 +10,7 @@ module Jobs invite = Invite.where(id: args[:invite_id]).first message = InviteMailer.send_invite(invite) - EmailSender.new(message, :invite).send + Email::Sender.new(message, :invite).send end end diff --git a/lib/jobs/test_email.rb b/lib/jobs/test_email.rb index 75c7a75e4..7bf00f9a1 100644 --- a/lib/jobs/test_email.rb +++ b/lib/jobs/test_email.rb @@ -1,4 +1,4 @@ -require_dependency 'email_sender' +require_dependency 'email/sender' module Jobs @@ -10,7 +10,7 @@ module Jobs raise Discourse::InvalidParameters.new(:to_address) unless args[:to_address].present? message = TestMailer.send_test(args[:to_address]) - EmailSender.new(message, :test_message).send + Email::Sender.new(message, :test_message).send end end diff --git a/lib/jobs/user_email.rb b/lib/jobs/user_email.rb index 432c5a295..47f7430b0 100644 --- a/lib/jobs/user_email.rb +++ b/lib/jobs/user_email.rb @@ -1,4 +1,4 @@ -require_dependency 'email_sender' +require_dependency 'email/sender' module Jobs @@ -69,7 +69,7 @@ module Jobs message.to = [args[:to_address]] end - EmailSender.new(message, args[:type], user).send + Email::Sender.new(message, args[:type], user).send end diff --git a/spec/components/email_sender_spec.rb b/spec/components/email/email_sender_spec.rb similarity index 87% rename from spec/components/email_sender_spec.rb rename to spec/components/email/email_sender_spec.rb index 9b8f1b1e7..c2a693c26 100644 --- a/spec/components/email_sender_spec.rb +++ b/spec/components/email/email_sender_spec.rb @@ -1,23 +1,23 @@ require 'spec_helper' -require 'email_sender' +require 'email/sender' -describe EmailSender do +describe Email::Sender do it "doesn't deliver mail when the message is nil" do Mail::Message.any_instance.expects(:deliver).never - EmailSender.new(nil, :hello).send + Email::Sender.new(nil, :hello).send end it "doesn't deliver when the to address is nil" do message = Mail::Message.new(body: 'hello') message.expects(:deliver).never - EmailSender.new(message, :hello).send + Email::Sender.new(message, :hello).send end it "doesn't deliver when the body is nil" do message = Mail::Message.new(to: 'eviltrout@test.domain') message.expects(:deliver).never - EmailSender.new(message, :hello).send + Email::Sender.new(message, :hello).send end context 'with a valid message' do @@ -29,7 +29,7 @@ describe EmailSender do message end - let(:email_sender) { EmailSender.new(message, :valid_type) } + let(:email_sender) { Email::Sender.new(message, :valid_type) } it 'calls deliver' do message.expects(:deliver).once @@ -91,7 +91,7 @@ describe EmailSender do end let(:user) { Fabricate(:user) } - let(:email_sender) { EmailSender.new(message, :valid_type, user) } + let(:email_sender) { Email::Sender.new(message, :valid_type, user) } before do email_sender.send diff --git a/spec/components/email_spec.rb b/spec/components/email/email_spec.rb similarity index 100% rename from spec/components/email_spec.rb rename to spec/components/email/email_spec.rb diff --git a/spec/components/email_styles_spec.rb b/spec/components/email/email_styles_spec.rb similarity index 76% rename from spec/components/email_styles_spec.rb rename to spec/components/email/email_styles_spec.rb index 025dd78b2..8608d0721 100644 --- a/spec/components/email_styles_spec.rb +++ b/spec/components/email/email_styles_spec.rb @@ -1,16 +1,16 @@ require 'spec_helper' require 'email' -describe EmailStyles do +describe Email::Styles do def style_exists(html, css_rule) - fragment = Nokogiri::HTML.fragment(EmailStyles.new(html).format) + fragment = Nokogiri::HTML.fragment(Email::Styles.new(html).format) element = fragment.at(css_rule) expect(element["style"]).not_to be_blank end it "returns blank from an empty string" do - EmailStyles.new("").format.should be_blank + Email::Styles.new("").format.should be_blank end it "attaches a style to h3 tags" do @@ -34,7 +34,7 @@ describe EmailStyles do end it "removes pre tags but keeps their contents" do - expect(EmailStyles.new("
hello
").format).to eq("hello") + expect(Email::Styles.new("
hello
").format).to eq("hello") end end diff --git a/spec/components/jobs/invite_email_spec.rb b/spec/components/jobs/invite_email_spec.rb index 2815bb214..b7186cd35 100644 --- a/spec/components/jobs/invite_email_spec.rb +++ b/spec/components/jobs/invite_email_spec.rb @@ -15,7 +15,7 @@ describe Jobs::InviteEmail do let (:invite) { Fabricate(:invite) } it 'delegates to the test mailer' do - EmailSender.any_instance.expects(:send) + Email::Sender.any_instance.expects(:send) InviteMailer.expects(:send_invite).with(invite).returns(mailer) Jobs::InviteEmail.new.execute(invite_id: invite.id) end diff --git a/spec/components/jobs/test_email_spec.rb b/spec/components/jobs/test_email_spec.rb index 4f53940e1..a8881fe0a 100644 --- a/spec/components/jobs/test_email_spec.rb +++ b/spec/components/jobs/test_email_spec.rb @@ -13,7 +13,7 @@ describe Jobs::TestEmail do let (:mailer) { Mail::Message.new(to: 'eviltrout@test.domain') } it 'delegates to the test mailer' do - EmailSender.any_instance.expects(:send) + Email::Sender.any_instance.expects(:send) TestMailer.expects(:send_test).with('eviltrout@test.domain').returns(mailer) Jobs::TestEmail.new.execute(to_address: 'eviltrout@test.domain') end diff --git a/spec/components/jobs/user_email_spec.rb b/spec/components/jobs/user_email_spec.rb index 79f117999..ae43956b5 100644 --- a/spec/components/jobs/user_email_spec.rb +++ b/spec/components/jobs/user_email_spec.rb @@ -31,7 +31,7 @@ describe Jobs::UserEmail do context 'to_address' do it 'overwrites a to_address when present' do UserNotifications.expects(:authorize_email).returns(mailer) - EmailSender.any_instance.expects(:send) + Email::Sender.any_instance.expects(:send) Jobs::UserEmail.new.execute(type: :authorize_email, user_id: user.id, to_address: 'jake@adventuretime.ooo') mailer.to.should == ['jake@adventuretime.ooo'] end @@ -42,7 +42,7 @@ describe Jobs::UserEmail do it "doesn't send an email to a user that's been recently seen" do user.update_column(:last_seen_at, 9.minutes.ago) - EmailSender.any_instance.expects(:send).never + Email::Sender.any_instance.expects(:send).never Jobs::UserEmail.new.execute(type: :private_message, user_id: user.id, post_id: post.id) end end @@ -51,7 +51,7 @@ describe Jobs::UserEmail do it 'passes a token as an argument when a token is present' do UserNotifications.expects(:forgot_password).with(user, {email_token: 'asdfasdf'}).returns(mailer) - EmailSender.any_instance.expects(:send) + Email::Sender.any_instance.expects(:send) Jobs::UserEmail.new.execute(type: :forgot_password, user_id: user.id, email_token: 'asdfasdf') end @@ -60,18 +60,18 @@ describe Jobs::UserEmail do it 'passes a post as an argument when a post_id is present' do UserNotifications.expects(:private_message).with(user, {post: post}).returns(mailer) - EmailSender.any_instance.expects(:send) + Email::Sender.any_instance.expects(:send) Jobs::UserEmail.new.execute(type: :private_message, user_id: user.id, post_id: post.id) end it "doesn't send the email if you've seen the post" do - EmailSender.any_instance.expects(:send).never + Email::Sender.any_instance.expects(:send).never PostTiming.record_timing(topic_id: post.topic_id, user_id: user.id, post_number: post.post_number, msecs: 6666) Jobs::UserEmail.new.execute(type: :private_message, user_id: user.id, post_id: post.id) end it "doesn't send the email if the user deleted the post" do - EmailSender.any_instance.expects(:send).never + Email::Sender.any_instance.expects(:send).never post.update_column(:user_deleted, true) Jobs::UserEmail.new.execute(type: :private_message, user_id: user.id, post_id: post.id) end @@ -84,19 +84,19 @@ describe Jobs::UserEmail do let!(:notification) { Fabricate(:notification, user: user, topic: post.topic, post_number: post.post_number)} it 'passes a notification as an argument when a notification_id is present' do - EmailSender.any_instance.expects(:send) + Email::Sender.any_instance.expects(:send) UserNotifications.expects(:user_mentioned).with(user, notification: notification, post: post).returns(mailer) Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id) end it "doesn't send the email if the notification has been seen" do - EmailSender.any_instance.expects(:send).never + Email::Sender.any_instance.expects(:send).never notification.update_column(:read, true) Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id) end it "doesn't send the email if the post has been user deleted" do - EmailSender.any_instance.expects(:send).never + Email::Sender.any_instance.expects(:send).never post.update_column(:user_deleted, true) Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id) end