Revert "Convert to Repository for DistrictAnalyticsQuery numbers" (#2479

) * Revert "Convert to Repository for DistrictAnalyticsQuery numbers (#2436)" This reverts commit 4341a40. * Disable intermittent failing test
simpledotorg · May 10, 2021 · 8f676bb · 8f676bb
1 parent 4341a40
commit 8f676bb
Show file tree

Hide file tree

Showing 29 changed files with 464 additions and 685 deletions.
diff --git a/.env.seed.test b/.env.seed.test
@@ -10,6 +10,6 @@ MAX_PATIENTS_TO_CREATE_SMALL=4
 MAX_PATIENTS_TO_CREATE_MEDIUM=6
 MAX_PATIENTS_TO_CREATE_LARGE=8
 
-MAX_BPS_TO_CREATE=5
+MAX_BPS_TO_CREATE=15
 
 SEED_TEST_MODE=true
diff --git a/app/controllers/reports/regions_controller.rb b/app/controllers/reports/regions_controller.rb
@@ -58,10 +58,6 @@ def details
 
     @show_current_period = true
     @period = Period.month(Time.current)
-    @period_range = Range.new(@period.advance(months: -5), @period)
-    regions = [@region, @region.facility_regions].flatten
-    @repository = Reports::Repository.new(regions, periods: @period_range)
-
     @dashboard_analytics = @region.dashboard_analytics(period: @period.type,
                                                        prev_periods: 6,
                                                        include_current_period: true)

diff --git a/app/models/facility.rb b/app/models/facility.rb
@@ -145,11 +145,15 @@ def source
   # ----------------
 
   def hypertension_follow_ups_by_period(*args)
-    patients.hypertension_follow_ups_by_period(*args).where(blood_pressures: {facility: self})
+    patients
+      .hypertension_follow_ups_by_period(*args)
+      .where(blood_pressures: {facility: self})
   end
 
   def diabetes_follow_ups_by_period(*args)
-    patients.diabetes_follow_ups_by_period(*args).where(blood_sugars: {facility: self})
+    patients
+      .diabetes_follow_ups_by_period(*args)
+      .where(blood_sugars: {facility: self})
   end
 
   # For compatibility w/ parent FacilityGroups

diff --git a/app/models/medication.rb b/app/models/medication.rb
@@ -2,8 +2,8 @@ class Medication < ActiveYaml::Base
   set_root_path "config/data"
   set_filename "medications"
 
-  CREATED_TIME = File.ctime("config/data/medications.yml").in_time_zone("UTC")
-  UPDATED_TIME = File.mtime("config/data/medications.yml").in_time_zone("UTC")
+  CREATED_TIME ||= File.ctime("config/data/medications.yml").in_time_zone("UTC")
+  UPDATED_TIME ||= File.mtime("config/data/medications.yml").in_time_zone("UTC")
 
   CATEGORIES = {
     hypertension_ccb: "Hypertension: CCB",

diff --git a/app/models/patient.rb b/app/models/patient.rb
@@ -74,17 +74,18 @@ class Patient < ApplicationRecord
   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 :follow_ups_by_period, ->(period, at_region: nil, current: true, last: nil) {
-    FollowUpsQuery.with(Encounter, period, at_region: at_region, current: current, time_column: "encountered_on", last: last)
+    follow_ups_with(Encounter, period, at_region: at_region, current: current, time_column: "encountered_on", last: last)
   }
 
   scope :diabetes_follow_ups_by_period, ->(period, at_region: nil, current: true, last: nil) {
-    FollowUpsQuery.with(BloodSugar, period, at_region: at_region, current: current, last: last).with_diabetes
+    follow_ups_with(BloodSugar, period, at_region: at_region, current: current, last: last)
+      .with_diabetes
   }
 
   scope :hypertension_follow_ups_by_period, ->(period, at_region: nil, current: true, last: nil) {
-    FollowUpsQuery.with(BloodPressure, period, at_region: at_region, current: current, last: last).with_hypertension
+    follow_ups_with(BloodPressure, period, at_region: at_region, current: current, last: last)
+      .with_hypertension
   }
 
   scope :contactable, -> {
@@ -104,6 +105,23 @@ class Patient < ApplicationRecord
   validates_associated :address, if: :address
   validates_associated :phone_numbers, if: :phone_numbers
 
+  def self.follow_ups_with(model_name, period, time_column: "recorded_at", at_region: nil, current: true, last: nil)
+    table_name = model_name.table_name.to_sym
+    time_column_with_table_name = "#{table_name}.#{time_column}"
+
+    relation = joins(table_name)
+      .where("patients.recorded_at < #{model_name.date_to_period_sql(time_column_with_table_name, period)}")
+      .group_by_period(period, time_column_with_table_name, current: current, last: last)
+      .distinct
+
+    if at_region.present?
+      facility_ids = at_region.facilities.map(&:id)
+      relation = relation.where(table_name => {facility_id: facility_ids})
+    end
+
+    relation
+  end
+
   def past_date_of_birth
     if date_of_birth.present? && date_of_birth > Date.current
       errors.add(:date_of_birth, "can't be in the future")

diff --git a/app/models/period.rb b/app/models/period.rb
@@ -159,11 +159,11 @@ def inspect
     "<Period type:#{type} value=#{value}>"
   end
 
-  def to_s(format = :mon_year)
+  def to_s
     if quarter?
       value.to_s
     else
-      value.to_s(format)
+      value.to_s(:mon_year)
     end
   end
 

diff --git a/app/models/reports/repository.rb b/app/models/reports/repository.rb
@@ -22,8 +22,8 @@ def initialize(regions, periods:)
     attr_reader :control_rate_query
     attr_reader :earliest_patient_data_query
     attr_reader :no_bp_measure_query
-    attr_reader :period_type
     attr_reader :periods
+    attr_reader :period_type
     attr_reader :regions
 
     delegate :cache, :logger, to: Rails
@@ -58,8 +58,6 @@ def initialize(regions, periods:)
       end
     end
 
-    # Adjusted patient counts are the patient counts from three months ago (the adjusted period) that
-    # are the basis for control rates. These counts DO include lost to follow up.
     memoize def adjusted_patient_counts_with_ltfu
       cumulative_assigned_patients_count.each_with_object({}) do |(entry, result), results|
         values = periods.each_with_object(Hash.new(0)) { |period, region_result|
@@ -69,8 +67,6 @@ def initialize(regions, periods:)
       end
     end
 
-    # Adjusted patient counts are the patient counts from three months ago (the adjusted period) that
-    # are the basis for control rates. These counts DO NOT include lost to follow up.
     memoize def adjusted_patient_counts
       cumulative_assigned_patients_count.each_with_object({}) do |(entry, result), results|
         values = periods.each_with_object(Hash.new(0)) { |period, region_result|
@@ -110,42 +106,6 @@ def initialize(regions, periods:)
       end
     end
 
-    memoize def registration_counts_by_user
-      region_entries = regions.map { |region| RegionEntry.new(region, __method__, period_type: period_type) }
-      region_entries.each_with_object({}) do |region_entry, sum|
-        sum[region_entry.slug] = RegisteredPatientsQuery.new.count(region_entry.region, period_type, group_by: :registration_user_id)
-      end
-    end
-
-    # Returns counts of the cumulative Registration counts done by Users within a Region.
-    # Returns a nested Hash structure in the following shape:
-    # {
-    #   slug => {
-    # .   period => {
-    # .     user_id_1 => cumulative_count,
-    # .     user_id_2 => cumulative_count,
-    # .  }
-    # }
-    #
-    memoize def cumulative_registration_counts_by_user
-      registration_counts_by_user.each_with_object({}) do |(slug, period_counts), totals|
-        totals[slug] = {}
-        # collect all the user ids in a region that we need to count for
-        user_ids = period_counts.each_with_object(Set.new) { |(period, user_counts), user_ids| user_ids.merge(user_counts.keys) }
-        # now sum up the running totals of registration counts for all those users for all periods
-        range = Range.new(earliest_patient_recorded_at_period[slug], periods.end)
-        range.each do |period|
-          totals[slug][period] ||= Hash.new(0)
-          user_ids.each do |user_id|
-            current = period_counts.dig(period, user_id) || 0
-            previous = totals.dig(slug, period.previous, user_id) || 0
-            totals[slug][period][user_id] = current + previous
-          end
-        end
-        totals
-      end
-    end
-
     # Returns the full range of registered patient counts for a Region. We do this via one SQL query for each Region, because its
     # fast and easy via the underlying query.
     memoize def complete_registration_counts
@@ -165,26 +125,26 @@ def initialize(regions, periods:)
     end
 
     memoize def ltfu_counts
-      region_period_cached_query(__method__) do |entry|
+      cached_query(__method__) do |entry|
         facility_ids = entry.region.facility_ids
         Patient.for_reports.where(assigned_facility: facility_ids).ltfu_as_of(entry.period.end).count
       end
     end
 
     memoize def controlled_patients_count
-      region_period_cached_query(__method__) do |entry|
+      cached_query(__method__) do |entry|
         control_rate_query.controlled(entry.region, entry.period).count
       end
     end
 
     memoize def uncontrolled_patients_count
-      region_period_cached_query(__method__) do |entry|
+      cached_query(__method__) do |entry|
         control_rate_query.uncontrolled(entry.region, entry.period).count
       end
     end
 
     memoize def missed_visits
-      region_period_cached_query(__method__) do |entry|
+      cached_query(__method__) do |entry|
         slug = entry.slug
         patient_count = denominator(entry.region, entry.period)
         controlled = controlled_patients_count[slug][entry.period]
@@ -194,21 +154,14 @@ def initialize(regions, periods:)
       end
     end
 
-    # We determine this rate via subtraction from 100 instead of via division to avoid confusing rounding errors.
     memoize def missed_visits_rate
-      region_period_cached_query(__method__) do |entry|
+      cached_query(__method__) do |entry|
         slug, period = entry.slug, entry.period
         remaining_percentages = controlled_patients_rate[slug][period] + uncontrolled_patients_rate[slug][period] + visited_without_bp_taken_rate[slug][period]
         100 - remaining_percentages
       end
     end
 
-    memoize def hypertension_follow_ups(group_by: nil)
-      regions.each_with_object({}) do |region, hsh|
-        hsh[region.slug] = FollowUpsQuery.new(region, period_type, group_by: group_by).hypertension
-      end
-    end
-
     # This method currently always returns the "excluding LTFU denominator".
     # Repository only returns "excluding LTFU" rates.
     # This only powers queries for children regions which do not require both variants of control rates, unlike Result.
@@ -218,30 +171,30 @@ def denominator(region, period)
     end
 
     memoize def controlled_patients_rate
-      region_period_cached_query(__method__) do |entry|
-        controlled = controlled_patients_count[entry.slug][entry.period]
+      cached_query(__method__) do |entry|
+        controlled = controlled_patients_count[entry.region.slug][entry.period]
         total = denominator(entry.region, entry.period)
         percentage(controlled, total)
       end
     end
 
     memoize def uncontrolled_patients_rate
-      region_period_cached_query(__method__) do |entry|
+      cached_query(__method__) do |entry|
         controlled = uncontrolled_patients_count[entry.region.slug][entry.period]
         total = denominator(entry.region, entry.period)
         percentage(controlled, total)
       end
     end
 
     memoize def visited_without_bp_taken
-      region_period_cached_query(__method__) do |entry|
+      cached_query(__method__) do |entry|
         no_bp_measure_query.call(entry.region, entry.period)
       end
     end
 
     memoize def visited_without_bp_taken_rate
-      region_period_cached_query(__method__) do |entry|
-        controlled = visited_without_bp_taken[entry.slug][entry.period]
+      cached_query(__method__) do |entry|
+        controlled = visited_without_bp_taken[entry.region.slug][entry.period]
         total = denominator(entry.region, entry.period)
         percentage(controlled, total)
       end
@@ -256,7 +209,7 @@ def denominator(region, period)
     #     region_2_slug: { period_1: value, period_2: value }
     #   }
     #
-    def region_period_cached_query(calculation, &block)
+    def cached_query(calculation, &block)
       items = cache_entries(calculation)
       cached_results = cache.fetch_multi(*items, force: bust_cache?) { |entry| block.call(entry) }
       cached_results.each_with_object({}) do |(entry, count), results|

diff --git a/app/queries/district_analytics_query.rb b/app/queries/district_analytics_query.rb
@@ -2,7 +2,7 @@ class DistrictAnalyticsQuery
   include BustCache
   include DashboardHelper
 
-  CACHE_VERSION = 4
+  CACHE_VERSION = 3
 
   attr_reader :region, :facilities
 
@@ -16,8 +16,6 @@ def initialize(region, period = :month, prev_periods = 3, from_time = Time.curre
     @district_name = @region.name
     @from_time = from_time
     @include_current_period = include_current_period
-    @current_period = Period.month(Date.current)
-    @range = Range.new(@current_period.advance(months: -prev_periods), @current_period)
   end
 
   def call
@@ -53,30 +51,26 @@ def total_assigned_patients
       .to_h
   end
 
-  def repository
-    @repository ||= Reports::Repository.new(facilities, periods: @range)
-  end
-
   def total_registered_patients
-    @total_registered_patients ||= @facilities.each_with_object({}) { |facility, result|
-      result[facility.id] = {
-        total_registered_patients: repository.cumulative_registrations.dig(facility.region.slug, @current_period)
-      }
-    }
-  end
+    @total_registered_patients ||=
+      Patient
+        .for_reports
+        .joins(:registration_facility)
+        .where(facilities: {id: facilities})
+        .group("facilities.id")
+        .count
+
+    return if @total_registered_patients.blank?
 
-  def period_to_dates(hsh)
-    return unless hsh
-    hsh.transform_keys { |k| k.to_date }
+    @total_registered_patients
+      .map { |facility_id, count| [facility_id, {total_registered_patients: count}] }
+      .to_h
   end
 
   def registered_patients_by_period
-    @facilities.each_with_object({}) { |facility, result|
-      counts = period_to_dates(repository.registration_counts[facility.region.slug])
-      next unless counts&.any?
+    result = ActivityService.new(region, group: [:registration_facility_id]).registrations
 
-      result[facility.id] = {registered_patients_by_period: counts}
-    }
+    group_by_date_and_facility(result, :registered_patients_by_period)
   end
 
   def follow_up_patients_by_period

diff --git a/app/queries/follow_ups_query.rb b/app/queries/follow_ups_query.rb