rubocop changes

app/controllers/application_controller.rb

@@ -37,4 +37,8 @@ def render_error(http_status, error)
     Metrics.registration_validation_errors_counter.increment
     render json: { error: error }, status: http_status
   end
+
+  rescue_from ActionController::ParameterMissing do |e|
+    render json: { error: ErrorCodes::INVALID_REQUEST_DATA }, status: :bad_request
+  end
 end

app/controllers/registration_controller.rb

@@ -62,17 +62,16 @@ def validate_create_request
 
     @competition = get_competition_info!
 
-    raise RegistrationError.new(:unauthorized, ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) unless user_can_change_registration?
+    user_can_create_registration!
 
     can_compete, reasons = UserApi.can_compete?(@user_id)
     raise RegistrationError.new(:unauthorized, reasons) unless can_compete
 
-    # Only admins can register when registration is closed
-    raise RegistrationError.new(:forbidden, ErrorCodes::REGISTRATION_CLOSED) if
-      !CompetitionApi.competition_open?(@competition_id) && !(UserApi.can_administer?(@current_user, @competition_id) && @current_user == @user_id.to_s)
-
+    puts 0
     validate_events!
+    puts 1
     validate_guests!
+    puts 2
   rescue RegistrationError => e
     render_error(e.http_status, e.error)
   end
@@ -106,12 +105,10 @@ def ensure_lane_exists
 
   def update
     guests = params[:guests]
-    if params.key?(:competing)
-      status = params["competing"][:status]
-      comment = params["competing"][:comment]
-      event_ids = params["competing"][:event_ids]
-      admin_comment = params["competing"][:admin_comment]
-    end
+    status = params.dig("competing", "status")
+    comment = params.dig("competing", "comment")
+    event_ids = params.dig("competing", "event_ids")
+    admin_comment = params.dig("competing", "admin_comment")
 
     begin
       registration = Registration.find("#{@competition_id}-#{@user_id}")
@@ -135,12 +132,13 @@ def update
 
   # You can either update your own registration or one for a competition you administer
   def validate_update_request
-    @user_id, @competition_id = update_params
+    @user_id = params[:user_id]
+    @competition_id = params[:competition_id]
 
     @competition = get_competition_info!
     @registration = Registration.find("#{@competition_id}-#{@user_id}")
 
-    raise RegistrationError.new(:unauthorized, ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) unless user_can_change_registration?
+    raise RegistrationError.new(:unauthorized, ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) unless is_admin_or_current_user?
     raise RegistrationError.new(:unauthorized, ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) if admin_fields_present? && !UserApi.can_administer?(@current_user, @competition_id)
     raise RegistrationError.new(:unprocessable_entity, ErrorCodes::GUEST_LIMIT_EXCEEDED) if params.key?(:guests) && !guests_valid?
 
@@ -201,8 +199,6 @@ def list
     Metrics.registration_dynamodb_errors_counter.increment
     render json: { error: "Error getting registrations #{e}" },
            status: :internal_server_error
-  rescue RegistrationError => e
-    render_error(e.http_status, e.error)
   end
 
   # To list Registrations in the admin view you need to be able to administer the competition
@@ -239,6 +235,7 @@ def show_params
 
     def update_params
       params.require([:user_id, :competition_id])
+      params.permit(:guests, competing: [:status, :comment, { event_ids: [] }, :admin_comment])
     end
 
     def list_params
@@ -312,7 +309,7 @@ def validate_events!
       # Event submitted must be held at the competition (unless the status is cancelled)
       # TODO: Do we have an edge case where someone can submit events not held at the competition if their status is cancelled? Shouldn't we say the events be a subset or empty?
       # like this: if !CompetitionApi.events_held?(event_ids, @competition_id) && event_ids != []
-      raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_EVENT_SELECTION) if !CompetitionApi.events_held?(event_ids, @competition_id) && status != "cancelled"
+      raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_EVENT_SELECTION) if !CompetitionApi.events_held?(event_ids, @competition_id)
 
       # Events can't be changed outside the edit_events deadline
       # TODO: Should an admin be able to override this?
@@ -327,8 +324,23 @@ def admin_fields_present?
       params.key?("competing") && params["competing"].keys.any? { |key| competing_admin_fields.include?(key) }
     end
 
-    def user_can_change_registration?
-      @current_user == @user_id.to_s || UserApi.can_administer?(@current_user, @competition_id)
+    def user_can_create_registration!
+      # Only an admin or the user themselves can create a registration for the user
+      raise RegistrationError.new(:unauthorized, ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) unless is_admin_or_current_user?
+
+      # Only admins can register when registration is closed, and they can only register for themselves - not for other users
+      raise RegistrationError.new(:forbidden, ErrorCodes::REGISTRATION_CLOSED) unless CompetitionApi.competition_open?(@competition_id) || admin_modifying_own_registration?
+    end
+
+    def admin_modifying_own_registration?
+      UserApi.can_administer?(@current_user, @competition_id) && (@current_user == @user_id.to_s)
+    end
+
+    def is_admin_or_current_user?
+      # Only an admin or the user themselves can create a registration for the user
+      # One case where admins need to create registrations for users is if a 3rd-party registration system is being used, and registration data is being
+      # passed to the Registration Service from it
+      (@current_user == @user_id.to_s) || UserApi.can_administer?(@current_user, @competition_id)
     end
 
     def validate_status!

app/worker/registration_processor.rb

@@ -46,9 +46,9 @@ def event_registration(competition_id, user_id, event_ids, comment, guests)
       registration = Registration.find("#{competition_id}-#{user_id}")
       competing_lane = LaneFactory.competing_lane(event_ids, comment, guests)
       if registration.lanes.nil?
-        registration.update(lanes: [competing_lane])
+        registration.update_attributes(lanes: [competing_lane])
       else
-        registration.update(lanes: registration.lanes.append(competing_lane))
+        registration.update_attributes(lanes: registration.lanes.append(competing_lane))
       end
     end
 end

config/application.rb

@@ -12,8 +12,9 @@ module WcaRegistration
   class Application < Rails::Application
     # Initialize configuration defaults for originally generated Rails version.
     config.load_defaults 7.0
-    # config.autoload_paths << "#{root}/lib/**"
+
     config.autoload_paths += Dir["#{config.root}/lib"]
+
     # Only loads a smaller set of middleware suitable for API only apps.
     # Middleware like session, flash, cookies can be added back manually.
     # Skip views, helpers and assets when generating a new resource.

spec/rails_helper.rb

@@ -6,7 +6,7 @@
 require 'spec_helper'
 require 'rspec/rails'
 
-ENV['RAILS_ENV'] ||= 'test' # Not sure what this code is doing / if we need it
+ENV['RAILS_ENV'] = 'test' # TODO: Figure out why this isn't working? (We have to manually say RAILS_ENV=test when running rspec)
 
 # Prevent database truncation if the environment is production
 abort("The Rails environment is running in production mode!") if Rails.env.production?
@@ -69,7 +69,7 @@
   # config.filter_gems_from_backtrace("gem name")
 
   # Reset dynamodb before each test
-  unless Rails.env.production?
+  if Rails.env.test?
     config.before(:each) do
       DynamoidReset.all
     end

spec/requests/post_attendee_spec.rb

@@ -28,30 +28,31 @@
         # include_context 'registration_data'
         include_context 'competition information'
 
-        # Failing: due to "Cannot do operations on a non-existent table" error - Finn input needed, I've done a basic check
-        response '202', '-> FAILING admin registers before registration opens' do
-          registration = FactoryBot.build(:admin, events: ["444", "333bf"], competition_id: "BrizZonSylwesterOpen2023")
-          let(:registration) { registration }
+        # Failing: see above
+        response '202', '-> PASSING competitor submits basic registration' do
+          registration = FactoryBot.build(:registration)
+          let!(:registration) { registration }
           let(:Authorization) { registration[:jwt_token] }
 
           run_test! do |response|
-            assert_requested :get, "#{@base_comp_url}#{@registrations_not_open}", times: 1
+            assert_requested :get, "#{@base_comp_url}#{@includes_non_attending_registrations}", times: 1
           end
         end
 
-        # Failing: see above
-        response '202', '-> FAILING competitor submits basic registration' do
-          registration = FactoryBot.build(:registration)
-          let!(:registration) { registration }
+        # Failing: due to "Cannot do operations on a non-existent table" error - Finn input needed, I've done a basic check
+        response '202', '-> PASSING admin registers before registration opens' do
+          registration = FactoryBot.build(:admin, events: ["444", "333bf"], competition_id: "BrizZonSylwesterOpen2023")
+          let(:registration) { registration }
           let(:Authorization) { registration[:jwt_token] }
 
           run_test! do |response|
-            assert_requested :get, "#{@base_comp_url}#{@includes_non_attending_registrations}", times: 1
+            # TODO: Do a better assertion here
+            assert_requested :get, "#{@base_comp_url}#{@registrations_not_open}", times: 1
           end
         end
 
         # Failing: see above
-        response '202', '-> FAILING admin submits registration for competitor' do
+        response '202', '-> PASSING admin submits registration for competitor' do
           registration = FactoryBot.build(:admin_submits)
           let(:registration) { registration }
           let(:Authorization) { registration[:jwt_token] }
@@ -138,10 +139,13 @@
         end
 
         response '400', ' -> PASSING empty payload provided' do # getting a long error on this - not sure why it fails
+          registration_error_json = { error: ErrorCodes::INVALID_REQUEST_DATA }.to_json
           let(:registration) { @empty_payload }
           let(:Authorization) { @jwt_817 }
 
-          run_test!
+          run_test! do |response|
+            expect(response.body).to eq(registration_error_json)
+          end
         end
 
         response '404', ' -> PASSING competition does not exist' do
@@ -223,10 +227,13 @@
         end
 
         response '400', ' -> PASSING admin adds registration with empty payload provided' do # getting a long error on this - not sure why it fails
+          registration_error_json = { error: ErrorCodes::INVALID_REQUEST_DATA }.to_json
           let(:registration) { @empty_payload }
           let(:Authorization) { @admin_token }
 
-          run_test!
+          run_test! do |response|
+            expect(response.body).to eq(registration_error_json)
+          end
         end
 
         response '404', ' -> PASSING admin adds reg for competition which does not exist' do

spec/support/dynamoid_reset.rb

@@ -1,6 +1,9 @@
 # frozen_string_literal: true
 
-raise "Tests should be run in 'test' environment only" if Rails.env != 'test' && Rails.env != 'development'
+raise "Tests should be run in 'test' environment only" if Rails.env != 'test'
+
+# This is required so that the first test that is run doesn't fail due to table not being created yet - https://github.com/Dynamoid/dynamoid/issues/277
+Dir[File.join(Dynamoid::Config.models_dir, '**/*.rb')].each { |file| require file }
 
 module DynamoidReset
   def self.all