diff --git a/app/models/invitation_log.rb b/app/models/invitation_log.rb index 83cb0f13e..199c34d6d 100644 --- a/app/models/invitation_log.rb +++ b/app/models/invitation_log.rb @@ -6,7 +6,7 @@ class InvitationLog < ApplicationRecord belongs_to :loggable, polymorphic: true belongs_to :initiator, class_name: 'Member', optional: true belongs_to :chapter, optional: true - has_many :entries, class_name: 'InvitationLogEntry', dependent: :destroy + has_many :entries, class_name: 'InvitationLogEntry', dependent: :destroy, autosave: false before_create :set_expires_at diff --git a/app/services/invitation_logger.rb b/app/services/invitation_logger.rb index fc81c31a0..9c5e31898 100644 --- a/app/services/invitation_logger.rb +++ b/app/services/invitation_logger.rb @@ -23,7 +23,7 @@ def log_success(member, invitation = nil) return unless @log entry = find_or_build_entry(member, invitation, :success) - return entry if entry.persisted? + return entry if entry.processed_at entry.assign_attributes(processed_at: Time.current) save_entry(entry, :success_count) @@ -33,7 +33,7 @@ def log_failure(member, invitation, error) return unless @log entry = find_or_build_entry(member, invitation, :failed) - return entry if entry.persisted? + return entry if entry.processed_at entry.assign_attributes( failure_reason: error.message, @@ -46,7 +46,7 @@ def log_skipped(member, invitation, reason) return unless @log entry = find_or_build_entry(member, invitation, :skipped) - return entry if entry.persisted? + return entry if entry.processed_at entry.assign_attributes( failure_reason: reason, @@ -78,11 +78,9 @@ def fail_batch(error) private def find_or_build_entry(member, invitation, status) - @log.entries.find_or_initialize_by( - member:, - invitation:, - status: - ) + @log.entries.find_or_create_by(member: member, invitation: invitation) do |entry| + entry.status = status + end end def save_entry(entry, counter) diff --git a/spec/services/invitation_logger_spec.rb b/spec/services/invitation_logger_spec.rb index 14cd4b3c9..6595009d8 100644 --- a/spec/services/invitation_logger_spec.rb +++ b/spec/services/invitation_logger_spec.rb @@ -129,4 +129,32 @@ expect(log.completed_at).to be_present end end + + describe 'cross-status retry (PR #2558 fix)' do + let(:logger) { described_class.new(workshop, initiator, 'students', :invite) } + let!(:log) { logger.start_batch } + + it 'returns existing failure entry when logging success for same member+invitation' do + error = StandardError.new('SMTP error') + failure_entry = logger.log_failure(member, invitation, error) + + success_entry = logger.log_success(member, invitation) + + expect(success_entry).to eq failure_entry + expect(success_entry.status).to eq 'failed' + expect(log.reload.success_count).to eq 0 + expect(log.reload.failure_count).to eq 1 + end + + it 'returns existing success entry when logging failure for same member+invitation' do + logger.log_success(member, invitation) + + error = StandardError.new('SMTP error') + failure_entry = logger.log_failure(member, invitation, error) + + expect(failure_entry.status).to eq 'success' + expect(log.reload.success_count).to eq 1 + expect(log.reload.failure_count).to eq 0 + end + end end