Merge pull request #2592 from mroderick/feature/delayed-email-async

feat: add configurable async email delivery for workshop invitations

Author: mroderick

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

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

app/models/concerns/workshop_invitation_manager_concerns.rb

@@ -2,14 +2,16 @@ module WorkshopInvitationManagerConcerns
   extend ActiveSupport::Concern
 
   included do
+    include AsyncEmailConcern
     include InstanceMethods
   end
 
   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

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)

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}" }
 

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

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