Hello,
+ ++ You have <%= @pending_requests.size %> pending extension <%= 'request'.pluralize(@pending_requests.size) %> + in <%= link_to @course.course_code, @course.course_link %> + (<%= [@course.course_name, @course.semester].compact_blank.join(', ') %>). +
+ +
+ Please review them on the <%= link_to 'requests page', @course.requests_link %>.
+ In the past <%= @period_label %> <%= @auto_approved_count %> <%= 'request'.pluralize(@auto_approved_count) %> <%= @auto_approved_count == 1 ? 'has' : 'have' %> been auto approved.
+
Here are the individual pending requests:
+Thanks!
diff --git a/app/views/pending_requests_mailer/pending_requests_email.text.erb b/app/views/pending_requests_mailer/pending_requests_email.text.erb new file mode 100644 index 00000000..53ec604d --- /dev/null +++ b/app/views/pending_requests_mailer/pending_requests_email.text.erb @@ -0,0 +1,16 @@ +Hello, + +You have <%= @pending_requests.size %> pending extension <%= 'request'.pluralize(@pending_requests.size) %> in <%= @course.course_code %> (<%= [@course.course_name, @course.semester].compact_blank.join(', ') %>): <%= @course.course_link %> + +Please review them on the requests page: <%= @course.requests_link %> +In the past <%= @period_label %> <%= @auto_approved_count %> <%= 'request'.pluralize(@auto_approved_count) %> <%= @auto_approved_count == 1 ? 'has' : 'have' %> been auto approved. + +Here are the individual pending requests: +<% @listed_requests.each do |request| -%> +* <%= request.user.name.presence || 'Unknown student' %>, <%= request.assignment.name %>, <%= pluralize(request.calculate_days_difference, 'day') %>: <%= request.request_link %> +<% end -%> +<% if @unlisted_count.positive? -%> +* …and <%= @unlisted_count %> more: <%= @course.requests_link %> +<% end -%> + +Thanks! diff --git a/spec/jobs/pending_requests_notification_job_spec.rb b/spec/jobs/pending_requests_notification_job_spec.rb index f499b911..9e2ef4be 100644 --- a/spec/jobs/pending_requests_notification_job_spec.rb +++ b/spec/jobs/pending_requests_notification_job_spec.rb @@ -1,7 +1,9 @@ require 'rails_helper' RSpec.describe PendingRequestsNotificationJob, type: :job do - let(:course) { create(:course, canvas_id: 'notif_123', course_name: 'CS 101', course_code: 'CS101') } + let(:course) do + create(:course, canvas_id: 'notif_123', course_name: 'CS 101', course_code: 'CS101', semester: 'Spring 2026') + end let(:student) { create(:user, canvas_uid: 'stu_notif_1', email: 'student_notif@example.com', name: 'Student') } let(:lms) { Lms.first } let(:course_to_lms) { CourseToLms.create!(course: course, lms: lms, external_course_id: 'ext_123') } @@ -20,14 +22,20 @@ ActionMailer::Base.deliveries.clear allow(ENV).to receive(:fetch).and_call_original allow(ENV).to receive(:fetch).with('DEFAULT_FROM_EMAIL').and_return('flextensions@berkeley.edu') - allow(ENV).to receive(:fetch).with('APP_HOST', nil).and_return('http://localhost:3000') + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with('APP_HOST').and_return('http://localhost:3000') + end + + def mail_bodies + mail = ActionMailer::Base.deliveries.last + [ mail.html_part.body.decoded, mail.text_part.body.decoded ] end describe '#perform' do it 'sends email when course has matching frequency and pending requests' do course.course_settings.update!(pending_notification_frequency: 'daily', pending_notification_email: 'prof@example.com') - Request.create!(course: course, assignment: assignment, user: student, status: 'pending', - reason: 'Need more time', requested_due_date: 5.days.from_now) + request = Request.create!(course: course, assignment: assignment, user: student, status: 'pending', + reason: 'Need more time', requested_due_date: 5.days.from_now) expect { described_class.perform_now('daily') }.to change { ActionMailer::Base.deliveries.count }.by(1) @@ -35,16 +43,44 @@ expect(mail.to).to eq([ 'prof@example.com' ]) expect(mail.subject).to include('1 Pending Extension Request') expect(mail.subject).to include('CS101') - expect(mail.body.encoded).to include("http://localhost:3000/courses/#{course.id}/requests") + + expect(mail_bodies).to all include( + "http://localhost:3000/courses/#{course.id}", + "http://localhost:3000/courses/#{course.id}/requests", + "http://localhost:3000/courses/#{course.id}/requests/#{request.id}", + 'CS 101, Spring 2026', + 'Student, HW1, 2 days', + 'In the past day 0 requests have been auto approved.' + ) end - it 'sends email to courses set to hourly' do + it 'counts recent auto-approvals within the frequency window' do + course.course_settings.update!(pending_notification_frequency: 'daily', pending_notification_email: 'prof@example.com') + Request.create!(course: course, assignment: assignment, user: student, status: 'pending', + reason: 'Need more time', requested_due_date: 5.days.from_now) + + recent_user = create(:user, canvas_uid: 'stu_notif_auto1', email: 'auto1@example.com') + Request.create!(course: course, assignment: assignment, user: recent_user, status: 'approved', + auto_approved: true, reason: 'Auto', requested_due_date: 4.days.from_now) + stale_user = create(:user, canvas_uid: 'stu_notif_auto2', email: 'auto2@example.com') + travel_to(2.days.ago) do + Request.create!(course: course, assignment: assignment, user: stale_user, status: 'approved', + auto_approved: true, reason: 'Auto', requested_due_date: 4.days.from_now) + end + + described_class.perform_now('daily') + + expect(mail_bodies).to all include('In the past day 1 request has been auto approved.') + end + + it 'sends email to courses set to hourly and words the window accordingly' do course.course_settings.update!(pending_notification_frequency: 'hourly', pending_notification_email: 'prof@example.com') Request.create!(course: course, assignment: assignment, user: student, status: 'pending', reason: 'Need more time', requested_due_date: 5.days.from_now) expect { described_class.perform_now('hourly') }.to change { ActionMailer::Base.deliveries.count }.by(1) expect(ActionMailer::Base.deliveries.last.to).to eq([ 'prof@example.com' ]) + expect(mail_bodies).to all include('In the past hour 0 requests have been auto approved.') end it 'does not send hourly digests to courses set to another frequency' do @@ -69,11 +105,12 @@ expect { described_class.perform_now('daily') }.not_to(change { ActionMailer::Base.deliveries.count }) end - it 'pluralizes correctly for multiple pending requests' do + it 'pluralizes correctly and lists each pending request' do course.course_settings.update!(pending_notification_frequency: 'daily', pending_notification_email: 'prof@example.com') - 2.times do |i| + requests = 2.times.map do |i| Request.create!(course: course, assignment: assignment, - user: create(:user, canvas_uid: "stu_multi_#{i}", email: "stu_multi_#{i}@example.com"), + user: create(:user, canvas_uid: "stu_multi_#{i}", email: "stu_multi_#{i}@example.com", + name: "Student #{i}"), status: 'pending', reason: 'Need time', requested_due_date: 5.days.from_now) end @@ -81,6 +118,11 @@ mail = ActionMailer::Base.deliveries.last expect(mail.subject).to include('2 Pending Extension Requests') + expected_lines = requests.flat_map do |request| + [ "#{request.user.name}, HW1, 2 days", + "http://localhost:3000/courses/#{course.id}/requests/#{request.id}" ] + end + expect(mail_bodies).to all include(*expected_lines) end it 'sends separate emails to multiple courses' do diff --git a/spec/mailers/pending_requests_mailer_spec.rb b/spec/mailers/pending_requests_mailer_spec.rb new file mode 100644 index 00000000..d94837bb --- /dev/null +++ b/spec/mailers/pending_requests_mailer_spec.rb @@ -0,0 +1,88 @@ +require 'rails_helper' + +RSpec.describe PendingRequestsMailer do + let(:course) do + create(:course, canvas_id: 'mailer_123', course_name: 'CS 101', course_code: 'CS101', semester: 'Spring 2026') + end + let(:course_settings) { course.course_settings } + let(:lms) { Lms.first } + let(:course_to_lms) { CourseToLms.create!(course: course, lms: lms, external_course_id: 'ext_mailer_123') } + let(:assignment) do + Assignment.create!( + name: 'HW1', + course_to_lms: course_to_lms, + due_date: 3.days.from_now, + external_assignment_id: 'asgn_mailer_1', + enabled: true + ) + end + + before do + course_settings.update!(pending_notification_frequency: 'daily', pending_notification_email: 'prof@example.com') + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with('DEFAULT_FROM_EMAIL').and_return('flextensions@berkeley.edu') + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with('APP_HOST').and_return('flextensions.example.com') + end + + def create_pending_request(name:, uid:) + Request.create!(course: course, assignment: assignment, status: 'pending', + reason: 'Need time', requested_due_date: 5.days.from_now, + user: create(:user, canvas_uid: uid, email: "#{uid}@example.com", name: name)) + end + + def build_mail(pending_requests, auto_approved_count: 0, frequency: 'daily') + described_class.pending_requests_email( + course_settings: course_settings, + pending_requests: pending_requests, + auto_approved_count: auto_approved_count, + frequency: frequency + ) + end + + describe '#pending_requests_email' do + it 'uses the reply email when set and defaults the host scheme to https' do + course_settings.update!(reply_email: 'staff@example.com') + request = create_pending_request(name: 'Alice', uid: 'mailer_stu_1') + + mail = build_mail([ request ]) + + expect(mail.from).to eq([ 'flextensions@berkeley.edu' ]) + expect(mail.reply_to).to eq([ 'staff@example.com' ]) + expect(mail.subject).to eq('1 Pending Extension Request - CS101') + expect(mail.html_part.body.decoded) + .to include("https://flextensions.example.com/courses/#{course.id}/requests/#{request.id}") + end + + it 'words the auto-approval window per frequency and singular count' do + request = create_pending_request(name: 'Alice', uid: 'mailer_stu_1') + + mail = build_mail([ request ], auto_approved_count: 1, frequency: 'weekly') + + expect(mail.html_part.body.decoded).to include('In the past week 1 request') + expect(mail.html_part.body.decoded).to include('has been auto approved.') + end + + it 'falls back to a placeholder for users without a name' do + request = create_pending_request(name: nil, uid: 'mailer_stu_1') + + mail = build_mail([ request ]) + + expect(mail.html_part.body.decoded).to include('Unknown student, HW1, 2 days') + end + + it 'caps the per-request list and links to the requests page for the rest' do + stub_const('PendingRequestsMailer::MAX_LISTED_REQUESTS', 2) + requests = 3.times.map { |i| create_pending_request(name: "Student #{i}", uid: "mailer_stu_#{i}") } + + mail = build_mail(requests) + + html = mail.html_part.body.decoded + expect(html).to include('Student 0') + expect(html).to include('Student 1') + expect(html).not_to include('Student 2') + expect(html).to include('and 1 more') + expect(mail.text_part.body.decoded).to include('and 1 more') + end + end +end