From 4140bd2d22e07178321620cf56253bc2783d0077 Mon Sep 17 00:00:00 2001 From: Duncan <52967253+dunkOnIT@users.noreply.github.com> Date: Fri, 24 May 2024 10:49:34 +0200 Subject: [PATCH] Allow a banned user to register for a competition which starts after their ban ends (#589) * added test case for ban end date * refactored ban end date check * rubocop changes --- app/helpers/competition_api.rb | 4 ++++ app/helpers/user_api.rb | 9 +++++++-- app/services/registration_checker.rb | 2 +- spec/factories/request_factory.rb | 4 ++++ spec/factories/response_factory.rb | 8 ++++++++ spec/services/registration_checker_spec.rb | 19 +++++++++++++++++-- 6 files changed, 41 insertions(+), 5 deletions(-) diff --git a/app/helpers/competition_api.rb b/app/helpers/competition_api.rb index 9653b2c1..ec07035a 100644 --- a/app/helpers/competition_api.rb +++ b/app/helpers/competition_api.rb @@ -53,6 +53,10 @@ def initialize(competition_json) @competition_id = competition_json['id'] end + def start_date + @competition_json['start_date'] + end + def within_event_change_deadline? return true if @competition_json['event_change_deadline_date'].nil? Time.now < @competition_json['event_change_deadline_date'] diff --git a/app/helpers/user_api.rb b/app/helpers/user_api.rb index a8e6ef82..3255af80 100644 --- a/app/helpers/user_api.rb +++ b/app/helpers/user_api.rb @@ -26,13 +26,18 @@ def self.get_user_info_pii(user_ids) HTTParty.post(competitor_info_path, headers: { WCA_API_HEADER => self.wca_token }, body: { ids: user_ids.to_a }) end - def self.can_compete?(user_id) + 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? permissions = Rails.cache.fetch("#{user_id}-permissions", expires_in: 5.minutes) do self.get_permissions(user_id) end - permissions['can_attend_competitions']['scope'] == '*' + + competition_permissions = permissions['can_attend_competitions'] + competition_start = DateTime.parse(competition_start_date) + ban_end = DateTime.parse(permissions['can_attend_competitions']['until'] || '3099-09-09') + + competition_permissions['scope'] == '*' || ban_end < competition_start end def self.can_administer?(user_id, competition_id) diff --git a/app/services/registration_checker.rb b/app/services/registration_checker.rb index 9082dfb9..2d55afa3 100644 --- a/app/services/registration_checker.rb +++ b/app/services/registration_checker.rb @@ -56,7 +56,7 @@ def user_can_create_registration! # Only organizers can register when registration is closed, and they can only register for themselves - not for other users raise RegistrationError.new(:forbidden, ErrorCodes::REGISTRATION_CLOSED) unless @competition_info.registration_open? || organizer_modifying_own_registration? - can_compete = UserApi.can_compete?(@requestee_user_id) + can_compete = UserApi.can_compete?(@requestee_user_id, @competition_info.start_date) raise RegistrationError.new(:unauthorized, ErrorCodes::USER_CANNOT_COMPETE) unless can_compete # Users cannot sign up for multiple competitions in a series diff --git a/spec/factories/request_factory.rb b/spec/factories/request_factory.rb index 25f9960c..8687e78f 100644 --- a/spec/factories/request_factory.rb +++ b/spec/factories/request_factory.rb @@ -39,6 +39,10 @@ user_id { 209943 } end + trait :unbanned_soon do + user_id { 209944 } + end + trait :incomplete do user_id { 999999 } end diff --git a/spec/factories/response_factory.rb b/spec/factories/response_factory.rb index 1ffe5783..f0fb002e 100644 --- a/spec/factories/response_factory.rb +++ b/spec/factories/response_factory.rb @@ -4,6 +4,10 @@ FactoryBot.define do factory :permissions_response, class: Hash do + transient do + ban_end_date { nil } # Specify as a string ISO-formatted date + end + organized_competitions { [] } can_attend_competitions { { 'scope' => '*' } } @@ -18,6 +22,10 @@ can_attend_competitions { { 'scope' => [] } } end + trait :unbanned_soon do + can_attend_competitions { { 'scope' => [], 'until' => ban_end_date } } + end + initialize_with { attributes.stringify_keys } end end diff --git a/spec/services/registration_checker_spec.rb b/spec/services/registration_checker_spec.rb index 87fb7d75..77626b30 100644 --- a/spec/services/registration_checker_spec.rb +++ b/spec/services/registration_checker_spec.rb @@ -269,10 +269,25 @@ end end - it 'banned user cant register' do + it 'can register if ban ends before competition starts', :focus do + registration_request = FactoryBot.build(:registration_request, :unbanned_soon) + competition_info = CompetitionInfo.new(FactoryBot.build(:competition)) + stub_request(:get, permissions_path(registration_request['user_id'])).to_return( + status: 200, + body: FactoryBot.build(:permissions_response, :unbanned_soon, ban_end_date: DateTime.parse(competition_info.start_date)-1).to_json, + headers: { content_type: 'application/json' }, + ) + + expect { RegistrationChecker.create_registration_allowed!(registration_request, competition_info, registration_request['submitted_by']) } + .not_to raise_error + end + + it 'cant register if ban ends after competition starts', :focus do registration_request = FactoryBot.build(:registration_request, :banned) competition_info = CompetitionInfo.new(FactoryBot.build(:competition)) - stub_request(:get, permissions_path(registration_request['user_id'])).to_return(status: 200, body: FactoryBot.build(:permissions_response, :banned).to_json, headers: { content_type: 'application/json' }) + stub_request(:get, permissions_path(registration_request['user_id'])).to_return( + status: 200, body: FactoryBot.build(:permissions_response, :banned).to_json, headers: { content_type: 'application/json' }, + ) expect { RegistrationChecker.create_registration_allowed!(registration_request, competition_info, registration_request['submitted_by'])