fix: deduplicate members in chapter_students and chapter_coaches

[mroderick]

Summary

Fixes a bug where members subscribed to multiple groups within a chapter were processed multiple times when sending workshop invitations with audience='everyone', causing batch failures and preventing remaining members from receiving invitations.

Root Cause

The Member.in_group() scope uses a SQL JOIN on the subscriptions table. When a member is subscribed to multiple groups of the same type (e.g., both "Students" and "Coaches" groups), the JOIN produces duplicate rows - one for each subscription.

# Before fix - returns duplicate rows for members with multiple subscriptions
scope :in_group, ->(members) { 
  not_banned.accepted_toc.joins(:groups).where(groups: { id: members.select(:id) }) 
}

This caused members to appear multiple times in the iteration, triggering a validation error in InvitationLogEntry which validates uniqueness of member_id scoped to (invitation_type, invitation_id).

Timeline

• Jan 2017 (commit 448acc7c): Original code included .uniq for deduplication
• May 2018 (commit d7a463da): Refactoring extracted in_group scope but accidentally removed .uniq

Impact

Batch Job Behavior

When duplicate members were processed:

1. Email sent successfully (first occurrence)
2. log_success() fails - InvitationLogEntry validation error
3. log_failure() ALSO fails - same validation error (same member/invitation)
4. Error bubbles up - batch catches and re-raises
5. Batch marked as failed - remaining emails NOT sent

Critical Consequences

• ✅ Emails sent up to point of failure
• ❌ Sent emails NOT logged as success
• ❌ Failures NOT logged either
• ❌ Remaining members never received invitations
• ❌ Admins had to manually retry multiple times

All Invitation Batches from Production

Workshop	Chapter	Audience	Status	Members Processed	Timestamp
3668	Berlin	everyone	❌ FAILED	99	2026-03-31 14:34
3668	Berlin	everyone	❌ FAILED	1	2026-03-31 14:44
3668	Berlin	everyone	❌ FAILED	1	2026-03-31 14:54
3696	Virtual East Asia	everyone	✅ completed	17	2026-04-01 05:44
3671	Brighton	everyone	❌ FAILED	493	2026-04-01 19:14
3671	Brighton	everyone	❌ FAILED	0	2026-04-01 19:24
3671	Brighton	everyone	❌ FAILED	1	2026-04-01 19:34
3693	Derby	students	✅ completed	21	2026-04-02 13:54
3691	Madrid	everyone	✅ completed	6	2026-04-03 10:04
3697	London	everyone	❌ FAILED	517	2026-04-03 15:54
3697	London	everyone	❌ FAILED	6	2026-04-03 16:04
3697	London	everyone	❌ FAILED	7	2026-04-03 16:14
3671	Brighton	coaches	✅ completed	268	2026-04-06 19:34
3677	Edinburgh	everyone	✅ completed	742	2026-04-07 12:04

Key Observations:

• All audience='everyone' batches on affected chapters failed
• audience='students' batches succeeded (Derby hadno duplicate student subscriptions)
• audience='coaches' batches succeeded (fewer duplicate coach subscriptions)
• Virtual East Asia (17 members) and Madrid (6 members) succeeded - small chapters with fewer duplicates
• Edinburgh (742 members) succeeded - likely no members with duplicate subscriptions at that time
• Retry pattern: 10-minute intervals between failed attempts (manual admin retries)

Email Impact Summary:

• Berlin: ~101 total emails sent across 3 attempts, remaining members missed
• Brighton: ~494 total emails sent across 3 "everyone" attempts (later succeeded with "coaches" only)
• London: ~530 total emails sent across 3 attempts, batch still failing

Note: Emails that succeeded in failed batches were NOT logged, making it impossible to determine exactly which members received invitations.

Affected Chapters

46 of 54 chapters (85%) have members with duplicate subscriptions:

#	Chapter	Unique Members	Duplicates
1	London	7,108	336
2	West London	2,142	79
3	South London	2,000	57
4	Brighton	1,461	57
5	Barcelona	1,025	55
6	Edinburgh	964	37
7	Berlin	779	35
8	Virtual UK	1,020	34
9	New York	1,062	30
10	Cambridge	705	30
11	Glasgow	629	30
12	Manchester	720	27
13	Birmingham	646	26
14	Helsinki	544	21
15	Shanghai	361	17
16	Oxford	492	16
17	Sydney	280	15
18	Amsterdam	177	14
19	Accra	211	14
20	Paris	202	12
21	Norwich	406	11
22	Nottingham	198	11
23	Houston	241	10
24	Delhi	108	7
25	Solent	122	7
26	Boston	151	6
27	Kent	301	6
28	Bournemouth	171	6
29	Virtual Central Europe	141	6
30	Belfast	129	5
31	Nairobi	137	5
32	Bristol	501	5
33	Savannah	132	4
34	Munich	14	4
35	Peterborough	190	4
36	Cape Town	81	4
37	Oslo	167	3
38	Florianopolis	59	3
39	Lagos, Nigeria	109	3
40	Winnipeg	56	2
41	Cardiff	105	2
42	Madrid	5	1
43	Brussels	29	1
44	Drammen	15	1
45	Virtual East Asia	16	1
46	Reading	182	1

~1,200 members affected across all chapters.

Solution

Add .distinct to chapter_students and chapter_coaches methods to deduplicate members before processing:

def chapter_students(chapter)
  Member.in_group(chapter.groups.students).distinct
end

def chapter_coaches(chapter)
  Member.in_group(chapter.groups.coaches).distinct
end

This restores the deduplication behavior that existed before the May 2018 refactoring.

Test Plan

Added comprehensive tests:

1. chapter_students returns unique members when subscribed to same group multiple times
2. chapter_coaches returns unique members when subscribed to same group multiple times
3. Members in both Students and Coaches groups receive exactly one invitation per role

All existing tests pass (30 examples, 0 failures).

Verification

Run locally:

bundle exec rspec spec/models/invitation_manager_spec.rb spec/models/invitation_manager_deduplication_spec.rb

To verify in production after deploy:

-- Check for duplicate batch errors (should decrease after fix)
SELECT il.id, il.loggable_id as workshop_id, c.name as chapter_name, 
       il.audience, il.status, il.success_count, il.error_message, il.started_at 
FROM invitation_logs il 
JOIN workshops w ON il.loggable_id = w.id 
JOIN chapters c ON w.chapter_id = c.id 
WHERE il.status = 'failed' 
ORDER BY il.started_at DESC;

[olleolleolle]

Good to have that!

[olleolleolle]

Ergonomics: I assume it'd be possible to have a describe "handling deduplication" block, wrapping these test cases, and placing them in a spec/models/invitation_manager_spec.rb, so that "flipping between test and implementation" works in an expected way.

[olleolleolle]

Suggested change

	subject(:manager) { InvitationManager.new }
	subject(:manager) { described_class.new }

Very minor in this test file, but in other cases, when it's repeated a lot in a test, it makes renaming classes more cumbersome.

[mroderick]

Thanks @olleolleolle! I've created #2554 to address the comments.