Skip to content

Commit 75ecc09

Browse files
committed
fix: deduplicate members in chapter_students and chapter_coaches
Members subscribed to multiple groups within a chapter were being processed multiple times when sending invitations with audience='everyone', causing 'Member has already been taken' validation errors. Root Cause: - Member.in_group() uses JOIN on subscriptions, returning duplicate rows when a member is subscribed to the same group type multiple times - 336 members in London chapter affected (subscribed to both Students and Coaches) - 35 members in Berlin chapter affected - 57 members in Brighton chapter affected Impact: - Workshop invitation batches failed mid-process (e.g., 517 of 7000+ invitations sent before failure) - InvitationLogEntry validates uniqueness of member_id per invitation, causing validation errors on second processing attempt - Admins had to manually retry batch sends multiple times Affected Chapters: - 46 of 54 chapters (85%) have members with duplicate subscriptions - ~1,200 members affected across all chapters - Top 5: London (336), West London (79), Brighton (57), South London (57), Barcelona (55), Edinburgh (37), Berlin (35) Timeline: - Original code (Jan 2017, commit 448acc7) included .uniq for deduplication - Refactoring (May 2018, commit d7a463d) extracted to scopes but accidentally removed the .uniq call Fix: Add .distinct to chapter_students and chapter_coaches methods, restoring the deduplication behavior that existed before the refactoring. Tests: - Add spec for unique members when subscribed to same group multiple times - Add spec for unique invitations per role when member is in both groups
1 parent c788c7b commit 75ecc09

File tree

2 files changed

+71
-2
lines changed

2 files changed

+71
-2
lines changed

app/models/invitation_manager.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@ def log_event_meeting_invitation_failure(context, member, error)
6262
end
6363

6464
def chapter_students(chapter)
65-
Member.in_group(chapter.groups.students)
65+
Member.in_group(chapter.groups.students).distinct
6666
end
6767

6868
def chapter_coaches(chapter)
69-
Member.in_group(chapter.groups.coaches)
69+
Member.in_group(chapter.groups.coaches).distinct
7070
end
7171
end
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
require 'rails_helper'
2+
3+
RSpec.describe InvitationManager do
4+
subject(:manager) { InvitationManager.new }
5+
6+
let(:chapter) { Fabricate(:chapter) }
7+
let(:member_in_both_groups) { Fabricate(:member, accepted_toc_at: Time.zone.now) }
8+
9+
describe '#chapter_students' do
10+
context 'when a member has multiple subscriptions to the same group type' do
11+
before do
12+
students_group1 = Fabricate(:group, name: 'Students', chapter: chapter)
13+
students_group2 = Fabricate(:group, name: 'Students', chapter: chapter)
14+
students_group1.members << member_in_both_groups
15+
students_group2.members << member_in_both_groups
16+
end
17+
18+
it 'returns unique members' do
19+
result = manager.send(:chapter_students, chapter)
20+
21+
expect(result.count).to eq(1)
22+
expect(result).to contain_exactly(member_in_both_groups)
23+
end
24+
end
25+
end
26+
27+
describe '#chapter_coaches' do
28+
context 'when a member has multiple subscriptions to the same group type' do
29+
before do
30+
coaches_group1 = Fabricate(:group, name: 'Coaches', chapter: chapter)
31+
coaches_group2 = Fabricate(:group, name: 'Coaches', chapter: chapter)
32+
coaches_group1.members << member_in_both_groups
33+
coaches_group2.members << member_in_both_groups
34+
end
35+
36+
it 'returns unique members' do
37+
result = manager.send(:chapter_coaches, chapter)
38+
39+
expect(result.count).to eq(1)
40+
expect(result).to contain_exactly(member_in_both_groups)
41+
end
42+
end
43+
end
44+
45+
describe 'sending invitations to members in both students and coaches groups' do
46+
let(:workshop) { Fabricate(:workshop, chapter: chapter) }
47+
let(:students_group) { Fabricate(:group, name: 'Students', chapter: chapter) }
48+
let(:coaches_group) { Fabricate(:group, name: 'Coaches', chapter: chapter) }
49+
50+
before do
51+
students_group.members << member_in_both_groups
52+
coaches_group.members << member_in_both_groups
53+
end
54+
55+
it 'sends one invitation per role when audience is everyone' do
56+
expect(WorkshopInvitation).to receive(:find_or_create_by)
57+
.with(workshop: workshop, member: member_in_both_groups, role: 'Student')
58+
.and_call_original
59+
.once
60+
61+
expect(WorkshopInvitation).to receive(:find_or_create_by)
62+
.with(workshop: workshop, member: member_in_both_groups, role: 'Coach')
63+
.and_call_original
64+
.once
65+
66+
manager.send_workshop_emails(workshop, 'everyone')
67+
end
68+
end
69+
end

0 commit comments

Comments
 (0)