diff --git a/app/models/competition.rb b/app/models/competition.rb index aa82e6da80..4ff33aab27 100644 --- a/app/models/competition.rb +++ b/app/models/competition.rb @@ -172,6 +172,7 @@ class Competition < ApplicationRecord event_change_deadline_date competition_series_id registration_version + newcomer_reserved_spots ).freeze VALID_NAME_RE = /\A([-&.:' [:alnum:]]+) (\d{4})\z/ VALID_ID_RE = /\A[a-zA-Z0-9]+\Z/ @@ -188,6 +189,9 @@ class Competition < ApplicationRecord MAX_MARKDOWN_LENGTH = 255 MAX_COMPETITOR_LIMIT = 5000 MAX_GUEST_LIMIT = 100 + NEWCOMER_MONTH_ENABLED = true + MAX_NEWCOMER_SPOTS_RESERVED_FRACTION = 0.5 + validates_inclusion_of :competitor_limit_enabled, in: [true, false], if: :competitor_limit_required? validates_numericality_of :competitor_limit, greater_than_or_equal_to: 1, less_than_or_equal_to: MAX_COMPETITOR_LIMIT, if: :competitor_limit_enabled? validates :competitor_limit_reason, presence: true, if: :competitor_limit_enabled? @@ -242,6 +246,30 @@ class Competition < ApplicationRecord validates :external_website, :external_registration_page, length: { maximum: MAX_URL_LENGTH } validates :contact, length: { maximum: MAX_MARKDOWN_LENGTH } + validate :validate_newcomer_reserved_spots + private def validate_newcomer_reserved_spots + return unless competitor_limit.present? + + if newcomer_reserved_spots > 0 && !NEWCOMER_MONTH_ENABLED + errors.add(:newcomer_reserved_spots, 'newcomer competitions are not allowed at present') + end + + max_newcomer_spots = (competitor_limit * MAX_NEWCOMER_SPOTS_RESERVED_FRACTION).to_i + if newcomer_reserved_spots > max_newcomer_spots + errors.add(:newcomer_reserved_spots, 'cant reserve more than 50% of spots for newcomers') + end + end + + # TODO: Make sure this query is optimized + def newcomers_competing + registrations.competing_status_accepted.includes(:user).select { |registration| registration.user.newcomer? } + end + + # TODO: What if there are no newcomer reserved spots? Is this represented as nil or 0 in the db? + def newcomer_reserved_spots_remaining + newcomer_reserved_spots - newcomers_competing.count + end + # Dirty old trick to deal with competition id changes (see other methods using # 'with_old_id' for more details). def persisted_events_id @@ -2351,6 +2379,7 @@ def to_form_data "enabled" => competitor_limit_enabled, "count" => competitor_limit, "reason" => competitor_limit_reason, + "newcomerReservedSpots" => newcomer_reserved_spots, }, "staff" => { "staffDelegateIds" => staff_delegates.to_a.pluck(:id), @@ -2452,6 +2481,7 @@ def form_errors "enabled" => errors[:competitor_limit_enabled], "count" => errors[:competitor_limit], "reason" => errors[:competitor_limit_reason], + "newcomer_reserved_spots" => errors[:newcomer_reserved_spots], }, "staff" => { "staffDelegateIds" => errors[:staff_delegate_ids], @@ -2589,6 +2619,7 @@ def self.form_data_to_attributes(form_data) competitor_limit_enabled: form_data.dig('competitorLimit', 'enabled'), competitor_limit: form_data.dig('competitorLimit', 'count'), competitor_limit_reason: form_data.dig('competitorLimit', 'reason'), + newcomer_reserved_spots: form_data.dig('competitorLimit', 'newcomerReservedSpots'), extra_registration_requirements: form_data.dig('registration', 'extraRequirements'), on_the_spot_registration: form_data.dig('registration', 'allowOnTheSpot'), on_the_spot_entry_fee_lowest_denomination: form_data.dig('entryFees', 'onTheSpotEntryFee'), @@ -2732,6 +2763,7 @@ def self.form_data_json_schema "enabled" => { "type" => ["boolean", "null"] }, "count" => { "type" => ["integer", "null"] }, "reason" => { "type" => ["string", "null"] }, + "newcomerReservedSpots" => { "type" => ["integer", "null"] }, }, }, "staff" => { diff --git a/app/models/user.rb b/app/models/user.rb index 7fbc80999f..7d9e2c1a14 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -387,6 +387,10 @@ def country Country.find_by_iso2(country_iso2) end + def newcomer? + person.nil? || wca_id.start_with?(Time.current.year.to_s) + end + def locale preferred_locale || I18n.default_locale end diff --git a/app/webpacker/components/CompetitionForm/FormSections/CompetitorLimit.js b/app/webpacker/components/CompetitionForm/FormSections/CompetitorLimit.js index c7399d80c7..89fc7bc82c 100644 --- a/app/webpacker/components/CompetitionForm/FormSections/CompetitorLimit.js +++ b/app/webpacker/components/CompetitionForm/FormSections/CompetitorLimit.js @@ -17,6 +17,7 @@ export default function CompetitorLimit() { + ); diff --git a/app/webpacker/components/RegistrationsV2/api/helper/error_codes.js b/app/webpacker/components/RegistrationsV2/api/helper/error_codes.js index a158b774a6..cf05130436 100644 --- a/app/webpacker/components/RegistrationsV2/api/helper/error_codes.js +++ b/app/webpacker/components/RegistrationsV2/api/helper/error_codes.js @@ -22,5 +22,6 @@ export const REGISTRATION_CLOSED = -4008; export const ORGANIZER_MUST_CANCEL_REGISTRATION = -4009; export const INVALID_WAITING_LIST_POSITION = -4010; export const QUALIFICATION_NOT_MET = -4012; +export const NO_UNRESERVED_SPOTS_REMAINING = -4013; export const PAYMENT_NOT_ENABLED = -6001; export const PAYMENT_NOT_READY = -6002; diff --git a/config/locales/en.yml b/config/locales/en.yml index 93cf206d0b..712e3eeeef 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1662,6 +1662,7 @@ en: enabled: "Competitor limit" count: "Maximum number of competitors" reason: "The reason for the competitor limit" + newcomer_reserved_spots: "Number of spots to reserve for newcomers" staff: staff_delegate_ids: "WCA Delegate(s)" trainee_delegate_ids: "WCA Trainee Delegate(s)" @@ -1746,6 +1747,7 @@ en: enabled: "Although it is not required by the Regulations to provide a competitor limit, it's highly recommended to do so." count: "The number of competitors allowed in this competition. For now this number is informational only, and does not yet prevent more people from registering. We are working on adding explicit support for this to the registration flow, but it requires some other work first." reason: "What is the reason for the limit on competitors? Please fill this out in English!" + newcomer_reserved_spots: "Only allowed during Newcomer Month - reserved spots can't be more than half of the Competitor Limit." staff: staff_delegate_ids: "WCA Delegates for the competition." trainee_delegate_ids: "WCA Trainee Delegates for the competition." @@ -1947,6 +1949,7 @@ en: -4010: "The Waiting List Position is not Valid" -4011: "You can only accept the first person on the waiting list" -4012: "You are not qualified to compete in the selected events" + -4013: "Cant accept non-newcomer when only reserved newcomer spots remain" -6001: "WCA Payment is not enabled for this competition" -6002: "You need to finish your registration before you can pay" #context: and when an error occured diff --git a/db/migrate/20241126084446_add_newcomer_reserved_spots_to_competitions.rb b/db/migrate/20241126084446_add_newcomer_reserved_spots_to_competitions.rb new file mode 100644 index 0000000000..d9bf6c37fc --- /dev/null +++ b/db/migrate/20241126084446_add_newcomer_reserved_spots_to_competitions.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddNewcomerReservedSpotsToCompetitions < ActiveRecord::Migration[7.2] + def change + add_column :Competitions, :newcomer_reserved_spots, :integer, default: 0, null: false + end +end diff --git a/lib/database_dumper.rb b/lib/database_dumper.rb index e472a23a95..1c0703498e 100644 --- a/lib/database_dumper.rb +++ b/lib/database_dumper.rb @@ -110,6 +110,7 @@ def self.actions_to_column_sanitizers(columns_by_action) registration_version forbid_newcomers forbid_newcomers_reason + newcomer_reserved_spots ), db_default: %w( connected_stripe_account_id diff --git a/lib/registrations/error_codes.rb b/lib/registrations/error_codes.rb index f69d7c9ea0..274324ad99 100644 --- a/lib/registrations/error_codes.rb +++ b/lib/registrations/error_codes.rb @@ -35,6 +35,7 @@ module ErrorCodes ORGANIZER_MUST_CANCEL_REGISTRATION = -4009 INVALID_WAITING_LIST_POSITION = -4010 QUALIFICATION_NOT_MET = -4012 + NO_UNRESERVED_SPOTS_REMAINING = -4013 # Payment Errors PAYMENT_NOT_ENABLED = -6001 diff --git a/lib/registrations/registration_checker.rb b/lib/registrations/registration_checker.rb index 9bdfe55f9d..cb2d33e4be 100644 --- a/lib/registrations/registration_checker.rb +++ b/lib/registrations/registration_checker.rb @@ -171,12 +171,22 @@ def contains_organizer_fields?(request, organizer_fields) def validate_update_status!(new_status, competition, current_user, target_user, registration, events) raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_REQUEST_DATA) unless Registration.competing_statuses.include?(new_status) - raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::COMPETITOR_LIMIT_REACHED) if - new_status == Registrations::Helper::STATUS_ACCEPTED && competition.competitor_limit_enabled? && - competition.registrations.competing_status_accepted.count >= competition.competitor_limit raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::ALREADY_REGISTERED_IN_SERIES) if new_status == Registrations::Helper::STATUS_ACCEPTED && existing_registration_in_series?(competition, target_user) + if new_status == Registrations::Helper::STATUS_ACCEPTED && competition.competitor_limit_enabled? + raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::COMPETITOR_LIMIT_REACHED) if + competition.registrations.competing_status_accepted.count >= competition.competitor_limit + + if !target_user.newcomer? && competition.newcomer_reserved_spots > 0 + available_spots = competition.competitor_limit - competition.registrations.competing_status_accepted.count + newcomer_reserved_spots_remaining = competition.newcomer_reserved_spots - competition.newcomers_competing.count + + raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::NO_UNRESERVED_SPOTS_REMAINING) if + (available_spots <= newcomer_reserved_spots_remaining) && competition.registration_currently_open? + end + end + # Otherwise, organizers can make any status change they want to return if current_user.can_manage_competition?(competition) diff --git a/spec/factories/competitions.rb b/spec/factories/competitions.rb index 864db6f92a..7293e2d6c9 100644 --- a/spec/factories/competitions.rb +++ b/spec/factories/competitions.rb @@ -99,8 +99,17 @@ refund_policy_percent { 0 } guests_entry_fee_lowest_denomination { 0 } + # TODO: This should change to :v3 registration_version { :v1 } + trait :newcomer_month do + registration_open + with_organizer + with_competitor_limit + competitor_limit { 4 } + newcomer_reserved_spots { 2 } + end + trait :enforces_qualifications do with_organizer qualification_results { true } diff --git a/spec/factories/persons.rb b/spec/factories/persons.rb index 8166f786a5..1aed8708e6 100644 --- a/spec/factories/persons.rb +++ b/spec/factories/persons.rb @@ -2,9 +2,13 @@ FactoryBot.define do factory :person do + transient do + wca_id_year { "2016" } + end + wca_id do mid = ('A'..'Z').to_a.sample(4).join - id = "2016#{mid}01" + id = "#{wca_id_year}#{mid}01" id = id.next while Person.exists?(wca_id: id) id end diff --git a/spec/factories/registrations.rb b/spec/factories/registrations.rb index 635d8e3023..ce0af058a6 100644 --- a/spec/factories/registrations.rb +++ b/spec/factories/registrations.rb @@ -41,10 +41,14 @@ competing_status { Registrations::Helper::STATUS_WAITING_LIST } end - trait :newcomer do + trait :first_timer do association :user, factory: [:user] end + trait :newcomer do + association :user, factory: [:user, :current_year_wca_id] + end + trait :paid do after(:create) do |registration| FactoryBot.create :registration_payment, registration: registration, user: registration.user, diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 4e2e76c0f7..d3e02e917c 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -220,6 +220,16 @@ end end + trait :current_year_wca_id do + transient do + person { + FactoryBot.create( + :person, name: name, countryId: Country.find_by_iso2(country_iso2).id, gender: gender, dob: dob.strftime("%F"), wca_id_year: Time.current.year.to_s + ) + } + end + end + trait :with_2fa do otp_required_for_login { true } otp_secret { User.generate_otp_secret } diff --git a/spec/lib/registrations/registration_checker_spec.rb b/spec/lib/registrations/registration_checker_spec.rb index f278e70792..982027f897 100644 --- a/spec/lib/registrations/registration_checker_spec.rb +++ b/spec/lib/registrations/registration_checker_spec.rb @@ -2278,6 +2278,155 @@ }.not_to raise_error end end + + describe '#update_registration_allowed!.reserved newcomer spots' do + let(:newcomer_month_comp) { FactoryBot.create(:competition, :newcomer_month) } + let(:non_newcomer_reg) { FactoryBot.create(:registration, competition: newcomer_month_comp) } + let(:newcomer_reg) { FactoryBot.create(:registration, :newcomer, competition: newcomer_month_comp) } + let(:first_timer_reg) { FactoryBot.create(:registration, :first_timer, competition: newcomer_month_comp) } + + describe 'only newcomer spots remain' do + before do + FactoryBot.create_list(:registration, 2, :accepted, competition: newcomer_month_comp) + end + + it 'organizer cant accept non-newcomer if only reserved newcomer spots remain' do + update_request = FactoryBot.build( + :update_request, + user_id: non_newcomer_reg.user.id, + competition_id: non_newcomer_reg.competition.id, + submitted_by: newcomer_month_comp.organizers.first.id, + competing: { 'status' => 'accepted' }, + ) + + expect { + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) + }.to raise_error(WcaExceptions::RegistrationError) do |error| + expect(error.error).to eq(Registrations::ErrorCodes::NO_UNRESERVED_SPOTS_REMAINING) + expect(error.status).to eq(:forbidden) + end + end + + it 'organizer can accept first-timer' do + update_request = FactoryBot.build( + :update_request, + user_id: first_timer_reg.user.id, + competition_id: first_timer_reg.competition.id, + submitted_by: newcomer_month_comp.organizers.first.id, + competing: { 'status' => 'accepted' }, + ) + + expect { + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) + }.not_to raise_error + end + + it 'organizer can accept newcomer who started competing this year' do + update_request = FactoryBot.build( + :update_request, + user_id: newcomer_reg.user.id, + competition_id: newcomer_reg.competition.id, + submitted_by: newcomer_month_comp.organizers.first.id, + competing: { 'status' => 'accepted' }, + ) + + expect { + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) + }.not_to raise_error + end + end + + describe 'reserved newcomer spots are full' do + before do + FactoryBot.create_list(:registration, 2, :newcomer, :accepted, competition: newcomer_month_comp) + end + + it 'organizer can still accept newcomers if all reserved newcomer spots are full' do + update_request = FactoryBot.build( + :update_request, + user_id: newcomer_reg.user.id, + competition_id: newcomer_reg.competition.id, + submitted_by: newcomer_month_comp.organizers.first.id, + competing: { 'status' => 'accepted' }, + ) + + expect { + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) + }.not_to raise_error + end + + it 'organizer can accept non-newcomer if all reserved newcomer spots are full' do + update_request = FactoryBot.build( + :update_request, + user_id: non_newcomer_reg.user.id, + competition_id: non_newcomer_reg.competition.id, + submitted_by: newcomer_month_comp.organizers.first.id, + competing: { 'status' => 'accepted' }, + ) + + expect { + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) + }.not_to raise_error + end + end + + it 'organizer cant accept newcomer if competition is full' do + FactoryBot.create_list(:registration, 4, :newcomer, :accepted, competition: newcomer_month_comp) + + update_request = FactoryBot.build( + :update_request, + user_id: newcomer_reg.user.id, + competition_id: newcomer_reg.competition.id, + submitted_by: newcomer_month_comp.organizers.first.id, + competing: { 'status' => 'accepted' }, + ) + + expect { + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) + }.to raise_error(WcaExceptions::RegistrationError) do |error| + expect(error.error).to eq(Registrations::ErrorCodes::COMPETITOR_LIMIT_REACHED) + expect(error.status).to eq(:forbidden) + end + end + + it 'organizer can accept non-newcomer into a newcomer reserved spot if registration is closed' do + closed_newcomer_comp = FactoryBot.create(:competition, :newcomer_month, :registration_closed) + normal_reg = FactoryBot.create(:registration, competition: closed_newcomer_comp) + FactoryBot.create_list(:registration, 2, :accepted, competition: closed_newcomer_comp) + + update_request = FactoryBot.build( + :update_request, + user_id: normal_reg.user.id, + competition_id: normal_reg.competition.id, + submitted_by: closed_newcomer_comp.organizers.first.id, + competing: { 'status' => 'accepted' }, + ) + + expect { + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) + }.not_to raise_error + end + + it 'takes newcomer registrations into account when calculating spots remaining' do + FactoryBot.create_list(:registration, 2, :accepted, competition: newcomer_month_comp) + FactoryBot.create(:registration, :accepted, :newcomer, competition: newcomer_month_comp) + + update_request = FactoryBot.build( + :update_request, + user_id: non_newcomer_reg.user.id, + competition_id: non_newcomer_reg.competition.id, + submitted_by: newcomer_month_comp.organizers.first.id, + competing: { 'status' => 'accepted' }, + ) + + expect { + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) + }.to raise_error(WcaExceptions::RegistrationError) do |error| + expect(error.error).to eq(Registrations::ErrorCodes::NO_UNRESERVED_SPOTS_REMAINING) + expect(error.status).to eq(:forbidden) + end + end + end end describe '#bulk_update' do diff --git a/spec/models/competition_spec.rb b/spec/models/competition_spec.rb index 1efb07fcc2..7fe769cb24 100644 --- a/spec/models/competition_spec.rb +++ b/spec/models/competition_spec.rb @@ -11,6 +11,20 @@ expect(competition.cellName).to eq "Foo: Test - 2015" end + it 'can reserve newcomer spots up to 50% of registrations' do + expect(FactoryBot.build(:competition, newcomer_reserved_spots: 50, competitor_limit: 100)).to be_valid + end + + it 'reserved newcomers spots cant exceed 50% of registrations' do + expect(FactoryBot.build(:competition, newcomer_reserved_spots: 51, competitor_limit: 100)).to be_invalid_with_errors( + newcomer_reserved_spots: ['cant reserve more than 50% of spots for newcomers'], + ) + + expect(FactoryBot.build(:competition, newcomer_reserved_spots: 50, competitor_limit: 99)).to be_invalid_with_errors( + newcomer_reserved_spots: ['cant reserve more than 50% of spots for newcomers'], + ) + end + it "rejects invalid names" do [ "foo (Test) - 2015", @@ -1583,4 +1597,30 @@ def change_and_check_activities(new_start_date, new_end_date) expect(new_competition).not_to be_valid end end + + context 'newcomer month competition' do + let(:newcomer_month_comp) { FactoryBot.create(:competition, :newcomer_month) } + let!(:newcomer_reg) { FactoryBot.create(:registration, :newcomer, competition: newcomer_month_comp) } + + context '#non_newcomers_competing' do + before do + FactoryBot.create_list(:registration, 2, :accepted, competition: newcomer_month_comp) + end + + it 'doesnt include newcomers in count' do + expect(newcomer_month_comp.registrations.count).to eq(3) + expect(newcomer_month_comp.non_newcomers_competing.count).to eq(2) + end + + it 'doesnt include non-newcomers in non-accepted states' do + FactoryBot.create(:registration, competition: newcomer_month_comp) + FactoryBot.create(:registration, :cancelled, competition: newcomer_month_comp) + FactoryBot.create(:registration, :rejected, competition: newcomer_month_comp) + FactoryBot.create(:registration, :waiting_list, competition: newcomer_month_comp) + + expect(newcomer_month_comp.registrations.count).to eq(7) + expect(newcomer_month_comp.non_newcomers_competing.count).to eq(2) + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 332c26d8c2..d29b08f8cd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -791,4 +791,21 @@ expect(user.teams_committees_at_least_senior_roles).not_to include(wrt_role) end end + + describe 'newcomer?' do + it 'true for user with no WCA ID' do + user = FactoryBot.create(:user) + expect(user.newcomer?).to eq(true) + end + + it 'true for user with current year WCA ID' do + user = FactoryBot.create(:user, :current_year_wca_id) + expect(user.newcomer?).to eq(true) + end + + it 'false for user with previous year WCA ID' do + user = FactoryBot.create(:user, :wca_id) + expect(user.newcomer?).to eq(false) + end + end end