v3 nits: Competition argument in validate_update and event_ids.nil check (#10161)

* moved event_ids present check

* event changes and bulk update changes

* removed competition id from requests in bulk update

* rubocop

* fixing request tests

Author: dunkOnIT

app/controllers/api/v1/registrations/registrations_controller.rb

@@ -60,21 +60,24 @@ def validate_create_request
 
   def update
     if params[:competing]
-      updated_registration = Registrations::Lanes::Competing.update!(params, @current_user.id)
+      updated_registration = Registrations::Lanes::Competing.update!(params, @competition, @current_user.id)
       return render json: { status: 'ok', registration: updated_registration.to_v2_json(admin: true, history: true) }, status: :ok
     end
     render json: { status: 'bad request', message: 'You need to supply at least one lane' }, status: :bad_request
   end
 
   def validate_update_request
-    Registrations::RegistrationChecker.update_registration_allowed!(params, @current_user)
+    @competition = Competition.find(params[:competition_id])
+    Registrations::RegistrationChecker.update_registration_allowed!(params, @competition, @current_user)
   end
 
   def bulk_update
     updated_registrations = {}
     update_requests = params[:requests]
+    competition = Competition.find(params[:competition_id])
+
     update_requests.each do |update|
-      updated_registrations[update['user_id']] = Registrations::Lanes::Competing.update!(update, @current_user)
+      updated_registrations[update['user_id']] = Registrations::Lanes::Competing.update!(update, competition, @current_user)
     end
 
     render json: { status: 'ok', updated_registrations: updated_registrations }

app/models/competition.rb

@@ -400,7 +400,7 @@ def main_event
   end
 
   def events_held?(desired_event_ids)
-    desired_event_ids.present? && (desired_event_ids & self.event_ids) == desired_event_ids
+    (desired_event_ids & self.event_ids) == desired_event_ids
   end
 
   def enforces_qualifications?

lib/registrations/lanes/competing.rb

@@ -19,17 +19,16 @@ def self.process!(lane_params, user_id, competition_id)
         registration.add_history_entry(changes, "worker", user_id, "Worker processed")
       end
 
-      def self.update!(update_params, current_user_id)
+      def self.update!(update_params, competition, current_user_id)
         guests = update_params[:guests]
         status = update_params.dig('competing', 'status')
         comment = update_params.dig('competing', 'comment')
         event_ids = update_params.dig('competing', 'event_ids')
         admin_comment = update_params.dig('competing', 'admin_comment')
         waiting_list_position = update_params.dig('competing', 'waiting_list_position')
         user_id = update_params[:user_id]
-        competition_id = update_params[:competition_id]
 
-        registration = Registration.find_by(competition_id: competition_id, user_id: user_id)
+        registration = Registration.find_by(competition_id: competition.id, user_id: user_id)
         old_status = registration.competing_status
 
         if old_status == Registrations::Helper::STATUS_WAITING_LIST || status == Registrations::Helper::STATUS_WAITING_LIST

lib/registrations/registration_checker.rb

@@ -15,12 +15,11 @@ def self.create_registration_allowed!(registration_request, current_user)
       validate_comment!(registration_request.dig('competing', 'comment'), competition)
     end
 
-    def self.update_registration_allowed!(update_request, current_user)
-      registration = Registration.find_by(competition_id: update_request['competition_id'], user_id: update_request['user_id'])
+    def self.update_registration_allowed!(update_request, competition, current_user)
+      registration = Registration.find_by(competition_id: competition.id, user_id: update_request['user_id'])
       raise WcaExceptions::RegistrationError.new(:not_found, Registrations::ErrorCodes::REGISTRATION_NOT_FOUND) unless registration.present?
 
       target_user = User.find(update_request['user_id'])
-      competition = Competition.find(update_request['competition_id'])
       waiting_list_position = update_request.dig('competing', 'waiting_list_position')
       comment = update_request.dig('competing', 'comment')
       guests = update_request['guests']
@@ -49,7 +48,7 @@ def self.bulk_update_allowed!(bulk_update_request, current_user)
 
       errors = {}
       bulk_update_request['requests'].each do |update_request|
-        update_registration_allowed!(update_request, current_user)
+        update_registration_allowed!(update_request, competition, current_user)
       rescue WcaExceptions::RegistrationError => e
         errors[update_request['user_id']] = e.error
       end
@@ -98,7 +97,8 @@ def can_administer_or_current_user?(competition, current_user, target_user)
       def validate_create_events!(request, competition)
         event_ids = request['competing']['event_ids']
         # Event submitted must be held at the competition
-        raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) unless competition.events_held?(event_ids)
+        raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) unless
+          event_ids.present? && competition.events_held?(event_ids)
 
         event_limit = competition.events_per_registration_limit
         raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) if event_limit.present? && event_ids.count > event_limit
@@ -197,7 +197,8 @@ def validate_update_status!(new_status, competition, current_user, target_user,
       end
 
       def validate_update_events!(event_ids, competition)
-        raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) unless competition.events_held?(event_ids)
+        raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) unless
+          event_ids.present? && competition.events_held?(event_ids)
 
         event_limit = competition.events_per_registration_limit
         raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) if event_limit.present? && event_ids.count > event_limit

spec/factories/registration_request_factory.rb

@@ -71,7 +71,7 @@
       submitted_by { other_user.id }
     end
 
-    initialize_with { attributes.stringify_keys }
+    initialize_with { attributes.compact.stringify_keys }
 
     after(:build) do |instance, evaluator|
       instance['guests'] = evaluator.guests if evaluator.guests
@@ -90,7 +90,7 @@
 
     requests do
       user_ids.map do |user_id|
-        FactoryBot.build(:update_request, user_id: user_id, competition_id: competition_id, competing: { 'status' => 'deleted' })
+        FactoryBot.build(:update_request, user_id: user_id, competing: { 'status' => 'deleted' })
       end
     end