Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/models/invitation_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
69 changes: 69 additions & 0 deletions spec/models/invitation_manager_deduplication_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
require 'rails_helper'

RSpec.describe InvitationManager do
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

subject(:manager) { InvitationManager.new }
Copy link
Copy Markdown
Collaborator

@olleolleolle olleolleolle Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


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