From 4361a66c0b5582d5faacbe31aa30d8dfdc1efe05 Mon Sep 17 00:00:00 2001 From: Daniel Nottingham Date: Wed, 3 Dec 2025 11:46:45 -0300 Subject: [PATCH 1/2] fix: Fix the N+1 query issue in EventProceduresController#index caused by repeated database queries in EventProcedures::BuildTotalAmountCents --- .../build_total_amount_cents.rb | 18 +++++++++--------- app/operations/event_procedures/list.rb | 3 ++- .../api/v1/event_procedures_request_spec.rb | 15 +++++++++++++++ spec/support/query_counter.rb | 16 ++++++++++++++++ 4 files changed, 42 insertions(+), 10 deletions(-) create mode 100644 spec/support/query_counter.rb diff --git a/app/operations/event_procedures/build_total_amount_cents.rb b/app/operations/event_procedures/build_total_amount_cents.rb index 83d79c4..04bd1e7 100644 --- a/app/operations/event_procedures/build_total_amount_cents.rb +++ b/app/operations/event_procedures/build_total_amount_cents.rb @@ -30,22 +30,22 @@ def base_amount_cents end def value_for_anesthetic_port_zero(event_procedure) - port = PortValue.find_by(cbhpm_id: event_procedure.cbhpm_id, anesthetic_port: ANESTHETIC_PORT_ZERO_AMOUNT) + port = event_procedure.cbhpm.port_values.find do |pv| + pv.anesthetic_port == ANESTHETIC_PORT_ZERO_AMOUNT + end port&.amount_cents.to_i end def find_cbhpm_procedure - CbhpmProcedure.find_by( - procedure_id: event_procedure.procedure_id, - cbhpm_id: event_procedure.cbhpm_id - ) + event_procedure.procedure.cbhpm_procedures.find do |cp| + cp.cbhpm_id == event_procedure.cbhpm_id + end end def find_port_value(cbhpm_procedure) - PortValue.find_by( - cbhpm_id: cbhpm_procedure.cbhpm_id, - anesthetic_port: cbhpm_procedure.anesthetic_port - ) + event_procedure.cbhpm.port_values.find do |pv| + pv.anesthetic_port == cbhpm_procedure.anesthetic_port + end end def apartment_multiplier diff --git a/app/operations/event_procedures/list.rb b/app/operations/event_procedures/list.rb index 094888b..5766b22 100644 --- a/app/operations/event_procedures/list.rb +++ b/app/operations/event_procedures/list.rb @@ -21,7 +21,8 @@ def filtered_query :patient, :hospital, :health_insurance, - cbhpm: :port_values + cbhpm: :port_values, + procedure: :cbhpm_procedures ) apply_all_filters(query) end diff --git a/spec/requests/api/v1/event_procedures_request_spec.rb b/spec/requests/api/v1/event_procedures_request_spec.rb index e45b827..849c294 100644 --- a/spec/requests/api/v1/event_procedures_request_spec.rb +++ b/spec/requests/api/v1/event_procedures_request_spec.rb @@ -28,6 +28,21 @@ end end + context "when checking for N+1 queries" do + it "does not have N+1 queries" do + create_list(:event_procedure, 2, user_id: user.id) + get(path, params: {}, headers: headers) # Warmup + + control_count = count_queries { get(path, params: {}, headers: headers) } + + create_list(:event_procedure, 5, user_id: user.id) + + final_count = count_queries { get(path, params: {}, headers: headers) } + + expect(final_count).to be <= control_count + end + end + context "when has filters by month" do it "returns event_procedures per month" do create_list(:event_procedure, 3, date: "2023-02-15", user_id: user.id) diff --git a/spec/support/query_counter.rb b/spec/support/query_counter.rb new file mode 100644 index 0000000..6324251 --- /dev/null +++ b/spec/support/query_counter.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module QueryCounter + def count_queries(&) + count = 0 + counter_f = lambda { |_name, _started, _finished, _unique_id, payload| + count += 1 unless payload[:name].in? %w[CACHE SCHEMA] + } + ActiveSupport::Notifications.subscribed(counter_f, "sql.active_record", &) + count + end +end + +RSpec.configure do |config| + config.include QueryCounter, type: :request +end From 28b59f79aaf50c9c637f444c17494249f0f76db5 Mon Sep 17 00:00:00 2001 From: Daniel Nottingham Date: Wed, 3 Dec 2025 12:10:55 -0300 Subject: [PATCH 2/2] refactor: creates indexed hashes for O(1) search --- .../build_total_amount_cents.rb | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/app/operations/event_procedures/build_total_amount_cents.rb b/app/operations/event_procedures/build_total_amount_cents.rb index 04bd1e7..acc927e 100644 --- a/app/operations/event_procedures/build_total_amount_cents.rb +++ b/app/operations/event_procedures/build_total_amount_cents.rb @@ -23,29 +23,30 @@ def base_amount_cents cbhpm_procedure = find_cbhpm_procedure return 0 unless cbhpm_procedure - port_value = find_port_value(cbhpm_procedure) - return value_for_anesthetic_port_zero(event_procedure) unless port_value + port_value = find_port_value(cbhpm_procedure.anesthetic_port) + return value_for_anesthetic_port_zero unless port_value port_value.amount_cents.to_i end - def value_for_anesthetic_port_zero(event_procedure) - port = event_procedure.cbhpm.port_values.find do |pv| - pv.anesthetic_port == ANESTHETIC_PORT_ZERO_AMOUNT - end - port&.amount_cents.to_i + def value_for_anesthetic_port_zero + port_values_by_anesthetic_port[ANESTHETIC_PORT_ZERO_AMOUNT]&.amount_cents.to_i end def find_cbhpm_procedure - event_procedure.procedure.cbhpm_procedures.find do |cp| - cp.cbhpm_id == event_procedure.cbhpm_id - end + cbhpm_procedures_by_cbhpm_id[event_procedure.cbhpm_id] end - def find_port_value(cbhpm_procedure) - event_procedure.cbhpm.port_values.find do |pv| - pv.anesthetic_port == cbhpm_procedure.anesthetic_port - end + def find_port_value(anesthetic_port) + port_values_by_anesthetic_port[anesthetic_port] + end + + def cbhpm_procedures_by_cbhpm_id + @cbhpm_procedures_by_cbhpm_id ||= event_procedure.procedure.cbhpm_procedures.index_by(&:cbhpm_id) + end + + def port_values_by_anesthetic_port + @port_values_by_anesthetic_port ||= event_procedure.cbhpm.port_values.index_by(&:anesthetic_port) end def apartment_multiplier