Add v2 matviews to nightly refresh (#2667)

* Start adding new mat views

* Add BPs per month

* Refactor and add specs for v2

* freeze

* linting

* Turn cascade off, it refreshes too many tables

See scenic-views/scenic#275

* Use RefreshMaterializedViews to refresh the new matviews

* Visits depend up on States, so refresh states first

* States depend up on Visits, so refresh Visits first

* Enforce a known good time for all these specs

* Enforce end of June for UTC, ET, and IST

* Freeze time to avoid intermittent failures

app/models/reporting_pipeline/matview.rb

@@ -3,7 +3,7 @@ class Matview < ActiveRecord::Base
     def self.refresh
       ActiveRecord::Base.transaction do
         ActiveRecord::Base.connection.execute("SET LOCAL TIME ZONE '#{Period::REPORTING_TIME_ZONE}'")
-        Scenic.database.refresh_materialized_view(table_name, concurrently: false, cascade: true)
+        Scenic.database.refresh_materialized_view(table_name, concurrently: false, cascade: false)
       end
     end
   end

app/models/reporting_pipeline/patient_blood_pressures_per_month.rb

@@ -0,0 +1,6 @@
+module ReportingPipeline
+  class PatientBloodPressuresPerMonth < Matview
+    self.table_name = "reporting_patient_blood_pressures_per_month"
+    belongs_to :patient
+  end
+end

app/services/refresh_materialized_views.rb

@@ -28,8 +28,11 @@ def benchmark_and_statsd(operation)
   end
 
   def call
-    benchmark_and_statsd("all") do
-      refresh
+    benchmark_and_statsd("all_v1") do
+      refresh_v1
+    end
+    benchmark_and_statsd("all_v2") do
+      refresh_v2
     end
   end
 
@@ -39,37 +42,44 @@ def self.tz
 
   delegate :tz, :set_last_updated_at, to: self
 
-  def refresh
-    # LatestBloodPressuresPerPatientPerMonth should be refreshed before
-    # LatestBloodPressuresPerPatientPerQuarter and LatestBloodPressuresPerPatient
+  V1_MATVIEWS = %w[
+    LatestBloodPressuresPerPatientPerMonth
+    LatestBloodPressuresPerPatient
+    LatestBloodPressuresPerPatientPerQuarter
+    BloodPressuresPerFacilityPerDay
+    PatientRegistrationsPerDayPerFacility
+    MaterializedPatientSummary
+  ].freeze
+  V2_MATVIEWS = %w[
+    ReportingPipeline::PatientBloodPressuresPerMonth
+    ReportingPipeline::PatientVisitsPerMonth
+    ReportingPipeline::PatientStatesPerMonth
+  ].freeze
+
+  # LatestBloodPressuresPerPatientPerMonth should be refreshed before
+  # LatestBloodPressuresPerPatientPerQuarter and LatestBloodPressuresPerPatient
+  def refresh_v1
     ActiveRecord::Base.transaction do
       ActiveRecord::Base.connection.execute("SET LOCAL TIME ZONE '#{tz}'")
-
-      benchmark_and_statsd("LatestBloodPressuresPerPatientPerMonth") do
-        LatestBloodPressuresPerPatientPerMonth.refresh
-      end
-
-      benchmark_and_statsd("LatestBloodPressuresPerPatient") do
-        LatestBloodPressuresPerPatient.refresh
-      end
-
-      benchmark_and_statsd("LatestBloodPressuresPerPatientPerQuarter") do
-        LatestBloodPressuresPerPatientPerQuarter.refresh
-      end
-
-      benchmark_and_statsd("BloodPressuresPerFacilityPerDay") do
-        BloodPressuresPerFacilityPerDay.refresh
-      end
-
-      benchmark_and_statsd("PatientRegistrationsPerDayPerFacility") do
-        PatientRegistrationsPerDayPerFacility.refresh
+      V1_MATVIEWS.each do |name|
+        benchmark_and_statsd(name) do
+          klass = name.constantize
+          klass.refresh
+        end
       end
+      set_last_updated_at
+    end
+  end
 
-      benchmark_and_statsd("MaterializedPatientSummary") do
-        MaterializedPatientSummary.refresh
+  def refresh_v2
+    ActiveRecord::Base.transaction do
+      ActiveRecord::Base.connection.execute("SET LOCAL TIME ZONE '#{tz}'")
+      V2_MATVIEWS.each do |name|
+        benchmark_and_statsd(name) do
+          klass = name.constantize
+          klass.refresh
+        end
       end
-
-      set_last_updated_at
     end
   end
 end

spec/controllers/reports/regions_controller_spec.rb

@@ -514,16 +514,18 @@ def refresh_views
     end
 
     it "calls csv service and returns 200 with csv data" do
-      facility
-      sign_in(cvho.email_authentication)
+      Timecop.freeze("June 15th 2020") do
+        facility
+        sign_in(cvho.email_authentication)
 
-      expect_any_instance_of(MonthlyDistrictDataService).to receive(:report).and_call_original
-      get :monthly_district_data_report, params: {id: region.slug, report_scope: "district", format: "csv"}
-      expect(response.status).to eq(200)
-      expect(response.body).to include("Monthly District Data: #{region.name} #{Date.current.strftime("%B %Y")}")
-      report_date = Date.current.strftime("%b-%Y").downcase
-      expected_filename = "monthly-district-data-#{region.slug}-#{report_date}.csv"
-      expect(response.headers["Content-Disposition"]).to include(%(filename="#{expected_filename}"))
+        expect_any_instance_of(MonthlyDistrictDataService).to receive(:report).and_call_original
+        get :monthly_district_data_report, params: {id: region.slug, report_scope: "district", format: "csv"}
+        expect(response.status).to eq(200)
+        expect(response.body).to include("Monthly District Data: #{region.name} #{Date.current.strftime("%B %Y")}")
+        report_date = Date.current.strftime("%b-%Y").downcase
+        expected_filename = "monthly-district-data-#{region.slug}-#{report_date}.csv"
+        expect(response.headers["Content-Disposition"]).to include(%(filename="#{expected_filename}"))
+      end
     end
 
     it "works for facility districts" do

spec/models/reporting_pipeline/patient_states_per_month_spec.rb

@@ -5,6 +5,17 @@
     it { should belong_to(:patient) }
   end
 
+  around do |example|
+    # We need to enforce a known time for this test, otherwise we will have intermittent failures. For example,
+    # if we use live system time, many of these specs will fail after 18:30 UTC (ie 14:30 ET) when on the last day of a month,
+    # because that falls into the next day in IST (our reporting time zone). So to prevent confusing failures for
+    # developers or CI during North American afternoons, we freeze to a time that will be the end of the month for
+    # UTC, ET, and IST. Timezones! 🤯
+    Timecop.freeze("June 30 2021 5:30 UTC") do # June 30th 23:00 IST time
+      example.run
+    end
+  end
+
   context "indicators" do
     describe "htn_care_state" do
       it "marks a dead patient dead" do
@@ -19,7 +30,7 @@
         patient_registered_13m_ago = Timecop.freeze(13.months.ago) { create(:patient) }
         Timecop.freeze(13.months.ago) { create(:blood_pressure, patient: patient_registered_13m_ago) }
 
-        described_class.refresh
+        RefreshMaterializedViews.new.refresh_v2
         with_reporting_time_zones do
           expect(described_class
             .where(htn_care_state: "lost_to_follow_up", month_date: Date.current.beginning_of_month)
@@ -34,7 +45,7 @@
         patient_registered_12m_ago = Timecop.freeze(12.months.ago) { create(:patient) }
         patient_registered_11m_ago = Timecop.freeze(11.months.ago) { create(:patient) }
 
-        described_class.refresh
+        RefreshMaterializedViews.new.refresh_v2
         with_reporting_time_zones do
           expect(described_class
             .where(htn_care_state: "lost_to_follow_up", month_date: Date.current.beginning_of_month)
@@ -56,7 +67,7 @@
         patient_with_recent_bp = Timecop.freeze(13.months.ago) { create(:patient) }
         Timecop.freeze(11.months.ago) { create(:blood_pressure, patient: patient_with_recent_bp) }
 
-        described_class.refresh
+        RefreshMaterializedViews.new.refresh_v2
         with_reporting_time_zones do
           expect(described_class
             .where(htn_care_state: "lost_to_follow_up", month_date: Date.current.beginning_of_month)
@@ -75,7 +86,7 @@
           create(:blood_pressure, patient: under_care_patient, recorded_at: june_2021[:under_12_months_ago])
           create(:blood_pressure, patient: ltfu_patient, recorded_at: june_2021[:over_12_months_ago])
 
-          described_class.refresh
+          RefreshMaterializedViews.new.refresh_v2
 
           with_reporting_time_zones do
             expect(described_class
@@ -100,7 +111,8 @@
           create(:blood_pressure, patient: under_care_patient, recorded_at: june_2021[:end_of_month] - 1.minute)
           create(:blood_pressure, patient: ltfu_patient, recorded_at: june_2021[:end_of_month] + 1.minute)
 
-          described_class.refresh
+          RefreshMaterializedViews.new.refresh_v2
+
           with_reporting_time_zones do
             expect(described_class
               .where(htn_care_state: "lost_to_follow_up", month_date: june_2021[:beginning_of_month])
@@ -121,7 +133,7 @@
           under_care_patient = create(:patient, recorded_at: june_2021[:under_12_months_ago])
           ltfu_patient = create(:patient, recorded_at: june_2021[:over_12_months_ago])
 
-          described_class.refresh
+          RefreshMaterializedViews.new.refresh_v2
           with_reporting_time_zones do
             expect(described_class
               .where(htn_care_state: "lost_to_follow_up", month_date: june_2021[:beginning_of_month])
@@ -147,7 +159,7 @@
         patient_2 = create(:patient, recorded_at: june_2021[:long_ago])
         create(:encounter, patient: patient_2, encountered_on: june_2021[:under_3_months_ago])
         patient_3 = create(:patient, recorded_at: june_2021[:long_ago])
-        described_class.refresh
+        RefreshMaterializedViews.new.refresh_v2
 
         with_reporting_time_zones do
           expect(described_class.where(htn_treatment_outcome_in_last_3_months: "missed_visit", month_date: june_2021[:now]).pluck(:patient_id))
@@ -180,7 +192,7 @@
           facility: patient_with_no_bp.registration_facility,
           patient: patient_with_no_bp,
           user: patient_with_no_bp.registration_user)
-        described_class.refresh
+        RefreshMaterializedViews.new.refresh_v2
 
         with_reporting_time_zones do
           expect(described_class.where(htn_treatment_outcome_in_last_3_months: "visited_no_bp", month_date: june_2021[:now]).pluck(:patient_id))
@@ -200,7 +212,7 @@
         patient_bp_over_3_months = create(:patient, recorded_at: june_2021[:long_ago])
         create(:blood_pressure, :with_encounter, patient: patient_bp_over_3_months, recorded_at: june_2021[:over_3_months_ago])
 
-        described_class.refresh
+        RefreshMaterializedViews.new.refresh_v2
 
         with_reporting_time_zones do
           expect(described_class.where(htn_treatment_outcome_in_last_3_months: "controlled", month_date: june_2021[:now]).pluck(:patient_id))
@@ -220,7 +232,7 @@
         patient_3 = create(:patient, recorded_at: june_2021[:now])
         patient_4 = create(:patient, recorded_at: june_2021[:over_3_months_ago])
 
-        described_class.refresh
+        RefreshMaterializedViews.new.refresh_v2
         with_reporting_time_zones do
           expect(described_class.find_by(patient_id: patient_1.id, month_string: june_2021[:month_string]).months_since_registration).to eq 11
           expect(described_class.find_by(patient_id: patient_2.id, month_string: june_2021[:month_string]).months_since_registration).to eq 12
@@ -243,7 +255,7 @@
 
         patient = create(:patient, registration_facility: registration_facility, assigned_facility: assigned_facility)
 
-        described_class.refresh
+        RefreshMaterializedViews.new.refresh_v2
 
         with_reporting_time_zones do
           patient_state = described_class.find_by(patient_id: patient.id, month_string: june_2021[:month_string])
@@ -275,7 +287,7 @@
 
         patient = create(:patient, registration_facility: registration_facility, assigned_facility: assigned_facility)
 
-        described_class.refresh
+        RefreshMaterializedViews.new.refresh_v2
 
         with_reporting_time_zones do
           patient_state = described_class.find_by(patient_id: patient.id, month_string: june_2021[:month_string])
@@ -306,7 +318,7 @@
 
         patient_no_bp = create(:patient, recorded_at: june_2021[:long_ago])
 
-        described_class.refresh
+        RefreshMaterializedViews.new.refresh_v2
 
         with_reporting_time_zones do
           controlled_state = described_class.find_by(patient_id: patient_controlled.id, month_string: june_2021[:month_string])
@@ -348,7 +360,7 @@ def patient_states(patient, from: nil, to: nil)
         create(:prescription_drug, patient: patient, device_created_at: eight_months_ago)
         create(:blood_pressure, :with_encounter, patient: patient, recorded_at: five_months_ago, systolic: 140, diastolic: 90)
 
-        described_class.refresh
+        RefreshMaterializedViews.new.refresh_v2
 
         with_reporting_time_zones do
           expect(patient_states(patient).pluck(:months_since_registration)).to eq((0..24).to_a)

spec/models/reporting_pipeline/patient_visits_per_month_spec.rb

@@ -5,6 +5,12 @@
     it { should belong_to(:patient) }
   end
 
+  around do |example|
+    Timecop.freeze("June 30 2021 5:30 UTC") do # June 30th 23:00 IST time
+      example.run
+    end
+  end
+
   describe "the visit definition" do
     it "considers a BP measurement as a visit" do
       bp = create(:blood_pressure, :with_encounter, recorded_at: june_2021[:now])

spec/services/refresh_materialized_views_spec.rb

@@ -24,10 +24,10 @@
     time = Time.current
     # Just adding enough data to smoke test this; we test these views
     # more thoroughly via various reporting specs
-    create_list(:blood_pressure, 2)
 
     expect {
       Timecop.freeze(time) do
+        create_list(:blood_pressure, 2)
         RefreshMaterializedViews.call
       end
     }.to change { LatestBloodPressuresPerPatientPerMonth.count }.from(0).to(2)
@@ -37,4 +37,16 @@
       .and change { PatientRegistrationsPerDayPerFacility.count }.from(0).to(2)
       .and change { RefreshMaterializedViews.last_updated_at }.from(nil).to(time)
   end
+
+  it "updates v2 matviews" do
+    time = Time.current
+    expect {
+      Timecop.freeze(time) do
+        create_list(:blood_pressure, 2)
+        RefreshMaterializedViews.call
+      end
+    }.to change { ReportingPipeline::PatientBloodPressuresPerMonth.count }.from(0).to(2)
+      .and change { ReportingPipeline::PatientStatesPerMonth.count }.from(0).to(2)
+      .and change { ReportingPipeline::PatientVisitsPerMonth.count }.from(0).to(2)
+  end
 end