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
29 changes: 24 additions & 5 deletions app/jobs/pending_requests_notification_job.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,35 @@
class PendingRequestsNotificationJob < ApplicationJob
queue_as :default

# Window the digest counts auto-approvals over, matched to how often the
# digest for that frequency goes out.
AUTO_APPROVAL_WINDOWS = {
'hourly' => 1.hour,
'daily' => 1.day,
'weekly' => 1.week
}.freeze

def perform(frequency)
window_start = AUTO_APPROVAL_WINDOWS.fetch(frequency).ago

CourseSettings.with_pending_notifications(frequency).includes(:course).find_each do |cs|
course = cs.course
pending_count = Request.where(course_id: course.id, status: 'pending').count
next if pending_count.zero?
pending_requests = course.requests.pending.includes(:user, :assignment).order(:created_at).to_a
next if pending_requests.empty?

# TODO: If the count is < ~50 (?) build and include summary links.
requests_url = "#{ENV.fetch('APP_HOST', nil)}/courses/#{course.id}/requests"
# Requests carry no approved-at timestamp, so updated_at is the closest
# proxy for when an auto-approval happened.
auto_approved_count = course.requests
.where(status: 'approved', auto_approved: true)
.where(updated_at: window_start..)
.count

PendingRequestsMailer.send_pending_request_notifications(cs, pending_count, requests_url)
PendingRequestsMailer.pending_requests_email(
course_settings: cs,
pending_requests: pending_requests,
auto_approved_count: auto_approved_count,
frequency: frequency
).deliver_now
end
end
end
52 changes: 25 additions & 27 deletions app/mailers/pending_requests_mailer.rb
Original file line number Diff line number Diff line change
@@ -1,40 +1,38 @@
# frozen_string_literal: true

# Templates and delivery helper for the "you have N pending extension requests"
# digest email that PendingRequestsNotificationJob sends. Wraps EmailService so
# the job doesn't need to know the template strings or mapping shape.
class PendingRequestsMailer
SUBJECT_TEMPLATE = '{{pending_count}} Pending Extension Request{{plural}} - {{course_code}}'
# The "you have N pending extension requests" digest email that
# PendingRequestsNotificationJob sends to course staff. Rendered from ERB views
# in app/views/pending_requests_mailer/.
class PendingRequestsMailer < ApplicationMailer
# How the digest describes the window auto-approvals are counted over
# ("In the past hour ..."), keyed by the course's notification frequency.
PERIOD_LABELS = {
'hourly' => 'hour',
'daily' => 'day',
'weekly' => 'week'
}.freeze

BODY_TEMPLATE = <<~BODY
Hello,
# Very large courses can accumulate hundreds of pending requests; cap the
# per-request list so the digest stays readable and link to the requests
# page for the rest.
MAX_LISTED_REQUESTS = 50

You have {{pending_count}} pending extension request{{plural}} in {{course_name}} ({{course_code}}).
def pending_requests_email(course_settings:, pending_requests:, auto_approved_count:, frequency:)
@course = course_settings.course
@pending_requests = pending_requests
@listed_requests = pending_requests.first(MAX_LISTED_REQUESTS)
@unlisted_count = pending_requests.size - @listed_requests.size
@auto_approved_count = auto_approved_count
@period_label = PERIOD_LABELS.fetch(frequency)

Please review them at: {{requests_url}}

Thank you,
Flextensions
BODY

def self.send_pending_request_notifications(course_settings, pending_count, requests_url)
course = course_settings.course
default_from = ENV.fetch('DEFAULT_FROM_EMAIL')
count = pending_requests.size

EmailService.send_email(
mail(
to: course_settings.pending_notification_email,
from: default_from,
reply_to: course_settings.reply_email.presence || default_from,
subject_template: SUBJECT_TEMPLATE,
body_template: BODY_TEMPLATE,
mapping: {
'pending_count' => pending_count.to_s,
'plural' => pending_count == 1 ? '' : 's',
'course_name' => course.course_name,
'course_code' => course.course_code,
'requests_url' => requests_url
},
deliver_later: false
subject: "#{count} Pending Extension Request#{'s' unless count == 1} - #{@course.course_code}"
)
end
end
15 changes: 15 additions & 0 deletions app/models/course.rb
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,21 @@ def external_course_id_for(lms_id)
(links.where.not(external_course_id: nil).first || links.first)&.external_course_id
end

# Absolute URL of the course page, for links embedded in outbound
# notifications (email, Slack). Falls back to a relative path when no host is
# configured so links still render in development.
def course_link
base_host = ENV['APP_HOST'].presence || Rails.application.routes.default_url_options[:host].presence
return Rails.application.routes.url_helpers.course_path(self) if base_host.blank?

normalized_host = base_host.start_with?('http://', 'https://') ? base_host : "https://#{base_host}"
"#{normalized_host.chomp('/')}/courses/#{id}"
end

def requests_link
"#{course_link}/requests"
end

# TODO: Add specs for these 3 simple methods
def students
enrollments.where(role: Enrollment::STUDENT_ROLE).map(&:user)
Expand Down
14 changes: 6 additions & 8 deletions app/models/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,12 @@ def send_email_response
)
end

# Absolute URL for reviewing this request, used in Slack and email
# notifications.
def request_link
"#{course.requests_link}/#{id}"
end

private

def flag_auto_approval_breakdown(reason)
Expand Down Expand Up @@ -379,12 +385,4 @@ def notify_slack(slack_message)
success = SlackNotifier.notify(slack_message, course.course_settings.slack_webhook_url)
Rails.logger.error "Failed to send Slack notification for request #{id} in course #{course.id}. Please check your webhook URL." unless success
end

def request_link
base_host = ENV['APP_HOST'].presence || Rails.application.routes.default_url_options[:host].presence
return Rails.application.routes.url_helpers.course_request_path(course, id) if base_host.blank?

normalized_host = base_host.start_with?('http://', 'https://') ? base_host : "https://#{base_host}"
"#{normalized_host.chomp('/')}/courses/#{course.id}/requests/#{id}"
end
end
27 changes: 27 additions & 0 deletions app/views/pending_requests_mailer/pending_requests_email.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<p>Hello,</p>

<p>
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(', ') %>).
</p>

<p>
Please review them on the <%= link_to 'requests page', @course.requests_link %>.<br>
In the past <%= @period_label %> <%= @auto_approved_count %> <%= 'request'.pluralize(@auto_approved_count) %> <%= @auto_approved_count == 1 ? 'has' : 'have' %> been auto approved.
</p>

<p>Here are the individual pending requests:</p>
<ul>
<% @listed_requests.each do |request| %>
<li>
<%= link_to "#{request.user.name.presence || 'Unknown student'}, #{request.assignment.name}, #{pluralize(request.calculate_days_difference, 'day')}",
request.request_link %>
</li>
<% end %>
<% if @unlisted_count.positive? %>
<li>…and <%= @unlisted_count %> more on the <%= link_to 'requests page', @course.requests_link %>.</li>
<% end %>
</ul>

<p>Thanks!</p>
16 changes: 16 additions & 0 deletions app/views/pending_requests_mailer/pending_requests_email.text.erb
Original file line number Diff line number Diff line change
@@ -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!
60 changes: 51 additions & 9 deletions spec/jobs/pending_requests_notification_job_spec.rb
Original file line number Diff line number Diff line change
@@ -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') }
Expand All @@ -20,31 +22,65 @@
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)

mail = ActionMailer::Base.deliveries.last
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
Expand All @@ -69,18 +105,24 @@
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

described_class.perform_now('daily')

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
Expand Down
88 changes: 88 additions & 0 deletions spec/mailers/pending_requests_mailer_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Loading