Additional Waiting List Tests

app/models/waiting_list.rb

@@ -8,6 +8,10 @@ def remove(entry_id)
   end
 
   def add(entry_id)
+    raise ArgumentError.new("Registration must have a competing_status of 'waiting_list' to be added to the waiting list") unless
+      Registration.find(entry_id).competing_status == Registrations::Helper::STATUS_WAITING_LIST
+    return if entries.include?(entry_id)
+
     if entries.nil?
       update_column :entries, [entry_id]
     else

lib/registrations/lanes/competing.rb

@@ -66,10 +66,10 @@ def self.update_waiting_list(competing_params, registration, waiting_list)
         status = competing_params['status']
         waiting_list_position = competing_params['waiting_list_position']
 
-        should_add = status == Registrations::Helper::STATUS_WAITING_LIST # TODO: Add case where waiting_list status is present but that matches the old_status
-        should_move = waiting_list_position.present? # TODO: Add case where waiting list pos is present but it matches the current position
+        should_add = status == Registrations::Helper::STATUS_WAITING_LIST && registration.waiting_list_position.nil?
+        should_move = waiting_list_position.present?
         should_remove = status.present? && registration.competing_status == Registrations::Helper::STATUS_WAITING_LIST &&
-                        status != Registrations::Helper::STATUS_WAITING_LIST # TODO: Consider adding cases for when not all of these are true?
+                        status != Registrations::Helper::STATUS_WAITING_LIST
 
         waiting_list.add(registration.id) if should_add
         waiting_list.move_to_position(registration.id, competing_params[:waiting_list_position].to_i) if should_move

lib/registrations/registration_checker.rb

@@ -31,7 +31,7 @@ def self.update_registration_allowed!(update_request, competition, current_user)
       validate_comment!(comment, competition, registration)
       validate_organizer_fields!(update_request, current_user, competition)
       validate_organizer_comment!(update_request)
-      validate_waiting_list_position!(waiting_list_position, competition) unless waiting_list_position.nil?
+      validate_waiting_list_position!(waiting_list_position, competition, registration) unless waiting_list_position.nil?
       validate_update_status!(new_status, competition, current_user, target_user, registration, events) unless new_status.nil?
       validate_update_events!(events, competition) unless events.nil?
       validate_qualifications!(update_request, competition, target_user)
@@ -149,7 +149,11 @@ def validate_organizer_comment!(request)
           !organizer_comment.nil? && organizer_comment.length > COMMENT_CHARACTER_LIMIT
       end
 
-      def validate_waiting_list_position!(waiting_list_position, competition)
+      def validate_waiting_list_position!(waiting_list_position, competition, registration)
+        # User must be on the wating list
+        raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_REQUEST_DATA) unless
+         registration.competing_status == Registrations::Helper::STATUS_WAITING_LIST
+
         # Floats are not allowed
         raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_WAITING_LIST_POSITION) if waiting_list_position.is_a? Float
 

spec/lib/registrations/registration_checker_spec.rb

@@ -1782,20 +1782,32 @@
     end
 
     describe '#update_registration_allowed!.validate_waiting_list_position!' do
-      let(:waiting_list) { FactoryBot.create(:waiting_list, holder: default_competition) }
+      let(:waiting_list) { default_competition.waiting_list }
+      let!(:waitlisted_registration) { FactoryBot.create(:registration, :waiting_list, competition: default_competition) }
 
       before do
-        waiting_list.add(FactoryBot.create(:registration, :waiting_list, competition: default_competition).user_id)
-        waiting_list.add(FactoryBot.create(:registration, :waiting_list, competition: default_competition).user_id)
-        waiting_list.add(FactoryBot.create(:registration, :waiting_list, competition: default_competition).user_id)
-        waiting_list.add(FactoryBot.create(:registration, :waiting_list, competition: default_competition).user_id)
+        FactoryBot.create_list(:registration, 4, :waiting_list, competition: default_competition)
+      end
+
+      it 'waiting list position can be updated' do
+        update_request = FactoryBot.build(
+          :update_request,
+          user_id: waitlisted_registration.user_id,
+          competition_id: waitlisted_registration.competition.id,
+          submitted_by: default_competition.organizers.first.id,
+          competing: { 'waiting_list_position' => 3 },
+        )
+
+        expect {
+          Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by']))
+        }.not_to raise_error
       end
 
       it 'must be an integer, not string' do
         update_request = FactoryBot.build(
           :update_request,
-          user_id: default_registration.user_id,
-          competition_id: default_registration.competition.id,
+          user_id: waitlisted_registration.user_id,
+          competition_id: waitlisted_registration.competition.id,
           submitted_by: default_competition.organizers.first.id,
           competing: { 'waiting_list_position' => 'b' },
         )
@@ -1809,12 +1821,10 @@
       end
 
       it 'can be an integer given as a string' do
-        default_competition.waiting_list.add(default_registration.user_id)
-
         update_request = FactoryBot.build(
           :update_request,
-          user_id: default_registration.user_id,
-          competition_id: default_registration.competition.id,
+          user_id: waitlisted_registration.user_id,
+          competition_id: waitlisted_registration.competition.id,
           submitted_by: default_competition.organizers.first.id,
           competing: { 'waiting_list_position' => '1' },
         )
@@ -1827,8 +1837,8 @@
       it 'must be an integer, not float' do
         update_request = FactoryBot.build(
           :update_request,
-          user_id: default_registration.user_id,
-          competition_id: default_registration.competition.id,
+          user_id: waitlisted_registration.user_id,
+          competition_id: waitlisted_registration.competition.id,
           submitted_by: default_competition.organizers.first.id,
           competing: { 'waiting_list_position' => 2.0 },
         )
@@ -1842,12 +1852,10 @@
       end
 
       it 'cannot move to less than position 1' do
-        waiting_list.add(default_registration.user_id)
-
         update_request = FactoryBot.build(
           :update_request,
-          user_id: default_registration.user_id,
-          competition_id: default_registration.competition.id,
+          user_id: waitlisted_registration.user_id,
+          competition_id: waitlisted_registration.competition.id,
           submitted_by: default_competition.organizers.first.id,
           competing: { 'waiting_list_position' => 0 },
         )
@@ -1861,12 +1869,10 @@
       end
 
       it 'cannot move to greater than the number of items in the waiting list' do
-        waiting_list.add(default_registration.user_id)
-
         update_request = FactoryBot.build(
           :update_request,
-          user_id: default_registration.user_id,
-          competition_id: default_registration.competition.id,
+          user_id: waitlisted_registration.user_id,
+          competition_id: waitlisted_registration.competition.id,
           submitted_by: default_competition.organizers.first.id,
           competing: { 'waiting_list_position' => 6 },
         )
@@ -1878,6 +1884,23 @@
           expect(error.error).to eq(Registrations::ErrorCodes::INVALID_WAITING_LIST_POSITION)
         end
       end
+
+      it 'registration must be on the waiting list' do
+        update_request = FactoryBot.build(
+          :update_request,
+          user_id: default_registration.user_id,
+          competition_id: default_registration.competition.id,
+          submitted_by: default_competition.organizers.first.id,
+          competing: { 'waiting_list_position' => 1 },
+        )
+
+        expect {
+          Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by']))
+        }.to raise_error(WcaExceptions::RegistrationError) do |error|
+          expect(error.status).to eq(:unprocessable_entity)
+          expect(error.error).to eq(Registrations::ErrorCodes::INVALID_REQUEST_DATA)
+        end
+      end
     end
 
     describe '#update_registration_allowed!.validate_qualifications!' do

spec/models/registration_spec.rb

@@ -63,7 +63,6 @@
     registration.user = user
     expect(registration).to be_valid
   end
-
   it "allows deleting a registration of a banned competitor" do
     user = FactoryBot.create(:user, :banned)
     registration.user = user
@@ -492,6 +491,18 @@
         expect(reg.waiting_list_position).to eq(6)
       end
 
+      it 'no change if we try to add a registration on the waiting list' do
+        reg1.update_lanes!({ user_id: reg1.user.id, competing: { status: 'waiting_list' } }.with_indifferent_access, reg1.user)
+
+        expect(reg1.waiting_list_position).to eq(1)
+        expect(reg2.waiting_list_position).to eq(2)
+        expect(reg3.waiting_list_position).to eq(3)
+        expect(reg4.waiting_list_position).to eq(4)
+        expect(reg5.waiting_list_position).to eq(5)
+
+        expect(waiting_list.entries.count).to eq(5)
+      end
+
       it 'removes from waiting list' do
         reg4.update_lanes!({ user_id: reg4.user.id, competing: { status: 'pending' } }.with_indifferent_access, reg4.user)
 
@@ -522,6 +533,31 @@
 
         expect(waiting_list.entries.count).to eq(5)
       end
+
+      it 'moves to the same position' do
+        reg5.update_lanes!({ user_id: reg5.user.id, competing: { waiting_list_position: 5 } }.with_indifferent_access, reg5.user)
+
+        expect(reg1.waiting_list_position).to eq(1)
+        expect(reg2.waiting_list_position).to eq(2)
+        expect(reg3.waiting_list_position).to eq(3)
+        expect(reg4.waiting_list_position).to eq(4)
+        expect(reg5.waiting_list_position).to eq(5)
+
+        expect(waiting_list.entries.count).to eq(5)
+      end
+
+      it 'move request for a registration that isnt in the waiting list' do
+        reg = FactoryBot.create(:registration, competition: competition)
+        reg.update_lanes!({ user_id: reg.user.id, competing: { waiting_list_position: 3 } }.with_indifferent_access, reg.user)
+
+        expect(reg.waiting_list_position).to eq(nil)
+
+        expect(reg1.waiting_list_position).to eq(1)
+        expect(reg2.waiting_list_position).to eq(2)
+        expect(reg3.waiting_list_position).to eq(3)
+        expect(reg4.waiting_list_position).to eq(4)
+        expect(reg5.waiting_list_position).to eq(5)
+      end
     end
   end
 end

spec/models/waiting_list_spec.rb

@@ -13,7 +13,9 @@
 
   describe 'add to waiting list' do
     it 'first competitor in the waiting list gets set to position 1' do
-      registration = FactoryBot.create(:registration, :waiting_list, competition: competition)
+      registration = FactoryBot.create(:registration, :pending, competition: competition)
+      registration.update!(competing_status: 'waiting_list')
+      waiting_list.add(registration.id)
       expect(competition.waiting_list.entries[0]).to eq(registration.id)
     end
 
@@ -22,6 +24,19 @@
       registration = FactoryBot.create(:registration, :waiting_list, competition: competition)
       expect(competition.waiting_list.entries[1]).to eq(registration.id)
     end
+
+    it 'doesnt get added if the registration is already on the list' do
+      registration = FactoryBot.create(:registration, :waiting_list, competition: competition)
+      waiting_list.add(registration.id)
+      expect(waiting_list.entries.count).to eq(1)
+    end
+
+    it 'must have waiting_list status to be added' do
+      registration = FactoryBot.create(:registration, :pending, competition: competition)
+      expect {
+        waiting_list.add(registration.id)
+      }.to raise_error(ArgumentError, "Registration must have a competing_status of 'waiting_list' to be added to the waiting list")
+    end
   end
 
   describe 'with populated waiting list' do