From 77a6b44f3288f4d7bf93065e22bb79d3483c8ef2 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Mon, 12 Oct 2026 13:28:04 +0200 Subject: [PATCH] fix(waitlist): declined members no longer auto-promoted A member who cancels after the close time keeps their waiting-list entry, but nothing stopped a later freed seat from being handed to that declined member: promotion eligible them on auto_rsvp alone, so someone who said "I can no longer attend" could be re-confirmed and emailed by another member's cancellation. The seat could end up spent past the freeze on a member who will not attend. promote_next now considers only invitations whose attending flag is still undecided. A declined invitation (attending false) stays on the printed list for the security handover but is never auto-promoted; the excluder handoff the previous commit carried is not needed since a cancelling member is already declined by the time promote_next runs. Accepted invitations with a lingering entry are skipped too. The admin removal path shares the same rule through WaitlistPromotionConcerns, collapsing the difference where member rejects kept the entry while admin removals destroyed it. --- .../concerns/waitlist_promotion_concerns.rb | 5 +-- app/models/waiting_list.rb | 15 +++++---- spec/models/waiting_list_spec.rb | 32 +++++++++++++------ 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/app/controllers/concerns/waitlist_promotion_concerns.rb b/app/controllers/concerns/waitlist_promotion_concerns.rb index c4a959e9a..e1fb6a4bb 100644 --- a/app/controllers/concerns/waitlist_promotion_concerns.rb +++ b/app/controllers/concerns/waitlist_promotion_concerns.rb @@ -16,10 +16,7 @@ def release_seat(**attributes) end def promote_next_waitlist_member - # Skip the cancelling member's own entry: promote_next must hand the freed - # seat to the next member, not redeliver it to the person who just gave it up. - promoted = WaitingList.promote_next(@invitation.workshop, @invitation.role, - excluding: @invitation) + promoted = WaitingList.promote_next(@invitation.workshop, @invitation.role) @workshop.send_attending_email(promoted, true) if promoted end end diff --git a/app/models/waiting_list.rb b/app/models/waiting_list.rb index 4e928ab2b..35bbb6787 100644 --- a/app/models/waiting_list.rb +++ b/app/models/waiting_list.rb @@ -23,16 +23,17 @@ def self.coaches(workshop) end # Pops the next auto-RSVP waitlist entry and confirms its invitation. The - # caller sends the attendance email for the promoted invitation. `excluding` - # keeps a member's own entry out of the pick, so a cancelling member never - # receives back the seat they just freed. - def self.promote_next(workshop, role, excluding: nil) + # caller sends the attendance email for the promoted invitation. Only + # undecided invitations are eligible: a member who declines the workshop + # keeps their entry on the printed list but is never auto-promoted, so a + # later freed seat always reaches a member who still wants it. + def self.promote_next(workshop, role) transaction do # SKIP LOCKED lets a concurrent promoter that holds another freed seat # take the next entry instead of racing on this one. - scope = by_workshop(workshop).where_role(role).where(auto_rsvp: true) - scope = scope.where.not(invitation: excluding) if excluding - next_spot = scope.order(:created_at).lock('FOR UPDATE SKIP LOCKED').first + next_spot = by_workshop(workshop).where_role(role).where(auto_rsvp: true) + .where('workshop_invitations.attending IS NULL') + .order(:created_at).lock('FOR UPDATE SKIP LOCKED').first return unless next_spot invitation = next_spot.invitation diff --git a/spec/models/waiting_list_spec.rb b/spec/models/waiting_list_spec.rb index a905c1a77..1abd0e0cb 100644 --- a/spec/models/waiting_list_spec.rb +++ b/spec/models/waiting_list_spec.rb @@ -44,30 +44,44 @@ expect(described_class.by_workshop(workshop).map(&:invitation)).to eq([later_invitation]) end - it 'skips the excluded entry and promotes the next one' do - own_invitation = Fabricate(:workshop_invitation, workshop:) + it 'skips a declined member and promotes the next eager one' do + declined_invitation = Fabricate(:workshop_invitation, workshop:) next_invitation = Fabricate(:workshop_invitation, workshop:, member: Fabricate(:member)) - described_class.add(own_invitation) + described_class.add(declined_invitation) described_class.add(next_invitation) + declined_invitation.update!(attending: false) - promoted = described_class.promote_next(workshop, 'Student', excluding: own_invitation) + promoted = described_class.promote_next(workshop, 'Student') expect(promoted).to eq(next_invitation) - expect(own_invitation.reload.attending).to be_nil - expect(described_class.where(invitation: own_invitation)).to exist + expect(declined_invitation.reload.attending).to be(false) + expect(described_class.where(invitation: declined_invitation)).to exist end - it 'returns nil when the only entry is excluded' do + it 'returns nil when every eligible entry is declined' do invitation = Fabricate(:workshop_invitation, workshop:) described_class.add(invitation) + invitation.update!(attending: false) - promoted = described_class.promote_next(workshop, 'Student', excluding: invitation) + promoted = described_class.promote_next(workshop, 'Student') expect(promoted).to be_nil - expect(invitation.reload.attending).to be_nil + expect(invitation.reload.attending).to be(false) expect(described_class.where(invitation:)).to exist end + it 'skips an accepted member whose entry lingers on the waiting list' do + accepted_invitation = Fabricate(:workshop_invitation, workshop:) + next_invitation = Fabricate(:workshop_invitation, workshop:, member: Fabricate(:member)) + described_class.add(accepted_invitation) + described_class.add(next_invitation) + accepted_invitation.update!(attending: true) + + promoted = described_class.promote_next(workshop, 'Student') + + expect(promoted).to eq(next_invitation) + end + it 'returns nil and promotes nothing for an empty waitlist' do expect(described_class.promote_next(workshop, 'Student')).to be_nil end