From 9adf2598251b3951ef58cf373604cecfe5d1090f Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Thu, 9 Apr 2026 10:17:25 +0200 Subject: [PATCH 1/2] fix: handle duplicate InvitationLogEntry creation on retry When re-sending workshop invitations, the InvitationLogger would attempt to create a new InvitationLogEntry for members with existing invitations, causing a uniqueness validation error: 'Member has already been taken'. This fix uses Rails' idiomatic find_or_initialize_by pattern to check if an entry already exists before creating a new one. If the entry exists (meaning the member was already processed), it returns the existing entry without incrementing the counter. Root cause: - InvitationLogEntry validates uniqueness on (member_id, invitation_type, invitation_id) - Re-sending invitations to the same workshop would try to log success for an already-logged member+invitation combination - This caused batch jobs to fail mid-process with validation errors Affected scenarios: - Re-sending invitations for the same workshop - Sending invitations with audience='everyone' (processes both students and coaches, where a member might be in both groups) - Retrying failed invitation batches Fix: - Replace create! with find_or_initialize_by in log_success, log_failure, and log_skipped methods - Return existing entry if already persisted - Only increment counter when creating a new entry - Extract shared helper methods for cleaner code Tests: - Add retry tests for all three logging methods to verify no duplicate entries are created on repeated calls with the same member+invitation --- app/services/invitation_logger.rb | 49 +++++++++++++++++-------- spec/services/invitation_logger_spec.rb | 25 +++++++++++++ 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/app/services/invitation_logger.rb b/app/services/invitation_logger.rb index 5241aa8c7..3cfc607d3 100644 --- a/app/services/invitation_logger.rb +++ b/app/services/invitation_logger.rb @@ -22,36 +22,37 @@ def start_batch def log_success(member, invitation = nil) return unless @log - @log.entries.create!( - member: member, - invitation: invitation, - status: :success, - processed_at: Time.current - ).tap { @log.increment!(:success_count) } + entry = find_or_build_entry(member, invitation, :success) + return entry if entry.persisted? + + entry.assign_attributes(processed_at: Time.current) + save_entry(entry, :success_count) end def log_failure(member, invitation, error) return unless @log - @log.entries.create!( - member: member, - invitation: invitation, - status: :failed, + entry = find_or_build_entry(member, invitation, :failed) + return entry if entry.persisted? + + entry.assign_attributes( failure_reason: error.message, processed_at: Time.current - ).tap { @log.increment!(:failure_count) } + ) + save_entry(entry, :failure_count) end def log_skipped(member, invitation, reason) return unless @log - @log.entries.create!( - member: member, - invitation: invitation, - status: :skipped, + entry = find_or_build_entry(member, invitation, :skipped) + return entry if entry.persisted? + + entry.assign_attributes( failure_reason: reason, processed_at: Time.current - ).tap { @log.increment!(:skipped_count) } + ) + save_entry(entry, :skipped_count) end def finish_batch(total_invitees) @@ -74,5 +75,21 @@ def fail_batch(error) ) end + private + + def find_or_build_entry(member, invitation, status) + @log.entries.find_or_initialize_by( + member: member, + invitation: invitation, + status: status + ) + end + + def save_entry(entry, counter) + entry.save! + @log.increment!(counter) + entry + end + attr_reader :log end diff --git a/spec/services/invitation_logger_spec.rb b/spec/services/invitation_logger_spec.rb index 934d228d2..14cd4b3c9 100644 --- a/spec/services/invitation_logger_spec.rb +++ b/spec/services/invitation_logger_spec.rb @@ -46,6 +46,14 @@ expect(entry.invitation).to eq invitation expect(log.reload.success_count).to eq 1 end + + it 'does not create duplicate entry on retry' do + entry1 = logger.log_success(member, invitation) + entry2 = logger.log_success(member, invitation) + + expect(entry2).to eq entry1 + expect(log.reload.success_count).to eq 1 + end end describe '#log_failure' do @@ -60,6 +68,15 @@ expect(entry.failure_reason).to eq 'SMTP error' expect(log.reload.failure_count).to eq 1 end + + it 'does not create duplicate entry on retry' do + error = StandardError.new('SMTP error') + entry1 = logger.log_failure(member, invitation, error) + entry2 = logger.log_failure(member, invitation, error) + + expect(entry2).to eq entry1 + expect(log.reload.failure_count).to eq 1 + end end describe '#log_skipped' do @@ -73,6 +90,14 @@ expect(entry.failure_reason).to eq 'Already invited' expect(log.reload.skipped_count).to eq 1 end + + it 'does not create duplicate entry on retry' do + entry1 = logger.log_skipped(member, invitation, 'Already invited') + entry2 = logger.log_skipped(member, invitation, 'Already invited') + + expect(entry2).to eq entry1 + expect(log.reload.skipped_count).to eq 1 + end end describe '#finish_batch' do From de2eae1c7e460ca8d0850c79be695cb11fafc582 Mon Sep 17 00:00:00 2001 From: Morgan Roderick <20321+mroderick@users.noreply.github.com> Date: Thu, 9 Apr 2026 10:50:46 +0200 Subject: [PATCH 2/2] Update app/services/invitation_logger.rb Use modern ruby :) Co-authored-by: Olle Jonsson --- app/services/invitation_logger.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/services/invitation_logger.rb b/app/services/invitation_logger.rb index 3cfc607d3..fc81c31a0 100644 --- a/app/services/invitation_logger.rb +++ b/app/services/invitation_logger.rb @@ -79,9 +79,9 @@ def fail_batch(error) def find_or_build_entry(member, invitation, status) @log.entries.find_or_initialize_by( - member: member, - invitation: invitation, - status: status + member:, + invitation:, + status: ) end