Perf: refactor DistributionsByCountry#report using raw SQL query

Author: coalest

app/controllers/distributions_by_county_controller.rb

@@ -4,7 +4,12 @@ class DistributionsByCountyController < ApplicationController
 
   def report
     setup_date_range_picker
-    distributions = current_organization.distributions.includes(:partner).during(helpers.selected_range)
-    @breakdown = DistributionByCountyReportService.new.get_breakdown(distributions)
+    start_date = helpers.selected_range.first.iso8601
+    end_date = helpers.selected_range.last.iso8601
+    @breakdown = DistributionSummaryByCountyQuery.new(
+      organization_id: current_organization.id,
+      start_date: start_date,
+      end_date: end_date
+    ).call
   end
 end

app/queries/distribution_summary_by_county_query.rb

@@ -0,0 +1,78 @@
+class DistributionSummaryByCountyQuery
+  DISTRIBUTION_BY_COUNTY_SQL = <<~SQL.squish.freeze
+    /* Calculate total item quantity and value per distribution */
+    WITH distribution_totals AS
+    (
+        SELECT DISTINCT d.id,
+                        d.partner_id,
+                        COALESCE(SUM(li.quantity) OVER (PARTITION BY d.id), 0) AS quantity,
+                        COALESCE(SUM(COALESCE(i.value_in_cents, 0) * li.quantity) OVER (PARTITION BY d.id), 0) AS amount
+        FROM distributions d
+        JOIN line_items li ON li.itemizable_id = d.id AND li.itemizable_type = 'Distribution'
+        JOIN items i ON i.id = li.item_id
+        WHERE d.issued_at BETWEEN :start_date AND :end_date
+            AND d.organization_id = :organization_id
+        GROUP BY d.id, li.id, i.id
+    ),
+    /* Match distribution totals with client share and counties.
+       If distribution has no associated county, set county name to "Unspecified"
+       and set region to ZZZ so it will be last when sorted  */
+    totals_by_county AS
+    (
+        SELECT dt.id,
+               dt.quantity,
+               dt.amount,
+               COALESCE(psa.client_share::float / 100, 1) AS percentage,
+               COALESCE(c.name, 'Unspecified') county_name,
+               COALESCE(c.region, 'ZZZ') county_region
+        FROM distribution_totals dt
+        JOIN partner_profiles pp ON pp.id = dt.partner_id
+        LEFT JOIN partner_served_areas psa ON psa.partner_profile_id = pp.partner_id
+        LEFT JOIN counties c ON c.id = psa.county_id
+        UNION
+        /* Previous behavior was to add a row for unspecified counties
+           even if all distributions have an associated county */
+        SELECT 0 AS id,
+               0 AS quantity,
+               0 AS amount,
+               1 AS percentage,
+               'Unspecified' AS county_name,
+               'ZZZ' AS county_region
+    )
+    /* Distribution value and quantities per county share may not be whole numbers,
+       so we cast to an integer for rounding purposes */
+    SELECT tbc.county_name AS name,
+           SUM((tbc.quantity * percentage)::int) AS quantity,
+           SUM((tbc.amount * percentage)::int) AS amount
+    FROM totals_by_county tbc
+    GROUP BY county_name, county_region
+    ORDER BY county_region ASC;
+  SQL
+
+  def initialize(organization_id:, start_date: nil, end_date: nil)
+    @organization_id = organization_id
+    @start_date = start_date || "1000-01-01"
+    @end_date = end_date || "3000-01-01"
+  end
+
+  def call
+    execute(to_sql(DISTRIBUTION_BY_COUNTY_SQL)).to_a
+  end
+
+  private
+
+  def execute(sql)
+    ActiveRecord::Base.connection.execute(sql)
+  end
+
+  def to_sql(query)
+    ActiveRecord::Base.sanitize_sql_array(
+      [
+        query,
+        organization_id: @organization_id,
+        start_date: @start_date,
+        end_date: @end_date
+      ]
+    )
+  end
+end

app/services/distribution_by_county_report_service.rb

@@ -50,9 +50,9 @@
               <tbody>
               <% @breakdown.each do |bd| %>
                 <tr>
-                  <td><%= bd.name %></td>
-                  <td class="numeric"><%= number_with_delimiter(bd.num_items) %></td>
-                  <td class="numeric"><%= dollar_presentation(bd.amount) %></td>
+                  <td><%= bd["name"] %></td>
+                  <td class="numeric"><%= number_with_delimiter(bd["quantity"]) %></td>
+                  <td class="numeric"><%= dollar_presentation(bd["amount"]) %></td>
                 </tr>
               <% end %>
 

app/views/distributions_by_county/report.html.erb

@@ -0,0 +1,67 @@
+RSpec.describe DistributionSummaryByCountyQuery do
+  let(:year) { Time.current.year }
+  let(:issued_at_last_year) { Time.current.change(year: year - 1).to_datetime }
+  let(:distributions) { [] }
+  let(:organization_id) { organization.id }
+  let(:start_date) { nil }
+  let(:end_date) { nil }
+  let(:params) { {organization_id:, start_date:, end_date:} }
+
+  include_examples "distribution_by_county"
+
+  before do
+    create(:storage_location, organization: organization)
+  end
+
+  describe "get_breakdown" do
+    it "will have 100% unspecified shows if no served_areas" do
+      create(:distribution, :with_items, item: item_1, organization: user.organization)
+      breakdown = DistributionSummaryByCountyQuery.new(**params).call
+      expect(breakdown.size).to eq(1)
+      expect(breakdown[0]["quantity"]).to eq(100)
+      expect(breakdown[0]["amount"]).to be_within(0.01).of(105000.0)
+    end
+
+    it "divides the item numbers and values according to the partner profile" do
+      create(:distribution, :with_items, item: item_1, organization: user.organization, partner: partner_1)
+      breakdown = DistributionSummaryByCountyQuery.new(**params).call
+      expect(breakdown.size).to eq(5)
+      expect(breakdown[4]["quantity"]).to eq(0)
+      expect(breakdown[4]["amount"]).to be_within(0.01).of(0)
+      3.times do |i|
+        expect(breakdown[i]["quantity"]).to eq(25)
+        expect(breakdown[i]["amount"]).to be_within(0.01).of(26250.0)
+      end
+    end
+
+    it "handles multiple partners with overlapping service areas properly" do
+      create(:distribution, :with_items, item: item_1, organization: user.organization, partner: partner_1, issued_at: issued_at_present)
+      create(:distribution, :with_items, item: item_1, organization: user.organization, partner: partner_2, issued_at: issued_at_present)
+      breakdown = DistributionSummaryByCountyQuery.new(**params).call
+      num_with_45 = 0
+      num_with_20 = 0
+      num_with_0 = 0
+      # The result will have at least 1 45 and at least 1 20, and 1 0.  Anything else will be either 45 or 25 or 20
+      breakdown.each do |sa|
+        if sa["quantity"] == 45
+          expect(sa["amount"]).to be_within(0.01).of(47250.0)
+          num_with_45 += 1
+        end
+
+        if sa["quantity"] == 25
+          expect(sa["amount"]).to be_within(0.01).of(26250.0)
+        end
+        if sa["quantity"] == 20
+          expect(sa["amount"]).to be_within(0.01).of(21000.0)
+          num_with_20 += 1
+        end
+        if sa["quantity"] == 0
+          expect(sa["amount"]).to be_within(0.01).of(0)
+        end
+      end
+      expect(num_with_45).to be > 0
+      expect(num_with_20).to be > 0
+      expect(num_with_0).to eq 0
+    end
+  end
+end