Refactor tests to use factories instead of fixtures (#285)

RSwag tests and their fixtures have been removed. All RSwag cases are now covered by service tests, as RSwag was being misused. Appropriate RSwag tests will be introduced in a later PR.

.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

@@ -1,15 +1,14 @@
 # frozen_string_literal: true
 
 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_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: registration_status } },
       comment: comment,
       admin_comment: admin_comment,
       guests: guests,
@@ -32,4 +31,3 @@ def self.payment_lane(fee_lowest_denominator, currency_code, payment_id)
     payment_lane
   end
 end
-# rubocop:enable Metrics/ParameterLists

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,11 +11,28 @@ 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
 
+  # Pre-validations
+  before_validation :set_is_competing
+
+  # Validations
+  validate :is_competing_consistency
+
+  # NOTE: There are more efficient ways to do this, see: https://github.com/thewca/wca-registration/issues/330
+  def self.accepted_competitors(competition_id)
+    where(competition_id: competition_id, is_competing: true).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]
   end
 
+  def attendee_id
+    attendee_id = "#{competition_id}-#{user_id}"
+    puts "returning attendee id: #{attendee_id}"
+    "#{competition_id}-#{user_id}"
+  end
+
   # Returns id's of the events with a non-cancelled state
   def registered_event_ids
     event_ids = []
@@ -80,7 +97,7 @@ def update_competing_lane!(update_params)
         if update_params[:status].present?
           lane.lane_state = update_params[:status]
 
-          lane.lane_details['event_details'].each do |event|
+          lane.lane_details[:event_details].each do |event|
             # NOTE: Currently event_registration_state is not used - when per-event registrations are added, we need to add validation logic to support cases like
             # limited registrations and waiting lists for certain events
             event['event_registration_state'] = update_params[:status]
@@ -97,12 +114,12 @@ def update_competing_lane!(update_params)
       lane
     end
     # TODO: In the future we will need to check if any of the other lanes have a status set to accepted
-    updated_is_attending = if update_params[:status].present?
+    updated_is_competing = if update_params[:status].present?
                              update_params[:status] == 'accepted'
                            else
-                             is_attending
+                             is_competing
                            end
-    update_attributes!(lanes: updated_lanes, is_attending: updated_is_attending) # TODO: Apparently update_attributes is deprecated in favor of update! - should we change?
+    update_attributes!(lanes: updated_lanes, is_competing: updated_is_competing) # TODO: Apparently update_attributes is deprecated in favor of update! - should we change?
   end
 
   def init_payment_lane(amount, currency_code, id)
@@ -137,10 +154,25 @@ def update_payment_lane(id, iso_amount, currency_iso, status)
   # Fields
   field :user_id, :string
   field :competition_id, :string
-  field :is_attending, :boolean
+  field :is_competing, :boolean
   field :hide_name_publicly, :boolean
   field :lanes, :array, of: Lane
 
   global_secondary_index hash_key: :user_id, projected_attributes: :all
   global_secondary_index hash_key: :competition_id, projected_attributes: :all
+
+  private
+
+    def set_is_competing
+      puts "executing set is competing for: #{attendee_id}"
+      self.is_competing = true if competing_status == 'accepted'
+    end
+
+    def is_competing_consistency
+      if is_competing
+        errors.add(:is_competing, 'cant be true unless competing_status is accepted') unless competing_status == 'accepted'
+      else
+        errors.add(:is_competing, 'must be true if competing_status is accepted') if competing_status == 'accepted'
+      end
+    end
 end