One invitation per member per event or workshop

app/controllers/workshops_controller.rb

@@ -44,10 +44,13 @@ def find_attending_invitation(workshop, user)
   end
 
   def find_or_create_invitation(workshop, user, role)
-    invitation = WorkshopInvitation.create_or_find_by(workshop:,
-                                                      member: user,
-                                                      role:)
-    invitation.persisted? ? invitation : WorkshopInvitation.find_by(workshop:, member: user, role:)
+    # Identity is workshop + member, matching InvitationManager; the member's
+    # role choice wins, so an existing invitation with the other role is updated.
+    invitation = WorkshopInvitation.find_or_create_by!(workshop:, member: user) { |record| record.role = role }
+    invitation.update!(role:) unless invitation.role.eql?(role)
+    invitation
+  rescue ActiveRecord::RecordNotUnique
+    WorkshopInvitation.find_by(workshop:, member: user)
   end
 
   def user_attending_or_waitlisted?(workshop, user)

app/mailers/event_invitation_mailer.rb

@@ -20,7 +20,9 @@ def invite_coach(event, member, invitation)
     @member = member
     @invitation = invitation
     @host_address = AddressPresenter.new(@event.venue.address) if @event.venue.present?
-    @everyone_is_invited = !event.audience
+    # Coach emails are labelled as such only when the event is actually for
+    # coaches; blank or missing audience means a general invitation.
+    @everyone_is_invited = !event.audience.eql?('Coaches')
 
     mail_to_member(member, @everyone_is_invited ? "Invitation: #{@event.name}" : "Coach Invitation: #{@event.name}",
                    &:html)

app/services/invitation_manager.rb

@@ -164,14 +164,16 @@ def chapter_coaches(chapter)
   end
 
   def create_invitation(workshop, member, role)
-    WorkshopInvitation.find_or_create_by!(workshop:, member:, role:)
+    # Identity is workshop + member; role applies only on create so a member
+    # subscribed as both student and coach gets one invite, not one per role.
+    WorkshopInvitation.find_or_create_by!(workshop:, member:) { |invitation| invitation.role = role }
   rescue StandardError => e
     log_invitation_failure(workshop, member, role, e)
     nil
   end
 
   def create_event_invitation(event, member, role)
-    Invitation.find_or_create_by!(event:, member:, role:)
+    Invitation.find_or_create_by!(event:, member:) { |invitation| invitation.role = role }
   rescue StandardError => e
     log_event_meeting_invitation_failure("event_id=#{event.id}", member, e)
     nil

spec/controllers/workshops_controller_spec.rb

@@ -25,6 +25,21 @@
       end
     end
 
+    context 'when the member has an invitation with a different role' do
+      let!(:invitation) do
+        Fabricate(:workshop_invitation, workshop:, member:, role: 'Student', attending: nil)
+      end
+
+      it 'updates the existing invitation to the requested role instead of creating a second one' do
+        expect do
+          post :rsvp, params: { id: workshop.id, role: 'Coach' }
+        end.not_to change(WorkshopInvitation, :count)
+
+        expect(invitation.reload.role).to eq('Coach')
+        expect(response).to redirect_to(invitation_path(invitation))
+      end
+    end
+
     context 'when the member does not have an invitation for the workshop and role' do
       it 'creates a new invitation and redirects' do
         expect do

spec/mailers/event_invitation_mailer_spec.rb

@@ -52,6 +52,15 @@
       expect(email.body.encoded).to match('hello@codebar.io')
     end
 
+    it 'sends a generic invitation if the event audience is blank' do
+      blank_audience_event = Fabricate(:event, name: 'Test event', audience: '')
+      blank_invitation = Fabricate(:invitation, event: blank_audience_event, member:)
+
+      described_class.invite_coach(blank_audience_event, member, blank_invitation).deliver_now
+
+      expect(email.subject).to eq("Invitation: #{blank_audience_event.name}")
+    end
+
     it 'sends a coach invitation of the event is for coaches' do
       email_subject = "Coach Invitation: #{event.name}"
       described_class.invite_coach(coach_event, member, invitation).deliver_now

spec/services/invitation_manager_spec.rb

@@ -30,67 +30,54 @@
 
     it 'can email only students' do
       event = Fabricate(:event, chapters: [chapter], audience: 'Students')
-      students.each do |student|
-        allow(Invitation).to receive(:find_or_create_by!).with(
-          event:, member: student, role: 'Student'
-        ).and_call_original
-      end
-
-      manager.send_event_emails(event, chapter)
 
-      students.each do |student|
-        expect(Invitation).to have_received(:find_or_create_by!).with(event:, member: student, role: 'Student')
-      end
+      expect do
+        manager.send_event_emails(event, chapter)
+      end.to change(Invitation, :count).by(students.count)
 
-      coaches.each do |student|
-        expect(Invitation).not_to have_received(:find_or_create_by!).with(event:, member: student, role: 'Coach')
-      end
+      expect(event.invitations.students.map(&:member)).to match_array(students)
+      expect(event.invitations.coaches).to be_empty
     end
 
     it 'can email only coaches' do
       event = Fabricate(:event, chapters: [chapter], audience: 'Coaches')
 
-      coaches.each do |student|
-        allow(Invitation).to receive(:find_or_create_by!).with(
-          event:, member: student, role: 'Coach'
-        ).and_call_original
-      end
-
-      manager.send_event_emails(event, chapter)
-
-      students.each do |student|
-        expect(Invitation).not_to have_received(:find_or_create_by!).with(event:, member: student, role: 'Student')
-      end
+      expect do
+        manager.send_event_emails(event, chapter)
+      end.to change(Invitation, :count).by(coaches.count)
 
-      coaches.each do |student|
-        expect(Invitation).to have_received(:find_or_create_by!).with(event:, member: student, role: 'Coach')
-      end
+      expect(event.invitations.coaches.map(&:member)).to match_array(coaches)
+      expect(event.invitations.students).to be_empty
     end
 
     it 'can email both students and coaches' do
       event = Fabricate(:event, chapters: [chapter])
 
-      students.each do |student|
-        allow(Invitation).to receive(:find_or_create_by!).with(
-          event:, member: student, role: 'Student'
-        ).and_call_original
-      end
+      expect do
+        manager.send_event_emails(event, chapter)
+      end.to change(Invitation, :count).by(students.count + coaches.count)
 
-      coaches.each do |student|
-        allow(Invitation).to receive(:find_or_create_by!).with(
-          event:, member: student, role: 'Coach'
-        ).and_call_original
-      end
+      expect(event.invitations.students.map(&:member)).to match_array(students)
+      expect(event.invitations.coaches.map(&:member)).to match_array(coaches)
+    end
 
-      manager.send_event_emails(event, chapter)
+    it 'sends one invitation to a member subscribed as both student and coach' do
+      dual_member = Fabricate(:member)
+      Fabricate(:students, chapter:, members: [dual_member])
+      Fabricate(:coaches, chapter:, members: [dual_member])
+      event = Fabricate(:event, chapters: [chapter])
 
-      students.each do |student|
-        expect(Invitation).to have_received(:find_or_create_by!).with(event:, member: student, role: 'Student')
-      end
+      expect do
+        manager.send_event_emails(event, chapter)
+      end.to change { Invitation.where(event:, member: dual_member).count }.by(1)
 
-      coaches.each do |student|
-        expect(Invitation).to have_received(:find_or_create_by!).with(event:, member: student, role: 'Coach')
-      end
+      invitation = Invitation.find_by(event:, member: dual_member)
+      # Coaches are invited first for events, so the coach pass wins for dual members.
+      expect(invitation.role).to eq('Coach')
+
+      deliveries_to_dual_member = ActionMailer::Base.deliveries.select { |e| e.to.include?(dual_member.email) }
+      expect(deliveries_to_dual_member.count).to eq(1)
+      expect(deliveries_to_dual_member.first.subject).to eq("Invitation: #{event.name}")
     end
 
     it 'emails only students that accepted toc' do
@@ -99,21 +86,11 @@
       first_student, *other_students = students
       first_student.update(accepted_toc_at: nil)
 
-      other_students.each do |other_student|
-        allow(Invitation).to(
-          receive(:find_or_create_by!)
-          .with(event:, member: other_student, role: 'Student')
-          .and_call_original
-        )
-      end
-
-      manager.send_event_emails(event, chapter)
-
-      expect(Invitation).not_to have_received(:find_or_create_by!).with(event:, member: first_student, role: 'Student')
+      expect do
+        manager.send_event_emails(event, chapter)
+      end.to change(Invitation, :count).by(other_students.count)
 
-      other_students.each do |other_student|
-        expect(Invitation).to have_received(:find_or_create_by!).with(event:, member: other_student, role: 'Student')
-      end
+      expect(event.invitations.students.map(&:member)).to match_array(other_students)
     end
 
     it 'emails only coaches that accepted toc' do
@@ -122,21 +99,11 @@
       first_coach, *other_coaches = coaches
       first_coach.update(accepted_toc_at: nil)
 
-      other_coaches.each do |other_coach|
-        allow(Invitation).to(
-          receive(:find_or_create_by!)
-          .with(event:, member: other_coach, role: 'Coach')
-          .and_call_original
-        )
-      end
-
-      manager.send_event_emails(event, chapter)
-
-      expect(Invitation).not_to have_received(:find_or_create_by!).with(event:, member: first_coach, role: 'Coach')
+      expect do
+        manager.send_event_emails(event, chapter)
+      end.to change(Invitation, :count).by(other_coaches.count)
 
-      other_coaches.each do |other_coach|
-        expect(Invitation).to have_received(:find_or_create_by!).with(event:, member: other_coach, role: 'Coach')
-      end
+      expect(event.invitations.coaches.map(&:member)).to match_array(other_coaches)
     end
   end
 
@@ -279,6 +246,16 @@
       expect(invitation.role).to eq('Student')
     end
 
+    it 'returns existing invitation with previously_new_record? as false when called with a different role' do
+      invitation1 = manager.send(:create_invitation, workshop, member, 'Student')
+
+      invitation2 = manager.send(:create_invitation, workshop, member, 'Coach')
+
+      expect(invitation2.previously_new_record?).to be false
+      expect(invitation2.id).to eq(invitation1.id)
+      expect(invitation2.role).to eq('Student')
+    end
+
     it 'returns existing invitation with previously_new_record? as false on duplicate call' do
       # First call creates the invitation
       invitation1 = manager.send(:create_invitation, workshop, member, 'Student')
@@ -462,17 +439,16 @@
         coaches_group.members << member_in_both_groups
       end
 
-      it 'creates one invitation per role when audience is everyone' do
+      it 'sends one invitation and one email when audience is everyone' do
         expect do
           manager.send_workshop_emails(workshop, 'everyone')
-        end.to change(WorkshopInvitation, :count).by(2)
-
-        student_invitation = WorkshopInvitation.find_by(workshop:, member: member_in_both_groups, role: 'Student')
-        coach_invitation = WorkshopInvitation.find_by(workshop:, member: member_in_both_groups, role: 'Coach')
+        end.to change(WorkshopInvitation, :count).by(1)
+           .and change { ActionMailer::Base.deliveries.count }.by(1)
 
-        expect(student_invitation).to be_present
-        expect(coach_invitation).to be_present
-        expect(student_invitation.id).not_to eq(coach_invitation.id)
+        invitation = WorkshopInvitation.find_by(workshop:, member: member_in_both_groups)
+        # Students are invited first for workshops, so the student pass wins for dual members.
+        expect(invitation.role).to eq('Student')
+        expect(ActionMailer::Base.deliveries.last.subject).to start_with('Workshop Invitation')
       end
     end
   end

spec/support/shared_examples/behaves_like_sending_workshop_emails.rb

@@ -3,7 +3,7 @@
     Fabricate(:students, chapter:, members: students)
 
     students.each do |student|
-      allow(WorkshopInvitation).to receive(:find_or_create_by!).with(workshop:, member: student, role: 'Student').and_call_original
+      allow(WorkshopInvitation).to receive(:find_or_create_by!).with(workshop:, member: student).and_call_original
     end
 
     expect do
@@ -12,7 +12,7 @@
                                                          .and change { WorkshopInvitation.where(workshop:, role: 'Student').count }.by(students.count)
 
     students.each do |student|
-      expect(WorkshopInvitation).to have_received(:find_or_create_by!).with(workshop:, member: student, role: 'Student')
+      expect(WorkshopInvitation).to have_received(:find_or_create_by!).with(workshop:, member: student)
     end
 
     # Verify emails were sent to the right recipients
@@ -25,7 +25,7 @@
     Fabricate(:coaches, chapter:, members: coaches)
 
     coaches.each do |coach|
-      allow(WorkshopInvitation).to receive(:find_or_create_by!).with(workshop:, member: coach, role: 'Coach').and_call_original
+      allow(WorkshopInvitation).to receive(:find_or_create_by!).with(workshop:, member: coach).and_call_original
     end
 
     expect do
@@ -34,7 +34,7 @@
                                                          .and change { WorkshopInvitation.where(workshop:, role: 'Coach').count }.by(coaches.count)
 
     coaches.each do |coach|
-      expect(WorkshopInvitation).to have_received(:find_or_create_by!).with(workshop:, member: coach, role: 'Coach')
+      expect(WorkshopInvitation).to have_received(:find_or_create_by!).with(workshop:, member: coach)
     end
 
     # Verify emails were sent to the right recipients
@@ -48,15 +48,15 @@
     Fabricate(:coaches, chapter:, members: coaches + [banned_coach])
 
     coaches.each do |coach|
-      allow(WorkshopInvitation).to receive(:find_or_create_by!).with(workshop:, member: coach, role: 'Coach').and_call_original
+      allow(WorkshopInvitation).to receive(:find_or_create_by!).with(workshop:, member: coach).and_call_original
     end
 
     manager.send(send_email, workshop, 'coaches')
 
     coaches.each do |coach|
-      expect(WorkshopInvitation).to have_received(:find_or_create_by!).with(workshop:, member: coach, role: 'Coach')
+      expect(WorkshopInvitation).to have_received(:find_or_create_by!).with(workshop:, member: coach)
     end
-    expect(WorkshopInvitation).not_to have_received(:find_or_create_by!).with(workshop:, member: banned_coach, role: 'Coach')
+    expect(WorkshopInvitation).not_to have_received(:find_or_create_by!).with(workshop:, member: banned_coach)
   end
 
   it 'sends emails when a WorkshopInvitation is created' do