Refactor tests to use factories instead of fixtures

.rubocop.yml

@@ -32,6 +32,9 @@ Naming/AccessorMethodName:
 Style/Alias:
   Enabled: false
 
+Style/NumericLiterals:
+  Enabled: false
+
 Style/EmptyMethod:
   EnforcedStyle: expanded
 

.ruby-version

@@ -1 +1 @@
-ruby-3.2.2
+3.2.2

app/controllers/registration_controller.rb

@@ -49,26 +49,10 @@ def create
     render json: { status: 'accepted', message: 'Started Registration Process' }, status: :accepted
   end
 
-  # For a user to register they need to
-  # 1) Need to actually be the user that they are trying to register
-  # 2) Be Eligible to Compete (complete profile + not banned)
-  # 3) Register for a competition that is open
-  # 4) Register for events that are actually held at the competition
-  # We need to do this in this order, so we don't leak user attributes
   def validate_create_request
-    @user_id = registration_params[:user_id]
     @competition_id = registration_params[:competition_id]
-    @event_ids = registration_params[:competing]['event_ids']
 
-    @competition = CompetitionApi.find!(@competition_id)
-
-    user_can_create_registration!
-
-    can_compete = UserApi.can_compete?(@user_id)
-    raise RegistrationError.new(:unauthorized, ErrorCodes::USER_PROFILE_INCOMPLETE) unless can_compete
-
-    validate_events!
-    raise RegistrationError.new(:unprocessable_entity, ErrorCodes::GUEST_LIMIT_EXCEEDED) if params.key?(:guests) && @competition.guest_limit_exceeded?(params[:guests])
+    RegistrationChecker.create_registration_allowed!(registration_params, CompetitionApi.find!(@competition_id), @current_user)
   rescue RegistrationError => e
     render_error(e.http_status, e.error)
   end
@@ -134,19 +118,7 @@ def validate_update_request
     @user_id = params[:user_id]
     @competition_id = params[:competition_id]
 
-    @competition = CompetitionApi.find!(@competition_id)
-    @registration = Registration.find("#{@competition_id}-#{@user_id}")
-
-    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) && @competition.guest_limit_exceeded?(params[:guests])
-
-    if params.key?(:competing)
-      validate_status! if params['competing'].key?(:status)
-      validate_events! if params['competing'].key?(:event_ids)
-      raise RegistrationError.new(:unprocessable_entity, ErrorCodes::USER_COMMENT_TOO_LONG) if params['competing'].key?(:comment) && !comment_valid?
-      raise RegistrationError.new(:unprocessable_entity, ErrorCodes::REQUIRED_COMMENT_MISSING) if !params['competing'].key?(:comment) && @competition.force_comment?
-    end
+    RegistrationChecker.update_registration_allowed!(params, CompetitionApi.find!(@competition_id), @current_user)
   rescue Dynamoid::Errors::RecordNotFound
     render_error(:not_found, ErrorCodes::REGISTRATION_NOT_FOUND)
   rescue RegistrationError => e
@@ -300,69 +272,4 @@ def get_single_registration(user_id, competition_id)
         },
       }
     end
-
-    def registration_exists?(user_id, competition_id)
-      Registration.find("#{competition_id}-#{user_id}")
-      true
-    rescue Dynamoid::Errors::RecordNotFound
-      false
-    end
-
-    def comment_valid?
-      params['competing'][:comment].length <= 240
-    end
-
-    def validate_events!
-      event_ids = params['competing'][:event_ids]
-      if defined?(@registration) && params['competing'].key?(:status) && params['competing'][:status] == 'cancelled'
-        # If status is cancelled, events can only be empty or match the old events list
-        # This allows for edge cases where an API user might send an empty event list/the old event list, or admin might want to remove events
-        raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_EVENT_SELECTION) unless event_ids == [] || event_ids == @registration.event_ids
-      else
-        # Event submitted must be held at the competition
-        raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_EVENT_SELECTION) unless @competition.events_held?(event_ids)
-      end
-
-      # Events can't be changed outside the edit_events deadline
-      # TODO: Should an admin be able to override this?
-      if @competition.event_change_deadline.present?
-        events_edit_deadline = Time.parse(@competition.event_change_deadline)
-        raise RegistrationError.new(:forbidden, ErrorCodes::EVENT_EDIT_DEADLINE_PASSED) if events_edit_deadline < Time.now
-      end
-    end
-
-    def admin_fields_present?
-      # There could be different admin fields in different lanes - define the admin fields per lane and check each
-      competing_admin_fields = ['admin_comment']
-
-      params.key?('competing') && params['competing'].keys.any? { |key| competing_admin_fields.include?(key) }
-    end
-
-    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 @competition.registration_open? || organizer_signing_up_themselves?
-    end
-
-    def organizer_signing_up_themselves?
-      @competition.is_organizer_or_delegate?(@current_user) && (@current_user.to_s == @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.to_s == @user_id.to_s) || UserApi.can_administer?(@current_user, @competition_id)
-    end
-
-    def validate_status!
-      raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_REQUEST_DATA) unless Registration::REGISTRATION_STATES.include?(params['competing'][:status])
-
-      raise RegistrationError.new(:unauthorized, ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) if
-        Registration::ADMIN_ONLY_STATES.include?(params['competing'][:status]) && !UserApi.can_administer?(@current_user, @competition_id)
-
-      raise RegistrationError.new(:forbidden, ErrorCodes::COMPETITOR_LIMIT_REACHED) if params['competing'][:status] == 'accepted' && Registration.count > @competition.competitor_limit
-    end
 end

app/helpers/competition_api.rb

@@ -46,19 +46,23 @@ def fetch_competition(competition_id)
 end
 
 class CompetitionInfo
+  attr_accessor :competition_id
+
   def initialize(competition_json)
     @competition_json = competition_json
+    @competition_id = competition_json['id']
   end
 
-  def event_change_deadline
-    @competition_json['event_change_deadline_date']
+  def within_event_change_deadline?
+    Time.now < @competition_json['event_change_deadline_date']
   end
 
   def competitor_limit
     @competition_json['competitor_limit']
   end
 
   def guest_limit_exceeded?(guest_count)
+    return false unless @competition_json['guests_per_registration_limit'].present?
     @competition_json['guest_entry_status'] == 'restricted' && @competition_json['guests_per_registration_limit'] < guest_count
   end
 
@@ -93,4 +97,12 @@ def is_organizer_or_delegate?(user_id)
   def name
     @competition_json['name']
   end
+
+  def registration_edits_allowed?
+    @competition_json['allow_registration_edits'] && within_event_change_deadline?
+  end
+
+  def user_can_cancel?
+    @competition_json['allow_registration_self_delete_after_acceptance']
+  end
 end

app/helpers/error_codes.rb

@@ -11,23 +11,23 @@ module ErrorCodes
   COMPETITION_API_5XX = -1001
 
   # User Errors
-  USER_IS_BANNED = -2001
-  USER_PROFILE_INCOMPLETE = -2002
+  USER_CANNOT_COMPETE = -2001
   USER_INSUFFICIENT_PERMISSIONS = -2003
 
   # Registration errors
   REGISTRATION_NOT_FOUND = -3000
 
   # Request errors
   INVALID_REQUEST_DATA = -4000
-  EVENT_EDIT_DEADLINE_PASSED = -4001
+  USER_EDITS_NOT_ALLOWED = -4001
   GUEST_LIMIT_EXCEEDED = -4002
   USER_COMMENT_TOO_LONG = -4003
   INVALID_EVENT_SELECTION = -4004
   REQUIRED_COMMENT_MISSING = -4005
   COMPETITOR_LIMIT_REACHED = -4006
   INVALID_REGISTRATION_STATUS = -4007
   REGISTRATION_CLOSED = -4008
+  ORGANIZER_MUST_CANCEL_REGISTRATION = -4009
 
   # Payment Errors
   PAYMENT_NOT_ENABLED = -3001

app/helpers/lane_factory.rb

@@ -3,13 +3,13 @@
 require 'time'
 # rubocop:disable Metrics/ParameterLists
 class LaneFactory
-  def self.competing_lane(event_ids = [], comment = '', guests = 0, admin_comment = '', registration_status = 'pending')
+  def self.competing_lane(event_ids: [], comment: '', guests: 0, admin_comment: '', registration_status: 'pending')
     competing_lane = Lane.new({})
     competing_lane.lane_name = 'competing'
     competing_lane.completed_steps = ['Event Registration']
-    competing_lane.lane_state = registration_status
+    competing_lane.lane_state = lane_state
     competing_lane.lane_details = {
-      event_details: event_ids.map { |event_id| { event_id: event_id } },
+      event_details: event_ids.map { |event_id| { event_id: event_id, event_registration_state: lane_state } },
       comment: comment,
       admin_comment: admin_comment,
       guests: guests,

app/helpers/mocks.rb

@@ -39,24 +39,10 @@ def self.permissions_mock(user_id)
           'scope' => '*',
         },
       }
-    when '209943' # Test banned User
+    when '209943', '999999' # Test banned/incomplete profile User
       {
         'can_attend_competitions' => {
           'scope' => [],
-          'reasons' => ErrorCodes::USER_IS_BANNED,
-        },
-        'can_organize_competitions' => {
-          'scope' => [],
-        },
-        'can_administer_competitions' => {
-          'scope' => [],
-        },
-      }
-    when '999999' # Test incomplete User
-      {
-        'can_attend_competitions' => {
-          'scope' => [],
-          'reasons' => ErrorCodes::USER_PROFILE_INCOMPLETE,
         },
         'can_organize_competitions' => {
           'scope' => [],

app/models/registration.rb

@@ -11,6 +11,11 @@ class Registration
   REGISTRATION_STATES = %w[pending waiting_list accepted cancelled].freeze
   ADMIN_ONLY_STATES = %w[pending waiting_list accepted].freeze # Only admins are allowed to change registration state to one of these states
 
+  # NOTE: this could be very inefficient? Not sure if there's a way to cache the total status numbers?
+  def self.accepted_competitors
+    all.select { |registration| registration.competing_status == 'accepted' }.count
+  end
+
   # Returns all event ids irrespective of registration status
   def event_ids
     lanes.filter_map { |x| x.lane_details['event_details'].pluck('event_id') if x.lane_name == 'competing' }[0]

app/services/registration_checker.rb

@@ -0,0 +1,140 @@
+# frozen_string_literal: true
+
+COMMENT_CHARACTER_LIMIT = 240
+
+class RegistrationChecker
+  def self.create_registration_allowed!(registration_request, competition_info, requesting_user)
+    @request = registration_request
+    @competition_info = competition_info
+    @requester_user_id = requesting_user
+
+    user_can_create_registration!
+    validate_create_events!
+    validate_guests!
+    validate_comment!
+    true
+  end
+
+  def self.update_registration_allowed!(update_request, competition_info, requesting_user)
+    @request = update_request
+    @competition_info = competition_info
+    @requester_user_id = requesting_user
+    @registration = Registration.find("#{update_request['competition_id']}-#{update_request['user_id']}")
+
+    user_can_modify_registration!
+    validate_guests!
+    validate_comment!
+    validate_admin_fields!
+    validate_admin_comment!
+    validate_update_status!
+    validate_update_events!
+  end
+
+  class << self
+    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 @competition_info.registration_open? || organizer_modifying_own_registration?
+
+      can_compete = UserApi.can_compete?(@request['user_id'])
+      raise RegistrationError.new(:unauthorized, ErrorCodes::USER_CANNOT_COMPETE) unless can_compete
+
+      can_compete
+    end
+
+    def user_can_modify_registration!
+      raise RegistrationError.new(:unauthorized, ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) unless is_admin_or_current_user?
+      raise RegistrationError.new(:forbidden, ErrorCodes::USER_EDITS_NOT_ALLOWED) unless @competition_info.registration_edits_allowed? || user_is_admin?
+    end
+
+    def organizer_modifying_own_registration?
+      UserApi.can_administer?(@requester_user_id, @competition_info.competition_id) && (@requester_user_id == @request['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
+      (@requester_user_id == @request['user_id'].to_s) || user_is_admin?
+    end
+
+    def user_is_admin?
+      UserApi.can_administer?(@requester_user_id, @competition_info.competition_id)
+    end
+
+    def validate_create_events!
+      event_ids = @request['competing']['event_ids']
+      # Event submitted must be held at the competition
+      raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_EVENT_SELECTION) unless @competition_info.events_held?(event_ids)
+    end
+
+    def validate_guests!
+      return unless @request.key?('guests')
+
+      raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_REQUEST_DATA) if @request['guests'] < 0
+      raise RegistrationError.new(:unprocessable_entity, ErrorCodes::GUEST_LIMIT_EXCEEDED) if @competition_info.guest_limit_exceeded?(@request['guests'])
+    end
+
+    def validate_comment!
+      if @request.key?('competing') && @request['competing'].key?('comment')
+        comment = @request['competing']['comment']
+
+        raise RegistrationError.new(:unprocessable_entity, ErrorCodes::USER_COMMENT_TOO_LONG) if comment.length > COMMENT_CHARACTER_LIMIT
+        raise RegistrationError.new(:unprocessable_entity, ErrorCodes::REQUIRED_COMMENT_MISSING) if @competition_info.force_comment? && comment == ''
+      else
+        raise RegistrationError.new(:unprocessable_entity, ErrorCodes::REQUIRED_COMMENT_MISSING) if @competition_info.force_comment?
+      end
+    end
+
+    def validate_admin_fields!
+      @admin_fields = ['admin_comment']
+
+      raise RegistrationError.new(:unauthorized, ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) if contains_admin_fields? && !UserApi.can_administer?(@requester_user_id, @competition_info.competition_id)
+    end
+
+    def validate_admin_comment!
+      if @request.key?('competing') && @request['competing'].key?('admin_comment')
+        admin_comment = @request['competing']['admin_comment']
+
+        raise RegistrationError.new(:unprocessable_entity, ErrorCodes::USER_COMMENT_TOO_LONG) if admin_comment.length > COMMENT_CHARACTER_LIMIT
+      end
+    end
+
+    def contains_admin_fields?
+      @request.key?('competing') && @request['competing'].keys.any? { |key| @admin_fields.include?(key) }
+    end
+
+    def validate_update_status!
+      return unless @request.key?('competing') && @request['competing'].key?('status')
+
+      old_status = @registration.competing_status
+      new_status = @request['competing']['status']
+
+      raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_REQUEST_DATA) unless Registration::REGISTRATION_STATES.include?(new_status)
+      raise RegistrationError.new(:forbidden, ErrorCodes::COMPETITOR_LIMIT_REACHED) if
+        new_status == 'accepted' && Registration.accepted_competitors >= @competition_info.competitor_limit
+
+      unless user_is_admin?
+        if new_status == 'pending'
+          raise RegistrationError.new(:unauthorized, ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) unless old_status == 'cancelled'
+        elsif new_status == 'cancelled'
+          raise RegistrationError.new(:unauthorized, ErrorCodes::ORGANIZER_MUST_CANCEL_REGISTRATION) if
+            !@competition_info.user_can_cancel? && @registration.competing_status == 'accepted'
+          raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_REQUEST_DATA) if
+            @request['competing'].key?('event_ids') && @registration.event_ids != @request['competing']['event_ids']
+        else
+          raise RegistrationError.new(:unauthorized, ErrorCodes::USER_INSUFFICIENT_PERMISSIONS)
+        end
+      end
+    end
+
+    def validate_update_events!
+      return unless @request.key?('competing') && @request['competing'].key?('event_ids')
+      raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_EVENT_SELECTION) if !@competition_info.events_held?(
+        @request['competing']['event_ids'],
+      )
+    end
+  end
+end

spec/domains/registrations_api_spec.rb

@@ -5,3 +5,4 @@
 require_relative '../requests/get_registrations_spec'
 require_relative '../requests/post_attendee_spec'
 require_relative '../requests/update_registration_spec'
+require_relative '../services/registration_checker_spec'