Skip to content

Commit

Permalink
Introduce registration version column instead of uses_v2_registrations (
Browse files Browse the repository at this point in the history
#10132)

* introduce registration version column

* rubocop

* fix sanitizer

* use integer in enum

* set default value for enum and correct setting registration version

* use new column in form

* eslint

* remove last references to uses_v2_registrations

* set registration_version to v1 in factory

* Change back occurences of registration_version to less intrusive checkbox

* Further distinguish between V2 and V3 in convenience methods

---------

Co-authored-by: Gregor Billing <[email protected]>
  • Loading branch information
FinnIckler and gregorbg authored Oct 28, 2024
1 parent 56400ac commit e752244
Show file tree
Hide file tree
Showing 16 changed files with 66 additions and 40 deletions.
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

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?
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

0 comments on commit e752244

Please sign in to comment.