From db407d0f46d7e7f0a7bfed998e7f94d49dcf1d76 Mon Sep 17 00:00:00 2001 From: Gregor Billing Date: Fri, 22 Nov 2024 21:23:45 +0900 Subject: [PATCH] Optimize admin registration panel query (#10300) * Straight-forward call optimizations * Deep SQL optimizations * Reorder includes * Simplify entry_fee calculation in registration model * Adapt frontend to load PII from user model --- .../registrations/registrations_controller.rb | 9 ++- app/models/competition.rb | 2 +- app/models/registration.rb | 69 +++++++++---------- app/models/registration_history_entry.rb | 2 +- .../AdministrationTableRow.jsx | 3 +- .../RegistrationAdministrationList.jsx | 2 +- 6 files changed, 43 insertions(+), 44 deletions(-) diff --git a/app/controllers/api/v1/registrations/registrations_controller.rb b/app/controllers/api/v1/registrations/registrations_controller.rb index 10604c591e..f6591ef080 100644 --- a/app/controllers/api/v1/registrations/registrations_controller.rb +++ b/app/controllers/api/v1/registrations/registrations_controller.rb @@ -106,10 +106,13 @@ def validate_list_admin def list_admin registrations = Registration.where(competition: @competition) render json: registrations.includes( - :user, + :events, + :competition_events, + :registration_competition_events, + competition: :competition_payment_integrations, + user: :delegate_role_metadata, registration_payments: :receipt, - registration_competition_events: { competition_event: :competition }, - registration_history_entries: :registration_history_change, + registration_history_entries: :registration_history_changes, ).map { |r| r.to_v2_json(admin: true, history: true, pii: true) } end diff --git a/app/models/competition.rb b/app/models/competition.rb index e9fec108a7..b0cc6c0eb0 100644 --- a/app/models/competition.rb +++ b/app/models/competition.rb @@ -980,7 +980,7 @@ def update_receive_registration_emails end def using_payment_integrations? - competition_payment_integrations.exists? && has_fees? + competition_payment_integrations.any? && has_fees? end def can_edit_registration_fees? diff --git a/app/models/registration.rb b/app/models/registration.rb index 1b2ab34477..c851de86ea 100644 --- a/app/models/registration.rb +++ b/app/models/registration.rb @@ -173,10 +173,12 @@ def best_solve(event, type) end def entry_fee - # DEPRECATION WARNING: Rails 7.0 has deprecated Enumerable.sum in favor of Ruby's native implementation - # available since 2.4. Sum of non-numeric elements requires an initial argument. - zero_money = Money.new 0, competition.currency_code - competition.base_entry_fee + competition_events.map(&:fee).sum(zero_money) + sum_lowest_denomination = competition.base_entry_fee + competition_events.map(&:fee_lowest_denomination).sum + + Money.new( + sum_lowest_denomination, + competition.currency_code, + ) end def paid_entry_fees @@ -253,7 +255,7 @@ def saved_and_unsaved_events def add_history_entry(changes, actor_type, actor_id, action, timestamp = Time.now.utc) new_entry = registration_history_entries.create(actor_type: actor_type, actor_id: actor_id, action: action, created_at: timestamp) changes.each_key do |key| - new_entry.registration_history_change.create(value: changes[key], key: key) + new_entry.registration_history_changes.create(value: changes[key], key: key) end end @@ -294,8 +296,8 @@ def compute_competing_status end def registration_history - registration_history_entries.includes([:registration_history_change]).map do |r| - changed_attributes = r.registration_history_change.each_with_object({}) do |change, attrs| + registration_history_entries.map do |r| + changed_attributes = r.registration_history_changes.each_with_object({}) do |change, attrs| attrs[change.key] = if change.key == 'event_ids' JSON.parse(change.value) # Assuming 'event_ids' is stored as JSON array in `to` else @@ -313,8 +315,10 @@ def registration_history end def to_v2_json(admin: false, history: false, pii: false) + private_attributes = pii ? %w[dob email] : nil + base_json = { - user: user.as_json(only: %w[id wca_id name gender country_iso2], methods: %w[country]), + user: user.as_json(only: %w[id wca_id name gender country_iso2], methods: %w[country], include: [], private_attributes: private_attributes), user_id: user_id, competing: { event_ids: event_ids, @@ -322,40 +326,33 @@ def to_v2_json(admin: false, history: false, pii: false) } if admin if competition.using_payment_integrations? - base_json.merge!({ - payment: { - has_paid: outstanding_entry_fees <= 0, - payment_statuses: registration_payments.sort_by(&:created_at).reverse!.map { |p| p.payment_status }, - payment_amount_iso: paid_entry_fees.cents, - payment_amount_human_readable: "#{paid_entry_fees.format} (#{paid_entry_fees.currency.name})", - updated_at: last_payment_date, - }, - }) + base_json.deep_merge!({ + payment: { + has_paid: outstanding_entry_fees <= 0, + payment_statuses: registration_payments.sort_by(&:created_at).reverse.map(&:payment_status), + payment_amount_iso: paid_entry_fees.cents, + payment_amount_human_readable: "#{paid_entry_fees.format} (#{paid_entry_fees.currency.name})", + updated_at: last_payment_date, + }, + }) end - base_json.merge!({ - guests: guests, - competing: { - event_ids: event_ids, - registration_status: competing_status, - registered_on: created_at, - comment: comments, - admin_comment: administrative_notes, - }, - }) + base_json.deep_merge!({ + guests: guests, + competing: { + registration_status: competing_status, + registered_on: created_at, + comment: comments, + admin_comment: administrative_notes, + }, + }) if competing_status == "waiting_list" base_json[:competing][:waiting_list_position] = waiting_list_position end end if history - base_json.merge!({ - history: registration_history || [], - }) - end - if pii - base_json.merge!({ - email: user.email, - dob: user.dob, - }) + base_json.deep_merge!({ + history: registration_history, + }) end base_json end diff --git a/app/models/registration_history_entry.rb b/app/models/registration_history_entry.rb index 98eb482202..dfbf8d0d58 100644 --- a/app/models/registration_history_entry.rb +++ b/app/models/registration_history_entry.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true class RegistrationHistoryEntry < ActiveRecord::Base - has_many :registration_history_change, dependent: :destroy + has_many :registration_history_changes, dependent: :destroy belongs_to :registration end diff --git a/app/webpacker/components/RegistrationsV2/RegistrationAdministration/AdministrationTableRow.jsx b/app/webpacker/components/RegistrationsV2/RegistrationAdministration/AdministrationTableRow.jsx index 0de0cfba64..23c7ec5a46 100644 --- a/app/webpacker/components/RegistrationsV2/RegistrationAdministration/AdministrationTableRow.jsx +++ b/app/webpacker/components/RegistrationsV2/RegistrationAdministration/AdministrationTableRow.jsx @@ -71,12 +71,11 @@ export default function TableRow({ dob, region, events, comments, email, timestamp, } = columnsExpanded; const { - id, wca_id: wcaId, name, country, + id, wca_id: wcaId, name, country, dob: dateOfBirth, email: emailAddress, } = registration.user; const { registered_on: registeredOn, event_ids: eventIds, comment, admin_comment: adminComment, } = registration.competing; - const { dob: dateOfBirth, email: emailAddress } = registration; const { payment_amount_iso: paymentAmount, updated_at: updatedAt, diff --git a/app/webpacker/components/RegistrationsV2/RegistrationAdministration/RegistrationAdministrationList.jsx b/app/webpacker/components/RegistrationsV2/RegistrationAdministration/RegistrationAdministrationList.jsx index 753379897b..00b68fe83a 100644 --- a/app/webpacker/components/RegistrationsV2/RegistrationAdministration/RegistrationAdministrationList.jsx +++ b/app/webpacker/components/RegistrationsV2/RegistrationAdministration/RegistrationAdministrationList.jsx @@ -278,7 +278,7 @@ export default function RegistrationAdministrationList({ competitionInfo }) { () => Object.fromEntries( (registrations ?? []).map((registration) => [ registration.user.id, - registration.email, + registration.user.email, ]), ), [registrations],