From 6ad00938bf7de471735bf21208d8e92f416ba0a4 Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 30 Oct 2024 19:10:52 +0200 Subject: [PATCH 01/12] moved event_ids present check --- app/models/competition.rb | 2 +- lib/registrations/registration_checker.rb | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/models/competition.rb b/app/models/competition.rb index 9dae077e2d..8276f23852 100644 --- a/app/models/competition.rb +++ b/app/models/competition.rb @@ -400,7 +400,7 @@ def main_event end def events_held?(desired_event_ids) - desired_event_ids.present? && (desired_event_ids & self.event_ids) == desired_event_ids + (desired_event_ids & self.event_ids) == desired_event_ids end def enforces_qualifications? diff --git a/lib/registrations/registration_checker.rb b/lib/registrations/registration_checker.rb index 7e8d22f508..d497c7cc92 100644 --- a/lib/registrations/registration_checker.rb +++ b/lib/registrations/registration_checker.rb @@ -197,7 +197,9 @@ def validate_update_status!(new_status, competition, current_user, target_user, end def validate_update_events!(event_ids, competition) - raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) unless competition.events_held?(event_ids) + + raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) unless + event_ids.present? && competition.events_held?(event_ids) && event_limit = competition.events_per_registration_limit raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) if event_limit.present? && event_ids.count > event_limit From 435547f9e049f768bed8412b89a892795cc496d4 Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 30 Oct 2024 19:42:20 +0200 Subject: [PATCH 02/12] event changes and bulk update changes --- .../registrations/registrations_controller.rb | 3 +- lib/registrations/registration_checker.rb | 10 +- .../factories/registration_request_factory.rb | 2 +- .../registration_checker_spec.rb | 160 +++++++++--------- 4 files changed, 88 insertions(+), 87 deletions(-) diff --git a/app/controllers/api/v1/registrations/registrations_controller.rb b/app/controllers/api/v1/registrations/registrations_controller.rb index a384080a56..c46b23c1aa 100644 --- a/app/controllers/api/v1/registrations/registrations_controller.rb +++ b/app/controllers/api/v1/registrations/registrations_controller.rb @@ -67,7 +67,8 @@ def update end def validate_update_request - Registrations::RegistrationChecker.update_registration_allowed!(params, @current_user) + competition = params[:competition_id] + Registrations::RegistrationChecker.update_registration_allowed!(params, competition, @current_user) end def bulk_update diff --git a/lib/registrations/registration_checker.rb b/lib/registrations/registration_checker.rb index d497c7cc92..4dd630efb0 100644 --- a/lib/registrations/registration_checker.rb +++ b/lib/registrations/registration_checker.rb @@ -15,7 +15,7 @@ def self.create_registration_allowed!(registration_request, current_user) validate_comment!(registration_request.dig('competing', 'comment'), competition) end - def self.update_registration_allowed!(update_request, current_user) + def self.update_registration_allowed!(update_request, competition, current_user) registration = Registration.find_by(competition_id: update_request['competition_id'], user_id: update_request['user_id']) raise WcaExceptions::RegistrationError.new(:not_found, Registrations::ErrorCodes::REGISTRATION_NOT_FOUND) unless registration.present? @@ -49,7 +49,7 @@ def self.bulk_update_allowed!(bulk_update_request, current_user) errors = {} bulk_update_request['requests'].each do |update_request| - update_registration_allowed!(update_request, current_user) + update_registration_allowed!(update_request, competition, current_user) rescue WcaExceptions::RegistrationError => e errors[update_request['user_id']] = e.error end @@ -98,7 +98,8 @@ def can_administer_or_current_user?(competition, current_user, target_user) def validate_create_events!(request, competition) event_ids = request['competing']['event_ids'] # Event submitted must be held at the competition - raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) unless competition.events_held?(event_ids) + raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) unless + event_ids.present? && competition.events_held?(event_ids) event_limit = competition.events_per_registration_limit raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) if event_limit.present? && event_ids.count > event_limit @@ -197,9 +198,8 @@ def validate_update_status!(new_status, competition, current_user, target_user, end def validate_update_events!(event_ids, competition) - raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) unless - event_ids.present? && competition.events_held?(event_ids) && + event_ids.present? && competition.events_held?(event_ids) event_limit = competition.events_per_registration_limit raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::INVALID_EVENT_SELECTION) if event_limit.present? && event_ids.count > event_limit diff --git a/spec/factories/registration_request_factory.rb b/spec/factories/registration_request_factory.rb index 34d8e84e40..0f5725e96b 100644 --- a/spec/factories/registration_request_factory.rb +++ b/spec/factories/registration_request_factory.rb @@ -71,7 +71,7 @@ submitted_by { other_user.id } end - initialize_with { attributes.stringify_keys } + initialize_with { attributes.compact.stringify_keys } after(:build) do |instance, evaluator| instance['guests'] = evaluator.guests if evaluator.guests diff --git a/spec/lib/registrations/registration_checker_spec.rb b/spec/lib/registrations/registration_checker_spec.rb index cee9d0bcff..7dd51e168e 100644 --- a/spec/lib/registrations/registration_checker_spec.rb +++ b/spec/lib/registrations/registration_checker_spec.rb @@ -710,7 +710,7 @@ update_request = FactoryBot.build(:update_request, competition_id: default_competition.id, user_id: default_user.id) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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::REGISTRATION_NOT_FOUND) expect(error.status).to eq(:not_found) @@ -725,7 +725,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) }.not_to raise_error end @@ -738,7 +738,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unauthorized) expect(error.error).to eq(Registrations::ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) @@ -756,7 +756,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:forbidden) expect(error.error).to eq(Registrations::ErrorCodes::USER_EDITS_NOT_ALLOWED) @@ -774,7 +774,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:forbidden) expect(error.error).to eq(Registrations::ErrorCodes::USER_EDITS_NOT_ALLOWED) @@ -789,7 +789,7 @@ submitted_by: default_competition.organizers.first.id, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -806,7 +806,7 @@ submitted_by: edit_deadline_passed.organizers.first.id, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -820,7 +820,7 @@ competing: { 'comment' => 'new comment' }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -836,7 +836,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::USER_COMMENT_TOO_LONG) @@ -854,7 +854,7 @@ competing: { 'comment' => at_character_limit }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -866,7 +866,7 @@ competing: { 'comment' => '' }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -882,7 +882,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::REQUIRED_COMMENT_MISSING) @@ -899,7 +899,7 @@ user_id: registration.user_id, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -917,7 +917,7 @@ competing: { 'status' => 'accepted' }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -932,7 +932,7 @@ competing: { 'comment' => 'heres a random different comment' }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -949,7 +949,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::USER_COMMENT_TOO_LONG) @@ -968,7 +968,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:forbidden) expect(error.error).to eq(Registrations::ErrorCodes::USER_EDITS_NOT_ALLOWED) @@ -986,7 +986,7 @@ competing: { 'organizer_comment' => 'this is an admin comment' }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1003,7 +1003,7 @@ competing: { 'organizer_comment' => 'this is an admin comment' }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1016,7 +1016,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unauthorized) expect(error.error).to eq(Registrations::ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) @@ -1032,7 +1032,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unauthorized) expect(error.error).to eq(Registrations::ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) @@ -1054,7 +1054,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::USER_COMMENT_TOO_LONG) @@ -1073,7 +1073,7 @@ competing: { 'organizer_comment' => at_character_limit }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1087,7 +1087,7 @@ guests: 4, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1103,7 +1103,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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::GUEST_LIMIT_EXCEEDED) expect(error.status).to eq(:unprocessable_entity) @@ -1122,7 +1122,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) }.not_to raise_error end @@ -1137,7 +1137,7 @@ guests: 0, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1153,7 +1153,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_REQUEST_DATA) @@ -1168,7 +1168,7 @@ guests: 99, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1181,7 +1181,7 @@ guests: 5, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1197,7 +1197,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:forbidden) expect(error.error).to eq(Registrations::ErrorCodes::USER_EDITS_NOT_ALLOWED) @@ -1216,7 +1216,7 @@ guests: 5, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1231,7 +1231,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_REQUEST_DATA) @@ -1248,7 +1248,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_REQUEST_DATA) @@ -1269,7 +1269,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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) @@ -1289,7 +1289,7 @@ competing: { 'status' => 'accepted' }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1301,7 +1301,7 @@ competing: { 'status' => 'deleted' }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1314,7 +1314,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_REQUEST_DATA) @@ -1331,7 +1331,7 @@ competing: { 'status' => 'pending' }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1349,7 +1349,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unauthorized) expect(error.error).to eq(Registrations::ErrorCodes::ORGANIZER_MUST_CANCEL_REGISTRATION) @@ -1369,7 +1369,7 @@ competing: { 'status' => 'deleted' }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1387,7 +1387,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:forbidden) expect(error.error).to eq(Registrations::ErrorCodes::USER_EDITS_NOT_ALLOWED) @@ -1408,7 +1408,7 @@ competing: { 'status' => 'deleted' }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1424,7 +1424,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unauthorized) expect(error.error).to eq(Registrations::ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) @@ -1464,7 +1464,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unauthorized) expect(error.error).to eq(Registrations::ErrorCodes::REGISTRATION_IS_REJECTED) @@ -1493,7 +1493,7 @@ submitted_by: default_competition.organizers.first.id, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1509,7 +1509,7 @@ submitted_by: admin.id, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1525,7 +1525,7 @@ submitted_by: competition.organizers.first.id, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1572,7 +1572,7 @@ competing: { 'event_ids' => ['333', '444', '555', 'minx'] }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1584,7 +1584,7 @@ competing: { 'event_ids' => ['333'] }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1596,7 +1596,7 @@ competing: { 'event_ids' => ['pyram', 'minx'] }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1609,7 +1609,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_EVENT_SELECTION) @@ -1625,7 +1625,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_EVENT_SELECTION) @@ -1641,7 +1641,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_EVENT_SELECTION) @@ -1657,7 +1657,7 @@ competing: { 'event_ids' => ['333', '555'] }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1671,7 +1671,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_EVENT_SELECTION) @@ -1686,7 +1686,7 @@ competing: { 'event_ids' => ['333', '333oh', '555', 'pyram', 'minx'] }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 @@ -1699,7 +1699,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:forbidden) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_EVENT_SELECTION) @@ -1717,7 +1717,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:forbidden) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_EVENT_SELECTION) @@ -1745,7 +1745,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_WAITING_LIST_POSITION) @@ -1764,7 +1764,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) }.not_to raise_error end @@ -1778,7 +1778,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:unprocessable_entity) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_WAITING_LIST_POSITION) @@ -1797,7 +1797,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:forbidden) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_WAITING_LIST_POSITION) @@ -1816,7 +1816,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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.status).to eq(:forbidden) expect(error.error).to eq(Registrations::ErrorCodes::INVALID_WAITING_LIST_POSITION) @@ -1870,7 +1870,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) }.not_to raise_error end @@ -1902,7 +1902,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) }.not_to raise_error end @@ -1915,7 +1915,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) }.not_to raise_error end @@ -1928,7 +1928,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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 @@ -1964,7 +1964,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + Registrations::RegistrationChecker.update_registration_allowed!(update_request, Competition.find(update_request['competition_id']), User.find(update_request['submitted_by'])) }.not_to raise_error end @@ -1977,7 +1977,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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 @@ -2019,7 +2019,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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::QUALIFICATION_NOT_MET) expect(error.status).to eq(:unprocessable_entity) @@ -2036,7 +2036,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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::QUALIFICATION_NOT_MET) expect(error.status).to eq(:unprocessable_entity) @@ -2053,7 +2053,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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::QUALIFICATION_NOT_MET) expect(error.status).to eq(:unprocessable_entity) @@ -2085,7 +2085,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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::QUALIFICATION_NOT_MET) expect(error.status).to eq(:unprocessable_entity) @@ -2106,7 +2106,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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::QUALIFICATION_NOT_MET) expect(error.status).to eq(:unprocessable_entity) @@ -2127,7 +2127,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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::QUALIFICATION_NOT_MET) expect(error.status).to eq(:unprocessable_entity) @@ -2148,7 +2148,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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::QUALIFICATION_NOT_MET) expect(error.status).to eq(:unprocessable_entity) @@ -2184,7 +2184,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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::ALREADY_REGISTERED_IN_SERIES) expect(error.status).to eq(:forbidden) @@ -2201,7 +2201,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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::ALREADY_REGISTERED_IN_SERIES) expect(error.status).to eq(:forbidden) @@ -2218,7 +2218,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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 From 1a9ff705bb11ffa34e13b1141e9dc1f2075b7e3c Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 30 Oct 2024 19:45:33 +0200 Subject: [PATCH 03/12] removed competition id from requests in bulk update --- spec/factories/registration_request_factory.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/factories/registration_request_factory.rb b/spec/factories/registration_request_factory.rb index 0f5725e96b..007eb201db 100644 --- a/spec/factories/registration_request_factory.rb +++ b/spec/factories/registration_request_factory.rb @@ -90,7 +90,7 @@ requests do user_ids.map do |user_id| - FactoryBot.build(:update_request, user_id: user_id, competition_id: competition_id, competing: { 'status' => 'deleted' }) + FactoryBot.build(:update_request, user_id: user_id, competing: { 'status' => 'deleted' }) end end From 00b1edf568b3b2e6a7b0a2fd6e64ba8060327173 Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 30 Oct 2024 19:55:10 +0200 Subject: [PATCH 04/12] rubocop --- lib/registrations/registration_checker.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/registrations/registration_checker.rb b/lib/registrations/registration_checker.rb index 4dd630efb0..714e5fd857 100644 --- a/lib/registrations/registration_checker.rb +++ b/lib/registrations/registration_checker.rb @@ -20,7 +20,6 @@ def self.update_registration_allowed!(update_request, competition, current_user) raise WcaExceptions::RegistrationError.new(:not_found, Registrations::ErrorCodes::REGISTRATION_NOT_FOUND) unless registration.present? target_user = User.find(update_request['user_id']) - competition = Competition.find(update_request['competition_id']) waiting_list_position = update_request.dig('competing', 'waiting_list_position') comment = update_request.dig('competing', 'comment') guests = update_request['guests'] From f48f356e6eac1d3352e132c8d1d6b2bf1264ac78 Mon Sep 17 00:00:00 2001 From: Duncan Date: Thu, 31 Oct 2024 05:55:33 +0200 Subject: [PATCH 05/12] fixing request tests --- .../api/v1/registrations/registrations_controller.rb | 10 ++++++---- lib/registrations/lanes/competing.rb | 5 ++--- lib/registrations/registration_checker.rb | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/controllers/api/v1/registrations/registrations_controller.rb b/app/controllers/api/v1/registrations/registrations_controller.rb index c46b23c1aa..c122cce281 100644 --- a/app/controllers/api/v1/registrations/registrations_controller.rb +++ b/app/controllers/api/v1/registrations/registrations_controller.rb @@ -60,22 +60,24 @@ def validate_create_request def update if params[:competing] - updated_registration = Registrations::Lanes::Competing.update!(params, @current_user.id) + updated_registration = Registrations::Lanes::Competing.update!(params, @competition, @current_user.id) return render json: { status: 'ok', registration: updated_registration.to_v2_json(admin: true, history: true) }, status: :ok end render json: { status: 'bad request', message: 'You need to supply at least one lane' }, status: :bad_request end def validate_update_request - competition = params[:competition_id] - Registrations::RegistrationChecker.update_registration_allowed!(params, competition, @current_user) + @competition = Competition.find(params[:competition_id]) + Registrations::RegistrationChecker.update_registration_allowed!(params, @competition, @current_user) end def bulk_update updated_registrations = {} update_requests = params[:requests] + competition = Competition.find(params[:competition_id]) + update_requests.each do |update| - updated_registrations[update['user_id']] = Registrations::Lanes::Competing.update!(update, @current_user) + updated_registrations[update['user_id']] = Registrations::Lanes::Competing.update!(update, competition, @current_user) end render json: { status: 'ok', updated_registrations: updated_registrations } diff --git a/lib/registrations/lanes/competing.rb b/lib/registrations/lanes/competing.rb index 68670d2edc..7f08bc46c7 100644 --- a/lib/registrations/lanes/competing.rb +++ b/lib/registrations/lanes/competing.rb @@ -19,7 +19,7 @@ def self.process!(lane_params, user_id, competition_id) registration.add_history_entry(changes, "worker", user_id, "Worker processed") end - def self.update!(update_params, current_user_id) + def self.update!(update_params, competition, current_user_id) guests = update_params[:guests] status = update_params.dig('competing', 'status') comment = update_params.dig('competing', 'comment') @@ -27,9 +27,8 @@ def self.update!(update_params, current_user_id) admin_comment = update_params.dig('competing', 'admin_comment') waiting_list_position = update_params.dig('competing', 'waiting_list_position') user_id = update_params[:user_id] - competition_id = update_params[:competition_id] - registration = Registration.find_by(competition_id: competition_id, user_id: user_id) + registration = Registration.find_by(competition_id: competition.id, user_id: user_id) old_status = registration.competing_status if old_status == Registrations::Helper::STATUS_WAITING_LIST || status == Registrations::Helper::STATUS_WAITING_LIST diff --git a/lib/registrations/registration_checker.rb b/lib/registrations/registration_checker.rb index 714e5fd857..d36dddab8c 100644 --- a/lib/registrations/registration_checker.rb +++ b/lib/registrations/registration_checker.rb @@ -16,7 +16,7 @@ def self.create_registration_allowed!(registration_request, current_user) end def self.update_registration_allowed!(update_request, competition, current_user) - registration = Registration.find_by(competition_id: update_request['competition_id'], user_id: update_request['user_id']) + registration = Registration.find_by(competition_id: competition.id, user_id: update_request['user_id']) raise WcaExceptions::RegistrationError.new(:not_found, Registrations::ErrorCodes::REGISTRATION_NOT_FOUND) unless registration.present? target_user = User.find(update_request['user_id']) From e51d9b5e3ce12c0a382678777a1c6d68ddc16dff Mon Sep 17 00:00:00 2001 From: Duncan Date: Thu, 31 Oct 2024 10:27:29 +0200 Subject: [PATCH 06/12] added >= competitor limit check --- lib/registrations/registration_checker.rb | 2 +- .../registration_checker_spec.rb | 23 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/registrations/registration_checker.rb b/lib/registrations/registration_checker.rb index 7e8d22f508..d2e38ebec4 100644 --- a/lib/registrations/registration_checker.rb +++ b/lib/registrations/registration_checker.rb @@ -167,7 +167,7 @@ def contains_organizer_fields?(request, organizer_fields) def validate_update_status!(new_status, competition, current_user, target_user, registration) raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_REQUEST_DATA) unless Registrations::Helper::REGISTRATION_STATES.include?(new_status) raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::COMPETITOR_LIMIT_REACHED) if - new_status == 'accepted' && Registration.accepted.count == competition.competitor_limit + new_status == 'accepted' && Registration.accepted.count >= competition.competitor_limit raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::ALREADY_REGISTERED_IN_SERIES) if new_status == 'accepted' && existing_registration_in_series?(competition, target_user) diff --git a/spec/lib/registrations/registration_checker_spec.rb b/spec/lib/registrations/registration_checker_spec.rb index cee9d0bcff..2281e5e2a6 100644 --- a/spec/lib/registrations/registration_checker_spec.rb +++ b/spec/lib/registrations/registration_checker_spec.rb @@ -1255,7 +1255,7 @@ end end - it 'organizer cant accept a user when registration list is full' do + it 'organizer cant accept a user when registration list is exactly full' do competitor_limit = FactoryBot.create(:competition, :with_competitor_limit, :with_organizer, competitor_limit: 3) limited_reg = FactoryBot.create(:registration, competition: competitor_limit) FactoryBot.create_list(:registration, 3, :accepted, competition: competitor_limit) @@ -1276,6 +1276,27 @@ end end + it 'organizer cant accept a user when registration list is over full' do + competitor_limit = FactoryBot.create(:competition, :with_competitor_limit, :with_organizer, competitor_limit: 3) + limited_reg = FactoryBot.create(:registration, competition: competitor_limit) + FactoryBot.create_list(:registration, 4, :accepted, competition: competitor_limit) + + update_request = FactoryBot.build( + :update_request, + user_id: limited_reg.user_id, + competition_id: limited_reg.competition.id, + submitted_by: competitor_limit.organizers.first.id, + competing: { 'status' => 'accepted' }, + ) + + expect { + Registrations::RegistrationChecker.update_registration_allowed!(update_request, 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 registrations up to the limit' do competitor_limit = FactoryBot.create(:competition, :with_competitor_limit, :with_organizer, competitor_limit: 3) limited_reg = FactoryBot.create(:registration, competition: competitor_limit) From 1b2583b84fcb0d5f52c28c9c7ec3d10d3a213010 Mon Sep 17 00:00:00 2001 From: Duncan Date: Thu, 31 Oct 2024 10:32:12 +0200 Subject: [PATCH 07/12] no competitor limit test --- .../registrations/registration_checker_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/lib/registrations/registration_checker_spec.rb b/spec/lib/registrations/registration_checker_spec.rb index 2281e5e2a6..a9fa7f2e28 100644 --- a/spec/lib/registrations/registration_checker_spec.rb +++ b/spec/lib/registrations/registration_checker_spec.rb @@ -1255,6 +1255,22 @@ end end + it 'organizer can accept registrations when there is no competitor limit', :tag do + no_competitor_limit = FactoryBot.create(:competition, :with_organizer) + registration = FactoryBot.create(:registration, competition: competitor_limit) + + update_request = FactoryBot.build( + :update_request, + user_id: registration.user_id, + competition_id: registration.competition.id, + submitted_by: no_competitor_limit.organizers.first.id, + competing: { 'status' => 'accepted' }, + ) + + expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + .not_to raise_error + end + it 'organizer cant accept a user when registration list is exactly full' do competitor_limit = FactoryBot.create(:competition, :with_competitor_limit, :with_organizer, competitor_limit: 3) limited_reg = FactoryBot.create(:registration, competition: competitor_limit) From a0b24e71db6ba4637d48e12de32006da0d611e28 Mon Sep 17 00:00:00 2001 From: Duncan Date: Thu, 31 Oct 2024 10:35:44 +0200 Subject: [PATCH 08/12] added test case for no competitor limit --- lib/registrations/registration_checker.rb | 2 +- spec/factories/competitions.rb | 1 + spec/lib/registrations/registration_checker_spec.rb | 6 +++--- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/registrations/registration_checker.rb b/lib/registrations/registration_checker.rb index 2ebf7dd37b..5a33f219b7 100644 --- a/lib/registrations/registration_checker.rb +++ b/lib/registrations/registration_checker.rb @@ -167,7 +167,7 @@ def contains_organizer_fields?(request, organizer_fields) def validate_update_status!(new_status, competition, current_user, target_user, registration) raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_REQUEST_DATA) unless Registrations::Helper::REGISTRATION_STATES.include?(new_status) raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::COMPETITOR_LIMIT_REACHED) if - new_status == 'accepted' && Registration.accepted.count >= competition.competitor_limit + new_status == 'accepted' && competition.competitor_limit > 0 && Registration.accepted.count >= competition.competitor_limit raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::ALREADY_REGISTERED_IN_SERIES) if new_status == 'accepted' && existing_registration_in_series?(competition, target_user) diff --git a/spec/factories/competitions.rb b/spec/factories/competitions.rb index 864db6f92a..044a80008d 100644 --- a/spec/factories/competitions.rb +++ b/spec/factories/competitions.rb @@ -94,6 +94,7 @@ external_registration_page { "https://www.worldcubeassociation.org" } competitor_limit_enabled { false } + competitor_limit { 0 } guests_enabled { true } on_the_spot_registration { false } refund_policy_percent { 0 } diff --git a/spec/lib/registrations/registration_checker_spec.rb b/spec/lib/registrations/registration_checker_spec.rb index 9a92aa3057..6dc7563423 100644 --- a/spec/lib/registrations/registration_checker_spec.rb +++ b/spec/lib/registrations/registration_checker_spec.rb @@ -1255,9 +1255,9 @@ end end - it 'organizer can accept registrations when there is no competitor limit', :tag do + it 'organizer can accept registrations when there is no competitor limit' do no_competitor_limit = FactoryBot.create(:competition, :with_organizer) - registration = FactoryBot.create(:registration, competition: competitor_limit) + registration = FactoryBot.create(:registration, competition: no_competitor_limit) update_request = FactoryBot.build( :update_request, @@ -1267,7 +1267,7 @@ competing: { 'status' => 'accepted' }, ) - expect { Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) } + 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 From 3425be2eac7651999b36ce78b1006c437721080e Mon Sep 17 00:00:00 2001 From: Duncan Date: Thu, 31 Oct 2024 10:37:32 +0200 Subject: [PATCH 09/12] added check for organizer modifying own rejected registration --- lib/registrations/registration_checker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/registrations/registration_checker.rb b/lib/registrations/registration_checker.rb index 5a33f219b7..328c9404d7 100644 --- a/lib/registrations/registration_checker.rb +++ b/lib/registrations/registration_checker.rb @@ -74,7 +74,7 @@ def user_can_create_registration!(competition, current_user, target_user) def user_can_modify_registration!(competition, current_user, target_user, registration) raise WcaExceptions::RegistrationError.new(:unauthorized, Registrations::ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) unless can_administer_or_current_user?(competition, current_user, target_user) raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::USER_EDITS_NOT_ALLOWED) unless competition.registration_edits_allowed? || current_user.can_manage_competition?(competition) - raise WcaExceptions::RegistrationError.new(:unauthorized, Registrations::ErrorCodes::REGISTRATION_IS_REJECTED) if user_is_rejected?(current_user, target_user, registration) + raise WcaExceptions::RegistrationError.new(:unauthorized, Registrations::ErrorCodes::REGISTRATION_IS_REJECTED) if user_is_rejected?(current_user, target_user, registration) && !organizer_modifying_own_registration? raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::ALREADY_REGISTERED_IN_SERIES) if existing_registration_in_series?(competition, target_user) && !current_user.can_manage_competition?(competition) end From 3f66d21619728b4f93e9643ee2cd42c1bac8e27f Mon Sep 17 00:00:00 2001 From: Duncan Date: Thu, 31 Oct 2024 11:39:51 +0200 Subject: [PATCH 10/12] changed competitor limit factory --- spec/factories/competitions.rb | 1 - spec/lib/registrations/registration_checker_spec.rb | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/factories/competitions.rb b/spec/factories/competitions.rb index 044a80008d..864db6f92a 100644 --- a/spec/factories/competitions.rb +++ b/spec/factories/competitions.rb @@ -94,7 +94,6 @@ external_registration_page { "https://www.worldcubeassociation.org" } competitor_limit_enabled { false } - competitor_limit { 0 } guests_enabled { true } on_the_spot_registration { false } refund_policy_percent { 0 } diff --git a/spec/lib/registrations/registration_checker_spec.rb b/spec/lib/registrations/registration_checker_spec.rb index 6dc7563423..3d71a0f943 100644 --- a/spec/lib/registrations/registration_checker_spec.rb +++ b/spec/lib/registrations/registration_checker_spec.rb @@ -1256,7 +1256,9 @@ end it 'organizer can accept registrations when there is no competitor limit' do - no_competitor_limit = FactoryBot.create(:competition, :with_organizer) + # Once created, a competition with no competitor limit will have a competitor_limit of 0 (check in rails console or database) + # However, a validation prevents you from creating a comp with a 0 competitor limit + no_competitor_limit = FactoryBot.create(:competition, :with_organizer, :skip_validations, competitor_limit: 0) registration = FactoryBot.create(:registration, competition: no_competitor_limit) update_request = FactoryBot.build( From ebbd78d1dc3f604e6e9308a53a81e746d4600e39 Mon Sep 17 00:00:00 2001 From: Gregor Billing Date: Fri, 1 Nov 2024 18:36:02 +0900 Subject: [PATCH 11/12] Fix test: Check 'competitor_limit_enabled' as expected --- app/models/competition.rb | 4 ---- lib/registrations/registration_checker.rb | 2 +- spec/lib/registrations/registration_checker_spec.rb | 4 +--- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/app/models/competition.rb b/app/models/competition.rb index 8276f23852..8f9423bdee 100644 --- a/app/models/competition.rb +++ b/app/models/competition.rb @@ -1112,10 +1112,6 @@ def entry_fee_required? ) end - def competitor_limit_enabled? - competitor_limit_enabled - end - def competitor_limit_required? confirmed? && created_at.present? && created_at > Date.new(2018, 9, 1) end diff --git a/lib/registrations/registration_checker.rb b/lib/registrations/registration_checker.rb index 328c9404d7..1f560ebc79 100644 --- a/lib/registrations/registration_checker.rb +++ b/lib/registrations/registration_checker.rb @@ -167,7 +167,7 @@ def contains_organizer_fields?(request, organizer_fields) def validate_update_status!(new_status, competition, current_user, target_user, registration) raise WcaExceptions::RegistrationError.new(:unprocessable_entity, Registrations::ErrorCodes::INVALID_REQUEST_DATA) unless Registrations::Helper::REGISTRATION_STATES.include?(new_status) raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::COMPETITOR_LIMIT_REACHED) if - new_status == 'accepted' && competition.competitor_limit > 0 && Registration.accepted.count >= competition.competitor_limit + new_status == 'accepted' && competition.competitor_limit_enabled? && Registration.accepted.count >= competition.competitor_limit raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::ALREADY_REGISTERED_IN_SERIES) if new_status == 'accepted' && existing_registration_in_series?(competition, target_user) diff --git a/spec/lib/registrations/registration_checker_spec.rb b/spec/lib/registrations/registration_checker_spec.rb index 3d71a0f943..6dc7563423 100644 --- a/spec/lib/registrations/registration_checker_spec.rb +++ b/spec/lib/registrations/registration_checker_spec.rb @@ -1256,9 +1256,7 @@ end it 'organizer can accept registrations when there is no competitor limit' do - # Once created, a competition with no competitor limit will have a competitor_limit of 0 (check in rails console or database) - # However, a validation prevents you from creating a comp with a 0 competitor limit - no_competitor_limit = FactoryBot.create(:competition, :with_organizer, :skip_validations, competitor_limit: 0) + no_competitor_limit = FactoryBot.create(:competition, :with_organizer) registration = FactoryBot.create(:registration, competition: no_competitor_limit) update_request = FactoryBot.build( From 3c51cf8ab52a60d4ce1d04f86f8a83560ed107c6 Mon Sep 17 00:00:00 2001 From: Duncan Date: Sat, 2 Nov 2024 04:35:58 +0200 Subject: [PATCH 12/12] fixed test failures --- lib/registrations/registration_checker.rb | 9 ++++++--- spec/lib/registrations/registration_checker_spec.rb | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/registrations/registration_checker.rb b/lib/registrations/registration_checker.rb index 1f560ebc79..6c3e7f1a67 100644 --- a/lib/registrations/registration_checker.rb +++ b/lib/registrations/registration_checker.rb @@ -72,9 +72,12 @@ def user_can_create_registration!(competition, current_user, target_user) end def user_can_modify_registration!(competition, current_user, target_user, registration) - raise WcaExceptions::RegistrationError.new(:unauthorized, Registrations::ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) unless can_administer_or_current_user?(competition, current_user, target_user) - raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::USER_EDITS_NOT_ALLOWED) unless competition.registration_edits_allowed? || current_user.can_manage_competition?(competition) - raise WcaExceptions::RegistrationError.new(:unauthorized, Registrations::ErrorCodes::REGISTRATION_IS_REJECTED) if user_is_rejected?(current_user, target_user, registration) && !organizer_modifying_own_registration? + raise WcaExceptions::RegistrationError.new(:unauthorized, Registrations::ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) unless + can_administer_or_current_user?(competition, current_user, target_user) + raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::USER_EDITS_NOT_ALLOWED) unless + competition.registration_edits_allowed? || current_user.can_manage_competition?(competition) + raise WcaExceptions::RegistrationError.new(:unauthorized, Registrations::ErrorCodes::REGISTRATION_IS_REJECTED) if + user_is_rejected?(current_user, target_user, registration) && !organizer_modifying_own_registration?(competition, current_user, target_user) raise WcaExceptions::RegistrationError.new(:forbidden, Registrations::ErrorCodes::ALREADY_REGISTERED_IN_SERIES) if existing_registration_in_series?(competition, target_user) && !current_user.can_manage_competition?(competition) end diff --git a/spec/lib/registrations/registration_checker_spec.rb b/spec/lib/registrations/registration_checker_spec.rb index 6dc7563423..692a42e746 100644 --- a/spec/lib/registrations/registration_checker_spec.rb +++ b/spec/lib/registrations/registration_checker_spec.rb @@ -1306,7 +1306,7 @@ ) expect { - Registrations::RegistrationChecker.update_registration_allowed!(update_request, User.find(update_request['submitted_by'])) + 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)