Merge pull request #2556 from mroderick/fix/invitation-log-retry-handling

fix: handle duplicate InvitationLogEntry creation on retry

Author: mroderick

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:,
+      invitation:,
+      status:
+    )
+  end
+
+  def save_entry(entry, counter)
+    entry.save!
+    @log.increment!(counter)
+    entry
+  end
+
   attr_reader :log
 end

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