From c0dc93f3cfe41f444d1fe29a190106d48590f0c9 Mon Sep 17 00:00:00 2001 From: Akshay Gupta Date: Thu, 17 Dec 2020 16:13:26 +0530 Subject: [PATCH] Improve the records_to_sync queries structurally to avoid sub-selects --- .../api/v3/blood_sugars_controller.rb | 2 +- app/controllers/api/v3/patients_controller.rb | 15 +++++------ .../concerns/api/v3/sync_to_user.rb | 27 +++++++++---------- app/models/appointment.rb | 4 +-- app/models/blood_pressure.rb | 4 +-- app/models/blood_sugar.rb | 5 +--- app/models/encounter.rb | 4 +-- app/models/facility.rb | 4 +++ app/models/patient.rb | 3 ++- app/models/prescription_drug.rb | 4 +-- spec/models/appointment_spec.rb | 25 ----------------- spec/models/blood_pressure_spec.rb | 25 ----------------- spec/models/blood_sugar_spec.rb | 25 ----------------- spec/models/encounter_spec.rb | 27 ------------------- spec/models/medical_history_spec.rb | 25 ----------------- spec/models/patient_spec.rb | 24 ----------------- spec/models/prescription_drug_spec.rb | 27 ------------------- 17 files changed, 30 insertions(+), 220 deletions(-) diff --git a/app/controllers/api/v3/blood_sugars_controller.rb b/app/controllers/api/v3/blood_sugars_controller.rb index ba4ab65acc..cd7f6a6d35 100644 --- a/app/controllers/api/v3/blood_sugars_controller.rb +++ b/app/controllers/api/v3/blood_sugars_controller.rb @@ -12,7 +12,7 @@ def sync_to_user private - def region_records + def model super.for_v3 end diff --git a/app/controllers/api/v3/patients_controller.rb b/app/controllers/api/v3/patients_controller.rb index cba44a368a..a7462dcdbf 100644 --- a/app/controllers/api/v3/patients_controller.rb +++ b/app/controllers/api/v3/patients_controller.rb @@ -13,22 +13,19 @@ def request_metadata {request_user_id: current_user.id, request_facility_id: current_facility.id} end - def region_records - super - .includes(:address, :phone_numbers, :business_identifiers) - end - def current_facility_records - region_records - .where(registration_facility: current_facility) + model + .where(id: current_facility.syncable_patients) .updated_on_server_since(current_facility_processed_since, limit) end def other_facility_records other_facilities_limit = limit - current_facility_records.count + other_patient_records = + current_sync_region.syncable_patients - current_facility.syncable_patients - region_records - .where.not(registration_facility: current_facility) + model + .where(id: other_patient_records) .updated_on_server_since(other_facilities_processed_since, other_facilities_limit) end diff --git a/app/controllers/concerns/api/v3/sync_to_user.rb b/app/controllers/concerns/api/v3/sync_to_user.rb index af720791dc..4bcdb3f79f 100644 --- a/app/controllers/concerns/api/v3/sync_to_user.rb +++ b/app/controllers/concerns/api/v3/sync_to_user.rb @@ -2,35 +2,32 @@ module Api::V3::SyncToUser extend ActiveSupport::Concern included do - def region_records - model = controller_name.classify.constantize - model.syncable_to_region(current_sync_region) - end - def current_facility_records - region_records - .where(patient: prioritized_patients) + model + .where(patient: current_facility.syncable_patients) .updated_on_server_since(current_facility_processed_since, limit) end def other_facility_records - other_facilities_limit = limit - current_facility_records.count + other_facilities_limit = limit - current_facility_records.size + other_patient_records = + current_sync_region.syncable_patients - current_facility.syncable_patients - region_records - .where.not(patient: prioritized_patients) + model + .where(patient: other_patient_records) .updated_on_server_since(other_facilities_processed_since, other_facilities_limit) end private - def records_to_sync - current_facility_records + other_facility_records + def model + controller_name.classify.constantize.for_sync end - def prioritized_patients - current_facility.registered_patients.with_discarded + def records_to_sync + current_facility_records + other_facility_records end - + def processed_until(records) records.last.updated_at.strftime(APIController::TIME_WITHOUT_TIMEZONE_FORMAT) if records.present? end diff --git a/app/models/appointment.rb b/app/models/appointment.rb index 509d69c0b1..7f280edb84 100644 --- a/app/models/appointment.rb +++ b/app/models/appointment.rb @@ -40,9 +40,7 @@ class Appointment < ApplicationRecord validates :device_created_at, presence: true validates :device_updated_at, presence: true - scope :syncable_to_region, ->(region) { - with_discarded.where(patient: Patient.syncable_to_region(region)) - } + scope :for_sync, -> { with_discarded } def self.all_overdue where(status: "scheduled") diff --git a/app/models/blood_pressure.rb b/app/models/blood_pressure.rb index 813ff47bd9..d21fb49c34 100644 --- a/app/models/blood_pressure.rb +++ b/app/models/blood_pressure.rb @@ -32,9 +32,7 @@ class BloodPressure < ApplicationRecord .where(arel_table[:diastolic].lt(THRESHOLDS[:hypertensive][:diastolic])) } - scope :syncable_to_region, ->(region) { - with_discarded.where(patient: Patient.syncable_to_region(region)) - } + scope :for_sync, -> { with_discarded } def critical? systolic >= THRESHOLDS[:critical][:systolic] || diastolic >= THRESHOLDS[:critical][:diastolic] diff --git a/app/models/blood_sugar.rb b/app/models/blood_sugar.rb index 1a524b75cf..a2cb03d6bc 100644 --- a/app/models/blood_sugar.rb +++ b/app/models/blood_sugar.rb @@ -30,10 +30,7 @@ class BloodSugar < ApplicationRecord V3_TYPES = %i[random post_prandial fasting].freeze scope :for_v3, -> { where(blood_sugar_type: V3_TYPES) } - - scope :syncable_to_region, ->(region) { - with_discarded.where(patient: Patient.syncable_to_region(region)) - } + scope :for_sync, -> { with_discarded } THRESHOLDS = { high: {random: 300, diff --git a/app/models/encounter.rb b/app/models/encounter.rb index be4b1af979..26a0ac2dd3 100644 --- a/app/models/encounter.rb +++ b/app/models/encounter.rb @@ -9,9 +9,7 @@ class Encounter < ApplicationRecord has_many :blood_pressures, through: :observations, source: :observable, source_type: "BloodPressure" has_many :blood_sugars, through: :observations, source: :observable, source_type: "BloodSugar" - scope :syncable_to_region, ->(region) { - with_discarded.where(patient: Patient.syncable_to_region(region)) - } + scope :for_sync, -> { with_discarded } def self.generate_id(facility_id, patient_id, encountered_on) UUIDTools::UUID diff --git a/app/models/facility.rb b/app/models/facility.rb index b9f4758706..bd5be8a51e 100644 --- a/app/models/facility.rb +++ b/app/models/facility.rb @@ -209,4 +209,8 @@ def valid_block errors.add(:zone, "not present in the facility group") end end + + def syncable_patients + registered_patients.with_discarded + end end diff --git a/app/models/patient.rb b/app/models/patient.rb index 16cded4ca2..d7b437df41 100644 --- a/app/models/patient.rb +++ b/app/models/patient.rb @@ -56,7 +56,8 @@ class Patient < ApplicationRecord attribute :call_result, :string - scope :syncable_to_region, ->(region) { region.syncable_patients } + scope :with_nested_sync_resources, -> { includes(:address, :phone_numbers, :business_identifiers) } + scope :for_sync, -> { with_discarded.with_nested_sync_resources } scope :search_by_address, ->(term) { joins(:address).merge(Address.search_by_street_or_village(term)) } scope :with_diabetes, -> { joins(:medical_history).merge(MedicalHistory.diabetes_yes) } scope :with_hypertension, -> { joins(:medical_history).merge(MedicalHistory.hypertension_yes) } diff --git a/app/models/prescription_drug.rb b/app/models/prescription_drug.rb index 6d970a93a3..05b047e8f1 100644 --- a/app/models/prescription_drug.rb +++ b/app/models/prescription_drug.rb @@ -21,9 +21,7 @@ class PrescriptionDrug < ApplicationRecord validates :is_protocol_drug, inclusion: {in: [true, false]} validates :is_deleted, inclusion: {in: [true, false]} - scope :syncable_to_region, ->(region) { - with_discarded.where(patient: Patient.syncable_to_region(region)) - } + scope :for_sync, -> { with_discarded } def self.prescribed_as_of(date) where("device_created_at <= ?", date.end_of_day) diff --git a/spec/models/appointment_spec.rb b/spec/models/appointment_spec.rb index 5d53c235e5..84835d04ff 100644 --- a/spec/models/appointment_spec.rb +++ b/spec/models/appointment_spec.rb @@ -104,31 +104,6 @@ expect(described_class.eligible_for_reminders(days_overdue: 3)).to be_empty end end - - describe ".syncable_to_region" do - it "returns all patients registered in the region" do - facility_group = create(:facility_group) - facility = create(:facility, facility_group: facility_group) - patient = create(:patient) - other_patient = create(:patient) - - allow(Patient).to receive(:syncable_to_region).with(facility_group).and_return([patient]) - - appointments = [ - create(:appointment, patient: patient, facility: facility), - create(:appointment, patient: patient, facility: facility).tap(&:discard), - create(:appointment, patient: patient) - ] - - _other_appointments = [ - create(:appointment, patient: other_patient, facility: facility), - create(:appointment, patient: other_patient, facility: facility).tap(&:discard), - create(:appointment, patient: other_patient) - ] - - expect(Appointment.syncable_to_region(facility_group)).to contain_exactly(*appointments) - end - end end context "For discarded patients" do diff --git a/spec/models/blood_pressure_spec.rb b/spec/models/blood_pressure_spec.rb index cad1851a8e..4a34f429b9 100644 --- a/spec/models/blood_pressure_spec.rb +++ b/spec/models/blood_pressure_spec.rb @@ -39,31 +39,6 @@ expect(BloodPressure.under_control).not_to include(bp_high_systolic, bp_high_diastolic, bp_high_both) end end - - describe ".syncable_to_region" do - it "returns all patients registered in the region" do - facility_group = create(:facility_group) - facility = create(:facility, facility_group: facility_group) - patient = create(:patient) - other_patient = create(:patient) - - allow(Patient).to receive(:syncable_to_region).with(facility_group).and_return([patient]) - - blood_pressures = [ - create(:blood_pressure, patient: patient, facility: facility), - create(:blood_pressure, patient: patient, facility: facility).tap(&:discard), - create(:blood_pressure, patient: patient) - ] - - _other_blood_pressures = [ - create(:blood_pressure, patient: other_patient, facility: facility), - create(:blood_pressure, patient: other_patient, facility: facility).tap(&:discard), - create(:blood_pressure, patient: other_patient) - ] - - expect(BloodPressure.syncable_to_region(facility_group)).to contain_exactly(*blood_pressures) - end - end end context "utility methods" do diff --git a/spec/models/blood_sugar_spec.rb b/spec/models/blood_sugar_spec.rb index 84cee1aa7e..0043199dd5 100644 --- a/spec/models/blood_sugar_spec.rb +++ b/spec/models/blood_sugar_spec.rb @@ -49,30 +49,5 @@ expect(BloodSugar.for_v3.count).to eq 3 end end - - describe ".syncable_to_region" do - it "returns all patients registered in the region" do - facility_group = create(:facility_group) - facility = create(:facility, facility_group: facility_group) - patient = create(:patient) - other_patient = create(:patient) - - allow(Patient).to receive(:syncable_to_region).with(facility_group).and_return([patient]) - - blood_sugars = [ - create(:blood_sugar, patient: patient, facility: facility), - create(:blood_sugar, patient: patient, facility: facility).tap(&:discard), - create(:blood_sugar, patient: patient) - ] - - _other_blood_sugars = [ - create(:blood_sugar, patient: other_patient, facility: facility), - create(:blood_sugar, patient: other_patient, facility: facility).tap(&:discard), - create(:blood_sugar, patient: other_patient) - ] - - expect(BloodSugar.syncable_to_region(facility_group)).to contain_exactly(*blood_sugars) - end - end end end diff --git a/spec/models/encounter_spec.rb b/spec/models/encounter_spec.rb index ca4a9b0f62..61da41b4de 100644 --- a/spec/models/encounter_spec.rb +++ b/spec/models/encounter_spec.rb @@ -32,31 +32,4 @@ expect(id_1).to eq(id_2) end end - - describe "Scopes" do - describe ".syncable_to_region" do - it "returns all patients registered in the region" do - facility_group = create(:facility_group) - facility = create(:facility, facility_group: facility_group) - patient = create(:patient) - other_patient = create(:patient) - - allow(Patient).to receive(:syncable_to_region).with(facility_group).and_return([patient]) - - encounters = [ - create(:encounter, patient: patient, facility: facility), - create(:encounter, patient: patient, facility: facility).tap(&:discard), - create(:encounter, patient: patient) - ] - - _other_encounters = [ - create(:encounter, patient: other_patient, facility: facility), - create(:encounter, patient: other_patient, facility: facility).tap(&:discard), - create(:encounter, patient: other_patient) - ] - - expect(Encounter.syncable_to_region(facility_group)).to contain_exactly(*encounters) - end - end - end end diff --git a/spec/models/medical_history_spec.rb b/spec/models/medical_history_spec.rb index d1b9cafe19..4bd4bfc0e3 100644 --- a/spec/models/medical_history_spec.rb +++ b/spec/models/medical_history_spec.rb @@ -10,31 +10,6 @@ it { should validate_presence_of(:device_updated_at) } end - describe "Scopes" do - describe ".syncable_to_region" do - it "returns all patients registered in the region" do - facility_group = create(:facility_group) - patient = create(:patient) - other_patient = create(:patient) - - allow(Patient).to receive(:syncable_to_region).with(facility_group).and_return([patient]) - - MedicalHistory.destroy_all - medical_histories = [ - create(:medical_history, patient: patient), - create(:medical_history, patient: patient).tap(&:discard) - ] - - _other_medical_histories = [ - create(:medical_history, patient: other_patient), - create(:medical_history, patient: other_patient).tap(&:discard) - ] - - expect(MedicalHistory.syncable_to_region(facility_group)).to contain_exactly(*medical_histories) - end - end - end - describe "Behavior" do it_behaves_like "a record that is deletable" end diff --git a/spec/models/patient_spec.rb b/spec/models/patient_spec.rb index 7fd863e7f4..2995f9551e 100644 --- a/spec/models/patient_spec.rb +++ b/spec/models/patient_spec.rb @@ -312,30 +312,6 @@ expect(Patient.not_contacted).not_to include(patient_could_not_be_contacted) end end - - describe ".syncable_to_region" do - it "returns all patients registered in the region" do - facility_group = create(:facility_group) - other_facility_group = create(:facility_group) - - facility = create(:facility, facility_group: facility_group) - other_facility = create(:facility, facility_group: other_facility_group) - - patients = [ - create(:patient, registration_facility: facility), - create(:patient, registration_facility: facility).tap(&:discard), - create(:patient, registration_facility: facility, assigned_facility: other_facility) - ] - - _other_patients = [ - create(:patient, registration_facility: other_facility), - create(:patient, registration_facility: other_facility).tap(&:discard), - create(:patient, registration_facility: other_facility, assigned_facility: facility) - ] - - expect(Patient.syncable_to_region(facility_group)).to contain_exactly(*patients) - end - end end context "Utility methods" do diff --git a/spec/models/prescription_drug_spec.rb b/spec/models/prescription_drug_spec.rb index cbfa37c568..0b2575d308 100644 --- a/spec/models/prescription_drug_spec.rb +++ b/spec/models/prescription_drug_spec.rb @@ -15,33 +15,6 @@ it_behaves_like "a record that is deletable" end - describe "Scopes" do - describe ".syncable_to_region" do - it "returns all patients registered in the region" do - facility_group = create(:facility_group) - facility = create(:facility, facility_group: facility_group) - patient = create(:patient) - other_patient = create(:patient) - - allow(Patient).to receive(:syncable_to_region).with(facility_group).and_return([patient]) - - prescription_drugs = [ - create(:prescription_drug, patient: patient, facility: facility), - create(:prescription_drug, patient: patient, facility: facility).tap(&:discard), - create(:prescription_drug, patient: patient) - ] - - _other_prescription_drugs = [ - create(:prescription_drug, patient: other_patient, facility: facility), - create(:prescription_drug, patient: other_patient, facility: facility).tap(&:discard), - create(:prescription_drug, patient: other_patient) - ] - - expect(PrescriptionDrug.syncable_to_region(facility_group)).to contain_exactly(*prescription_drugs) - end - end - end - describe ".prescribed_as_of" do def remove_drug(prescription_drug, time) prescription_drug.update(is_deleted: true, device_updated_at: time)