Merge pull request #1150 from ElixirTeSS/digest-email-fix

Learning path subscription email fix

Author: fbacall

app/mailers/subscription_mailer.rb

@@ -13,7 +13,8 @@ def digest(sub, dig)
                    else
                      []
                    end
-    subs = pluralize(@digest.total_count, "new #{@subscription.subscribable_type.downcase}")
+    resource_type = I18n.t("features.#{@subscription.subscribable_type.underscore.pluralize}.short").downcase
+    subs = pluralize(@digest.total_count, "new #{resource_type}")
     subject = "#{TeSS::Config.site['title_short']} #{sub.frequency} digest - #{subs} matching your criteria"
     mail(subject: subject, to: sub.user.email) do |format|
       format.html

app/views/subscription_mailer/_events.html.erb

@@ -5,7 +5,7 @@
     </td>
     <td width="95%">
       <% digest.each do |result| -%>
-        <%= link_to(digest_event_title(result), result, style: 'color: #f57d20; font-weight: bold; text-decoration: none') %>
+        <%= link_to(digest_event_title(result), result, style: 'color: #f57d20; font-weight: bold;') %>
         <br/>
         <%= result.title %><br/><br/>
       <% end -%>

…/subscription_mailer/_materials.html.erb …/subscription_mailer/_resources.html.erbapp/views/subscription_mailer/_materials.html.erb renamed to app/views/subscription_mailer/_resources.html.erb app/views/subscription_mailer/_materials.html.erb renamed to app/views/subscription_mailer/_resources.html.erb

@@ -4,8 +4,10 @@
     <td width="20"></td>
     <td>
       <% digest.each do |result| -%>
-        <%= link_to(result.title, result, style: 'color: #f57d20; font-weight: bold; text-decoration: none') %><br/>
-        <%= truncate(result.description, length: 120, separator: ' ') %><br/><br/>
+        <%= link_to(result.title, result, style: 'color: #f57d20; font-weight: bold;') %><br/>
+        <% if result.respond_to?(:description) %>
+          <%= truncate(result.description, length: 120, separator: ' ') %><br/><br/>
+        <% end %>
       <% end -%>
     </td>
   </tr>

app/views/subscription_mailer/digest.html.erb

@@ -27,7 +27,12 @@ Dear <%= @user.profile.firstname || @user.username %>,<br/>
   <%= pluralize(@digest.total_count, "new #{@subscription.subscribable_type.downcase}") -%>
   registered in <%= TeSS::Config.site['title_short'] %>:
 </p>
-<%= render partial: @subscription.subscribable_type.downcase.pluralize, locals: { digest: @digest } %>
+
+<% if @subscription.subscribable_type == 'Event' %>
+  <%= render partial: 'events', locals: { digest: @digest } %>
+<% else %>
+  <%= render partial: 'resources', locals: { digest: @digest } %>
+<% end %>
 
 <p>
   <% if @digest.count < @digest.total_count %>
@@ -36,7 +41,7 @@ Dear <%= @user.profile.firstname || @user.username %>,<br/>
   <%= link_to("View all results on #{TeSS::Config.site['title_short']}", subscription_results_url(@subscription)) %>
 </p>
 
-<% if @collections&.any? %>
+<% if @collections&.any? && ['Event', 'Material'].include?(@subscription.subscribable_type) %>
 <p>
   Collections you might like to update:
 </p>

app/views/subscription_mailer/digest.text.erb

@@ -23,7 +23,7 @@ View all results on <%= TeSS::Config.site['title_short'] %>:
 <%= subscription_results_url(@subscription) %>
 
 
-<% if @collections&.any? %>
+<% if @collections&.any? && ['Event', 'Material'].include?(@subscription.subscribable_type) %>
 Collections you might like to update:
 <% @collections.each do |collection| %>
   <%= collection.title %>: <%= send("curate_#{@subscription.subscribable_type.downcase.pluralize}_collection_url", collection) %>

test/controllers/subscriptions_controller_test.rb

@@ -9,7 +9,8 @@ class SubscriptionsControllerTest < ActionController::TestCase
 
     get :index
 
-    assert_select '.subscription', count: 3
+    assert users(:regular_user).subscriptions.any?
+    assert_select '.subscription', count: users(:regular_user).subscriptions.count
   end
 
   test "should not list other user's subscriptions" do

test/fixtures/subscriptions.yml

@@ -32,3 +32,11 @@ event_subscription:
   facets: { "times" : ["good", "great"] }
   user: admin
   subscribable_type: Event
+
+learning_path_subscription:
+  frequency: 1
+  last_checked_at: 1986-11-23 10:16:33
+  query: bananas
+  facets: { "type": [ "fruit", "veg" ] }
+  user: regular_user
+  subscribable_type: LearningPath

test/mailers/previews/subscription_mailer_preview.rb

@@ -11,4 +11,9 @@ def last_material_digest
     SubscriptionMailer.digest(sub, sub.digest)
   end
 
+  def last_learning_path_digest
+    sub = Subscription.where(subscribable_type: 'LearningPath').last
+    SubscriptionMailer.digest(sub, sub.digest)
+  end
+
 end

test/mailers/subscription_mailer_test.rb

@@ -106,4 +106,33 @@ class SubscriptionMailerTest < ActionMailer::TestCase
     assert html.include? collections(:one).title
     assert html.include? @routes.curate_materials_collection_url(collaborating_collection)
   end
+
+  test 'html learning path digest' do
+    collaborating_collection = Collection.create!(title: 'collab', user: users(:regular_user))
+    collaborating_collection.collaborators << users(:admin)
+    sub = subscriptions(:learning_path_subscription)
+    lp = [
+      learning_paths(:one),
+      learning_paths(:two)
+    ]
+    digest = MockSearchResults.new(lp)
+    email = SubscriptionMailer.digest(sub, digest)
+
+    assert_emails 1 do
+      email.deliver_now
+    end
+
+    assert_equal [TeSS::Config.sender_email], email.from
+    assert_equal [sub.user.email], email.to
+    assert_equal "#{TeSS::Config.site['title_short']} daily digest - #{lp.length} new learning paths matching your criteria", email.subject
+
+    html = email.html_part.body.to_s
+
+    lp.each do |l|
+      assert html.include?(@routes.learning_path_url(l)), "Learning Path URL was missing from email: #{@routes.learning_path_url(l)}"
+    end
+
+    assert html.include?(@routes.unsubscribe_subscription_url(sub, code: sub.unsubscribe_code)), 'Expected unsubscribe link'
+    refute html.include?('Collections') # Curate feature is not available for learning paths
+  end
 end