diff --git a/app/controllers/admin/events_controller.rb b/app/controllers/admin/events_controller.rb index 26126af25..eab8cdb58 100644 --- a/app/controllers/admin/events_controller.rb +++ b/app/controllers/admin/events_controller.rb @@ -45,7 +45,7 @@ def invite authorize @event @event.chapters.each do |chapter| - InvitationManager.new.send_event_emails(@event, chapter) + InvitationManager.new.send_event_emails(@event, chapter, current_user.id) end redirect_to admin_event_path(@event), notice: 'Invitations will be emailed out soon.' diff --git a/app/services/invitation_logger.rb b/app/services/invitation_logger.rb index 9c5e31898..ef41c128a 100644 --- a/app/services/invitation_logger.rb +++ b/app/services/invitation_logger.rb @@ -1,9 +1,10 @@ class InvitationLogger - def initialize(loggable, initiator, audience, action) + def initialize(loggable, initiator, audience, action, chapter_id: nil) @loggable = loggable @initiator = initiator @audience = audience @action = action + @chapter_id = chapter_id @log = nil end @@ -11,7 +12,7 @@ def start_batch @log = InvitationLog.create!( loggable: @loggable, initiator: @initiator, - chapter_id: @loggable.try(:chapter_id), + chapter_id: resolved_chapter_id, audience: @audience, action: @action, started_at: Time.current, @@ -77,6 +78,10 @@ def fail_batch(error) private + def resolved_chapter_id + @chapter_id || @loggable.try(:chapter_id) || raise(ArgumentError, 'chapter_id is required') + end + def find_or_build_entry(member, invitation, status) @log.entries.find_or_create_by(member: member, invitation: invitation) do |entry| entry.status = status diff --git a/app/services/invitation_manager.rb b/app/services/invitation_manager.rb index 9cffe1c21..4c101993a 100644 --- a/app/services/invitation_manager.rb +++ b/app/services/invitation_manager.rb @@ -1,12 +1,23 @@ class InvitationManager - def send_event_emails(event, chapter) + def send_event_emails(event, chapter, initiator_id = nil) # rubocop:disable Metrics/AbcSize return 'The event is not invitable' unless event.invitable? - invite_coaches_to_event(event, chapter) unless event.audience.eql?('Students') - invite_students_to_event(event, chapter) unless event.audience.eql?('Coaches') - rescue StandardError => e - Rollbar.error(e, event_id: event.id, chapter_id: chapter.id) - raise + audience = event_audience(event) + logger = invitation_logger(event, initiator_id, audience, chapter.id) + + result = start_invitation_batch(logger) + return result if result.is_a?(String) + + total = 0 + begin + total += invite_coaches_to_event(event, chapter, logger) unless event.audience.eql?('Students') + total += invite_students_to_event(event, chapter, logger) unless event.audience.eql?('Coaches') + logger&.finish_batch(total) + rescue StandardError => e + Rollbar.error(e, event_id: event.id, chapter_id: chapter.id) + logger&.fail_batch(e) + raise + end end handle_asynchronously :send_event_emails @@ -42,25 +53,14 @@ def send_workshop_attendance_reminders(workshop) def send_workshop_emails(workshop, audience, initiator_id = nil) return 'The workshop is not invitable' unless workshop.invitable? - initiator = initiator_id ? Member.find_by(id: initiator_id) : nil - logger = initiator ? InvitationLogger.new(workshop, initiator, audience, :invite) : nil - - if logger - begin - logger.start_batch - rescue ActiveRecord::RecordNotUnique - return 'A batch is already running for this workshop and audience' - end - end + logger = invitation_logger(workshop, initiator_id, audience, workshop.chapter_id) + result = start_invitation_batch(logger) + return result if result.is_a?(String) total = 0 begin - if audience.in?(%w[students everyone]) - total += invite_students_to_workshop(workshop, logger) - end - if audience.in?(%w[coaches everyone]) - total += invite_coaches_to_workshop(workshop, logger) - end + total += invite_students_to_workshop(workshop, logger) if audience.in?(%w[students everyone]) + total += invite_coaches_to_workshop(workshop, logger) if audience.in?(%w[coaches everyone]) logger&.finish_batch(total) rescue StandardError => e logger&.fail_batch(e) @@ -72,25 +72,14 @@ def send_workshop_emails(workshop, audience, initiator_id = nil) def send_virtual_workshop_emails(workshop, audience, initiator_id = nil) return 'The workshop is not invitable' unless workshop.invitable? - initiator = initiator_id ? Member.find_by(id: initiator_id) : nil - logger = initiator ? InvitationLogger.new(workshop, initiator, audience, :invite) : nil - - if logger - begin - logger.start_batch - rescue ActiveRecord::RecordNotUnique - return 'A batch is already running for this workshop and audience' - end - end + logger = invitation_logger(workshop, initiator_id, audience, workshop.chapter_id) + result = start_invitation_batch(logger) + return result if result.is_a?(String) total = 0 begin - if audience.in?(%w[students everyone]) - total += invite_students_to_virtual_workshop(workshop, logger) - end - if audience.in?(%w[coaches everyone]) - total += invite_coaches_to_virtual_workshop(workshop, logger) - end + total += invite_students_to_virtual_workshop(workshop, logger) if audience.in?(%w[students everyone]) + total += invite_coaches_to_virtual_workshop(workshop, logger) if audience.in?(%w[coaches everyone]) logger&.finish_batch(total) rescue StandardError => e logger&.fail_batch(e) @@ -99,14 +88,6 @@ def send_virtual_workshop_emails(workshop, audience, initiator_id = nil) end handle_asynchronously :send_virtual_workshop_emails - def send_waiting_list_emails(workshop) - workshop = WorkshopPresenter.decorate(workshop) - - retrieve_and_notify_waitlisted(workshop, role: 'Coach') if workshop.coach_spaces? - retrieve_and_notify_waitlisted(workshop, role: 'Student') if workshop.student_spaces? - end - handle_asynchronously :send_waiting_list_emails - def send_workshop_waiting_list_reminders(workshop) workshop_mailer = workshop.virtual? ? VirtualWorkshopInvitationMailer : WorkshopInvitationMailer workshop.invitations.on_waiting_list.not_reminded.each do |invitation| @@ -116,28 +97,54 @@ def send_workshop_waiting_list_reminders(workshop) end handle_asynchronously :send_workshop_waiting_list_reminders + def send_waiting_list_emails(workshop) + workshop = WorkshopPresenter.decorate(workshop) + + retrieve_and_notify_waitlisted(workshop, role: 'Coach') if workshop.coach_spaces? + retrieve_and_notify_waitlisted(workshop, role: 'Student') if workshop.student_spaces? + end + handle_asynchronously :send_waiting_list_emails + private - def invite_students_to_event(event, chapter) + def invite_students_to_event(event, chapter, logger = nil) + count = 0 chapter_students(chapter).each do |student| - invitation = Invitation.new(event: event, member: student, role: 'Student') - next unless invitation.save + invitation = create_event_invitation(event, student, 'Student') + next unless invitation - EventInvitationMailer.invite_student(event, student, invitation).deliver_later + if invitation.previously_new_record? + count += 1 + send_email_with_logging(logger, student, invitation) do + EventInvitationMailer.invite_student(event, student, invitation).deliver_later + end + else + logger&.log_skipped(student, invitation, 'Already invited to this event') + end rescue StandardError => e log_event_meeting_invitation_failure("event_id=#{event.id}", student, e) end + count end - def invite_coaches_to_event(event, chapter) + def invite_coaches_to_event(event, chapter, logger = nil) + count = 0 chapter_coaches(chapter).each do |coach| - invitation = Invitation.new(event: event, member: coach, role: 'Coach') - next unless invitation.save + invitation = create_event_invitation(event, coach, 'Coach') + next unless invitation - EventInvitationMailer.invite_coach(event, coach, invitation).deliver_later + if invitation.previously_new_record? + count += 1 + send_email_with_logging(logger, coach, invitation) do + EventInvitationMailer.invite_coach(event, coach, invitation).deliver_later + end + else + logger&.log_skipped(coach, invitation, 'Already invited to this event') + end rescue StandardError => e log_event_meeting_invitation_failure("event_id=#{event.id}", coach, e) end + count end def log_event_meeting_invitation_failure(context, member, error) @@ -157,14 +164,19 @@ def chapter_coaches(chapter) end def create_invitation(workshop, member, role) - invitation = WorkshopInvitation.find_or_initialize_by(workshop: workshop, member: member, role: role) - invitation.save! if invitation.new_record? - invitation + WorkshopInvitation.find_or_create_by!(workshop: workshop, member: member, role: role) rescue StandardError => e log_invitation_failure(workshop, member, role, e) nil end + def create_event_invitation(event, member, role) + Invitation.find_or_create_by!(event: event, member: member, role: role) + rescue StandardError => e + log_event_meeting_invitation_failure("event_id=#{event.id}", member, e) + nil + end + def log_invitation_failure(workshop, member, role, error) Rails.logger.error( '[InvitationManager] Failed to create invitation: ' \ @@ -216,6 +228,13 @@ def invite_members(workshop, logger, members, role = 'Coach') count end + 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_later + waiting_list.destroy + end + end + def send_email_with_logging(logger, member, invitation) if logger begin @@ -229,10 +248,28 @@ def send_email_with_logging(logger, member, invitation) end end - 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_later - waiting_list.destroy + def event_audience(event) + case event.audience + when 'Students' then 'students' + when 'Coaches' then 'coaches' + else 'everyone' + end + end + + def invitation_logger(loggable, initiator_id, audience, chapter_id) + initiator = Member.find_by(id: initiator_id) + return nil unless initiator + + InvitationLogger.new(loggable, initiator, audience, :invite, chapter_id: chapter_id) + end + + def start_invitation_batch(logger) + return unless logger + + begin + logger.start_batch + rescue ActiveRecord::RecordNotUnique + 'A batch is already running for this loggable and audience' end end end diff --git a/db/migrate/20260729151201_add_chapter_id_to_invitation_logs_unique_active_index.rb b/db/migrate/20260729151201_add_chapter_id_to_invitation_logs_unique_active_index.rb new file mode 100644 index 000000000..6ce77ddf9 --- /dev/null +++ b/db/migrate/20260729151201_add_chapter_id_to_invitation_logs_unique_active_index.rb @@ -0,0 +1,29 @@ +class AddChapterIdToInvitationLogsUniqueActiveIndex < ActiveRecord::Migration[8.1] + disable_ddl_transaction! + + def up + remove_index :invitation_logs, + name: 'index_invitation_logs_unique_active', + algorithm: :concurrently + + add_index :invitation_logs, + %i[loggable_type loggable_id chapter_id audience action status], + unique: true, + where: "status = 'running'", + name: 'index_invitation_logs_unique_active', + algorithm: :concurrently + end + + def down + remove_index :invitation_logs, + name: 'index_invitation_logs_unique_active', + algorithm: :concurrently + + add_index :invitation_logs, + %i[loggable_type loggable_id audience action status], + unique: true, + where: "status = 'running'", + name: 'index_invitation_logs_unique_active', + algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index b2be0a713..9283847eb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_07_02_102700) do +ActiveRecord::Schema[8.1].define(version: 2026_07_29_151201) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -201,6 +201,7 @@ create_table "events", id: :serial, force: :cascade do |t| t.boolean "announce_only" t.string "audience" + t.string "check_in_code" t.text "coach_description" t.string "coach_questionnaire" t.integer "coach_spaces" @@ -228,6 +229,7 @@ t.string "url" t.integer "venue_id" t.boolean "virtual", default: false, null: false + t.index ["check_in_code"], name: "index_events_on_check_in_code", unique: true t.index ["date_and_time"], name: "index_events_on_date_and_time" t.index ["slug"], name: "index_events_on_slug", unique: true t.index ["venue_id"], name: "index_events_on_venue_id" @@ -327,7 +329,7 @@ t.index ["created_at"], name: "index_invitation_logs_on_created_at" t.index ["expires_at"], name: "index_invitation_logs_on_expires_at" t.index ["initiator_id"], name: "index_invitation_logs_on_initiator_id" - t.index ["loggable_type", "loggable_id", "audience", "action", "status"], name: "index_invitation_logs_unique_active", unique: true, where: "((status)::text = 'running'::text)" + t.index ["loggable_type", "loggable_id", "chapter_id", "audience", "action", "status"], name: "index_invitation_logs_unique_active", unique: true, where: "((status)::text = 'running'::text)" t.index ["loggable_type", "loggable_id"], name: "index_invitation_logs_on_loggable" t.index ["status"], name: "index_invitation_logs_on_status" end @@ -339,6 +341,7 @@ t.integer "member_id" t.text "note" t.string "role" + t.string "source" t.string "token" t.datetime "updated_at", precision: nil t.boolean "verified" @@ -592,6 +595,7 @@ t.datetime "reminded_at", precision: nil t.string "role" t.datetime "rsvp_time", precision: nil + t.string "source" t.string "token" t.text "tutorial" t.datetime "updated_at", precision: nil @@ -616,6 +620,7 @@ create_table "workshops", id: :serial, force: :cascade do |t| t.integer "chapter_id" + t.string "check_in_code" t.integer "coach_spaces", default: 0 t.datetime "created_at", precision: nil t.datetime "date_and_time", precision: nil @@ -632,6 +637,7 @@ t.datetime "updated_at", precision: nil t.boolean "virtual", default: false t.index ["chapter_id"], name: "index_workshops_on_chapter_id" + t.index ["check_in_code"], name: "index_workshops_on_check_in_code", unique: true t.index ["date_and_time"], name: "index_workshops_on_date_and_time" end diff --git a/spec/controllers/admin/events_controller_spec.rb b/spec/controllers/admin/events_controller_spec.rb new file mode 100644 index 000000000..5f386b5f6 --- /dev/null +++ b/spec/controllers/admin/events_controller_spec.rb @@ -0,0 +1,45 @@ +RSpec.describe Admin::EventsController, type: :controller do + let(:admin) { Fabricate(:member) } + let(:event) { Fabricate(:event, chapters: [chapter]) } + let(:chapter) { Fabricate(:chapter) } + + before do + admin.add_role(:organiser, event) + login(admin) + end + + describe 'POST #invite' do + it 'passes current_user.id to send_event_emails' do + manager = instance_spy(InvitationManager) + allow(InvitationManager).to receive(:new).and_return(manager) + + post :invite, params: { event_id: event.slug } + + expect(manager).to have_received(:send_event_emails).with(event, chapter, admin.id) + end + + it 'redirects to the event page' do + post :invite, params: { event_id: event.slug } + + expect(response).to redirect_to(admin_event_path(event)) + end + + it 'sets a notice' do + post :invite, params: { event_id: event.slug } + + expect(flash[:notice]).to eq('Invitations will be emailed out soon.') + end + end + + describe 'POST #invite with unauthorised user' do + it 'redirects unauthorised users' do + unauthorised = Fabricate(:member) + login(unauthorised) + + post :invite, params: { event_id: event.slug } + + expect(response).to redirect_to(root_path) + expect(flash[:notice]).to eq('You can\'t be here') + end + end +end diff --git a/spec/fabricators/invitation_log_fabricator.rb b/spec/fabricators/invitation_log_fabricator.rb index 8652bac15..2078fa9ef 100644 --- a/spec/fabricators/invitation_log_fabricator.rb +++ b/spec/fabricators/invitation_log_fabricator.rb @@ -4,4 +4,5 @@ audience 'students' action 'invite' status 'running' + chapter_id { |attrs| attrs[:loggable].try(:chapter_id) } end diff --git a/spec/services/invitation_manager_logging_spec.rb b/spec/services/invitation_manager_logging_spec.rb index 8c31c46b6..f8fef3105 100644 --- a/spec/services/invitation_manager_logging_spec.rb +++ b/spec/services/invitation_manager_logging_spec.rb @@ -51,9 +51,11 @@ end it 'prevents duplicate concurrent batches when start_batch is called' do - Fabricate(:invitation_log, loggable: workshop, audience: 'students', action: 'invite', status: :running) + Fabricate(:invitation_log, loggable: workshop, audience: 'students', action: 'invite', + status: :running, chapter_id: workshop.chapter_id) - logger = InvitationLogger.new(workshop, initiator, 'students', :invite) + logger = InvitationLogger.new(workshop, initiator, 'students', :invite, + chapter_id: workshop.chapter_id) expect { logger.start_batch }.to raise_error(ActiveRecord::RecordNotUnique) end @@ -80,6 +82,18 @@ expect(log.status).to eq 'completed' expect(log.error_message).to be_nil end + + it 'returns early and sends no emails when a batch is already running' do + Fabricate(:invitation_log, loggable: workshop, audience: 'students', action: 'invite', + status: :running, chapter_id: workshop.chapter_id) + + allow(WorkshopInvitationMailer).to receive(:invite_student).and_call_original + result = manager.send_workshop_emails_without_delay(workshop, 'students', initiator.id) + + expect(result).to eq 'A batch is already running for this loggable and audience' + expect(WorkshopInvitationMailer).not_to have_received(:invite_student) + expect(InvitationLog.count).to eq 1 + end end describe '#send_virtual_workshop_emails with logging' do @@ -97,5 +111,66 @@ log = InvitationLog.last expect(log.success_count).to eq students.count end + + it 'returns early and sends no emails when a batch is already running' do + Fabricate(:invitation_log, loggable: workshop, audience: 'students', action: 'invite', + status: :running, chapter_id: workshop.chapter_id) + + allow(VirtualWorkshopInvitationMailer).to receive(:invite_student).and_call_original + result = manager.send_virtual_workshop_emails_without_delay(workshop, 'students', initiator.id) + + expect(result).to eq 'A batch is already running for this loggable and audience' + expect(VirtualWorkshopInvitationMailer).not_to have_received(:invite_student) + expect(InvitationLog.count).to eq 1 + end + end + + describe '#send_event_emails with logging' do + let(:event) { Fabricate(:event, chapters: [chapter]) } + + it 'creates an InvitationLog with the chapter and initiator' do + expect do + manager.send_event_emails(event, chapter, initiator.id) + end.to change(InvitationLog, :count).by(1) + + log = InvitationLog.last + expect(log.loggable).to eq event + expect(log.initiator).to eq initiator + expect(log.chapter_id).to eq chapter.id + expect(log.audience).to eq 'everyone' + expect(log.status).to eq 'completed' + end + + it 'logs successful email sends' do + manager.send_event_emails(event, chapter, initiator.id) + + log = InvitationLog.last + expect(log.success_count).to eq(students.count + coaches.count) + expect(log.failure_count).to eq 0 + end + + it 'returns early and sends no emails when a batch is already running' do + Fabricate(:invitation_log, loggable: event, audience: 'everyone', action: 'invite', + status: :running, chapter_id: chapter.id) + + allow(EventInvitationMailer).to receive(:invite_student).and_call_original + result = manager.send_event_emails_without_delay(event, chapter, initiator.id) + + expect(result).to eq 'A batch is already running for this loggable and audience' + expect(EventInvitationMailer).not_to have_received(:invite_student) + expect(InvitationLog.count).to eq 1 + end + + it 'logs skipped members on a second run without resending emails' do + manager.send_event_emails(event, chapter, initiator.id) + + allow(EventInvitationMailer).to receive(:invite_student).and_call_original + manager.send_event_emails(event, chapter, initiator.id) + + expect(EventInvitationMailer).not_to have_received(:invite_student) + log = InvitationLog.last + expect(log.skipped_count).to eq(students.count + coaches.count) + expect(log.success_count).to eq 0 + end end end diff --git a/spec/services/invitation_manager_spec.rb b/spec/services/invitation_manager_spec.rb index 418acc953..8f9451865 100644 --- a/spec/services/invitation_manager_spec.rb +++ b/spec/services/invitation_manager_spec.rb @@ -29,17 +29,19 @@ it 'can email only students' do event = Fabricate(:event, chapters: [chapter], audience: 'Students') students.each do |student| - allow(Invitation).to receive(:new).with(event: event, member: student, role: 'Student').and_call_original + allow(Invitation).to receive(:find_or_create_by!).with( + event: event, member: student, role: 'Student' + ).and_call_original end manager.send_event_emails(event, chapter) students.each do |student| - expect(Invitation).to have_received(:new).with(event: event, member: student, role: 'Student') + expect(Invitation).to have_received(:find_or_create_by!).with(event: event, member: student, role: 'Student') end coaches.each do |student| - expect(Invitation).not_to have_received(:new).with(event: event, member: student, role: 'Coach') + expect(Invitation).not_to have_received(:find_or_create_by!).with(event: event, member: student, role: 'Coach') end end @@ -47,17 +49,19 @@ event = Fabricate(:event, chapters: [chapter], audience: 'Coaches') coaches.each do |student| - allow(Invitation).to receive(:new).with(event: event, member: student, role: 'Coach').and_call_original + allow(Invitation).to receive(:find_or_create_by!).with( + event: event, member: student, role: 'Coach' + ).and_call_original end manager.send_event_emails(event, chapter) students.each do |student| - expect(Invitation).not_to have_received(:new).with(event: event, member: student, role: 'Student') + expect(Invitation).not_to have_received(:find_or_create_by!).with(event: event, member: student, role: 'Student') end coaches.each do |student| - expect(Invitation).to have_received(:new).with(event: event, member: student, role: 'Coach') + expect(Invitation).to have_received(:find_or_create_by!).with(event: event, member: student, role: 'Coach') end end @@ -65,21 +69,25 @@ event = Fabricate(:event, chapters: [chapter]) students.each do |student| - allow(Invitation).to receive(:new).with(event: event, member: student, role: 'Student').and_call_original + allow(Invitation).to receive(:find_or_create_by!).with( + event: event, member: student, role: 'Student' + ).and_call_original end coaches.each do |student| - allow(Invitation).to receive(:new).with(event: event, member: student, role: 'Coach').and_call_original + allow(Invitation).to receive(:find_or_create_by!).with( + event: event, member: student, role: 'Coach' + ).and_call_original end manager.send_event_emails(event, chapter) students.each do |student| - expect(Invitation).to have_received(:new).with(event: event, member: student, role: 'Student') + expect(Invitation).to have_received(:find_or_create_by!).with(event: event, member: student, role: 'Student') end coaches.each do |student| - expect(Invitation).to have_received(:new).with(event: event, member: student, role: 'Coach') + expect(Invitation).to have_received(:find_or_create_by!).with(event: event, member: student, role: 'Coach') end end @@ -91,7 +99,7 @@ other_students.each do |other_student| allow(Invitation).to( - receive(:new) + receive(:find_or_create_by!) .with(event: event, member: other_student, role: 'Student') .and_call_original ) @@ -99,10 +107,10 @@ manager.send_event_emails(event, chapter) - expect(Invitation).not_to have_received(:new).with(event: event, member: first_student, role: 'Student') + expect(Invitation).not_to have_received(:find_or_create_by!).with(event: event, member: first_student, role: 'Student') other_students.each do |other_student| - expect(Invitation).to have_received(:new).with(event: event, member: other_student, role: 'Student') + expect(Invitation).to have_received(:find_or_create_by!).with(event: event, member: other_student, role: 'Student') end end @@ -114,7 +122,7 @@ other_coaches.each do |other_coach| allow(Invitation).to( - receive(:new) + receive(:find_or_create_by!) .with(event: event, member: other_coach, role: 'Coach') .and_call_original ) @@ -122,35 +130,12 @@ manager.send_event_emails(event, chapter) - expect(Invitation).not_to have_received(:new).with(event: event, member: first_coach, role: 'Coach') + expect(Invitation).not_to have_received(:find_or_create_by!).with(event: event, member: first_coach, role: 'Coach') other_coaches.each do |other_coach| - expect(Invitation).to have_received(:new).with(event: event, member: other_coach, role: 'Coach') + expect(Invitation).to have_received(:find_or_create_by!).with(event: event, member: other_coach, role: 'Coach') end end - - it 'sends invitation emails for all eligible members' do - event = Fabricate(:event, chapters: [chapter]) - - expect do - manager.send_event_emails(event, chapter) - end.to change { ActionMailer::Base.deliveries.count }.by(students.count + coaches.count) - end - - it 'reports batch-level failures to Rollbar' do - event = Fabricate(:event, chapters: [chapter]) - allow(event).to receive(:invitable?).and_raise(StandardError.new('DB connection error')) - - allow(Rollbar).to receive(:error) - - expect { manager.send_event_emails(event, chapter) }.to raise_error(StandardError, 'DB connection error') - - expect(Rollbar).to have_received(:error).with( - an_instance_of(StandardError), - event_id: event.id, - chapter_id: chapter.id - ) - end end describe '#send_monthly_attendance_reminder_emails' do @@ -314,8 +299,8 @@ describe '#create_invitation resilience' do let(:member) { Fabricate(:member) } - it 'returns nil when find_or_initialize_by raises an exception' do - allow(WorkshopInvitation).to receive(:find_or_initialize_by) + 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') @@ -324,27 +309,26 @@ end it 'logs error with member_id and workshop_id but no PII' do - allow(WorkshopInvitation).to receive(:find_or_initialize_by) + allow(WorkshopInvitation).to receive(:find_or_create_by!) .and_raise(StandardError.new('database error')) - - logged_message = nil - allow(Rails.logger).to receive(:error) { |message| logged_message = message } + allow(Rails.logger).to receive(:error) manager.send(:create_invitation, workshop, member, 'Student') - expect(Rails.logger).to have_received(:error) - expect(logged_message).to include("member_id=#{member.id}") - expect(logged_message).to include("workshop_id=#{workshop.id}") - expect(logged_message).to include('role=Student') - expect(logged_message).not_to include(member.email) - expect(logged_message).not_to match(/#{Regexp.escape(member.name)}/) + expect(Rails.logger).to have_received(: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 match(/\b#{Regexp.escape(member.name)}\b/) + end 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_initialize_by) do + allow(WorkshopInvitation).to receive(:find_or_create_by!) do call_count += 1 if call_count == 1 raise StandardError, 'database error' diff --git a/spec/support/shared_examples/behaves_like_sending_workshop_emails.rb b/spec/support/shared_examples/behaves_like_sending_workshop_emails.rb index 24e319a56..428bf6c35 100644 --- a/spec/support/shared_examples/behaves_like_sending_workshop_emails.rb +++ b/spec/support/shared_examples/behaves_like_sending_workshop_emails.rb @@ -3,7 +3,7 @@ Fabricate(:students, chapter: chapter, members: students) students.each do |student| - allow(WorkshopInvitation).to receive(:find_or_initialize_by).with(workshop: workshop, member: student, role: 'Student').and_call_original + allow(WorkshopInvitation).to receive(:find_or_create_by!).with(workshop: workshop, member: student, role: 'Student').and_call_original end expect do @@ -12,7 +12,7 @@ .and change { WorkshopInvitation.where(workshop: workshop, role: 'Student').count }.by(students.count) students.each do |student| - expect(WorkshopInvitation).to have_received(:find_or_initialize_by).with(workshop: workshop, member: student, role: 'Student') + expect(WorkshopInvitation).to have_received(:find_or_create_by!).with(workshop: workshop, member: student, role: 'Student') end # Verify emails were sent to the right recipients @@ -25,7 +25,7 @@ Fabricate(:coaches, chapter: chapter, members: coaches) coaches.each do |coach| - allow(WorkshopInvitation).to receive(:find_or_initialize_by).with(workshop: workshop, member: coach, role: 'Coach').and_call_original + allow(WorkshopInvitation).to receive(:find_or_create_by!).with(workshop: workshop, member: coach, role: 'Coach').and_call_original end expect do @@ -34,7 +34,7 @@ .and change { WorkshopInvitation.where(workshop: workshop, role: 'Coach').count }.by(coaches.count) coaches.each do |coach| - expect(WorkshopInvitation).to have_received(:find_or_initialize_by).with(workshop: workshop, member: coach, role: 'Coach') + expect(WorkshopInvitation).to have_received(:find_or_create_by!).with(workshop: workshop, member: coach, role: 'Coach') end # Verify emails were sent to the right recipients @@ -48,15 +48,15 @@ Fabricate(:coaches, chapter: chapter, members: coaches + [banned_coach]) coaches.each do |coach| - allow(WorkshopInvitation).to receive(:find_or_initialize_by).with(workshop: workshop, member: coach, role: 'Coach').and_call_original + allow(WorkshopInvitation).to receive(:find_or_create_by!).with(workshop: workshop, member: coach, role: 'Coach').and_call_original end manager.send(send_email, workshop, 'coaches') coaches.each do |coach| - expect(WorkshopInvitation).to have_received(:find_or_initialize_by).with(workshop: workshop, member: coach, role: 'Coach') + expect(WorkshopInvitation).to have_received(:find_or_create_by!).with(workshop: workshop, member: coach, role: 'Coach') end - expect(WorkshopInvitation).not_to have_received(:find_or_initialize_by).with(workshop: workshop, member: banned_coach, role: 'Coach') + expect(WorkshopInvitation).not_to have_received(:find_or_create_by!).with(workshop: workshop, member: banned_coach, role: 'Coach') end it 'sends emails when a WorkshopInvitation is created' do @@ -71,13 +71,13 @@ it 'does not send emails when invitation creation returns nil' do Fabricate(:students, chapter: chapter, members: students) - allow(WorkshopInvitation).to receive(:find_or_initialize_by).and_return(nil).exactly(students.count) + allow(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 }) - expect(WorkshopInvitation).to have_received(:find_or_initialize_by).exactly(students.count).times + expect(WorkshopInvitation).to have_received(:find_or_create_by!).exactly(students.count).times end it 'does not send duplicate emails when members are already invited' do