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/db/migrate/20260408220120_fix_duplicate_subscriptions_and_add_unique_index.rb b/db/migrate/20260408220120_fix_duplicate_subscriptions_and_add_unique_index.rb new file mode 100644 index 000000000..e83cc074b --- /dev/null +++ b/db/migrate/20260408220120_fix_duplicate_subscriptions_and_add_unique_index.rb @@ -0,0 +1,30 @@ +class FixDuplicateSubscriptionsAndAddUniqueIndex < ActiveRecord::Migration[8.1] + disable_ddl_transaction! + + def up + execute <<~SQL + DELETE FROM subscriptions + WHERE id NOT IN ( + SELECT MIN(id) + FROM subscriptions + GROUP BY member_id, group_id + ) + AND (member_id, group_id) IN ( + SELECT member_id, group_id + FROM subscriptions + GROUP BY member_id, group_id + HAVING COUNT(*) > 1 + ) + SQL + + add_index :subscriptions, + %i[member_id group_id], + unique: true, + name: 'index_subscriptions_on_member_id_group_id', + algorithm: :concurrently + end + + def down + remove_index :subscriptions, name: 'index_subscriptions_on_member_id_group_id' + end +end diff --git a/db/schema.rb b/db/schema.rb index 1e14f430b..c9369956d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_03_30_193245) do +ActiveRecord::Schema[8.1].define(version: 2026_04_08_220120) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -525,6 +525,7 @@ t.integer "member_id" t.datetime "updated_at", precision: nil t.index ["group_id"], name: "index_subscriptions_on_group_id" + t.index ["member_id", "group_id"], name: "index_subscriptions_on_member_id_group_id", unique: true t.index ["member_id"], name: "index_subscriptions_on_member_id" 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