Skip to content

Commit 44c2ff9

Browse files
authored
Merge pull request #2558 from mroderick/fix/invitation-log-retry-v2
fix: handle duplicate InvitationLogEntry on retry (v2)
2 parents 8b4c101 + 72f387c commit 44c2ff9

File tree

3 files changed

+35
-9
lines changed

3 files changed

+35
-9
lines changed

app/models/invitation_log.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ class InvitationLog < ApplicationRecord
66
belongs_to :loggable, polymorphic: true
77
belongs_to :initiator, class_name: 'Member', optional: true
88
belongs_to :chapter, optional: true
9-
has_many :entries, class_name: 'InvitationLogEntry', dependent: :destroy
9+
has_many :entries, class_name: 'InvitationLogEntry', dependent: :destroy, autosave: false
1010

1111
before_create :set_expires_at
1212

app/services/invitation_logger.rb

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ def log_success(member, invitation = nil)
2323
return unless @log
2424

2525
entry = find_or_build_entry(member, invitation, :success)
26-
return entry if entry.persisted?
26+
return entry if entry.processed_at
2727

2828
entry.assign_attributes(processed_at: Time.current)
2929
save_entry(entry, :success_count)
@@ -33,7 +33,7 @@ def log_failure(member, invitation, error)
3333
return unless @log
3434

3535
entry = find_or_build_entry(member, invitation, :failed)
36-
return entry if entry.persisted?
36+
return entry if entry.processed_at
3737

3838
entry.assign_attributes(
3939
failure_reason: error.message,
@@ -46,7 +46,7 @@ def log_skipped(member, invitation, reason)
4646
return unless @log
4747

4848
entry = find_or_build_entry(member, invitation, :skipped)
49-
return entry if entry.persisted?
49+
return entry if entry.processed_at
5050

5151
entry.assign_attributes(
5252
failure_reason: reason,
@@ -78,11 +78,9 @@ def fail_batch(error)
7878
private
7979

8080
def find_or_build_entry(member, invitation, status)
81-
@log.entries.find_or_initialize_by(
82-
member:,
83-
invitation:,
84-
status:
85-
)
81+
@log.entries.find_or_create_by(member: member, invitation: invitation) do |entry|
82+
entry.status = status
83+
end
8684
end
8785

8886
def save_entry(entry, counter)

spec/services/invitation_logger_spec.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,32 @@
129129
expect(log.completed_at).to be_present
130130
end
131131
end
132+
133+
describe 'cross-status retry (PR #2558 fix)' do
134+
let(:logger) { described_class.new(workshop, initiator, 'students', :invite) }
135+
let!(:log) { logger.start_batch }
136+
137+
it 'returns existing failure entry when logging success for same member+invitation' do
138+
error = StandardError.new('SMTP error')
139+
failure_entry = logger.log_failure(member, invitation, error)
140+
141+
success_entry = logger.log_success(member, invitation)
142+
143+
expect(success_entry).to eq failure_entry
144+
expect(success_entry.status).to eq 'failed'
145+
expect(log.reload.success_count).to eq 0
146+
expect(log.reload.failure_count).to eq 1
147+
end
148+
149+
it 'returns existing success entry when logging failure for same member+invitation' do
150+
logger.log_success(member, invitation)
151+
152+
error = StandardError.new('SMTP error')
153+
failure_entry = logger.log_failure(member, invitation, error)
154+
155+
expect(failure_entry.status).to eq 'success'
156+
expect(log.reload.success_count).to eq 1
157+
expect(log.reload.failure_count).to eq 0
158+
end
159+
end
132160
end

0 commit comments

Comments
 (0)