From 072b2665d5b804a0d12002c3f14b546313a9924b Mon Sep 17 00:00:00 2001 From: FinnIckler Date: Fri, 15 Dec 2023 12:08:52 +0100 Subject: [PATCH] Add Public Waiting List Tab (#295) * Waiting List Frontend * implement waiting list management * run rubocop * fix eslint * correctly show the number of waiting competitors * check length of waiting too * don't force organizers to update the waitlist everytime someone is approved * adding tests for waiting list * waiting list tests passing * refactored waiting list functions * added cache invalidations * corrected typing of test values and updated type acceptance tests * added tests for waiting list position outside of min/max boundary * re-enabled cache * caching tests * Fixed caches not updating and introduced a method in lib * corrected cache behaviour * removed puts statements * removed get/set and refactored to use minmax * removed caching-test config files * remove cache_test fro mgemfile * refactored list_waiting to use get_by_status * skipping jwt validation on list_waiting temproarily * removed caching of waiting list * removed commented code and unnecessary docs * fix issue with worker not loading lane * fix issue with multiple updates not waiting before the other completes * fix typo * Add Segment for Waiting List * run eslint * added redis to backend-test compose file --------- Co-authored-by: Duncan --- .../api/registration/get/get_registrations.ts | 17 + Frontend/src/index.dev.jsx | 5 + .../register/components/CompetingStep.jsx | 2 +- .../components/RegistrationActions.jsx | 6 +- .../components/RegistrationEditor.jsx | 33 +- .../pages/waiting/components/WaitingList.jsx | 53 ++++ Frontend/src/pages/waiting/index.jsx | 12 + Frontend/src/routes.jsx | 5 + Frontend/src/ui/PageTabs.jsx | 7 + app/controllers/registration_controller.rb | 51 ++- app/helpers/error_codes.rb | 2 + app/helpers/lane_factory.rb | 4 +- app/models/lane.rb | 50 --- app/models/registration.rb | 98 ++++-- app/services/registration_checker.rb | 31 +- config/routes.rb | 4 +- docker-compose.backend-test.yml | 19 +- docs/example_payloads/registration.json | 38 --- docs/example_payloads/registrations.json | 0 lib/lane.rb | 118 +++++++ spec/factories/registration_factory.rb | 21 +- spec/models/registration_spec.rb | 299 ++++++++++++++++-- spec/services/registration_checker_spec.rb | 103 +++++- spec/support/dynamoid_reset.rb | 2 +- 24 files changed, 813 insertions(+), 167 deletions(-) create mode 100644 Frontend/src/pages/waiting/components/WaitingList.jsx create mode 100644 Frontend/src/pages/waiting/index.jsx delete mode 100644 app/models/lane.rb delete mode 100644 docs/example_payloads/registration.json delete mode 100644 docs/example_payloads/registrations.json create mode 100644 lib/lane.rb diff --git a/Frontend/src/api/registration/get/get_registrations.ts b/Frontend/src/api/registration/get/get_registrations.ts index 940b64fc..1c66909a 100644 --- a/Frontend/src/api/registration/get/get_registrations.ts +++ b/Frontend/src/api/registration/get/get_registrations.ts @@ -64,6 +64,7 @@ export async function getAllRegistrations( } throw new BackendError(error.error, response.status) } + return addUserInfo(data!) } @@ -77,3 +78,19 @@ export async function getSingleRegistration( { needsAuthentication: true } ) as Promise<{ registration: components['schemas']['registrationAdmin'] }> } + +export async function getWaitingCompetitors( + competitionId: string +): Promise { + const registrations = (await backendFetch( + `/registrations/${competitionId}/waiting`, + 'GET', + { + needsAuthentication: false, + } + )) as components['schemas']['registrationAdmin'][] + + return (await addUserInfo( + registrations + )) as components['schemas']['registrationAdmin'][] +} diff --git a/Frontend/src/index.dev.jsx b/Frontend/src/index.dev.jsx index c24b6181..8b2a5ff6 100644 --- a/Frontend/src/index.dev.jsx +++ b/Frontend/src/index.dev.jsx @@ -21,6 +21,7 @@ import Registrations from './pages/registrations' import Schedule from './pages/schedule' import TestLogin from './pages/test/login' import TestLogout from './pages/test/logout' +import Waiting from './pages/waiting' import { BASE_ROUTE } from './routes' import App from './ui/App' import Competition from './ui/Competition' @@ -103,6 +104,10 @@ const router = createBrowserRouter([ path: 'register', element: , }, + { + path: `waiting`, + element: , + }, { path: 'tabs/:tab_id', element: , diff --git a/Frontend/src/pages/register/components/CompetingStep.jsx b/Frontend/src/pages/register/components/CompetingStep.jsx index 6a4a4d65..e3b4b017 100644 --- a/Frontend/src/pages/register/components/CompetingStep.jsx +++ b/Frontend/src/pages/register/components/CompetingStep.jsx @@ -345,7 +345,7 @@ export default function CompetingStep({ nextStep }) { <> } /> diff --git a/Frontend/src/pages/registration_administration/components/RegistrationActions.jsx b/Frontend/src/pages/registration_administration/components/RegistrationActions.jsx index 73087401..6ba44b6b 100644 --- a/Frontend/src/pages/registration_administration/components/RegistrationActions.jsx +++ b/Frontend/src/pages/registration_administration/components/RegistrationActions.jsx @@ -77,9 +77,9 @@ export default function RegistrationActions({ } } - const changeStatus = async (attendees, status) => { - attendees.forEach((attendee) => { - updateRegistrationMutation( + const changeStatus = (attendees, status) => { + attendees.forEach(async (attendee) => { + await updateRegistrationMutation( { user_id: attendee, competing: { diff --git a/Frontend/src/pages/registration_edit/components/RegistrationEditor.jsx b/Frontend/src/pages/registration_edit/components/RegistrationEditor.jsx index 5dab2e85..883646f6 100644 --- a/Frontend/src/pages/registration_edit/components/RegistrationEditor.jsx +++ b/Frontend/src/pages/registration_edit/components/RegistrationEditor.jsx @@ -8,6 +8,7 @@ import { Button, Checkbox, Header, + Input, Message, Segment, TextArea, @@ -28,6 +29,8 @@ export default function RegistrationEditor() { const [comment, setComment] = useState('') const [adminComment, setAdminComment] = useState('') const [status, setStatus] = useState('') + const [waitingListPosition, setWaitingListPosition] = useState(0) + const [guests, setGuests] = useState(0) const [selectedEvents, setSelectedEvents] = useState([]) const [registration, setRegistration] = useState({}) const [isCheckingRefunds, setIsCheckingRefunds] = useState(false) @@ -73,6 +76,10 @@ export default function RegistrationEditor() { setAdminComment( serverRegistration.registration.competing.admin_comment ?? '' ) + setWaitingListPosition( + serverRegistration.registration.competing.waiting_list_position ?? 0 + ) + setGuests(serverRegistration.registration.guests ?? 0) } }, [serverRegistration]) @@ -127,22 +134,24 @@ export default function RegistrationEditor() { event_ids: selectedEvents, comment, admin_comment: adminComment, + waiting_list_position: waitingListPosition, }, competition_id: competitionInfo.id, }) } }, [ - adminComment, - comment, + hasChanges, commentIsValid, - competitionInfo.id, eventsAreValid, - hasChanges, - selectedEvents, - status, + maxEvents, updateRegistrationMutation, user_id, - maxEvents, + status, + selectedEvents, + comment, + adminComment, + waitingListPosition, + competitionInfo.id, ]) const registrationEditDeadlinePassed = moment( @@ -228,6 +237,16 @@ export default function RegistrationEditor() { checked={status === 'cancelled'} onChange={(_, data) => setStatus(data.value)} /> +
+
Guests
+ setGuests(data.value)} + /> {registrationEditDeadlinePassed ? ( diff --git a/Frontend/src/pages/waiting/components/WaitingList.jsx b/Frontend/src/pages/waiting/components/WaitingList.jsx new file mode 100644 index 00000000..74a10ce2 --- /dev/null +++ b/Frontend/src/pages/waiting/components/WaitingList.jsx @@ -0,0 +1,53 @@ +import { useQuery } from '@tanstack/react-query' +import React, { useContext } from 'react' +import { Table, TableFooter } from 'semantic-ui-react' +import { CompetitionContext } from '../../../api/helper/context/competition_context' +import { getWaitingCompetitors } from '../../../api/registration/get/get_registrations' +import { setMessage } from '../../../ui/events/messages' +import LoadingMessage from '../../../ui/messages/loadingMessage' + +export default function WaitingList() { + const { competitionInfo } = useContext(CompetitionContext) + const { isLoading, data: waiting } = useQuery({ + queryKey: ['waiting', competitionInfo.id], + queryFn: () => getWaitingCompetitors(competitionInfo.id), + retry: false, + onError: (err) => { + setMessage(err.message, 'error') + }, + }) + return isLoading ? ( + + ) : ( + + + + Name + Position + + + + {waiting?.length ? ( + waiting + .sort( + (w1, w2) => + w1.competing.waiting_list_position - + w2.competing.waiting_list_position + ) // Once a waiting list is established, we just care about the order of the waitlisted competitors + .map((w, i) => ( + + {w.user.name} + + {w.competing.waiting_list_position === 0 + ? 'Not yet assigned' + : i + 1} + + + )) + ) : ( + No one on the Waiting List. + )} + +
+ ) +} diff --git a/Frontend/src/pages/waiting/index.jsx b/Frontend/src/pages/waiting/index.jsx new file mode 100644 index 00000000..9616981b --- /dev/null +++ b/Frontend/src/pages/waiting/index.jsx @@ -0,0 +1,12 @@ +import React from 'react' +import { Header, Segment } from 'semantic-ui-react' +import WaitingList from './components/WaitingList' + +export default function Waiting() { + return ( + +
Waiting List:
+ +
+ ) +} diff --git a/Frontend/src/routes.jsx b/Frontend/src/routes.jsx index 050ea3a3..1223111b 100644 --- a/Frontend/src/routes.jsx +++ b/Frontend/src/routes.jsx @@ -9,6 +9,7 @@ import RegistrationAdministration from './pages/registration_administration' import RegistrationEdit from './pages/registration_edit' import Registrations from './pages/registrations' import Schedule from './pages/schedule' +import Waiting from './pages/waiting' import App from './ui/App' import Competition from './ui/Competition' import CustomTab from './ui/CustomTab' @@ -72,6 +73,10 @@ const routes = [ path: 'tabs/:tab_id', element: , }, + { + path: `waiting`, + element: , + }, { path: 'registrations', element: , diff --git a/Frontend/src/ui/PageTabs.jsx b/Frontend/src/ui/PageTabs.jsx index 272f9fae..717044d8 100644 --- a/Frontend/src/ui/PageTabs.jsx +++ b/Frontend/src/ui/PageTabs.jsx @@ -21,6 +21,7 @@ export default function PageTabs() { } if (canAdminCompetition) { optionalTabs.push(registrationsMenuConfig) + optionalTabs.push(waitingMenuConfig) } if (new Date(competitionInfo.registration_open) < Date.now()) { optionalTabs.push(competitorsMenuConfig) @@ -143,6 +144,12 @@ const registrationsMenuConfig = { icon: 'list ul', label: 'Registrations', } +const waitingMenuConfig = { + key: 'waiting', + route: 'waiting', + icon: 'clock', + label: 'Waiting list', +} const competitorsMenuConfig = { key: 'competitors', route: 'registrations', diff --git a/app/controllers/registration_controller.rb b/app/controllers/registration_controller.rb index 7993127d..402a0940 100644 --- a/app/controllers/registration_controller.rb +++ b/app/controllers/registration_controller.rb @@ -8,7 +8,7 @@ require_relative '../helpers/error_codes' class RegistrationController < ApplicationController - skip_before_action :validate_token, only: [:list] + skip_before_action :validate_token, only: [:list, :list_waiting] # The order of the validations is important to not leak any non public info via the API # That's why we should always validate a request first, before taking any other before action # before_actions are triggered in the order they are defined @@ -32,7 +32,7 @@ def create lane_name: 'competing', step: 'Event Registration', step_details: { - registration_status: 'waiting', + registration_status: 'pending', event_ids: event_ids, comment: comment, guests: guests, @@ -90,28 +90,31 @@ def update comment = params.dig('competing', 'comment') event_ids = params.dig('competing', 'event_ids') admin_comment = params.dig('competing', 'admin_comment') + waiting_list_position = params.dig('competing', 'waiting_list_position') begin registration = Registration.find("#{@competition_id}-#{@user_id}") old_status = registration.competing_status - updated_registration = registration.update_competing_lane!({ status: status, comment: comment, event_ids: event_ids, admin_comment: admin_comment, guests: guests }) if old_status == 'accepted' && status != 'accepted' Registration.decrement_competitors_count(@competition_id) elsif old_status != 'accepted' && status == 'accepted' Registration.increment_competitors_count(@competition_id) end + updated_registration = registration.update_competing_lane!({ status: status, comment: comment, event_ids: event_ids, admin_comment: admin_comment, guests: guests, waiting_list_position: waiting_list_position }) + render json: { status: 'ok', registration: { user_id: updated_registration['user_id'], - guests: updated_registration.guests, + guests: updated_registration['guests'], competing: { event_ids: updated_registration.registered_event_ids, registration_status: updated_registration.competing_status, registered_on: updated_registration['created_at'], comment: updated_registration.competing_comment, admin_comment: updated_registration.admin_comment, + waiting_list_position: updated_registration.competing_waiting_list_position, }, } } - rescue StandardError => e + rescue Dynamoid::Errors::Error => e puts e Metrics.registration_dynamodb_errors_counter.increment render json: { error: "Error Updating Registration: #{e.message}" }, @@ -170,7 +173,39 @@ def list competition_id = list_params registrations = get_registrations(competition_id, only_attending: true) render json: registrations - rescue StandardError => e + rescue Dynamoid::Errors::Error => e + # Render an error response + puts e + Metrics.registration_dynamodb_errors_counter.increment + render json: { error: "Error getting registrations #{e}" }, + status: :internal_server_error + end + + def mine + my_registrations = Registration.where(user_id: @current_user).map { |x| { competition_id: x.competition_id, status: x.competing_status } } + render json: { registrations: my_registrations } + rescue Dynamoid::Errors::Error => e + # Render an error response + puts e + Metrics.registration_dynamodb_errors_counter.increment + render json: { error: "Error getting registrations #{e}" }, + status: :internal_server_error + end + + def list_waiting + competition_id = list_params + + waiting = Registration.get_registrations_by_status(competition_id, 'waiting_list').map do |registration| + { + user_id: registration[:user_id], + competing: { + event_ids: registration.event_ids, + waiting_list_position: registration.competing_waiting_list_position || 0, + }, + } + end + render json: waiting + rescue Dynamoid::Errors::Error => e # Render an error response puts e Metrics.registration_dynamodb_errors_counter.increment @@ -190,7 +225,7 @@ def validate_list_admin def list_admin registrations = get_registrations(@competition_id) render json: registrations - rescue StandardError => e + rescue Dynamoid::Errors::Error => e puts e # Is there a reason we aren't using an error code here? Metrics.registration_dynamodb_errors_counter.increment @@ -254,6 +289,7 @@ def get_registrations(competition_id, only_attending: false) registered_on: x['created_at'], comment: x.competing_comment, admin_comment: x.admin_comment, + waiting_list_position: x.competing_waiting_list_position, }, payment: { payment_status: x.payment_status, @@ -275,6 +311,7 @@ def get_single_registration(user_id, competition_id) registered_on: registration['created_at'], comment: registration.competing_comment, admin_comment: registration.admin_comment, + waiting_list_position: registration.competing_waiting_list_position, }, payment: { payment_status: registration.payment_status, diff --git a/app/helpers/error_codes.rb b/app/helpers/error_codes.rb index 18b9ddf2..7ff4434c 100644 --- a/app/helpers/error_codes.rb +++ b/app/helpers/error_codes.rb @@ -28,6 +28,8 @@ module ErrorCodes INVALID_REGISTRATION_STATUS = -4007 REGISTRATION_CLOSED = -4008 ORGANIZER_MUST_CANCEL_REGISTRATION = -4009 + INVALID_WAITING_LIST_POSITION = -4010 + MUST_ACCEPT_WAITING_LIST_LEADER = -4011 # Payment Errors PAYMENT_NOT_ENABLED = -3001 diff --git a/app/helpers/lane_factory.rb b/app/helpers/lane_factory.rb index d417bcde..b900fc23 100644 --- a/app/helpers/lane_factory.rb +++ b/app/helpers/lane_factory.rb @@ -2,7 +2,8 @@ require 'time' class LaneFactory - def self.competing_lane(event_ids: [], comment: '', admin_comment: '', registration_status: 'pending') + # TODO: try again to set waiting_list_position form the tests, instead of directly in the lane factory + def self.competing_lane(event_ids: [], comment: '', admin_comment: '', registration_status: 'pending', waiting_list_position: nil) competing_lane = Lane.new({}) competing_lane.lane_name = 'competing' competing_lane.completed_steps = ['Event Registration'] @@ -11,6 +12,7 @@ def self.competing_lane(event_ids: [], comment: '', admin_comment: '', registrat 'event_details' => event_ids.map { |event_id| { event_id: event_id, event_registration_state: registration_status } }, 'comment' => comment, 'admin_comment' => admin_comment, + 'waiting_list_position' => waiting_list_position.to_i, } competing_lane end diff --git a/app/models/lane.rb b/app/models/lane.rb deleted file mode 100644 index e87ebe8f..00000000 --- a/app/models/lane.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true - -class Lane - attr_accessor :lane_name, :lane_state, :completed_steps, :lane_details - - EVENT_IDS = %w[333 222 444 555 666 777 333bf 333oh clock minx pyram skewb sq1 444bf 555bf 333mbf 333fm].freeze - - def initialize(args) - @lane_name = args['lane_name'] - @lane_state = args['lane_state'] || 'waiting' - @completed_steps = args['completed_steps'] || [] - @lane_details = args['lane_details'] || {} - end - - def dynamoid_dump - self.to_json - end - - def ==(other) - @lane_name == other.lane_name && @lane_state == other.lane_state && @completed_steps == other.completed_steps && @lane_details == other.lane_details - end - - def self.dynamoid_load(serialized_str) - parsed = JSON.parse serialized_str - Lane.new(parsed) - end - - def update_events(new_event_ids) - if @lane_name == 'competing' - current_event_ids = @lane_details['event_details'].pluck('event_id') - - # Update events list with new events - new_event_ids.each do |id| - next if current_event_ids.include?(id) - new_details = { - 'event_id' => id, - # NOTE: Currently event_registration_state is not used - when per-event registrations are added, we need to add validation logic to support cases like - # limited registrations and waiting lists for certain events - 'event_registration_state' => @lane_state, - } - @lane_details['event_details'] << new_details - end - - # Remove events not in the new events list - @lane_details['event_details'].delete_if do |event| - !(new_event_ids.include?(event['event_id'])) - end - end - end -end diff --git a/app/models/registration.rb b/app/models/registration.rb index c9c4ad8d..2b6c8d51 100644 --- a/app/models/registration.rb +++ b/app/models/registration.rb @@ -1,8 +1,10 @@ # frozen_string_literal: true -require_relative 'lane' require_relative '../lib/redis_helper' require 'time' +# Requiring even though it's in lib because the work needs to find it too +require_relative '../../lib/lane' + class Registration include Dynamoid::Document @@ -13,13 +15,23 @@ class Registration ADMIN_ONLY_STATES = %w[pending waiting_list accepted].freeze # Only admins are allowed to change registration state to one of these states # Pre-validations - before_validation :set_is_competing + before_validation :set_competing_status # Validations - validate :is_competing_consistency + validate :competing_status_consistency def self.accepted_competitors(competition_id) - where(competition_id: competition_id, is_competing: true).count + where(competition_id: competition_id, competing_status: 'accepted').count + end + + def self.get_registrations_by_status(competition_id, status) + result = Rails.cache.fetch("#{competition_id}-#{status}_registrations", expires_in: 60.minutes) do + Registration.where(competition_id: competition_id, competing_status: status).to_a + end + unless result.is_a? Array + return [] + end + result end def self.accepted_competitors_count(competition_id) @@ -49,7 +61,10 @@ def attendee_id "#{competition_id}-#{user_id}" end - # Returns id's of the events with a non-cancelled state + def competing_lane + lanes.find { |x| x.lane_name == 'competing' } + end + def registered_event_ids event_ids = [] @@ -68,8 +83,8 @@ def event_details competing_lane.lane_details['event_details'] end - def competing_status - lanes&.filter_map { |x| x.lane_state if x.lane_name == 'competing' }&.first + def competing_waiting_list_position + competing_lane.lane_details['waiting_list_position'] end def competing_comment @@ -96,11 +111,27 @@ def payment_history lanes.filter_map { |x| x.lane_details['payment_history'] if x.lane_name == 'payment' }[0] end + def update_competing_waiting_list_position(new_position) + updated_lanes = lanes.map do |lane| + if lane.lane_name == 'competing' + lane.lane_details['waiting_list_position'] = new_position + end + lane + end + update_attributes!(lanes: updated_lanes, competing_status: competing_lane.lane_state, guests: guests) + end + def update_competing_lane!(update_params) + has_waiting_list_changed = waiting_list_changed?(update_params) + updated_lanes = lanes.map do |lane| if lane.lane_name == 'competing' # Update status for lane and events + if has_waiting_list_changed + update_waiting_list(lane, update_params) + end + if update_params[:status].present? lane.lane_state = update_params[:status] @@ -113,6 +144,7 @@ def update_competing_lane!(update_params) lane.lane_details['comment'] = update_params[:comment] if update_params[:comment].present? lane.lane_details['admin_comment'] = update_params[:admin_comment] if update_params[:admin_comment].present? + if update_params[:event_ids].present? && update_params[:status] != 'cancelled' lane.update_events(update_params[:event_ids]) end @@ -120,17 +152,19 @@ def update_competing_lane!(update_params) lane end # TODO: In the future we will need to check if any of the other lanes have a status set to accepted - updated_is_competing = if update_params[:status].present? - update_params[:status] == 'accepted' - else - is_competing - end updated_guests = if update_params[:guests].present? update_params[:guests] else guests end - update_attributes!(lanes: updated_lanes, is_competing: updated_is_competing, guests: updated_guests) # TODO: Apparently update_attributes is deprecated in favor of update! - should we change? + updated_values = update_attributes!(lanes: updated_lanes, competing_status: competing_lane.lane_state, guests: updated_guests) + if has_waiting_list_changed + # Update waiting list caches + Rails.cache.delete("#{competition_id}-waiting_list_registrations") + Rails.cache.delete("#{competition_id}-waiting_list_boundaries") + competing_lane.get_waiting_list_boundaries(competition_id) + end + updated_values end def init_payment_lane(amount, currency_code, id) @@ -162,11 +196,20 @@ def update_payment_lane(id, iso_amount, currency_iso, status) update_attributes!(lanes: updated_lanes) end + def update_waiting_list(lane, update_params) + update_params[:waiting_list_position]&.to_i + lane.add_to_waiting_list(competition_id) if update_params[:status] == 'waiting_list' + lane.accept_from_waiting_list if update_params[:status] == 'accepted' + lane.remove_from_waiting_list(competition_id) if update_params[:status] == 'cancelled' || update_params[:status] == 'pending' + lane.move_within_waiting_list(competition_id, update_params[:waiting_list_position].to_i) if + update_params[:waiting_list_position].present? && update_params[:waiting_list_position] != competing_waiting_list_position + end + # Fields field :user_id, :string field :guests, :integer field :competition_id, :string - field :is_competing, :boolean + field :competing_status, :string field :hide_name_publicly, :boolean field :lanes, :array, of: Lane @@ -175,15 +218,26 @@ def update_payment_lane(id, iso_amount, currency_iso, status) private - def set_is_competing - self.is_competing = true if competing_status == 'accepted' + def set_competing_status + self.competing_status = lanes&.filter_map { |x| x.lane_state if x.lane_name == 'competing' }&.first end - def is_competing_consistency - if is_competing - errors.add(:is_competing, 'cant be true unless competing_status is accepted') unless competing_status == 'accepted' - else - errors.add(:is_competing, 'must be true if competing_status is accepted') if competing_status == 'accepted' - end + def competing_status_consistency + errors.add(:competing_status, '') unless competing_status == lanes&.filter_map { |x| x.lane_state if x.lane_name == 'competing' }&.first + end + + def waiting_list_changed?(update_params) + waiting_list_position_changed?(update_params) || waiting_list_status_changed?(update_params) + end + + def waiting_list_position_changed?(update_params) + return false unless update_params[:waiting_list_position].present? + update_params[:waiting_list_position] != competing_waiting_list_position + end + + def waiting_list_status_changed?(update_params) + lane_state_present = competing_status == 'waiting_list' || update_params[:status] == 'waiting_list' + states_are_different = competing_status != update_params[:status] + lane_state_present && states_are_different end end diff --git a/app/services/registration_checker.rb b/app/services/registration_checker.rb index 0c370470..ef45ff41 100644 --- a/app/services/registration_checker.rb +++ b/app/services/registration_checker.rb @@ -27,6 +27,7 @@ def self.update_registration_allowed!(update_request, competition_info, requesti validate_comment! validate_organizer_fields! validate_organizer_comment! + validate_waiting_list_position! validate_update_status! validate_update_events! end @@ -68,7 +69,7 @@ def validate_create_events! def validate_guests! return unless @request.key?('guests') - raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_REQUEST_DATA) if @request['guests'] < 0 + raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_REQUEST_DATA) if @request['guests'].to_i < 0 raise RegistrationError.new(:unprocessable_entity, ErrorCodes::GUEST_LIMIT_EXCEEDED) if @competition_info.guest_limit_exceeded?(@request['guests']) end @@ -86,7 +87,7 @@ def validate_comment! end def validate_organizer_fields! - @organizer_fields = ['organizer_comment'] + @organizer_fields = ['organizer_comment', 'waiting_list_position'] raise RegistrationError.new(:unauthorized, ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) if contains_organizer_fields? && !@competition_info.is_organizer_or_delegate?(@requester_user_id) end @@ -97,6 +98,25 @@ def validate_organizer_comment! !organizer_comment.nil? && organizer_comment.length > COMMENT_CHARACTER_LIMIT end + def validate_waiting_list_position! + return if (waiting_list_position = @request.dig('competing', 'waiting_list_position')).nil? + + # Floats are not allowed + raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_WAITING_LIST_POSITION) if waiting_list_position.is_a? Float + + # We convert strings to integers and then check if they are an integer + converted_position = Integer(waiting_list_position, exception: false) + raise RegistrationError.new(:unprocessable_entity, ErrorCodes::INVALID_WAITING_LIST_POSITION) unless converted_position.is_a? Integer + + boundaries = @registration.competing_lane.get_waiting_list_boundaries(@competition_info.competition_id) + if boundaries['waiting_list_position_min'].nil? && boundaries['waiting_list_position_max'].nil? + raise RegistrationError.new(:forbidden, ErrorCodes::INVALID_WAITING_LIST_POSITION) if converted_position != 1 + else + raise RegistrationError.new(:forbidden, ErrorCodes::INVALID_WAITING_LIST_POSITION) if + converted_position < boundaries['waiting_list_position_min'] || converted_position > boundaries['waiting_list_position_max'] + end + end + def contains_organizer_fields? @request['competing']&.keys&.any? { |key| @organizer_fields.include?(key) } end @@ -109,7 +129,12 @@ def validate_update_status! raise RegistrationError.new(:forbidden, ErrorCodes::COMPETITOR_LIMIT_REACHED) if new_status == 'accepted' && Registration.accepted_competitors_count(@competition_info.competition_id) == @competition_info.competitor_limit - # Organizers can make any status change they want to - no checks performed + # Organizers cant accept someone from the waiting list who isn't in the leading position + min_waiting_list_position = @registration.competing_lane.get_waiting_list_boundaries(@registration.competition_id)['waiting_list_position_min'] + raise RegistrationError.new(:forbidden, ErrorCodes::MUST_ACCEPT_WAITING_LIST_LEADER) if + current_status == 'waiting_list' && new_status == 'accepted' && @registration.competing_waiting_list_position != min_waiting_list_position + + # Otherwise, organizers can make any status change they want to return if @competition_info.is_organizer_or_delegate?(@requester_user_id) # A user (ie not an organizer) is only allowed to: diff --git a/config/routes.rb b/config/routes.rb index 3814c646..bed1ba4b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -14,8 +14,10 @@ get '/api/v1/register', to: 'registration#show' post '/api/v1/register', to: 'registration#create' patch '/api/v1/register', to: 'registration#update' - get '/api/v1/registrations/:competition_id/admin', to: 'registration#list_admin' + get '/api/v1/registrations/mine', to: 'registration#mine' get '/api/v1/registrations/:competition_id', to: 'registration#list' + get '/api/v1/registrations/:competition_id/admin', to: 'registration#list_admin' + get '/api/v1/registrations/:competition_id/waiting', to: 'registration#list_waiting' get '/api/v1/:competition_id/payment', to: 'registration#payment_ticket' post '/api/v1/:competition_id/import', to: 'registration#import' end diff --git a/docker-compose.backend-test.yml b/docker-compose.backend-test.yml index 887b7c0d..b40240cd 100644 --- a/docker-compose.backend-test.yml +++ b/docker-compose.backend-test.yml @@ -9,6 +9,7 @@ services: environment: LOCALSTACK_ENDPOINT: "http://localstack:4566" PROMETHEUS_EXPORTER: "prometheus_exporter" + REDIS_URL: "redis://redis:6379" RAILS_ENV: test volumes: - .:/app @@ -16,12 +17,13 @@ services: tty: true command: > bash -c 'bundle install && yarn install && bin/rake db:seed && - RAILS_ENV=test bundle exec rspec' + RAILS_ENV=test bundle exec rspec --exclude-pattern spec/cache' networks: - wca-registration depends_on: - localstack - prometheus_exporter + - redis prometheus_exporter: @@ -76,11 +78,26 @@ services: networks: - wca-registration + # Use redis for tests that enable redis + redis: + container_name: redis + image: redis:latest + ports: + - '6379:6379' + volumes: + - cache:/data + networks: + - wca-registration + volumes: gems_volume_handler: driver: local gems_volume_worker: driver: local + cache: + driver: local + + networks: wca-registration: diff --git a/docs/example_payloads/registration.json b/docs/example_payloads/registration.json deleted file mode 100644 index 9bc24c36..00000000 --- a/docs/example_payloads/registration.json +++ /dev/null @@ -1,38 +0,0 @@ -{ - "attendee_id":"CubingZANationalChampionship2023-158816", - "competition_id":"CubingZANationalChampionship2023", - "user_id":"158816", - "is_attending":"True", - "lanes":[{ - "lane_name":"competitor", - "lane_state":"Accepted", - "completed_steps":[1,2,3], - "lane_details":{ - "event_details":[ - { - "event_id":"333", - "event_cost":"5", - "event_cost_currency":"$", - "event_registration_state":"accepted" - }, - { - "event_id":"333MBF", - "event_cost":"10.5", - "event_cost_currency":"$", - "event_registration_state":"waiting-list" - }, - ], - "custom_data": {} - }, - "payment_reference":"PI-1235231", - "payment_amount":"10", - "transaction_currency":"$", - "discount_percentage":"0", - "discount_amount":"0", - "last_action":"created", - "last_action_datetime":"2023-01-01T00:00:00Z", - "last_action_user":"158816" - } - ], - "hide_name_publicly":"False" -} diff --git a/docs/example_payloads/registrations.json b/docs/example_payloads/registrations.json deleted file mode 100644 index e69de29b..00000000 diff --git a/lib/lane.rb b/lib/lane.rb new file mode 100644 index 00000000..d7894519 --- /dev/null +++ b/lib/lane.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +class Lane + attr_accessor :lane_name, :lane_state, :completed_steps, :lane_details + + EVENT_IDS = %w[333 222 444 555 666 777 333bf 333oh clock minx pyram skewb sq1 444bf 555bf 333mbf 333fm].freeze + + def initialize(args) + @lane_name = args['lane_name'] + @lane_state = args['lane_state'] || 'waiting' + @completed_steps = args['completed_steps'] || [] + @lane_details = args['lane_details'] || {} + end + + def dynamoid_dump + self.to_json + end + + def ==(other) + @lane_name == other.lane_name && @lane_state == other.lane_state && @completed_steps == other.completed_steps && @lane_details == other.lane_details + end + + def self.dynamoid_load(serialized_str) + parsed = JSON.parse serialized_str + Lane.new(parsed) + end + + def move_within_waiting_list(competition_id, new_position) + if new_position < lane_details['waiting_list_position'] + cascade_waiting_list(competition_id, new_position, lane_details['waiting_list_position']+1) + else + cascade_waiting_list(competition_id, lane_details['waiting_list_position'], new_position+1, -1) + end + lane_details['waiting_list_position'] = new_position + end + + def add_to_waiting_list(competition_id) + boundaries = get_waiting_list_boundaries(competition_id) + waiting_list_max = boundaries['waiting_list_position_max'] + waiting_list_min = boundaries['waiting_list_position_min'] + + if waiting_list_max.nil? && waiting_list_min.nil? + lane_details['waiting_list_position'] = 1 + else + lane_details['waiting_list_position'] = waiting_list_max+1 + end + end + + def remove_from_waiting_list(competition_id) + max_position = get_waiting_list_boundaries(competition_id)['waiting_list_position_max'] + cascade_waiting_list(competition_id, lane_details['waiting_list_position'], max_position+1, -1) + lane_details['waiting_list_position'] = nil + end + + def accept_from_waiting_list + lane_details['waiting_list_position'] = nil + end + + def get_waiting_list_boundaries(competition_id) + Rails.cache.fetch("#{competition_id}-waiting_list_boundaries", expires_in: 60.minutes) do + waiting_list_registrations = Registration.get_registrations_by_status(competition_id, 'waiting_list') + + # We aren't just counting the number of registrations in the waiting list. When a registration is + # accepted from the waiting list, we don't "move up" the waiting_list_position of the registrations + # behind it - so we can't assume that the position 1 is the min, or that the count of waiting_list + # registrations is the max. + + waiting_list_positions = waiting_list_registrations.map(&:competing_waiting_list_position).compact + + if waiting_list_positions.any? + waiting_list_position_min, waiting_list_position_max = waiting_list_positions.minmax + else + waiting_list_position_min = waiting_list_position_max = nil + end + + { 'waiting_list_position_min' => waiting_list_position_min, 'waiting_list_position_max' => waiting_list_position_max } + end + end + + def update_events(new_event_ids) + if @lane_name == 'competing' + current_event_ids = @lane_details['event_details'].pluck('event_id') + + # Update events list with new events + new_event_ids.each do |id| + next if current_event_ids.include?(id) + new_details = { + 'event_id' => id, + # NOTE: Currently event_registration_state is not used - when per-event registrations are added, we need to add validation logic to support cases like + # limited registrations and waiting lists for certain events + 'event_registration_state' => @lane_state, + } + @lane_details['event_details'] << new_details + end + + # Remove events not in the new events list + @lane_details['event_details'].delete_if do |event| + !(new_event_ids.include?(event['event_id'])) + end + end + end + + private + + # Used for propagating a change in waiting_list_position to all affected registrations + # increment_value is the value by which position should be shifted - usually 1 or -1 + # Lower waiting_list_position = higher up the waiting list (1 on waiting list will be accepted before 10) + def cascade_waiting_list(competition_id, start_at, stop_at, increment_value = 1) + waiting_list_registrations = Registration.get_registrations_by_status(competition_id, 'waiting_list') + + waiting_list_registrations.each do |reg| + current_position = reg.competing_waiting_list_position.to_i + if current_position >= start_at && current_position < stop_at + reg.update_competing_waiting_list_position(current_position + increment_value) + end + end + end +end diff --git a/spec/factories/registration_factory.rb b/spec/factories/registration_factory.rb index e06a7261..98187d90 100644 --- a/spec/factories/registration_factory.rb +++ b/spec/factories/registration_factory.rb @@ -9,13 +9,22 @@ comment { '' } guests { 0 } registration_status { 'incoming' } - organizer_comment { nil } + organizer_comment { '' } + waiting_list_position { nil } end user_id { rand(100000..200000).to_s } competition_id { 'CubingZANationalChampionship2023' } attendee_id { "#{competition_id}-#{user_id}" } - lanes { [LaneFactory.competing_lane(event_ids: events, comment: comment, registration_status: registration_status)] } + lanes { + [LaneFactory.competing_lane( + event_ids: events, + comment: comment, + registration_status: registration_status, + admin_comment: organizer_comment, + waiting_list_position: waiting_list_position, + )] + } end trait :admin do @@ -28,12 +37,4 @@ factory :organizer_registration, traits: [:organizer] factory :admin_registration, traits: [:admin] - - after(:create) do |registration, evaluator| - unless evaluator.organizer_comment.nil? - attendee_id = "#{evaluator.competition_id}-#{evaluator.user_id}" - registration = Registration.find(attendee_id) - registration.update_competing_lane!({ organizer_comment: evaluator.organizer_comment }) - end - end end diff --git a/spec/models/registration_spec.rb b/spec/models/registration_spec.rb index e38a75d6..30221eef 100644 --- a/spec/models/registration_spec.rb +++ b/spec/models/registration_spec.rb @@ -3,18 +3,18 @@ require 'rails_helper' describe Registration do - describe 'validations#is_competing_consistency' do - it 'passes if is_competing is true and status is accepted' do - registration = FactoryBot.build(:registration, registration_status: 'accepted', is_competing: true) + describe 'validations#competing_status_consistency' do + it 'passes if competing_status and competing lane status match' do + registration = FactoryBot.build(:registration, registration_status: 'accepted') expect(registration).to be_valid end - it 'adds an error if is_competing is true and status is not accepted' do - registration = FactoryBot.build(:registration, registration_status: 'pending', is_competing: true) + it 'incorrect competing status get corrected when validation is run' do + registration = FactoryBot.build(:registration, registration_status: 'pending') + registration.competing_status = 'accepted' - expect(registration).not_to be_valid - expect(registration.errors[:is_competing]).to include('cant be true unless competing_status is accepted') + expect(registration).to be_valid end end @@ -25,28 +25,285 @@ expect(registration.competing_status).to eq('accepted') end - it 'given accepted status, it sets is_competing to true' do - registration = FactoryBot.create(:registration, registration_status: 'pending') - registration.update_competing_lane!({ status: 'accepted' }) - expect(registration.is_competing).to eq(true) - end - - it 'accepted given cancelled, it sets is_competing to false' do + it 'accepted given cancelled, it sets competing_status accordingly' do registration = FactoryBot.create(:registration, registration_status: 'accepted') registration.update_competing_lane!({ status: 'cancelled' }) - expect(registration.is_competing).to eq(false) + expect(registration.competing_status).to eq('cancelled') end - it 'accepted given pending, it sets is_competing to false' do + it 'accepted given pending, it sets competing_status accordingly' do registration = FactoryBot.create(:registration, registration_status: 'accepted') registration.update_competing_lane!({ status: 'pending' }) - expect(registration.is_competing).to eq(false) + expect(registration.competing_status).to eq('pending') end - it 'accepted given waiting_list, it sets is_competing to false' do + it 'accepted given waiting_list, it sets competing_status' do registration = FactoryBot.create(:registration, registration_status: 'accepted') registration.update_competing_lane!({ status: 'waiting_list' }) - expect(registration.is_competing).to eq(false) + expect(registration.competing_status).to eq('waiting_list') + end + end + + describe '#update_competing_lane!.waiting_list' do + # TODO: Needs more logic to test whether the logic paths for update_waiting_list (status are same, not change in waiting list position, etc) + + describe '#waiting_list.get_waiting_list_boundaries' do + it 'both are nil when there are no other registrations' do + registration = FactoryBot.create(:registration, registration_status: 'pending') + + boundaries = registration.competing_lane.get_waiting_list_boundaries(registration.competition_id) + expect(boundaries['waiting_list_position_min']).to eq(nil) + expect(boundaries['waiting_list_position_max']).to eq(nil) + end + + it 'both are 1 when there is only one registrations' do + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + + boundaries = registration.competing_lane.get_waiting_list_boundaries(registration.competition_id) + expect(boundaries['waiting_list_position_min']).to eq(1) + expect(boundaries['waiting_list_position_max']).to eq(1) + end + + it 'equal to lowest and highest position when there are multiple registrations' do + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 2) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 3) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 4) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 5) + + boundaries = registration.competing_lane.get_waiting_list_boundaries(registration.competition_id) + expect(boundaries['waiting_list_position_min']).to eq(1) + expect(boundaries['waiting_list_position_max']).to eq(5) + end + end + + describe '#waiting_list.add_to_waiting_list' do + it 'first competitor in the waiting list gets set to position 1' do + registration = FactoryBot.create(:registration, registration_status: 'pending') + registration.update_competing_lane!({ status: 'waiting_list' }) + expect(registration.competing_waiting_list_position).to eq(1) + end + + it 'second competitor gets set to position 2' do + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + registration = FactoryBot.create(:registration, registration_status: 'pending') + registration.update_competing_lane!({ status: 'waiting_list' }) + expect(registration.competing_waiting_list_position).to eq(2) + end + end + + describe '#waiting_list.caching_tests' do + let(:redis_store) { ActiveSupport::Cache.lookup_store(:redis_cache_store, { url: EnvConfig.REDIS_URL }) } + let(:cache) { Rails.cache } + + before do + allow(Rails).to receive(:cache).and_return(redis_store) + Rails.cache.clear + end + + describe '#waiting_list.cached_waiting_list_boundaries' do + it 'both are nil when there are no other registrations' do + registration = FactoryBot.create(:registration, registration_status: 'pending') + registration.competing_lane.get_waiting_list_boundaries(registration.competition_id) + + boundaries = Rails.cache.read("#{registration.competition_id}-waiting_list_boundaries") + expect(boundaries['waiting_list_position_min']).to eq(nil) + expect(boundaries['waiting_list_position_max']).to eq(nil) + end + + it 'both are 1 when there is only one registrations' do + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + registration.competing_lane.get_waiting_list_boundaries(registration.competition_id) + + boundaries = Rails.cache.read("#{registration.competition_id}-waiting_list_boundaries") + expect(boundaries['waiting_list_position_min']).to eq(1) + expect(boundaries['waiting_list_position_max']).to eq(1) + end + + it 'equal to lowest and highest position when there are multiple registrations' do + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 2) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 3) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 4) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 5) + registration.competing_lane.get_waiting_list_boundaries(registration.competition_id) + + boundaries = Rails.cache.read("#{registration.competition_id}-waiting_list_boundaries") + expect(boundaries['waiting_list_position_min']).to eq(1) + expect(boundaries['waiting_list_position_max']).to eq(5) + end + end + + describe '#waiting_list.verify_caches_after_updates' do + it 'is empty, then has 1 competitor after theyre added to waiting list' do + registration = FactoryBot.create(:registration, registration_status: 'pending') + + waiting_list_registrations = Rails.cache.read("#{registration.competition_id}-waiting_list_registrations") + expect(waiting_list_registrations).to eq(nil) + + registration.update_competing_lane!({ status: 'waiting_list' }) + + waiting_list_registrations = Rails.cache.read("#{registration.competition_id}-waiting_list_registrations") + expect(waiting_list_registrations.count).to eq(1) + end + + it 'two competitors yields an array of 2 stored in cache' do + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + registration = FactoryBot.create(:registration, registration_status: 'pending') + registration.update_competing_lane!({ status: 'waiting_list' }) + + waiting_list_registrations = Rails.cache.read("#{registration.competition_id}-waiting_list_registrations") + expect(waiting_list_registrations.count).to eq(2) + end + + it 'waiting list boundaries are updated after update_competing_lane is called' do + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + registration = FactoryBot.create(:registration, registration_status: 'pending') + registration.update_competing_lane!({ status: 'waiting_list' }) + + boundaries = Rails.cache.read("#{registration.competition_id}-waiting_list_boundaries") + expect(boundaries['waiting_list_position_min']).to eq(1) + expect(boundaries['waiting_list_position_max']).to eq(2) + end + end + end + + describe '#waiting_list.move_within_waiting_list' do + it 'gets moved to the correct position' do + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 2) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 3) + registration_4 = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 4) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 5) + + registration_4.update_competing_lane!({ waiting_list_position: '2' }) + registration_4.reload + + expect(registration_4.competing_waiting_list_position).to eq(2) + end + + it 'when moved forward on the list, everything between its new and old position gets moved back' do + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + registration_2 = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 2) + registration_3 = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 3) + registration_4 = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 4) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 5) + + registration_4.update_competing_lane!({ waiting_list_position: '2' }) + registration_4.reload + + registration_2.reload + registration_3.reload + + expect(registration_2.competing_waiting_list_position).to eq(3) + expect(registration_3.competing_waiting_list_position).to eq(4) + end + + it 'when moved forward, nothing outside the affected range changes' do + registration_1 = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 2) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 3) + registration_4 = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 4) + registration_5 = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 5) + + registration_4.update_competing_lane!({ waiting_list_position: 2 }) + registration_4.reload + + registration_1.reload + registration_5.reload + + expect(registration_1.competing_waiting_list_position).to eq(1) + expect(registration_5.competing_waiting_list_position).to eq(5) + end + + it 'if moved backward, everything, everything between new and old position gets moved forward' do + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + registration_2 = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 2) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 3) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 4) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 5) + + registration_2.update_competing_lane!({ waiting_list_position: 4 }) + registration_2.reload + + expect(registration_2.competing_waiting_list_position).to eq(4) + end + + it 'if moved backward, everything in front of its old position doesnt change' do + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + registration_2 = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 2) + registration_3 = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 3) + registration_4 = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 4) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 5) + + registration_2.update_competing_lane!({ waiting_list_position: 4 }) + registration_2.reload + + registration_3.reload + registration_4.reload + + expect(registration_3.competing_waiting_list_position).to eq(2) + expect(registration_4.competing_waiting_list_position).to eq(3) + end + end + + describe '#waiting_list.accept_from_waiting_list' do + it 'when accepted, waiting_list_position gets set to nil' do + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + registration.update_competing_lane!({ status: 'accepted' }) + expect(registration.competing_waiting_list_position).to eq(nil) + end + + it 'if waiting list is empty, new min/max should be nil' do + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + registration.update_competing_lane!({ status: 'accepted' }) + waiting_list_boundaries = registration.competing_lane.get_waiting_list_boundaries(registration.competition_id) + expect(waiting_list_boundaries['waiting_list_position_min']).to eq(nil) + expect(waiting_list_boundaries['waiting_list_position_max']).to eq(nil) + end + + it 'if waiting list isnt empty, new min should be one greater than the accepted registrations old waiting list position' do + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 2) + registration.update_competing_lane!({ status: 'accepted' }) + waiting_list_boundaries = registration.competing_lane.get_waiting_list_boundaries(registration.competition_id) + expect(waiting_list_boundaries['waiting_list_position_min']).to eq(2) + expect(waiting_list_boundaries['waiting_list_position_max']).to eq(2) + end + end + + describe '#waiting_list.remove_from_waiting_list' do + it 'change from waiting_list to cancelled' do + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + registration.update_competing_lane!({ status: 'cancelled' }) + expect(registration.competing_status).to eq('cancelled') + end + + it 'change from waiting_list to pending' do + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + registration.update_competing_lane!({ status: 'pending' }) + expect(registration.competing_status).to eq('pending') + end + + it 'removing from waiting list changes waiting_list_position to nil' do + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + registration.update_competing_lane!({ status: 'pending' }) + expect(registration.competing_waiting_list_position).to eq(nil) + end + + it 'all registrations behind removed registration decrement their waiting_list_position' do + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + registration_2 = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 2) + registration_3 = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 3) + + registration.update_competing_lane!({ status: 'pending' }) + + registration_2.reload + registration_3.reload + + expect(registration_2.competing_waiting_list_position).to eq(1) + expect(registration_3.competing_waiting_list_position).to eq(2) + end end end @@ -76,12 +333,12 @@ describe '#set_is_competing' do it 'persists a true state to the database' do registration = FactoryBot.create(:registration, registration_status: 'accepted') - expect(Registration.find(registration.attendee_id).is_competing).to eq(true) + expect(Registration.find(registration.attendee_id).competing_status).to eq('accepted') end it 'persists a false state to the database' do registration = FactoryBot.create(:registration, registration_status: 'pending') - expect(Registration.find(registration.attendee_id).is_competing).to eq(nil) + expect(Registration.find(registration.attendee_id).competing_status).to eq('pending') end end end diff --git a/spec/services/registration_checker_spec.rb b/spec/services/registration_checker_spec.rb index 52e22463..4f022c58 100644 --- a/spec/services/registration_checker_spec.rb +++ b/spec/services/registration_checker_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' require_relative '../../app/helpers/competition_api' -# Add a test where one comp has a lot of competitors and another doesnt but you can still accept, to ensure that we're checking the reg count +# TODO: Add a test where one comp has a lot of competitors and another doesnt but you can still accept, to ensure that we're checking the reg count # for the COMPETITION, not all registrations RSpec.shared_examples 'invalid user status updates' do |old_status, new_status| @@ -527,6 +527,19 @@ expect(error.error).to eq(ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) end end + + it 'user cant submit waiting_list_position' do + registration = FactoryBot.create(:registration) + competition_info = CompetitionInfo.new(FactoryBot.build(:competition)) + update_request = FactoryBot.build(:update_request, user_id: registration[:user_id], competing: { 'waiting_list_position' => '1' }) + + expect { + RegistrationChecker.update_registration_allowed!(update_request, competition_info, update_request['submitted_by']) + }.to raise_error(RegistrationError) do |error| + expect(error.http_status).to eq(:unauthorized) + expect(error.error).to eq(ErrorCodes::USER_INSUFFICIENT_PERMISSIONS) + end + end end describe '#update_registration_allowed!.validate_organizer_comment!' do @@ -932,4 +945,92 @@ end end end + + describe '#update_registration_allowed!.validate_waiting_list_position!' do + it 'must be an integer, not string' do + registration = FactoryBot.create(:registration) + competition_info = CompetitionInfo.new(FactoryBot.build(:competition)) + update_request = FactoryBot.build(:update_request, :organizer_for_user, user_id: registration[:user_id], competing: { 'waiting_list_position' => 'b' }) + + expect { + RegistrationChecker.update_registration_allowed!(update_request, competition_info, update_request['submitted_by']) + }.to raise_error(RegistrationError) do |error| + expect(error.http_status).to eq(:unprocessable_entity) + expect(error.error).to eq(ErrorCodes::INVALID_WAITING_LIST_POSITION) + end + end + + it 'can be an integer given as a string' do + registration = FactoryBot.create(:registration) + competition_info = CompetitionInfo.new(FactoryBot.build(:competition)) + update_request = FactoryBot.build(:update_request, :organizer_for_user, user_id: registration[:user_id], competing: { 'waiting_list_position' => '1' }) + + expect { + RegistrationChecker.update_registration_allowed!(update_request, competition_info, update_request['submitted_by']) + }.not_to raise_error + end + + it 'must be an integer, not float' do + registration = FactoryBot.create(:registration) + competition_info = CompetitionInfo.new(FactoryBot.build(:competition)) + update_request = FactoryBot.build(:update_request, :organizer_for_user, user_id: registration[:user_id], competing: { 'waiting_list_position' => 2.0 }) + + expect { + RegistrationChecker.update_registration_allowed!(update_request, competition_info, update_request['submitted_by']) + }.to raise_error(RegistrationError) do |error| + expect(error.http_status).to eq(:unprocessable_entity) + expect(error.error).to eq(ErrorCodes::INVALID_WAITING_LIST_POSITION) + end + end + + it 'organizer cant accept anyone except the min position on the waiting list' do + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => '1') + competition_info = CompetitionInfo.new(FactoryBot.build(:competition)) + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => '2') + update_request = FactoryBot.build(:update_request, :organizer_for_user, user_id: registration[:user_id], competing: { 'status' => 'accepted' }) + + expect { + RegistrationChecker.update_registration_allowed!(update_request, competition_info, update_request['submitted_by']) + }.to raise_error(RegistrationError) do |error| + expect(error.http_status).to eq(:forbidden) + expect(error.error).to eq(ErrorCodes::MUST_ACCEPT_WAITING_LIST_LEADER) + end + end + + it 'cannot move to less than current min position' do + competition_info = CompetitionInfo.new(FactoryBot.build(:competition)) + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 1) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 2) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 3) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 4) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 5) + + update_request = FactoryBot.build(:update_request, :organizer_for_user, user_id: registration[:user_id], competing: { 'waiting_list_position' => '10' }) + + expect { + RegistrationChecker.update_registration_allowed!(update_request, competition_info, update_request['submitted_by']) + }.to raise_error(RegistrationError) do |error| + expect(error.http_status).to eq(:forbidden) + expect(error.error).to eq(ErrorCodes::INVALID_WAITING_LIST_POSITION) + end + end + + it 'cannot move to greater than current max position' do + competition_info = CompetitionInfo.new(FactoryBot.build(:competition)) + registration = FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 6) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 2) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 3) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 4) + FactoryBot.create(:registration, registration_status: 'waiting_list', 'waiting_list_position' => 5) + + update_request = FactoryBot.build(:update_request, :organizer_for_user, user_id: registration[:user_id], competing: { 'waiting_list_position' => '1' }) + + expect { + RegistrationChecker.update_registration_allowed!(update_request, competition_info, update_request['submitted_by']) + }.to raise_error(RegistrationError) do |error| + expect(error.http_status).to eq(:forbidden) + expect(error.error).to eq(ErrorCodes::INVALID_WAITING_LIST_POSITION) + end + end + end end diff --git a/spec/support/dynamoid_reset.rb b/spec/support/dynamoid_reset.rb index 6b375931..77ff3119 100644 --- a/spec/support/dynamoid_reset.rb +++ b/spec/support/dynamoid_reset.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -raise "Tests should be run in 'test' environment only" if Rails.env != 'test' +raise "Tests should be run in 'test' environment only" unless Rails.env == 'test' || Rails.env == 'cache-test' # This is required so that the first test that is run doesn't fail due to table not being created yet - https://github.com/Dynamoid/dynamoid/issues/277 Dir[File.join(Dynamoid::Config.models_dir, '**/*.rb')].each { |file| require file }