diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb index a009ace51..8aa4cc1d7 100644 --- a/app/jobs/application_job.rb +++ b/app/jobs/application_job.rb @@ -1,2 +1,6 @@ class ApplicationJob < ActiveJob::Base + rescue_from(Exception) do |exception| + Rails.error.report(exception) + raise + end end diff --git a/app/models/concerns/async_email_concern.rb b/app/models/concerns/async_email_concern.rb new file mode 100644 index 000000000..ac5c678fc --- /dev/null +++ b/app/models/concerns/async_email_concern.rb @@ -0,0 +1,11 @@ +module AsyncEmailConcern + extend ActiveSupport::Concern + + private + + def async_email_enabled?(chapter) + return false if chapter.nil? + return false if Rails.application.config.async_email_chapter_ids.empty? + Rails.application.config.async_email_chapter_ids.include?(chapter.id) + end +end \ No newline at end of file diff --git a/app/models/concerns/workshop_invitation_manager_concerns.rb b/app/models/concerns/workshop_invitation_manager_concerns.rb index 23719264d..c83b8a7c7 100644 --- a/app/models/concerns/workshop_invitation_manager_concerns.rb +++ b/app/models/concerns/workshop_invitation_manager_concerns.rb @@ -2,6 +2,7 @@ module WorkshopInvitationManagerConcerns extend ActiveSupport::Concern included do + include AsyncEmailConcern include InstanceMethods end @@ -9,7 +10,8 @@ module InstanceMethods def send_workshop_attendance_reminders(workshop) workshop_mailer = workshop.virtual? ? VirtualWorkshopInvitationMailer : WorkshopInvitationMailer workshop.attendances.not_reminded.each do |invitation| - workshop_mailer.send(:attending_reminder, workshop, invitation.member, invitation).deliver_now + deliver_method = async_email_enabled?(workshop.chapter) ? :deliver_later : :deliver_now + workshop_mailer.send(:attending_reminder, workshop, invitation.member, invitation).public_send(deliver_method) invitation.update(reminded_at: Time.zone.now) end end @@ -86,7 +88,8 @@ def send_waiting_list_emails(workshop) def send_workshop_waiting_list_reminders(workshop) workshop_mailer = workshop.virtual? ? VirtualWorkshopInvitationMailer : WorkshopInvitationMailer workshop.invitations.on_waiting_list.not_reminded.each do |invitation| - workshop_mailer.send(:waiting_list_reminder, workshop, invitation.member, invitation).deliver_now + deliver_method = async_email_enabled?(workshop.chapter) ? :deliver_later : :deliver_now + workshop_mailer.send(:waiting_list_reminder, workshop, invitation.member, invitation).public_send(deliver_method) invitation.update(reminded_at: Time.zone.now) end end @@ -113,26 +116,30 @@ def log_invitation_failure(workshop, member, role, error) end def invite_coaches_to_virtual_workshop(workshop, logger = nil) + deliver_method = async_email_enabled?(workshop.chapter) ? :deliver_later : :deliver_now invite_members(workshop, logger, chapter_coaches(workshop.chapter)) do |coach, invitation| - VirtualWorkshopInvitationMailer.invite_coach(workshop, coach, invitation).deliver_now + VirtualWorkshopInvitationMailer.invite_coach(workshop, coach, invitation).public_send(deliver_method) end end def invite_coaches_to_workshop(workshop, logger = nil) + deliver_method = async_email_enabled?(workshop.chapter) ? :deliver_later : :deliver_now invite_members(workshop, logger, chapter_coaches(workshop.chapter)) do |coach, invitation| - WorkshopInvitationMailer.invite_coach(workshop, coach, invitation).deliver_now + WorkshopInvitationMailer.invite_coach(workshop, coach, invitation).public_send(deliver_method) end end def invite_students_to_virtual_workshop(workshop, logger = nil) + deliver_method = async_email_enabled?(workshop.chapter) ? :deliver_later : :deliver_now invite_members(workshop, logger, chapter_students(workshop.chapter), 'Student') do |student, invitation| - VirtualWorkshopInvitationMailer.invite_student(workshop, student, invitation).deliver_now + VirtualWorkshopInvitationMailer.invite_student(workshop, student, invitation).public_send(deliver_method) end end def invite_students_to_workshop(workshop, logger = nil) + deliver_method = async_email_enabled?(workshop.chapter) ? :deliver_later : :deliver_now invite_members(workshop, logger, chapter_students(workshop.chapter), 'Student') do |member, invitation| - WorkshopInvitationMailer.invite_student(workshop, member, invitation).deliver_now + WorkshopInvitationMailer.invite_student(workshop, member, invitation).public_send(deliver_method) end end @@ -169,7 +176,8 @@ def send_email_with_logging(logger, member, invitation) def retrieve_and_notify_waitlisted(workshop, role:) WaitingList.by_workshop(workshop).where_role(role).each do |waiting_list| - WorkshopInvitationMailer.notify_waiting_list(waiting_list.invitation).deliver_now + deliver_method = async_email_enabled?(waiting_list.invitation.workshop.chapter) ? :deliver_later : :deliver_now + WorkshopInvitationMailer.notify_waiting_list(waiting_list.invitation).public_send(deliver_method) waiting_list.destroy end end diff --git a/config/application.rb b/config/application.rb index a8af5e442..5bc449221 100644 --- a/config/application.rb +++ b/config/application.rb @@ -31,9 +31,18 @@ class Application < Rails::Application # and https://discuss.rubyonrails.org/t/cve-2022-32224-possible-rce-escalation-bug-with-serialized-columns-in-active-record/81017 config.active_record.yaml_column_permitted_classes = [Symbol, Date, Time, ActiveSupport::TimeWithZone, ActiveSupport::TimeZone, ActiveSupport::HashWithIndifferentAccess] - config.active_record.belongs_to_required_by_default = true +config.active_record.belongs_to_required_by_default = true - if ENV["RAILS_LOG_TO_STDOUT"].present? +# ActiveJob adapter for async email delivery +config.active_job.queue_adapter = :delayed_job + +# Feature flag: chapters that use async email delivery +# Empty = no chapters use async (all sync) +# "1" = only chapter 1 uses async +# "1,7" = chapters 1 and 7 use async +config.async_email_chapter_ids = ENV['ASYNC_EMAIL_CHAPTER_IDS']&.split(',')&.map(&:to_i) || [] + +if ENV["RAILS_LOG_TO_STDOUT"].present? $stdout.sync = true config.rails_semantic_logger.add_file_appender = false config.semantic_logger.add_appender(io: $stdout, formatter: config.rails_semantic_logger.format) diff --git a/config/environments/test.rb b/config/environments/test.rb index bcf11a2e7..c66414de3 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -11,12 +11,12 @@ # While tests run files are not watched, reloading is not necessary. config.enable_reloading = false - # Eager loading loads your entire application. When running a single test locally, - # this is usually not necessary, and can slow down your test suite. However, it's - # recommended that you enable it in continuous integration systems to ensure eager - # loading is working properly before deploying your code. + # Eager loading loads your entire application. config.eager_load = ENV['CI'].present? + # Use delayed_job adapter for async email tests + config.active_job.queue_adapter = :delayed_job + # Configure public file server for tests with cache-control for performance. config.public_file_server.headers = { 'cache-control' => "public, max-age=#{1.hour.to_i}" } diff --git a/spec/models/concerns/async_email_concern_spec.rb b/spec/models/concerns/async_email_concern_spec.rb new file mode 100644 index 000000000..57e6bd669 --- /dev/null +++ b/spec/models/concerns/async_email_concern_spec.rb @@ -0,0 +1,34 @@ +RSpec.describe AsyncEmailConcern do + let(:concern_class) do + Class.new { include AsyncEmailConcern } + end + let(:instance) { concern_class.new } + + describe "#async_email_enabled?" do + context "when ASYNC_EMAIL_CHAPTER_IDS is empty" do + before { Rails.application.config.async_email_chapter_ids = [] } + + it "returns false" do + expect(instance.send(:async_email_enabled?, OpenStruct.new(id: 1))).to be false + end + end + + context "when chapter is in async list" do + before { Rails.application.config.async_email_chapter_ids = [1, 7] } + + it "returns true for matching chapter" do + expect(instance.send(:async_email_enabled?, OpenStruct.new(id: 1))).to be true + end + + it "returns false for non-matching chapter" do + expect(instance.send(:async_email_enabled?, OpenStruct.new(id: 2))).to be false + end + end + + context "when chapter is nil" do + it "returns false" do + expect(instance.send(:async_email_enabled?, nil)).to be false + end + end + end +end \ No newline at end of file diff --git a/spec/models/invitation_manager_spec.rb b/spec/models/invitation_manager_spec.rb index 7059e415d..516464159 100644 --- a/spec/models/invitation_manager_spec.rb +++ b/spec/models/invitation_manager_spec.rb @@ -364,4 +364,80 @@ expect(log.skipped_count).to eq(students.count) end end + + describe '#send_workshop_emails async behavior' do + let!(:chapter) { Fabricate(:chapter, id: 1) } + + context 'when chapter is in ASYNC_EMAIL_CHAPTER_IDS' do + before { Rails.application.config.async_email_chapter_ids = [1] } + + it 'sends invitation emails' do + Fabricate(:students, chapter: chapter, members: students) + Fabricate(:coaches, chapter: chapter, members: coaches) + + expect(WorkshopInvitationMailer).to receive(:invite_student).at_least(:once).and_call_original + expect(WorkshopInvitationMailer).to receive(:invite_coach).at_least(:once).and_call_original + + manager.send_workshop_emails(workshop, 'everyone') + end + end + + context 'when chapter is NOT in ASYNC_EMAIL_CHAPTER_IDS' do + before { Rails.application.config.async_email_chapter_ids = [99] } + + it 'sends emails synchronously' do + Fabricate(:students, chapter: chapter, members: students) + Fabricate(:coaches, chapter: chapter, members: coaches) + + expect do + manager.send_workshop_emails(workshop, 'everyone') + end.to change { ActionMailer::Base.deliveries.count }.by(students.count + coaches.count) + + expect(Delayed::Job.count).to eq(0) + end + end + + context 'when ASYNC_EMAIL_CHAPTER_IDS is empty' do + before { Rails.application.config.async_email_chapter_ids = [] } + + it 'sends emails synchronously' do + Fabricate(:students, chapter: chapter, members: students) + Fabricate(:coaches, chapter: chapter, members: coaches) + + expect do + manager.send_workshop_emails(workshop, 'everyone') + end.to change { ActionMailer::Base.deliveries.count }.by(students.count + coaches.count) + + expect(Delayed::Job.count).to eq(0) + end + end + end + + describe '#send_workshop_attendance_reminders async behavior' do + let!(:chapter) { Fabricate(:chapter, id: 1) } + + context 'when chapter is in ASYNC_EMAIL_CHAPTER_IDS' do + before { Rails.application.config.async_email_chapter_ids = [1] } + + it 'sends attendance reminder emails' do + invitation = Fabricate(:attending_workshop_invitation, workshop: workshop) + + expect(WorkshopInvitationMailer).to receive(:attending_reminder).at_least(:once).and_call_original + + manager.send_workshop_attendance_reminders(workshop) + end + end + + context 'when chapter is NOT in ASYNC_EMAIL_CHAPTER_IDS' do + before { Rails.application.config.async_email_chapter_ids = [99] } + + it 'uses deliver_now' do + invitation = Fabricate(:attending_workshop_invitation, workshop: workshop) + + expect do + manager.send_workshop_attendance_reminders(workshop) + end.to change { ActionMailer::Base.deliveries.count }.by(1) + end + end + end end