Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/mailers/subscription_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/views/subscription_mailer/_events.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 -%>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) %>

Copilot AI Sep 24, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using respond_to? for feature detection is fragile and can hide real issues. Consider using a more explicit check like result.is_a?(Material) or checking if the result has a description attribute directly with result.try(:description) if using Rails.

Suggested change
<% if result.respond_to?(:description) %>
<% if result.try(:description) %>

Copilot uses AI. Check for mistakes.
<%= truncate(result.description, length: 120, separator: ' ') %><br/><br/>
<% end %>
<% end -%>
</td>
</tr>
Expand Down
9 changes: 7 additions & 2 deletions app/views/subscription_mailer/digest.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>
Expand All @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion app/views/subscription_mailer/digest.text.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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) %>
Expand Down
3 changes: 2 additions & 1 deletion test/controllers/subscriptions_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/subscriptions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions test/mailers/previews/subscription_mailer_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
29 changes: 29 additions & 0 deletions test/mailers/subscription_mailer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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