From 265d59263bfc03f69ecfaa87ab63aeb470a779aa Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Wed, 24 Sep 2025 11:25:41 +0100 Subject: [PATCH 1/2] Fix broken learning path subscription email digest --- app/mailers/subscription_mailer.rb | 3 +- .../subscription_mailer/_events.html.erb | 2 +- ...materials.html.erb => _resources.html.erb} | 6 ++-- app/views/subscription_mailer/digest.html.erb | 9 ++++-- app/views/subscription_mailer/digest.text.erb | 2 +- test/fixtures/subscriptions.yml | 8 +++++ .../previews/subscription_mailer_preview.rb | 5 ++++ test/mailers/subscription_mailer_test.rb | 29 +++++++++++++++++++ 8 files changed, 57 insertions(+), 7 deletions(-) rename app/views/subscription_mailer/{_materials.html.erb => _resources.html.erb} (53%) diff --git a/app/mailers/subscription_mailer.rb b/app/mailers/subscription_mailer.rb index 3a20b74f9..4ebf69ad4 100644 --- a/app/mailers/subscription_mailer.rb +++ b/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 diff --git a/app/views/subscription_mailer/_events.html.erb b/app/views/subscription_mailer/_events.html.erb index e61e343e4..e1a6b800d 100644 --- a/app/views/subscription_mailer/_events.html.erb +++ b/app/views/subscription_mailer/_events.html.erb @@ -5,7 +5,7 @@ <% 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;') %>
<%= result.title %>

<% end -%> diff --git a/app/views/subscription_mailer/_materials.html.erb b/app/views/subscription_mailer/_resources.html.erb similarity index 53% rename from app/views/subscription_mailer/_materials.html.erb rename to app/views/subscription_mailer/_resources.html.erb index 0d10ee439..5b55aa36e 100644 --- a/app/views/subscription_mailer/_materials.html.erb +++ b/app/views/subscription_mailer/_resources.html.erb @@ -4,8 +4,10 @@ <% digest.each do |result| -%> - <%= link_to(result.title, result, style: 'color: #f57d20; font-weight: bold; text-decoration: none') %>
- <%= truncate(result.description, length: 120, separator: ' ') %>

+ <%= link_to(result.title, result, style: 'color: #f57d20; font-weight: bold;') %>
+ <% if result.respond_to?(:description) %> + <%= truncate(result.description, length: 120, separator: ' ') %>

+ <% end %> <% end -%> diff --git a/app/views/subscription_mailer/digest.html.erb b/app/views/subscription_mailer/digest.html.erb index 1afef044c..4ea85a398 100644 --- a/app/views/subscription_mailer/digest.html.erb +++ b/app/views/subscription_mailer/digest.html.erb @@ -27,7 +27,12 @@ Dear <%= @user.profile.firstname || @user.username %>,
<%= pluralize(@digest.total_count, "new #{@subscription.subscribable_type.downcase}") -%> registered in <%= TeSS::Config.site['title_short'] %>:

-<%= 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 %>

<% if @digest.count < @digest.total_count %> @@ -36,7 +41,7 @@ Dear <%= @user.profile.firstname || @user.username %>,
<%= link_to("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:

diff --git a/app/views/subscription_mailer/digest.text.erb b/app/views/subscription_mailer/digest.text.erb index a180ecc85..b0eee38c3 100644 --- a/app/views/subscription_mailer/digest.text.erb +++ b/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) %> diff --git a/test/fixtures/subscriptions.yml b/test/fixtures/subscriptions.yml index 15712685c..498a7b601 100644 --- a/test/fixtures/subscriptions.yml +++ b/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 diff --git a/test/mailers/previews/subscription_mailer_preview.rb b/test/mailers/previews/subscription_mailer_preview.rb index 2fac8ceb4..255ee87f0 100644 --- a/test/mailers/previews/subscription_mailer_preview.rb +++ b/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 diff --git a/test/mailers/subscription_mailer_test.rb b/test/mailers/subscription_mailer_test.rb index 48d2f8652..cc8ec5d32 100644 --- a/test/mailers/subscription_mailer_test.rb +++ b/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 From 01e0613ff334ce305adf362f693b72e168b1107d Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Wed, 24 Sep 2025 12:52:23 +0100 Subject: [PATCH 2/2] Test fix --- test/controllers/subscriptions_controller_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/controllers/subscriptions_controller_test.rb b/test/controllers/subscriptions_controller_test.rb index f4da084fa..f841218fe 100644 --- a/test/controllers/subscriptions_controller_test.rb +++ b/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