From 8c8645f1583cd9970e08d18f28c4f777d70a20f8 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Thu, 28 Nov 2013 17:20:56 -0500 Subject: [PATCH] FIX: Code and Emoticon formatting in HTML emails. --- Gemfile | 2 + Gemfile_rails4.lock | 2 + app/controllers/admin/email_controller.rb | 2 +- app/helpers/user_notifications_helper.rb | 16 +++++++ app/views/user_notifications/digest.html.erb | 47 ++++++++++++++++++++ app/views/user_notifications/digest.text.erb | 9 ++-- lib/email/renderer.rb | 28 ++++-------- lib/email/sender.rb | 10 +++-- lib/email/styles.rb | 9 ++-- spec/components/email/styles_spec.rb | 8 +--- spec/mailers/user_notifications_spec.rb | 10 ++++- 11 files changed, 100 insertions(+), 43 deletions(-) create mode 100644 app/views/user_notifications/digest.html.erb diff --git a/Gemfile b/Gemfile index e4c993cb252..05fc760bc8b 100644 --- a/Gemfile +++ b/Gemfile @@ -59,6 +59,8 @@ gem 'redis', :require => ["redis", "redis/connection/hiredis"] gem 'active_model_serializers' +gem 'truncate_html' + # we had issues with latest, stick to the rev till we figure this out # PR that makes it all hang together welcome gem 'ember-rails' diff --git a/Gemfile_rails4.lock b/Gemfile_rails4.lock index bedecffc9f3..3a5acf86b28 100644 --- a/Gemfile_rails4.lock +++ b/Gemfile_rails4.lock @@ -442,6 +442,7 @@ GEM polyglot polyglot (>= 0.3.1) trollop (2.0) + truncate_html (0.9.2) tzinfo (0.3.38) uglifier (2.3.1) execjs (>= 0.3.0) @@ -543,6 +544,7 @@ DEPENDENCIES therubyracer thin timecop + truncate_html uglifier unf unicorn diff --git a/app/controllers/admin/email_controller.rb b/app/controllers/admin/email_controller.rb index 9450a448e68..7a7a4260b90 100644 --- a/app/controllers/admin/email_controller.rb +++ b/app/controllers/admin/email_controller.rb @@ -22,7 +22,7 @@ class Admin::EmailController < Admin::AdminController def preview_digest params.require(:last_seen_at) - renderer = Email::Renderer.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])) render json: MultiJson.dump(html_content: renderer.html, text_content: renderer.text) end diff --git a/app/helpers/user_notifications_helper.rb b/app/helpers/user_notifications_helper.rb index 79a9861d0d2..08939cf46de 100644 --- a/app/helpers/user_notifications_helper.rb +++ b/app/helpers/user_notifications_helper.rb @@ -17,4 +17,20 @@ module UserNotificationsHelper fragment.to_html.html_safe 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_site_link + "#{@site_name}" + end + + def email_excerpt(html) + html_string = TruncateHtml::HtmlString.new(html) + raw Sanitize.clean(TruncateHtml::HtmlTruncator.new(html_string, length: 1000).truncate, Sanitize::Config::RELAXED) + end end diff --git a/app/views/user_notifications/digest.html.erb b/app/views/user_notifications/digest.html.erb new file mode 100644 index 00000000000..1937897327c --- /dev/null +++ b/app/views/user_notifications/digest.html.erb @@ -0,0 +1,47 @@ + + + + + + + +
+ + +
+ <%= raw(t 'user_notifications.digest.why', site_link: html_site_link, last_seen_at: @last_seen_at) %> + + <%- if @featured_topics.present? %> +

<%=t 'user_notifications.digest.top_topics' %>

+ + <%- @featured_topics.each_with_index do |t, i| %> + <%= link_to t.title, t.relative_url %> + + <%- if t.best_post.present? %> +
+ <%= email_excerpt(t.best_post.cooked) %> +
+ <%- end %> + + <%- if i < @featured_topics.size - 1 %>
<% end %> + + <%- end %> + <%- end %> + + <%- if @new_topics.present? %> +

<%=t 'user_notifications.digest.other_new_topics' %>

+ + <%- @new_topics.each do |t| %> +
    +
  • <%= link_to t.title, t.relative_url %> <%- if t.category %>[<%= t.category.name %>]<%- end %>
    • +
+ <%- end -%> + + <%- end -%> + + + <%=raw(t :'user_notifications.digest.unsubscribe', + site_link: html_site_link, + unsubscribe_link: link_to(t('user_notifications.digest.click_here'), email_unsubscribe_path(key: @user.temporary_key))) %> + +
diff --git a/app/views/user_notifications/digest.text.erb b/app/views/user_notifications/digest.text.erb index 65f5754f4fd..edf9622ddd3 100644 --- a/app/views/user_notifications/digest.text.erb +++ b/app/views/user_notifications/digest.text.erb @@ -10,11 +10,10 @@ <%= raw(@markdown_linker.create(t.title, t.relative_url)) %> <%- if t.best_post.present? %> -
<%= raw(t.best_post.excerpt(1000, + <%= raw(t.best_post.excerpt(1000, strip_links: true, text_entities: true, - markdown_images: true)) %>
- + markdown_images: true)) %> -------------------------------------------------------------------------------- <%- end %> @@ -35,9 +34,9 @@ <%= raw(@markdown_linker.references) %> -<%=raw(t :'user_notifications.digest.unsubscribe', +<%=raw(t :'user_notifications.digest.unsubscribe', site_link: site_link, - unsubscribe_link: raw(@markdown_linker.create(t('user_notifications.digest.click_here'), email_unsubscribe_path(key: @user.temporary_key)))) %> + unsubscribe_link: raw(@markdown_linker.create(t('user_notifications.digest.click_here'), email_unsubscribe_path(key: @user.temporary_key)))) %> <%= raw(@markdown_linker.references) %> diff --git a/lib/email/renderer.rb b/lib/email/renderer.rb index 61894888492..a22a72571ae 100644 --- a/lib/email/renderer.rb +++ b/lib/email/renderer.rb @@ -9,32 +9,22 @@ module Email 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 + return @text if @text + @text = (@message.text_part ? @message.text_part : @message).body.to_s.force_encoding('UTF-8') end def html - style = Email::Styles.new(PrettyText.cook(text)) - style.format_basic - if @opts[:html_template] + if @message.html_part + style = Email::Styles.new(@message.html_part.body.to_s) + style.format_basic style.format_html - - ActionView::Base.new(Rails.configuration.paths["app/views"]).render( - template: 'email/template', - format: :html, - locals: { html_body: style.to_html, logo_url: logo_url } - ) else - style.to_html + style = Email::Styles.new(PrettyText.cook(text)) + style.format_basic end + + style.to_html end end diff --git a/lib/email/sender.rb b/lib/email/sender.rb index 6993740e2b6..957952bfc38 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -21,15 +21,17 @@ module Email def send return if @message.blank? return if @message.to.blank? - return if @message.body.blank? + + if @message.text_part + return if @message.text_part.body.to_s.blank? + else + return if @message.body.to_s.blank? + end @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) unless @message.html_part diff --git a/lib/email/styles.rb b/lib/email/styles.rb index 64a92900242..6dd86133bb8 100644 --- a/lib/email/styles.rb +++ b/lib/email/styles.rb @@ -13,7 +13,9 @@ module Email def format_basic @fragment.css('img').each do |img| - if img['src'] =~ /\/assets\/emoji\// + next if img['class'] == 'site-logo' + + if img['class'] == "emoji" || img['src'] =~ /plugins\/emoji/ img['width'] = 20 img['height'] = 20 else @@ -57,11 +59,6 @@ module Email style('div.digest-post', 'margin-left: 15px; margin-top: 20px; max-width: 694px;') style('div.digest-post h1', 'font-size: 20px;') style('span.footer-notice', 'color:#666; font-size:80%') - - @fragment.css('pre').each do |pre| - pre.replace(pre.text) - end - end def to_html diff --git a/spec/components/email/styles_spec.rb b/spec/components/email/styles_spec.rb index 9ba4e66d166..cfa75c8bae6 100644 --- a/spec/components/email/styles_spec.rb +++ b/spec/components/email/styles_spec.rb @@ -30,7 +30,7 @@ describe Email::Styles do end it "adds a width and height to images with an emoji path" do - frag = basic_fragment("") + frag = basic_fragment("") expect(frag.at("img")["width"]).to eq("20") expect(frag.at("img")["height"]).to eq("20") end @@ -85,12 +85,6 @@ describe Email::Styles do expect(frag.at('li')['style']).to be_present end - it "removes pre tags but keeps their contents" do - style = Email::Styles.new("
hello
") - style.format_basic - style.format_html - expect(style.to_html).to eq("hello") - end end diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index 2361761c955..a98af2af70c 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -37,7 +37,15 @@ describe UserNotifications do its(:to) { should == [user.email] } its(:subject) { should be_present } its(:from) { should == [SiteSetting.notification_email] } - its(:body) { should be_present } + + it 'should have a html body' do + subject.html_part.body.to_s.should be_present + end + + it 'should have a text body' do + subject.html_part.body.to_s.should be_present + end + end end