resolved html emails including html tags

fixes CNVS-5493

test steps:

Trigger the following emails and ensure that the HTML and plain text
versions of the email appear proper.

* conversation message received
* new announcement
* new discussion entry
* new discussion topic
* updated wiki page
* summary (daily or weekly)

Change-Id: I9638badb500eeb6cb23e4a83bd8395cd5dc40608
Reviewed-on: https://gerrit.instructure.com/20296
Reviewed-by: Jon Jensen <jon@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: Eric Berry <ericb@instructure.com>
QA-Review: Eric Berry <ericb@instructure.com>

Gemfile

@@ -81,6 +81,7 @@ gem 'xml-simple',     '1.0.12', :require => 'xmlsimple'
 gem 'yui-compressor', '0.9.4'
 gem 'foreigner',      '0.9.2'
 gem 'crocodoc-ruby',  '0.0.1', :require => 'crocodoc'
+gem 'regru-premailer', '1.7.7', :require => 'premailer'
 
 group :assets do
   gem 'compass-rails', '1.0.3'

app/messages/conversation_message.email.erb

@@ -11,7 +11,6 @@
 
 <%= asset.body %>
 
-
 <% if asset.has_media_objects? %>
 <%= t :audio_comment, "This message includes media comments. To listen or reply, click the link below:" %>
 <% else %>

app/messages/conversation_message.email.html.erb

@@ -17,7 +17,7 @@
     background-image:     -ms-linear-gradient(left, #ddd, #aaa, #eee);
     background-image:      -o-linear-gradient(left, #ddd, #aaa, #eee);" />
 
-<p><%= asset.body.gsub(/\n/,'</p><p>') %></p>
+<%= format_message(asset.body).first %>
 
 <hr style="border: 0;
     height: 1px;

app/messages/conversation_message.summary.erb

@@ -10,4 +10,4 @@
 
 <%= t :body, "%{user_name} just sent you a message in Canvas:", :user_name => content(:user_name) %>
 
-<%= strip_and_truncate(asset.body, :max_length => 200) %>
+<%= truncate_text(asset.body, :max_length => 200) %>

app/messages/new_announcement.email.erb

@@ -1,12 +1,11 @@
-<% define_content :link do %>
+<% define_content :link do -%>
   <%= HostUrl.protocol %>://<%= HostUrl.context_host(asset.context) %>/<%= asset.context.class.base_ar_class.to_s.downcase.pluralize %>/<%= asset.context_id %>/announcements/<%= asset.id %>
 <% end %>
-
-<% define_content :subject do %>
+<% define_content :subject do -%>
   <%= before_label asset.title %> <%= asset.context.name %>
 <% end %>
+<%= html_to_simple_text(asset.message, :base_url => "#{HostUrl.protocol}://#{HostUrl.context_host(asset.context)}") %>
 
-<%= html_to_text(asset.message) %>
 <% if asset.attachment %><%= before_label :file_included, "File Included" %> <%= asset.attachment.display_name %> - <%= asset.attachment.readable_size %>
   <%= HostUrl.protocol %>://<%= HostUrl.context_host(asset.context) %>/<%= asset.context.class.base_ar_class.to_s.downcase.pluralize %>/<%= asset.context_id %>/files/<%= asset.attachment_id %>/download<% end %>
 

app/messages/new_announcement.email.html.erb

@@ -6,7 +6,7 @@
   <%= before_label asset.title %> <%= asset.context.name %>
 <% end %>
 
-<p><%= asset.message.gsub(/\n/, '</p><p>') %></p>
+<%= html_to_simple_html(asset.message, :base_url => "#{HostUrl.protocol}://#{HostUrl.context_host(asset.context)}") %>
 
 <% if asset.attachment %>
 <div style="margin-top: 20px; padding: 10px; border: 1px solid #f1f1f1; background-color: #f3f3f3;">

app/messages/new_announcement.summary.erb

@@ -6,4 +6,4 @@
   <%= before_label asset.title %> <%= asset.context.name %>
 <% end %>
 
-<%= strip_and_truncate(asset.message, :max_length => 500) %> 
+<%= truncate_text(html_to_simple_text(asset.message, :base_url => "#{HostUrl.protocol}://#{HostUrl.context_host(asset.context)}"), :max_length => 500) %> 

app/messages/new_discussion_entry.email.erb

@@ -8,7 +8,7 @@
 
 <%= t :body, "%{user} posted a new comment on the thread %{discussion_topic} for %{course}:", :user => asset.user.short_name, :discussion_topic => asset.title, :course => asset.context.name %>
 
-<%= html_to_text(asset.message) %>
+<%= html_to_simple_text(asset.message, :base_url => "#{HostUrl.protocol}://#{HostUrl.context_host(asset.context)}") %>
 
 <%= t :details, "Join the conversation here:" %>
 <%= content :link %>

app/messages/new_discussion_entry.email.html.erb

@@ -16,7 +16,7 @@
     background-image:     -ms-linear-gradient(left, #ddd, #aaa, #eee);
     background-image:      -o-linear-gradient(left, #ddd, #aaa, #eee);" />
 
-<p><%= html_to_text(asset.message).gsub(/\n/, '</p><p>') %></p>
+<%= html_to_simple_html(asset.message, :base_url => "#{HostUrl.protocol}://#{HostUrl.context_host(asset.context)}") %>
 
 <hr style="border: 0;
     height: 1px;

app/messages/new_discussion_entry.summary.erb

@@ -6,4 +6,4 @@
   <%= t :subject, "New Comment for %{discussion_topic}: %{course}", :discussion_topic => asset.title, :course => asset.context.name %>
 <% end %>
 
-<%= strip_and_truncate(asset.message, :max_length => 200) %> 
+<%= truncate_text(html_to_simple_text(asset.message, :base_url => "#{HostUrl.protocol}://#{HostUrl.context_host(asset.context)}"), :max_length => 200) %> 

app/messages/new_discussion_entry.twitter.erb

@@ -1,7 +1,4 @@
 <% define_content :link do -%>
 <%= HostUrl.protocol %>://<%= HostUrl.context_host(asset.context) %>/<%= asset.context.class.to_s.downcase.pluralize %>/<%= asset.context_id %>/discussion_topics/<%= asset.discussion_topic_id %>
 <% end -%>
-<%= t :body, "Canvas Alert - Comment: %{discussion_topic}, %{course}.", :discussion_topic => asset.title, :course => asset.context.name %>
-
-<%= strip_and_truncate(asset.message, :max_length => 50) %>
-
+<%= t :body, "Canvas Alert - Comment: %{discussion_topic}, %{course}.", :discussion_topic => asset.title, :course => asset.context.name %>

app/messages/new_discussion_topic.email.erb

@@ -10,7 +10,7 @@
 
 <%= asset.title %> 
 
-<%= strip_and_truncate(asset.message, :max_length => 200) %> 
+<%= html_to_simple_text(asset.message, :base_url => "#{HostUrl.protocol}://#{HostUrl.context_host(asset.context)}") %> 
 <% if asset.attachment %><%= before_label :file_included, "File Included" %> <%= asset.attachment.display_name %> - <%= asset.attachment.readable_size %>
   <%= HostUrl.protocol %>://<%= HostUrl.context_host(asset.context) %>/<%= asset.context.class.to_s.downcase.pluralize %>/<%= asset.context_id %>/files/<%= asset.attachment_id %>/download<% end %>
 

app/messages/new_discussion_topic.email.html.erb

@@ -18,7 +18,7 @@
 
 <p><strong><%= asset.title %></strong></p>
 
-<p><%= strip_and_truncate(asset.message, :max_length => 5000).gsub(/\n/, '</p><p>') %></p>
+<%= html_to_simple_html(asset.message, :base_url => "#{HostUrl.protocol}://#{HostUrl.context_host(asset.context)}") %>
 
 <hr style="border: 0;
     height: 1px;

app/messages/new_discussion_topic.summary.erb

@@ -6,4 +6,4 @@
     <%= t :subject, "New Discussion - %{discussion_topic}: %{course}", :discussion_topic => asset.title, :course => asset.context.name %>
 <% end %>
 
-<%= strip_and_truncate(asset.message, :max_length => 200) %> 
+<%= truncate_text(html_to_simple_text(asset.message, :base_url => "#{HostUrl.protocol}://#{HostUrl.context_host(asset.context)}"), :max_length => 200) %> 

app/messages/summaries.email.erb

@@ -1,11 +1,9 @@
-<% define_content :link do %>
-  <%= HostUrl.protocol %>://<%= HostUrl.context_host(user.pseudonym.try(:account)) %>
-<% end %>
-
-<% define_content :subject do %>
+<% define_content :link do -%>
+  <%= HostUrl.protocol %>://<%= primary_host %>
+<% end -%>
+<% define_content :subject do -%>
   <%= t :subject, "Recent Canvas Notifications" %>
-<% end %>
-
+<% end -%>
 <% m = (delayed_messages || []).find{|m| m.frequency == Notification::FREQ_WEEKLY} || delayed_messages.first %>
 <% if !m || m.frequency == Notification::FREQ_DAILY %>
   <%= t :daily_body, "You're signed up to receive a daily report of some notifications from your Canvas account.  Below is the report for %{date}:", :date => (date_string(force_zone(m.send_at), :no_words) rescue t("#date.days.today_lower", "today")) %>
@@ -18,11 +16,11 @@
 
 <%= delayed_message.name_of_topic %>
 
-    <%= indent(delayed_message.summary, 4) %>
+    <%= indent(html_to_simple_text(delayed_message.summary, :base_url => content(:link)), 4) %>
   
     <%= delayed_message.link %>
 <% end %>
 
 
 <%= t :notifications_link, "You can change your notification settings by visiting the following page:" %>
-<%= HostUrl.protocol %>://<%= HostUrl.context_host(user.pseudonym.try(:account))  %>/profile/communication
+<%= content(:link)  %>/profile/communication

app/messages/summaries.email.html.erb

@@ -1,5 +1,5 @@
 <% define_content :link do %>
-  <%= HostUrl.protocol %>://<%= HostUrl.context_host(user.pseudonym.try(:account)) %>
+  <%= HostUrl.protocol %>://<%= primary_host %>
 <% end %>
 
 <% define_content :subject do %>
@@ -29,8 +29,8 @@
       background-image:     -ms-linear-gradient(left, #ddd, #aaa, #eee);
       background-image:      -o-linear-gradient(left, #ddd, #aaa, #eee);" />
 
-  <p><strong><%= delayed_message.name_of_topic %></strong></p>
-  <p><%= delayed_message.formatted_summary %></p>
+  <p><strong><%= html_to_simple_text(delayed_message.name_of_topic, :base_url => content(:link)) %></strong></p>
+  <p><%= html_to_simple_html(delayed_message.summary, :base_url => content(:link)) %></p>
   <p><a href="<%= delayed_message.link %>"><%= t :view, "Click to view" %></a></p>
 <% end %>
 
@@ -42,4 +42,4 @@
     background-image:     -ms-linear-gradient(left, #ddd, #aaa, #eee);
     background-image:      -o-linear-gradient(left, #ddd, #aaa, #eee);" />
 
-<p><a href="<%= HostUrl.protocol %>://<%= HostUrl.context_host(user.pseudonym.try(:account)) %>/profile/communication"><%= t 'click_to_preferences', "Click here to edit your notification preferences" %></a></p>
+<p><a href="<%= content(:link) %>/profile/communication"><%= t 'click_to_preferences', "Click here to edit your notification preferences" %></a></p>

app/messages/summaries.facebook.erb

@@ -1 +1 @@
-<%= t :new_notifications, "You've received *%{notification_count} new notifications* in #Canvas#.", :notification_count => delayed_messages.length, :wrapper => { "*" => '<b>\1</b>', "#" => "<a href=\"#{HostUrl.context_host(delayed_messages.first.try(:context))}/dashboard\">\\1</a>" } %>
+<%= t :new_notifications, "You've received *%{notification_count} new notifications* in #Canvas#.", :notification_count => delayed_messages.length, :wrapper => { "*" => '<b>\1</b>', "#" => "<a href=\"#{HostUrl.protocol}://#{primary_host}/dashboard\">\\1</a>" } %>

app/messages/summaries.sms.erb

@@ -1 +1,4 @@
-<%= t :notifications_sms, "%{notifications_count} new notifications at %{url}", :notifications_count => delayed_messages.length, :url => HostUrl.default_host %>
+<% define_content :link do -%>
+<%= HostUrl.protocol %>://<%= primary_host %>/profile/communication
+<% end %>
+<%= t :notifications_sms, "%{notifications_count} new notifications at %{url}", :notifications_count => delayed_messages.length, :url => content(:link) %>

app/messages/summaries.twitter.erb

@@ -1,4 +1,4 @@
 <% define_content :link do -%>
-<%= HostUrl.protocol %>://<%= HostUrl.context_host(user.pseudonym.try(:account)) %>/profile/communication
+<%= HostUrl.protocol %>://<%= primary_host %>/profile/communication
 <% end %>
 <%= t :tweet, "Canvas Alert - %{notification_count} new notifications", :notification_count => delayed_messages.length %>

app/messages/updated_wiki_page.email.erb

@@ -10,7 +10,7 @@
 
 <%= asset.title.titleize %> 
 
-<%= strip_and_truncate(asset.body, :max_length => 200) %> 
+<%= truncate_text(html_to_simple_text(asset.body, :base_url => "#{HostUrl.protocol}://#{HostUrl.context_host(asset.context)}"), :max_length => 200) %> 
 
 
 <%= t :link_message, "You can review it here: %{link}", :link => content(:link) %>

app/messages/updated_wiki_page.email.html.erb

@@ -8,8 +8,8 @@
 
 <p><%= t :body, "A page has been updated on the wiki for %{title} that may make your life easier.", :title => asset.context.name %></p>
 
-<p><strong><%= asset.title.titleize %></strong></p>
+<p><strong><%= html_to_simple_text(asset.title).titleize %></strong></p>
 
-<p><%= strip_and_truncate(asset.body, :max_length => 200) %></p>
+<p><%= truncate_text(html_to_simple_text(asset.body, :base_url => "#{HostUrl.protocol}://#{HostUrl.context_host(asset.context)}"), :max_length => 200) %></p>
 
 <p><a href="<%= content :link %>"><%= t :link_message, "Click here to review it" %></a></p>

app/messages/updated_wiki_page.sms.erb

@@ -1,5 +1,3 @@
 <%= t :updated_message, "%{title}, %{user} just updated:", :title => asset.title, :user => asset.context.name %>
 
-<%= strip_and_truncate(asset.body, :max_length => 50) %>
-
 <%= t :more_info, "More info at %{url}", :url => HostUrl.context_host(asset.context) %>

app/messages/updated_wiki_page.summary.erb

@@ -6,4 +6,4 @@
   <%= t :subject, "Updated Wiki Page: %{title}, %{user}", :title => asset.title.titleize, :user => asset.context.name %>
 <% end %>
 
-<%= strip_and_truncate(asset.body, :max_length => 200) %> 
+<%= truncate_text(html_to_simple_text(asset.body), :max_length => 200) %> 

lib/text_helper.rb

@@ -43,6 +43,22 @@ module TextHelper
     doc.text.strip
   end
 
+  # Converts a string of html to plain text using the Premailer gem.
+  def html_to_simple_text(html_str, opts={})
+    return "" if html_str.blank?
+    pm = Premailer.new(html_str, { :with_html_string => true, :input_encoding => 'UTF-8', :adapter => :nokogiri }.merge(opts))
+    pm.to_plain_text
+  end
+
+  def html_to_simple_html(html_str, opts={})
+    return "" if html_str.blank?
+    text = html_to_simple_text(html_str, opts)
+    text.gsub!(/^([\*\-]+\n*)$/, '') # Remove the H tag markers
+    text.gsub!(/\n{3,}/, "\n\n")     # Remove the triple breaks left by the H tag marker removals
+    html = format_message(text).first
+    "<p>#{html}</p>".html_safe
+  end
+
   def quote_clump(quote_lines)
     txt = "<div class='quoted_text_holder'><a href='#' class='show_quoted_text_link'>#{TextHelper.escape_html(I18n.t('lib.text_helper.quoted_text_toggle', "show quoted text"))}</a><div class='quoted_text' style='display: none;'>"
     txt += quote_lines.join("\n")
@@ -73,7 +89,8 @@ module TextHelper
   }xi
 
   # Converts a plaintext message to html, with newlinification, quotification, and linkification
-  def format_message(message, url=nil, notification_id=nil)
+  def format_message(message, opts={ :url => nil, :notification_id => nil })
+    return '' unless message
     # insert placeholders for the links we're going to generate, before we go and escape all the html
     links = []
     placeholder_blocks = []
@@ -85,7 +102,7 @@ module TextHelper
         s = $1
         link = s
         link = "http://#{link}" if link[0,3] == 'www'
-        link = add_notification_to_link(link, notification_id) if notification_id
+        link = add_notification_to_link(link, opts[:notification_id]) if opts[:notification_id]
         link = URI.escape(link).gsub("'", "%27")
         links << link
         "<a href='#{ERB::Util.h(link)}'>#{ERB::Util.h(s)}</a>"
@@ -116,11 +133,11 @@ module TextHelper
     end
     processed_lines << quote_clump(quote_block) if !quote_block.empty?
     message = processed_lines.join("\n")
-    if url
-      url = add_notification_to_link(url, notification_id) if notification_id
-      links.unshift url
+    if opts[:url]
+      url = add_notification_to_link(opts[:url], opts[:notification_id]) if opts[:notification_id]
+      links.unshift opts[:url]
     end
-    links.unshift message
+    links.unshift message.html_safe
   end
 
   def add_notification_to_link(url, notification_id)

spec/lib/text_helper_spec.rb

@@ -252,6 +252,82 @@ describe TextHelper do
     end
   end
 
+  describe "simplify html" do
+    before(:each) do
+      @body = <<-END.strip_heredoc.strip
+        <p><strong>This is a bold tag</strong></p>
+        <p><em>This is an em tag</em></p>
+        <p><h1>This is an h1 tag</h1></p>
+        <p><h2>This is an h2 tag</h2></p>
+        <p><h3>This is an h3 tag</h3></p>
+        <p><h4>This is an h4 tag</h4></p>
+        <p><h5>This is an h5 tag</h5></p>
+        <p><h6>This is an h6 tag</h6></p>
+        <p><a href="http://foo.com">Link to Foo</a></p>
+        <p><img src="http://google.com/someimage.png" width="50" height="50" alt="Some Image" title="Some Image" /></p>
+      END
+    end
+
+    it "should convert simple tags to plain text" do
+      text = th.html_to_simple_text(@body)
+      text.should == <<-END.strip_heredoc.strip
+        This is a bold tag
+
+        This is an em tag
+
+        *****************
+        This is an h1 tag
+        *****************
+
+        -----------------
+        This is an h2 tag
+        -----------------
+
+        This is an h3 tag
+        -----------------
+
+        This is an h4 tag
+        -----------------
+
+        This is an h5 tag
+        -----------------
+
+        This is an h6 tag
+        -----------------
+
+        Link to Foo ( http://foo.com )
+      END
+    end
+
+    it "should convert simple tags to minimal html" do
+      html = th.html_to_simple_html(@body).gsub("\r\n", "\n") # gsub only for test matching
+      html.should == <<-END.strip_heredoc.strip
+        <p>This is a bold tag<br/>
+        <br/>
+        This is an em tag<br/>
+        <br/>
+        This is an h1 tag<br/>
+        <br/>
+        This is an h2 tag<br/>
+        <br/>
+        This is an h3 tag<br/>
+        <br/>
+        This is an h4 tag<br/>
+        <br/>
+        This is an h5 tag<br/>
+        <br/>
+        This is an h6 tag<br/>
+        <br/>
+        Link to Foo ( <a href='http://foo.com'>http://foo.com</a> )</p>
+      END
+    end
+
+    it "should convert relative links to absolute links" do
+      html = th.html_to_simple_html("<a href=\"/this/is/a/relative/link\">Relative Link</a>", :base_url => "http://example.com")
+      html.should == "<p>Relative Link ( <a href='http://example.com/this/is/a/relative/link'>http://example.com/this/is/a/relative/link</a> )</p>"
+    end
+  end
+
   context "markdown" do
     context "safety" do
       it "should escape Strings correctly" do

spec/messages/conversation_message.email.erb_spec.rb

@@ -24,7 +24,10 @@ describe 'conversation_message.email' do
     course_with_teacher
     student_in_course
     conversation = @teacher.initiate_conversation([@user])
-    message = conversation.add_message("some message")
+    message = conversation.add_message("this
+is
+a
+message")
     generate_message(:conversation_message, :email, message)
   end
 end