Improve Sync To User performance (batch 2)

app/controllers/api/v3/blood_sugars_controller.rb

@@ -12,7 +12,7 @@ def sync_to_user
 
   private
 
-  def region_records
+  def model_sync_scope
     super.for_v3
   end
 

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_sync_scope
+      .where(id: current_facility.syncable_patients.pluck(:id))
       .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.pluck(:id) - current_facility.syncable_patients.pluck(:id)
 
-    region_records
-      .where.not(registration_facility: current_facility)
+    model_sync_scope
+      .where(id: other_patient_records)
       .updated_on_server_since(other_facilities_processed_since, other_facilities_limit)
   end
 

app/controllers/concerns/api/v3/sync_to_user.rb

@@ -2,33 +2,30 @@ 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_sync_scope
+        .where(patient: current_facility.syncable_patients.pluck(:id))
         .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.pluck(:id) - current_facility.syncable_patients.pluck(:id)
 
-      region_records
-        .where.not(patient: prioritized_patients)
+      model_sync_scope
+        .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_sync_scope
+      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)

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")

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]

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,

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

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

app/models/medical_history.rb

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

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) }

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)

app/models/region.rb

@@ -88,10 +88,10 @@ def syncable_patients
     case region_type
       when "block"
         registered_patients.with_discarded
-          .union(assigned_patients.with_discarded)
+          .or(assigned_patients.with_discarded)
           .union(appointed_patients.with_discarded)
       else
-        registered_patients
+        registered_patients.with_discarded
     end
   end
 

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

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

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

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

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

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