Skip to content

Commit

Permalink
Improve the records_to_sync queries structurally to avoid sub-selects
Browse files Browse the repository at this point in the history
  • Loading branch information
kitallis committed Dec 17, 2020
1 parent 1e7b012 commit 886f8d3
Show file tree
Hide file tree
Showing 18 changed files with 31 additions and 223 deletions.
2 changes: 1 addition & 1 deletion app/controllers/api/v3/blood_sugars_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def sync_to_user

private

def region_records
def model
super.for_v3
end

Expand Down
15 changes: 6 additions & 9 deletions app/controllers/api/v3/patients_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
27 changes: 12 additions & 15 deletions app/controllers/concerns/api/v3/sync_to_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions app/models/appointment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
4 changes: 1 addition & 3 deletions app/models/blood_pressure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
5 changes: 1 addition & 4 deletions app/models/blood_sugar.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 1 addition & 3 deletions app/models/encounter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions app/models/facility.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 1 addition & 3 deletions app/models/medical_history.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ class MedicalHistory < ApplicationRecord
enum hypertension: MEDICAL_HISTORY_ANSWERS, _prefix: true
enum diagnosed_with_hypertension: MEDICAL_HISTORY_ANSWERS, _prefix: true

scope :syncable_to_region, ->(region) {
with_discarded.where(patient: Patient.syncable_to_region(region))
}
scope :for_sync, -> { with_discarded }

def indicates_hypertension_risk?
prior_heart_attack_boolean || prior_stroke_boolean
Expand Down
3 changes: 2 additions & 1 deletion app/models/patient.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand Down
4 changes: 1 addition & 3 deletions app/models/prescription_drug.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
25 changes: 0 additions & 25 deletions spec/models/appointment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 0 additions & 25 deletions spec/models/blood_pressure_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 0 additions & 25 deletions spec/models/blood_sugar_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
27 changes: 0 additions & 27 deletions spec/models/encounter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
25 changes: 0 additions & 25 deletions spec/models/medical_history_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 0 additions & 24 deletions spec/models/patient_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 0 additions & 27 deletions spec/models/prescription_drug_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 886f8d3

Please sign in to comment.