Skip to content
This repository has been archived by the owner on Jan 3, 2025. It is now read-only.

Commit

Permalink
Add qualification restrictions (#409)
Browse files Browse the repository at this point in the history
* added a mock for personal records

* mocks for qualifications

* started writing code for qualification check

* finished basic test cases

* fixed qualifications not being mocked

* changed 1/0 to true/false and removed puts

* fixed failure on update request

* fixed failure on update request again

* qualifications refactor started

* added nil test cases for qualification not enforced

* added tests for all nil cases

* all create and update tests added - no ranking quali yet

* rubocop fix

* added rankings test cases

* refactored qualification fetching code - still needs tests

* rubocop changes

* added request tests

* fixed rubocop issues

* comp api changes, all tests passing

* qualification fetch refactor

* actaully can't remember

* tests passing

* create tests passing/refactored

* update tests passing

* added whenDate support for qualifications

* rubocop changes

* removed bad helper files

* refactors

* refactors

* all tests passing with refactred stubs

* weird changes to make it run locally

* rubocop changes

* minor diff changes

* attempted fix at require_relative issue

* removed create tests

* split up helper functions into different files

* fixed commenting issue

* review changes

* rubocop changes

* added quali controller tests
  • Loading branch information
dunkOnIT authored Aug 12, 2024
1 parent 9624ca2 commit 4d92dba
Show file tree
Hide file tree
Showing 15 changed files with 535 additions and 42 deletions.
8 changes: 6 additions & 2 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,13 @@ def performance_profile(&)
end
end

def render_error(http_status, error)
def render_error(http_status, error, data = nil)
Metrics.registration_validation_errors_counter.increment
render json: { error: error }, status: http_status
if data.present?
render json: { error: error, data: data }, status: http_status
else
render json: { error: error }, status: http_status
end
end

rescue_from ActionController::ParameterMissing do |e|
Expand Down
43 changes: 43 additions & 0 deletions app/services/registration_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ def self.create_registration_allowed!(registration_request, competition_info, re

user_can_create_registration!
validate_create_events!
validate_qualifications!
validate_guests!
validate_comment!
end
Expand All @@ -30,6 +31,7 @@ def self.update_registration_allowed!(update_request, competition_info, requesti
validate_waiting_list_position!
validate_update_status!
validate_update_events!
validate_qualifications!
rescue Dynamoid::Errors::RecordNotFound
# We capture and convert the error so that it can be included in the error array of a bulk update request
raise RegistrationError.new(:not_found, ErrorCodes::REGISTRATION_NOT_FOUND)
Expand Down Expand Up @@ -89,6 +91,19 @@ def validate_create_events!
raise RegistrationError.new(:forbidden, ErrorCodes::INVALID_EVENT_SELECTION) if event_limit.present? && event_ids.count > event_limit
end

def validate_qualifications!
return unless @competition_info.enforces_qualifications?
# TODO: Read the request payload in as an object, and handle cases where expected values aren't found
event_ids = @request.dig('competing', 'event_ids')

unqualified_events = event_ids.map do |event|
qualification = @competition_info.get_qualification_for(event)
event if qualification.present? && !competitor_qualifies_for_event?(event, qualification)
end.compact

raise RegistrationError.new(:unprocessable_entity, ErrorCodes::QUALIFICATION_NOT_MET, unqualified_events) unless unqualified_events.empty?
end

def validate_guests!
return if (guests = @request['guests'].to_i).nil?

Expand Down Expand Up @@ -200,5 +215,33 @@ def existing_registration_in_series?
end
false
end

def competitor_qualifies_for_event?(event, qualification)
competitor_qualification_results = UserApi.qualifications(@requestee_user_id)
result_type = qualification['resultType']

competitor_pr = competitor_qualification_results.find { |result| result['eventId'] == event && result['type'] == result_type }
return false if competitor_pr.blank?

begin
pr_date = Date.parse(competitor_pr['on_or_before'])
qualification_date = Date.parse(qualification['whenDate'])
rescue ArgumentError
return false
end

return false unless pr_date <= qualification_date

case qualification['type']
when 'anyResult', 'ranking'
# By this point the user definitely has a result.
# Ranking qualifications are enforced when registration closes, so it is effectively an anyResult ranking when registering
true
when 'attemptResult'
competitor_pr['best'].to_i < qualification['level']
else
false
end
end
end
end
46 changes: 30 additions & 16 deletions lib/competition_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def self.find(competition_id)
nil
end

def self.comp_api_url(competition_id)
def self.url(competition_id)
"#{EnvConfig.WCA_HOST}/api/v0/competitions/#{competition_id}"
end

Expand All @@ -21,21 +21,35 @@ def self.find!(competition_id)
CompetitionInfo.new(competition_json)
end

# This is how you make a private class method
class << self
def fetch_competition(competition_id)
Rails.cache.fetch(competition_id, expires_in: 5.minutes) do
response = HTTParty.get(CompetitionApi.comp_api_url(competition_id))
case response.code
when 200
JSON.parse response.body
when 404
Metrics.registration_competition_api_error_counter.increment
raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND)
else
Metrics.registration_competition_api_error_counter.increment
raise RegistrationError.new(response.code.to_i, ErrorCodes::COMPETITION_API_5XX)
end
def self.fetch_qualifications(competition_id)
Rails.cache.fetch("#{competition_id}/qualifications", expires_in: 5.minutes) do
response = HTTParty.get("#{url(competition_id)}/qualifications")
case response.code
when 200
@status = 200
response.parsed_response
when 404
Metrics.registration_competition_api_error_counter.increment
raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND)
else
Metrics.registration_competition_api_error_counter.increment
raise RegistrationError.new(response.code.to_i, ErrorCodes::COMPETITION_API_5XX)
end
end
end

private_class_method def self.fetch_competition(competition_id)
Rails.cache.fetch(competition_id, expires_in: 5.minutes) do
response = HTTParty.get(CompetitionApi.url(competition_id))
case response.code
when 200
response.parsed_response
when 404
Metrics.registration_competition_api_error_counter.increment
raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND)
else
Metrics.registration_competition_api_error_counter.increment
raise RegistrationError.new(response.code.to_i, ErrorCodes::COMPETITION_API_5XX)
end
end
end
Expand Down
14 changes: 14 additions & 0 deletions lib/competition_info.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class CompetitionInfo
def initialize(competition_json)
@competition_json = competition_json
@competition_id = competition_json['id']
@qualifications = fetch_qualifications
end

def start_date
Expand Down Expand Up @@ -81,4 +82,17 @@ def user_can_cancel?
def other_series_ids
@competition_json['competition_series_ids']&.reject { |id| id == competition_id }
end

private def fetch_qualifications
return nil unless enforces_qualifications?
@qualifications = CompetitionApi.fetch_qualifications(@competition_id)
end

def enforces_qualifications?
@competition_json['qualification_results'] && !@competition_json['allow_registration_without_qualification']
end

def get_qualification_for(event)
@qualifications[event]
end
end
1 change: 1 addition & 0 deletions lib/error_codes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ module ErrorCodes
ORGANIZER_MUST_CANCEL_REGISTRATION = -4009
INVALID_WAITING_LIST_POSITION = -4010
MUST_ACCEPT_WAITING_LIST_LEADER = -4011
QUALIFICATION_NOT_MET = -4012

# Payment Errors
PAYMENT_NOT_ENABLED = -6001
Expand Down
5 changes: 3 additions & 2 deletions lib/registration_error.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
# frozen_string_literal: true

class RegistrationError < StandardError
attr_reader :http_status, :error
attr_reader :http_status, :error, :data

def initialize(http_status, error)
def initialize(http_status, error, data = nil)
@http_status = http_status
@error = error
@data = data
end
end
8 changes: 8 additions & 0 deletions lib/user_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ def self.competitor_info_path
"#{EnvConfig.WCA_HOST}/api/internal/v1/users/competitor-info"
end

def self.competitor_qualifications_path(user_id)
"#{EnvConfig.WCA_HOST}/api/v0/results/#{user_id}/qualification_data"
end

def self.get_permissions(user_id)
HTTParty.get(permissions_path(user_id), headers: { WCA_API_HEADER => self.wca_token })
end
Expand All @@ -20,6 +24,10 @@ def self.get_user_info_pii(user_ids)
HTTParty.post(UserApi.competitor_info_path, headers: { WCA_API_HEADER => self.wca_token }, body: { ids: user_ids.to_a })
end

def self.qualifications(user_id)
HTTParty.get(UserApi.competitor_qualifications_path(user_id), headers: { WCA_API_HEADER => self.wca_token })
end

def self.can_compete?(user_id, competition_start_date)
# All User Related cache Keys should start with the UserID, so we could invalidate them on user update
# TODO: Move this to it's own cache helper class so this is guaranteed?
Expand Down
44 changes: 39 additions & 5 deletions spec/controllers/registration_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require 'rails_helper'
require_relative '../support/qualification_results_faker'

describe RegistrationController do
describe '#create' do
Expand All @@ -16,7 +17,7 @@

it 'successfully creates a registration' do
@competition = FactoryBot.build(:competition)
stub_request(:get, CompetitionApi.comp_api_url(@competition['id'])).to_return(
stub_request(:get, CompetitionApi.url(@competition['id'])).to_return(
status: 200,
body: @competition.except('qualifications').to_json,
headers: { 'Content-Type' => 'application/json' },
Expand All @@ -31,13 +32,46 @@
created_registration = Registration.find("#{@competition['id']}-#{@registration_request['user_id']}")
expect(created_registration.event_ids).to eq(@registration_request['competing']['event_ids'])
end

context 'with qualification' do
before do
@competition = FactoryBot.build(:competition, :has_qualifications)
stub_json(CompetitionApi.url(@competition['id']), 200, @competition.except('qualifications'))
stub_json(CompetitionApi.url("#{@competition['id']}/qualifications"), 200, @competition['qualifications'])
end

it 'registration succeeds when qualifications are met' do
stub_json(UserApi.competitor_qualifications_path(@registration_request['user_id']), 200, QualificationResultsFaker.new.qualification_results)

request.headers['Authorization'] = @registration_request['jwt_token']
post :create, params: @registration_request, as: :json
sleep 0.1 # Give the queue time to work off the registration - perhaps this should be a queue length query instead?

created_registration = Registration.find("#{@competition['id']}-#{@registration_request['user_id']}")

expect(response.code).to eq('202')
expect(created_registration.event_ids).to eq(@registration_request['competing']['event_ids'])
end

it 'registration fails when qualifications not met' do
stub_json(UserApi.competitor_qualifications_path(@registration_request['user_id']), 200, [])

request.headers['Authorization'] = @registration_request['jwt_token']
post :create, params: @registration_request, as: :json
sleep 0.1 # Give the queue time to work off the registration - perhaps this should be a queue length query instead?

expect(response.code).to eq('422')
expect(response.body).to eq({ error: -4012 }.to_json)
expect { Registration.find("#{@competition['id']}-#{@registration_request['user_id']}") }.to raise_error(Dynamoid::Errors::RecordNotFound)
end
end
end

describe '#update' do
# NOTE: This code only needs to run once before the assertions, but before(:create) doesnt work because `request` defined then
before do
@competition = FactoryBot.build(:competition)
stub_request(:get, CompetitionApi.comp_api_url(@competition['id'])).to_return(status: 200, body: @competition.to_json)
stub_json(CompetitionApi.url(@competition['id']), 200, @competition.except('qualifications'))
stub_request(:post, EmailApi.registration_email_path).to_return(status: 200, body: { emails_sent: 1 }.to_json)

@registration = FactoryBot.create(:registration)
Expand Down Expand Up @@ -91,17 +125,17 @@

describe '#bulk_update' do
before do
stub_request(:post, EmailApi.registration_email_path).to_return(status: 200, body: { emails_sent: 1 }.to_json)

@competition = FactoryBot.build(:competition, mock_competition: true)
stub_request(:get, CompetitionApi.comp_api_url(@competition['id'])).to_return(status: 200, body: @competition.to_json)
stub_json(CompetitionApi.url(@competition['id']), 200, @competition.except('qualifications'))
stub_request(:post, EmailApi.registration_email_path).to_return(status: 200, body: { emails_sent: 1 }.to_json)

stub_request(:get, UserApi.permissions_path(1400)).to_return(
status: 200,
body: FactoryBot.build(:permissions_response, :admin).to_json,
headers: { content_type: 'application/json' },
)
end

# TODO: Consider refactor into separate contexts with one expect() per it-block
it 'returns a 422 if there are validation errors' do
registration = FactoryBot.create(:registration)
Expand Down
60 changes: 51 additions & 9 deletions spec/factories/competition_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
latitude_degrees { -26.21117 }
longitude_degrees { 28.06449 }
country_iso2 { 'ZA' }
qualifications { nil }
qualification_results { false }
allow_registration_without_qualification { false }
guest_entry_status { 'restricted' }
guests_per_registration_limit { 2 }
event_change_deadline_date { 1.week.from_now.iso8601 }
Expand All @@ -39,15 +42,59 @@

initialize_with { attributes.stringify_keys }

transient do
mock_competition { false }
end

trait :no_guest_limit do
guest_entry_status { 'free' }
guests_per_registration_limit { nil }
end

trait :has_qualifications do
today = Time.zone.today.iso8601

transient do
extra_qualifications { {} }
standard_qualifications {
{
'333' => { 'type' => 'attemptResult', 'resultType' => 'single', 'whenDate' => today, 'level' => 1000 },
'555' => { 'type' => 'attemptResult', 'resultType' => 'average', 'whenDate' => today, 'level' => 6000 },
'pyram' => { 'type' => 'ranking', 'resultType' => 'single', 'whenDate' => today, 'level' => 100 },
'minx' => { 'type' => 'ranking', 'resultType' => 'average', 'whenDate' => today, 'level' => 200 },
'222' => { 'type' => 'anyResult', 'resultType' => 'single', 'whenDate' => today, 'level' => 0 },
'555bf' => { 'type' => 'anyResult', 'resultType' => 'average', 'whenDate' => today, 'level' => 0 },
}
}
end

qualifications { standard_qualifications.merge(extra_qualifications) }
qualification_results { true }
allow_registration_without_qualification { false }
end

trait :has_hard_qualifications do
today = Time.zone.today.iso8601

transient do
extra_qualifications { {} }
standard_qualifications {
{
'333' => { 'type' => 'attemptResult', 'resultType' => 'single', 'whenDate' => today, 'level' => 10 },
'555' => { 'type' => 'attemptResult', 'resultType' => 'average', 'whenDate' => today, 'level' => 60 },
'pyram' => { 'type' => 'ranking', 'resultType' => 'single', 'whenDate' => (Time.zone.today-1).iso8601, 'level' => 10 },
'minx' => { 'type' => 'ranking', 'resultType' => 'average', 'whenDate' => (Time.zone.today-1).iso8601, 'level' => 20 },
'222' => { 'type' => 'anyResult', 'resultType' => 'single', 'whenDate' => (Time.zone.today-1).iso8601, 'level' => 0 },
'555bf' => { 'type' => 'anyResult', 'resultType' => 'average', 'whenDate' => (Time.zone.today-1).iso8601, 'level' => 0 },
}
}
end

qualifications { standard_qualifications.merge(extra_qualifications) }
qualification_results { true }
allow_registration_without_qualification { false }
end

trait :qualifications_not_enforced do
allow_registration_without_qualification { true }
end

trait :closed do
registration_opened? { false }
event_change_deadline_date { '2022-06-14T00:00:00.000Z' }
Expand All @@ -64,10 +111,5 @@
trait :series do
competition_series_ids { ['CubingZANationalChampionship2023', 'CubingZAWarmup2023'] }
end

# TODO: Create a flag that returns either the raw JSON (for mocking) or a CompetitionInfo object
after(:create) do |competition, evaluator|
stub_request(:get, comp_api_url(competition['competition_id'])).to_return(status: evalutor.mocked_status_code, body: competition) if evaluator.mock_competition
end
end
end
1 change: 0 additions & 1 deletion spec/factories/request_factory.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# frozen_string_literal: true

require 'factory_bot_rails'
require_relative '../support/jwt_token_generator'

FactoryBot.define do
factory :registration_request, class: Hash do
Expand Down
Loading

0 comments on commit 4d92dba

Please sign in to comment.