fix: make bulk invitation creation resilient to individual failures

- Use find_or_create_by instead of create to handle duplicate invitations
- Add exception handling to catch database errors and continue processing
- Log errors at ERROR level with member_id and context IDs (workshop/event/meeting)
- Add tests for exception handling and bulk resilience
- Update existing tests to use find_or_create_by

This fixes issue #2542 where workshop invitations would fail silently if any
individual member's invitation creation raised an exception, causing the
entire bulk invitation process to abort and leaving remaining members without
invitations.

Author: mroderick

app/models/concerns/workshop_invitation_manager_concerns.rb

@@ -51,8 +51,19 @@ def send_workshop_waiting_list_reminders(workshop)
     private
 
     def create_invitation(workshop, member, role)
-      invitation = WorkshopInvitation.create(workshop: workshop, member: member, role: role)
-      invitation.persisted? ? invitation : nil
+      WorkshopInvitation.find_or_create_by(workshop: workshop, member: member, role: role)
+    rescue StandardError => e
+      log_invitation_failure(workshop, member, role, e)
+      nil
+    end
+
+    def log_invitation_failure(workshop, member, role, error)
+      Rails.logger.error(
+        '[InvitationManager] Failed to create invitation: ' \
+        "workshop_id=#{workshop.id}, chapter_id=#{workshop.chapter_id}, " \
+        "member_id=#{member.id}, role=#{role}, " \
+        "error=#{error.class.name}: #{error.message}"
+      )
     end
 
     def invite_coaches_to_virtual_workshop(workshop)

app/models/invitation_manager.rb

@@ -20,7 +20,11 @@ def send_monthly_attendance_reminder_emails(monthly)
   def send_meeting_emails(meeting)
     meeting.invitees.not_banned.each do |invitee|
       invitation = MeetingInvitation.new(meeting: meeting, member: invitee, role: 'Participant')
-      MeetingInvitationMailer.invite(meeting, invitee, invitation).deliver_now if invitation.save
+      next unless invitation.save
+
+      MeetingInvitationMailer.invite(meeting, invitee, invitation).deliver_now
+    rescue StandardError => e
+      log_event_meeting_invitation_failure("meeting_id=#{meeting.id}", invitee, e)
     end
   end
   handle_asynchronously :send_meeting_emails
@@ -30,17 +34,33 @@ def send_meeting_emails(meeting)
   def invite_students_to_event(event, chapter)
     chapter_students(chapter).each do |student|
       invitation = Invitation.new(event: event, member: student, role: 'Student')
-      EventInvitationMailer.invite_student(event, student, invitation).deliver_now if invitation.save
+      next unless invitation.save
+
+      EventInvitationMailer.invite_student(event, student, invitation).deliver_now
+    rescue StandardError => e
+      log_event_meeting_invitation_failure("event_id=#{event.id}", student, e)
     end
   end
 
   def invite_coaches_to_event(event, chapter)
     chapter_coaches(chapter).each do |coach|
       invitation = Invitation.new(event: event, member: coach, role: 'Coach')
-      EventInvitationMailer.invite_coach(event, coach, invitation).deliver_now if invitation.save
+      next unless invitation.save
+
+      EventInvitationMailer.invite_coach(event, coach, invitation).deliver_now
+    rescue StandardError => e
+      log_event_meeting_invitation_failure("event_id=#{event.id}", coach, e)
     end
   end
 
+  def log_event_meeting_invitation_failure(context, member, error)
+    Rails.logger.error(
+      '[InvitationManager] Failed to create invitation: ' \
+      "#{context}, member_id=#{member.id}, " \
+      "error=#{error.class.name}: #{error.message}"
+    )
+  end
+
   def chapter_students(chapter)
     Member.in_group(chapter.groups.students)
   end

spec/models/invitation_manager_spec.rb

@@ -20,7 +20,7 @@
     include_examples 'sending workshop emails'
   end
 
-  context '#send_event_emails' do
+  describe '#send_event_emails' do
     let!(:student_group) { Fabricate(:students, chapter: chapter, members: students) }
     let!(:coaches_group) { Fabricate(:coaches, chapter: chapter, members: coaches) }
 
@@ -31,7 +31,7 @@
       end
 
       coaches.each do |student|
-        expect(Invitation).to_not receive(:new).with(event: event, member: student, role: 'Coach').and_call_original
+        expect(Invitation).not_to receive(:new).with(event: event, member: student, role: 'Coach').and_call_original
       end
 
       manager.send_event_emails(event, chapter)
@@ -41,7 +41,7 @@
       event = Fabricate(:event, chapters: [chapter], audience: 'Coaches')
 
       students.each do |student|
-        expect(Invitation).to_not receive(:new).with(event: event, member: student, role: 'Student').and_call_original
+        expect(Invitation).not_to receive(:new).with(event: event, member: student, role: 'Student').and_call_original
       end
 
       coaches.each do |student|
@@ -71,17 +71,17 @@
       first_student, *other_students = students
       first_student.update(accepted_toc_at: nil)
 
-      expect(Invitation).to_not(
-        receive(:new).
-        with(event: event, member: first_student, role: 'Student').
-        and_call_original
+      expect(Invitation).not_to(
+        receive(:new)
+        .with(event: event, member: first_student, role: 'Student')
+        .and_call_original
       )
 
       other_students.each do |other_student|
         expect(Invitation).to(
-          receive(:new).
-          with(event: event, member: other_student, role: 'Student').
-          and_call_original
+          receive(:new)
+          .with(event: event, member: other_student, role: 'Student')
+          .and_call_original
         )
       end
 
@@ -94,25 +94,25 @@
       first_coach, *other_coaches = coaches
       first_coach.update(accepted_toc_at: nil)
 
-      expect(Invitation).to_not(
-        receive(:new).
-        with(event: event, member: first_coach, role: 'Coach').
-        and_call_original
+      expect(Invitation).not_to(
+        receive(:new)
+        .with(event: event, member: first_coach, role: 'Coach')
+        .and_call_original
       )
 
       other_coaches.each do |other_coach|
         expect(Invitation).to(
-          receive(:new).
-          with(event: event, member: other_coach, role: 'Coach').
-          and_call_original
+          receive(:new)
+          .with(event: event, member: other_coach, role: 'Coach')
+          .and_call_original
         )
       end
 
       manager.send_event_emails(event, chapter)
     end
   end
 
-  describe '#send_monthly_attendance_reminder_emails', wip: true do
+  describe '#send_monthly_attendance_reminder_emails', :wip do
     it 'emails all attending members' do
       meeting = Fabricate(:meeting)
       attendees = Fabricate.times(2, :attending_meeting_invitation, meeting: meeting).map(&:member)
@@ -137,11 +137,11 @@
       end
 
       manager.send_workshop_attendance_reminders(workshop)
-      invitations.each { |invitation| expect(invitation.reload.reminded_at).to_not be_nil }
+      invitations.each { |invitation| expect(invitation.reload.reminded_at).not_to be_nil }
     end
   end
 
-  describe '#send_workshop_waiting_list_reminders', wip: true do
+  describe '#send_workshop_waiting_list_reminders', :wip do
     it 'emails everyone that hasn\'t already been reminded from the workshop\'s waitinglist' do
       workshop = Fabricate(:workshop)
       invitations = Fabricate.times(2, :waitinglist_invitation, workshop: workshop)
@@ -154,12 +154,12 @@
       end
 
       reminded_invitations.each do |invitation|
-        expect(WorkshopInvitationMailer).to_not receive(:waiting_list_reminder)
+        expect(WorkshopInvitationMailer).not_to receive(:waiting_list_reminder)
           .with(workshop, invitation.member, invitation)
       end
 
       manager.send_workshop_waiting_list_reminders(workshop)
-      invitations.each { |invitation| expect(invitation.reload.reminded_at).to_not be_nil }
+      invitations.each { |invitation| expect(invitation.reload.reminded_at).not_to be_nil }
     end
   end
 
@@ -240,4 +240,50 @@
       manager.send_meeting_emails(meeting)
     end
   end
+
+  describe '#create_invitation resilience' do
+    let(:member) { Fabricate(:member) }
+
+    it 'returns nil when find_or_create_by raises an exception' do
+      allow(WorkshopInvitation).to receive(:find_or_create_by)
+        .and_raise(StandardError.new('database error'))
+
+      result = manager.send(:create_invitation, workshop, member, 'Student')
+
+      expect(result).to be_nil
+    end
+
+    it 'logs error with member_id and workshop_id but no PII' do
+      allow(WorkshopInvitation).to receive(:find_or_create_by)
+        .and_raise(StandardError.new('database error'))
+
+      expect(Rails.logger).to receive(:error) do |message|
+        expect(message).to include("member_id=#{member.id}")
+        expect(message).to include("workshop_id=#{workshop.id}")
+        expect(message).to include('role=Student')
+        expect(message).not_to include(member.email)
+        expect(message).not_to include(member.name)
+      end
+
+      manager.send(:create_invitation, workshop, member, 'Student')
+    end
+
+    it 'continues processing when invitation creation fails for one member' do
+      Fabricate(:students, chapter: chapter, members: students)
+      call_count = 0
+
+      allow(WorkshopInvitation).to receive(:find_or_create_by) do
+        call_count += 1
+        if call_count == 1
+          raise StandardError.new('database error')
+        end
+
+        WorkshopInvitation.new(persisted?: true)
+      end
+
+      expect do
+        manager.send_workshop_emails(workshop, 'students')
+      end.not_to raise_error
+    end
+  end
 end

spec/support/shared_examples/behaves_like_sending_workshop_emails.rb

@@ -1,20 +1,20 @@
 RSpec.shared_examples 'sending workshop emails' do
   it 'creates an invitation for each student' do
-    student_group = Fabricate(:students, chapter: chapter, members: students)
+    Fabricate(:students, chapter: chapter, members: students)
 
     students.each do |student|
-      expect(WorkshopInvitation).to receive(:create).with(workshop: workshop, member: student, role: 'Student').and_call_original
+      expect(WorkshopInvitation).to receive(:find_or_create_by).with(workshop: workshop, member: student, role: 'Student').and_call_original
       expect(mailer).to receive(:invite_student).and_call_original
     end
 
     manager.send(send_email, workshop, 'students')
   end
 
   it 'creates an invitation for each coach' do
-    coach_group = Fabricate(:coaches, chapter: chapter, members: coaches)
+    Fabricate(:coaches, chapter: chapter, members: coaches)
 
     coaches.each do |coach|
-      expect(WorkshopInvitation).to receive(:create).with(workshop: workshop, member: coach, role: 'Coach').and_call_original
+      expect(WorkshopInvitation).to receive(:find_or_create_by).with(workshop: workshop, member: coach, role: 'Coach').and_call_original
       expect(mailer).to receive(:invite_coach).and_call_original
     end
 
@@ -23,32 +23,32 @@
 
   it 'does not invite banned coaches' do
     banned_coach = Fabricate(:banned_member)
-    coach_group = Fabricate(:coaches, chapter: chapter, members: coaches + [banned_coach])
+    Fabricate(:coaches, chapter: chapter, members: coaches + [banned_coach])
 
     coaches.each do |coach|
-      expect(WorkshopInvitation).to receive(:create).with(workshop: workshop, member: coach, role: 'Coach').and_call_original
+      expect(WorkshopInvitation).to receive(:find_or_create_by).with(workshop: workshop, member: coach, role: 'Coach').and_call_original
     end
-    expect(WorkshopInvitation).to_not receive(:create).with(workshop: workshop, member: banned_coach, role: 'Coach')
+    expect(WorkshopInvitation).not_to receive(:find_or_create_by).with(workshop: workshop, member: banned_coach, role: 'Coach')
 
     manager.send(send_email, workshop, 'coaches')
   end
 
   it 'sends emails when a WorkshopInvitation is created' do
-    student_group = Fabricate(:students, chapter: chapter, members: students)
-    coach_group = Fabricate(:coaches, chapter: chapter, members: coaches)
+    Fabricate(:students, chapter: chapter, members: students)
+    Fabricate(:coaches, chapter: chapter, members: coaches)
 
     expect do
       manager.send(send_email, workshop, 'everyone')
     end.to change { ActionMailer::Base.deliveries.count }.by(students.count + coaches.count)
   end
 
-  it "does not send emails when no invitation is created" do
-    student_group = Fabricate(:students, chapter: chapter, members: students)
+  it 'does not send emails when invitation creation returns nil' do
+    Fabricate(:students, chapter: chapter, members: students)
 
-    expect(WorkshopInvitation).to receive(:create).and_return(WorkshopInvitation.new).exactly(students.count)
+    expect(WorkshopInvitation).to receive(:find_or_create_by).and_return(nil).exactly(students.count)
 
     expect do
       manager.send(send_email, workshop, 'students')
-    end.not_to change { ActionMailer::Base.deliveries.count }
+    end.not_to(change { ActionMailer::Base.deliveries.count })
   end
 end