From 75ecc09b975cbdc911e3510ff969f3a90d94e72c Mon Sep 17 00:00:00 2001
From: Morgan Roderick <morgan@roderick.dk>
Date: Wed, 8 Apr 2026 20:00:23 +0200
Subject: [PATCH] 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 448acc7c) included .uniq for deduplication
- Refactoring (May 2018, commit d7a463da) 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
---
 app/models/invitation_manager.rb              |  4 +-
 .../invitation_manager_deduplication_spec.rb  | 69 +++++++++++++++++++
 2 files changed, 71 insertions(+), 2 deletions(-)
 create mode 100644 spec/models/invitation_manager_deduplication_spec.rb

diff --git a/app/models/invitation_manager.rb b/app/models/invitation_manager.rb
index 53838c50d..60cc0c571 100644
--- a/app/models/invitation_manager.rb
+++ b/app/models/invitation_manager.rb
@@ -62,10 +62,10 @@ def log_event_meeting_invitation_failure(context, member, error)
   end
 
   def chapter_students(chapter)
-    Member.in_group(chapter.groups.students)
+    Member.in_group(chapter.groups.students).distinct
   end
 
   def chapter_coaches(chapter)
-    Member.in_group(chapter.groups.coaches)
+    Member.in_group(chapter.groups.coaches).distinct
   end
 end
diff --git a/spec/models/invitation_manager_deduplication_spec.rb b/spec/models/invitation_manager_deduplication_spec.rb
new file mode 100644
index 000000000..22438b5b6
--- /dev/null
+++ b/spec/models/invitation_manager_deduplication_spec.rb
@@ -0,0 +1,69 @@
+require 'rails_helper'
+
+RSpec.describe InvitationManager do
+  subject(:manager) { InvitationManager.new }
+
+  let(:chapter) { Fabricate(:chapter) }
+  let(:member_in_both_groups) { Fabricate(:member, accepted_toc_at: Time.zone.now) }
+
+  describe '#chapter_students' do
+    context 'when a member has multiple subscriptions to the same group type' do
+      before do
+        students_group1 = Fabricate(:group, name: 'Students', chapter: chapter)
+        students_group2 = Fabricate(:group, name: 'Students', chapter: chapter)
+        students_group1.members << member_in_both_groups
+        students_group2.members << member_in_both_groups
+      end
+
+      it 'returns unique members' do
+        result = manager.send(:chapter_students, chapter)
+
+        expect(result.count).to eq(1)
+        expect(result).to contain_exactly(member_in_both_groups)
+      end
+    end
+  end
+
+  describe '#chapter_coaches' do
+    context 'when a member has multiple subscriptions to the same group type' do
+      before do
+        coaches_group1 = Fabricate(:group, name: 'Coaches', chapter: chapter)
+        coaches_group2 = Fabricate(:group, name: 'Coaches', chapter: chapter)
+        coaches_group1.members << member_in_both_groups
+        coaches_group2.members << member_in_both_groups
+      end
+
+      it 'returns unique members' do
+        result = manager.send(:chapter_coaches, chapter)
+
+        expect(result.count).to eq(1)
+        expect(result).to contain_exactly(member_in_both_groups)
+      end
+    end
+  end
+
+  describe 'sending invitations to members in both students and coaches groups' do
+    let(:workshop) { Fabricate(:workshop, chapter: chapter) }
+    let(:students_group) { Fabricate(:group, name: 'Students', chapter: chapter) }
+    let(:coaches_group) { Fabricate(:group, name: 'Coaches', chapter: chapter) }
+
+    before do
+      students_group.members << member_in_both_groups
+      coaches_group.members << member_in_both_groups
+    end
+
+    it 'sends one invitation per role when audience is everyone' do
+      expect(WorkshopInvitation).to receive(:find_or_create_by)
+        .with(workshop: workshop, member: member_in_both_groups, role: 'Student')
+        .and_call_original
+        .once
+
+      expect(WorkshopInvitation).to receive(:find_or_create_by)
+        .with(workshop: workshop, member: member_in_both_groups, role: 'Coach')
+        .and_call_original
+        .once
+
+      manager.send_workshop_emails(workshop, 'everyone')
+    end
+  end
+end