Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce registration version column instead of uses_v2_registrations #10132

Merged
merged 11 commits into from
Oct 28, 2024
1 change: 0 additions & 1 deletion app/controllers/competitions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@ def new
competitor_limit_enabled: true,
base_entry_fee_lowest_denomination: 0,
guests_entry_fee_lowest_denomination: 0,
uses_v2_registrations: true,
)

assign_editing_user(@competition)
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/registrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def export

def import
@competition = competition_from_params
if @competition.uses_new_registration_service?
if @competition.uses_microservice_registrations?
redirect_to Microservices::Registrations.registration_import_path(@competition.id)
end
end
Expand Down Expand Up @@ -210,7 +210,7 @@ def do_add
end
ActiveRecord::Base.transaction do
user, locked_account_created = user_for_registration!(params[:registration_data])
if @competition.uses_new_registration_service?
if @competition.uses_microservice_registrations?
Microservices::Registrations.add_registration(@competition.id,
user.id,
params[:registration_data][:event_ids],
Expand Down
43 changes: 23 additions & 20 deletions app/models/competition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ class Competition < ApplicationRecord
restricted: 2,
}, prefix: true

NEW_REG_SYSTEM_DEFAULT = :v2
gregorbg marked this conversation as resolved.
Show resolved Hide resolved

enum :registration_version, [:v1, :v2, :v3], prefix: true, default: NEW_REG_SYSTEM_DEFAULT

CLONEABLE_ATTRIBUTES = %w(
cityName
countryId
Expand Down Expand Up @@ -169,7 +173,7 @@ class Competition < ApplicationRecord
waiting_list_deadline_date
event_change_deadline_date
competition_series_id
uses_v2_registrations
registration_version
).freeze
VALID_NAME_RE = /\A([-&.:' [:alnum:]]+) (\d{4})\z/
VALID_ID_RE = /\A[a-zA-Z0-9]+\Z/
Expand Down Expand Up @@ -428,7 +432,7 @@ def confirmed_or_visible?
end

def registration_full?
competitor_count = uses_new_registration_service? ? Microservices::Registrations.competitor_count_by_competition(id) : registrations.accepted_and_paid_pending_count
competitor_count = uses_microservice_registrations? ? Microservices::Registrations.competitor_count_by_competition(id) : registrations.accepted_and_paid_pending_count
competitor_limit_enabled? && competitor_count >= competitor_limit
end

Expand Down Expand Up @@ -566,7 +570,7 @@ def registration_full_message

def reg_warnings
warnings = {}
warnings[:uses_v2_registrations] = I18n.t('competitions.messages.uses_v2_registrations') if uses_new_registration_service?
warnings[:uses_v2_registrations] = I18n.t('competitions.messages.uses_v2_registrations') if uses_new_registration_system?
if registration_range_specified? && !registration_past?
if self.announced?
if (self.registration_open - self.announced_at) < REGISTRATION_OPENING_EARLIEST
Expand Down Expand Up @@ -619,7 +623,7 @@ def user_can_pre_register?(user)
# We allow pre-registration at any time for the old registration system (because we have control over the foreign keys there)
# Otherwise, if it's using the new system, we only allow registrations when it's announced
# because the probability of an ID change is pretty low in that case and when it does happen, WST can handle it
system_compatible = !uses_new_registration_service? || announced?
system_compatible = !uses_microservice_registrations? || announced?

is_competition_manager && system_compatible
end
Expand All @@ -632,7 +636,6 @@ def being_cloned_from
def build_clone
Competition.new(attributes.slice(*CLONEABLE_ATTRIBUTES)).tap do |clone|
clone.being_cloned_from_id = id
clone.uses_v2_registrations = true

Competition.reflections.each_key do |association_name|
case association_name
Expand Down Expand Up @@ -738,16 +741,16 @@ def trainee_delegate_ids
@trainee_delegate_ids || trainee_delegates.map(&:id).join(",")
end

def enable_v2_registrations!
update_column :uses_v2_registrations, true
def uses_microservice_registrations?
self.registration_version_v2?
end

def uses_new_registration_service?
self.uses_v2_registrations
def uses_new_registration_system?
self.uses_microservice_registrations? || self.registration_version_v3?
gregorbg marked this conversation as resolved.
Show resolved Hide resolved
end

def should_render_register_v2?(user)
uses_new_registration_service? && user.cannot_register_for_competition_reasons(self).empty?
uses_new_registration_system? && user.cannot_register_for_competition_reasons(self).empty?
end

before_validation :unpack_delegate_organizer_ids
Expand Down Expand Up @@ -1000,7 +1003,7 @@ def registration_status
end

def any_registrations?
if uses_new_registration_service?
if uses_microservice_registrations?
Microservices::Registrations.competitor_count_by_competition(id) > 0
else
self.registrations.any?
Expand Down Expand Up @@ -1602,7 +1605,7 @@ def psych_sheet_event(event, sort_by)
raise "Unknown 'sort_by' in psych sheet computation: #{sort_by}"
end

if self.uses_new_registration_service?
if self.uses_microservice_registrations?
# We deliberately don't go through the cached `microservice_registrations` table here, because then we
# would need to separately check which of the cached registrations are accepted
# and which of those are registered for the specified event. Querying the MS directly is much more efficient.
Expand Down Expand Up @@ -1893,9 +1896,9 @@ def persons_wcif(authorized: false)
:wcif_extensions,
]
# V2 registrations store the event IDs in the microservice data, not in the monolith
includes_associations << :events unless self.uses_new_registration_service?
includes_associations << :events unless self.uses_microservice_registrations?

registrations_relation = self.uses_new_registration_service? ? self.microservice_registrations : self.registrations
registrations_relation = self.uses_microservice_registrations? ? self.microservice_registrations : self.registrations

# NOTE: we're including non-competing registrations so that they can have job
# assignments as well. These registrations don't have accepted?, but they
Expand Down Expand Up @@ -2047,13 +2050,13 @@ def set_wcif_events!(wcif_events, current_user)

# Takes an array of partial Person WCIF and updates the fields that are not immutable.
def update_persons_wcif!(wcif_persons, current_user)
registrations_relation = self.uses_new_registration_service? ? self.microservice_registrations : self.registrations
registrations_relation = self.uses_microservice_registrations? ? self.microservice_registrations : self.registrations
registration_includes = [
{ assignments: [:schedule_activity] },
:user,
:wcif_extensions,
]
registration_includes << :registration_competition_events unless self.uses_new_registration_service?
registration_includes << :registration_competition_events unless self.uses_microservice_registrations?
registrations = registrations_relation.includes(registration_includes)
competition_activities = all_activities
new_assignments = []
Expand Down Expand Up @@ -2407,7 +2410,7 @@ def to_form_data
"admin" => {
"isConfirmed" => confirmed?,
"isVisible" => showAtAll?,
"usesV2Registrations" => uses_new_registration_service?,
"usesNewRegistrationSystem" => uses_new_registration_system?,
},
"cloning" => {
"fromId" => being_cloned_from_id,
Expand Down Expand Up @@ -2614,7 +2617,7 @@ def self.form_data_to_attributes(form_data)
showAtAll: form_data.dig('admin', 'isVisible'),
being_cloned_from_id: form_data.dig('cloning', 'fromId'),
clone_tabs: form_data.dig('cloning', 'cloneTabs'),
uses_v2_registrations: form_data.dig('admin', 'usesV2Registrations'),
registration_version: form_data.dig('admin', 'usesNewRegistrationSystem') ? NEW_REG_SYSTEM_DEFAULT : :v1,
}
end

Expand Down Expand Up @@ -2677,7 +2680,7 @@ def disconnect_all_payment_integrations
end

def can_change_registration_system?
registration_not_yet_opened? && (uses_new_registration_service? || self.registrations.empty?)
registration_not_yet_opened? && (uses_microservice_registrations? || self.registrations.empty?)
end

# Our React date picker unfortunately behaves weirdly in terms of backend data
Expand Down Expand Up @@ -2846,7 +2849,7 @@ def self.form_data_json_schema
"properties" => {
"isConfirmed" => { "type" => "boolean" },
"isVisible" => { "type" => "boolean" },
"usesV2Registrations" => { "type" => "boolean" },
"usesNewRegistrationSystem" => { "type" => "boolean" },
},
},
"cloning" => {
Expand Down
4 changes: 2 additions & 2 deletions app/views/competitions/_nav.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
] : [])
}
if @competition.use_wca_registration?
pending_registrations = if @competition.uses_new_registration_service?
pending_registrations = if @competition.uses_microservice_registrations?
Microservices::Registrations.registrations_by_competition(@competition.id, 'pending', cache: true).length
else
@competition.registrations.pending.count
Expand Down Expand Up @@ -138,7 +138,7 @@
unless @competition.registration_not_yet_opened?
event_icons = @competition.events.map do |event|
{ text: event.id, path: competition_psych_sheet_event_path(@competition, event.id), cubing_icon: event.id, title: event.name }
end unless @competition.uses_new_registration_service?
end unless @competition.uses_new_registration_system?
nav_items << {
text: t('.menu.competitors'),
path: competition_registrations_path(@competition),
Expand Down
2 changes: 1 addition & 1 deletion app/views/registrations/edit_registrations.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<% provide(:title, I18n.t('registrations.list.title', comp: @competition.name)) %>

<%= render layout: 'nav' do %>
<% if @competition.uses_new_registration_service? %>
<% if @competition.uses_new_registration_system? %>
<%= react_component('RegistrationsV2/RegistrationAdministration', { competitionInfo: @competition.to_competition_info }) %>
<% else %>
<p>
Expand Down
2 changes: 1 addition & 1 deletion app/views/registrations/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<% provide(:title, I18n.t('registrations.list.title', comp: @competition.name)) %>

<%= render layout: "nav" do %>
<% if @competition.uses_new_registration_service? %>
<% if @competition.uses_new_registration_system? %>
<%= react_component('RegistrationsV2/Registrations', { competitionInfo: @competition.to_competition_info }) %>
<% else %>
<% cache [
Expand Down
2 changes: 1 addition & 1 deletion app/views/registrations/register.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<% opens_in_days = distance_of_time_in_words_to_now(@competition.registration_open) %>
<% closes_in_days = distance_of_time_in_words_to_now(@competition.registration_close) %>

<% if !@competition.uses_new_registration_service? && @competition.registration_not_yet_opened? %>
<% if !@competition.uses_new_registration_system? && @competition.registration_not_yet_opened? %>
<%= alert :info, t('registrations.will_open_html', days: opens_in_days, time: wca_local_time(@competition.registration_open)) %>
<%# Don't show this alert if we're showing the registration's details: this message is included in it! %>
<% elsif @competition.registration_past? && !@registration&.show_details?(current_user) %>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

<p>
You can approve or delete this registration
<%= link_to "here", @registration.competition.uses_new_registration_service? ?
<%= link_to "here", @registration.competition.uses_new_registration_system? ?
edit_registration_v2_url(competition_id: @registration.competition.id, user_id: @registration.user.id) :
edit_registration_url(@registration) %>.
</p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ import SubSection from '../../wca/FormBuilder/SubSection';

export default function Admin() {
const { isAdminView, isPersisted, canChangeRegistrationSystem } = useStore();

if (!isPersisted || !isAdminView) return null;

return (
<SubSection section="admin">
<InputBoolean id="isConfirmed" />
<InputBoolean id="isVisible" />
<InputBoolean id="usesV2Registrations" disabled={!canChangeRegistrationSystem} />
<InputBoolean id="usesNewRegistrationSystem" disabled={!canChangeRegistrationSystem} />
</SubSection>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export default function EventRestrictions() {
eventLimitation,
},
admin: {
usesV2Registrations,
usesNewRegistrationSystem,
},
} = useFormObject();

Expand All @@ -52,7 +52,7 @@ export default function EventRestrictions() {

return (
<SubSection section="eventRestrictions">
{ usesV2Registrations && (
{ usesNewRegistrationSystem && (
<SubSection section="forbidNewcomers">
<InputBoolean id="enabled" />
<ConditionalSection showIf={newcomers}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ import { useFormObject } from '../../wca/FormBuilder/provider/FormObjectProvider
export default function NameDetails() {
const { hasAnyRegistrations, isPersisted, isAdminView } = useStore();

const { name, admin: { usesV2Registrations } } = useFormObject();
const { name, admin: { usesNewRegistrationSystem } } = useFormObject();

const nameAlreadyShort = !name || name.length <= competitionMaxShortNameLength;
const disableIdAndShortName = !isAdminView && nameAlreadyShort;

// ID change on V1 is always possible, because we have control over the Foreign Keys.
// Otherwise, only competitions without registrations can change their ID.
const regSystemSupportsIdChange = !usesV2Registrations || !hasAnyRegistrations;
const regSystemSupportsIdChange = !usesNewRegistrationSystem || !hasAnyRegistrations;

return (
<>
Expand Down
4 changes: 2 additions & 2 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1619,7 +1619,7 @@ en:
admin:
is_confirmed: "Do not let organizers edit this competition"
is_visible: "Make competition visible to the public"
uses_v_2_registrations: "Use the new Registration System"
uses_new_registration_system: "Use the new Registration System"
competition_id: "ID"
name: "Name"
short_name: "Competition short name"
Expand Down Expand Up @@ -1700,7 +1700,7 @@ en:
admin:
is_confirmed: ""
is_visible: ""
uses_v_2_registrations: "If this is disabled, changing the registration system will cause technical issues. Please contact WST for assistance if you still need to change the registration system."
uses_new_registration_system: "If this is disabled, changing the registration system will cause technical issues. Please contact WST for assistance if you still need to change the registration system."
competition_id: ""
name: "The full name of the competition including the year at the end (e.g., Danish Open 2016). Be sure to capitalize. The name must contain only alphanumeric characters, dashes(-), ampersands(&), periods(.), colons(:), apostrophes('), and spaces( )."
short_name: "A short name for displaying. If the name of the competition is below %{short_name_limit} characters this should not be edited and should be the same as the normal name."
Expand Down
24 changes: 24 additions & 0 deletions db/migrate/20241025161404_make_registration_version_an_enum.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

class MakeRegistrationVersionAnEnum < ActiveRecord::Migration[7.2]
def up
add_column :Competitions, :registration_version, :integer, default: 0, null: false

# Update values based on the old boolean column
Competition.where(uses_v2_registrations: true).update_all(registration_version: Competition.registration_versions[:v2])

# Remove the old column
remove_column :Competitions, :uses_v2_registrations
end

def down
# Add back the original boolean column
add_column :Competitions, :uses_v2_registrations, :boolean, default: false, null: false

# Map back enum values to the boolean
Competition.where(registration_version: Competition.registration_versions[:v2]).update_all(uses_v2_registrations: true)

# Remove the enum column and rename the boolean column back
remove_column :Competitions, :registration_version
end
end
4 changes: 2 additions & 2 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.2].define(version: 2024_10_09_111904) do
ActiveRecord::Schema[7.2].define(version: 2024_10_25_161404) do
create_table "Competitions", id: { type: :string, limit: 32, default: "" }, charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t|
t.string "name", limit: 50, default: "", null: false
t.string "cityName", limit: 50, default: "", null: false
Expand Down Expand Up @@ -79,9 +79,9 @@
t.integer "events_per_registration_limit"
t.boolean "force_comment_in_registration"
t.integer "posting_by"
t.boolean "uses_v2_registrations", default: false, null: false
t.boolean "forbid_newcomers", default: false, null: false
t.string "forbid_newcomers_reason"
t.integer "registration_version", default: 0, null: false
t.index ["cancelled_at"], name: "index_Competitions_on_cancelled_at"
t.index ["countryId"], name: "index_Competitions_on_countryId"
t.index ["end_date"], name: "index_Competitions_on_end_date"
Expand Down
2 changes: 1 addition & 1 deletion lib/database_dumper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def self.actions_to_column_sanitizers(columns_by_action)
competition_series_id
use_wca_live_for_scoretaking
allow_registration_without_qualification
uses_v2_registrations
registration_version
forbid_newcomers
forbid_newcomers_reason
),
Expand Down
1 change: 1 addition & 0 deletions spec/factories/competitions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
use_wca_registration { false }
registration_open { 54.weeks.ago.change(usec: 0) }
registration_close { 1.weeks.from_now.change(usec: 0) }
registration_version { :v1 }

trait :with_valid_submitted_results do
announced
Expand Down