Perf: refactor DistributionsByCountry#report using raw SQL query

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
+        LEFT 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/views/distributions_by_county/report.html.erb

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

spec/factories/partners.rb

@@ -24,16 +24,16 @@
     send_reminders { true }
     organization_id { Organization.try(:first).try(:id) || create(:organization).id }
 
+    transient do
+      without_profile { false }
+    end
+
     trait :approved do
       status { :approved }
     end
 
     trait :uninvited do
       status { :uninvited }
-
-      transient do
-        without_profile { false }
-      end
     end
 
     trait :awaiting_review do

spec/queries/distribution_summary_by_county_query_spec.rb

@@ -0,0 +1,79 @@
+RSpec.describe DistributionSummaryByCountyQuery do
+  let(:organization) { create(:organization, name: "Some Unique Name") }
+  let(:item_1) { create(:item, value_in_cents: 1050, organization: organization) }
+  let(:partner_1) do
+    create(:partner, organization:, without_profile: true) do |p|
+      p.profile = create(:partner_profile, partner: p, organization:) do |pp|
+        pp.served_areas = create_list(:partners_served_area, 4, partner_profile: pp, client_share: 25) do |sa|
+          sa.county = create(:county)
+        end
+      end
+    end
+  end
+  let(:partner_2) do
+    create(:partner, organization:, without_profile: true) do |p|
+      p.profile = create(:partner_profile, partner: p, organization:) do |pp|
+        pp.served_areas = create_list(:partners_served_area, 5, partner_profile: pp, client_share: 20) do |sa, i|
+          # create one overlapping service area
+          sa.county = i.zero? ? partner_1.profile.served_areas[0].county : create(:county)
+        end
+      end
+    end
+  end
+
+  let(:now) { Time.current.to_datetime }
+
+  let(:params) { {organization_id: organization.id, start_date: nil, end_date: nil} }
+
+  describe "call" do
+    it "will have 100% unspecified shows if no served_areas" do
+      create(:distribution, :with_items, item: item_1, organization: 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: 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: organization, partner: partner_1, issued_at: now)
+      create(:distribution, :with_items, item: item_1, organization: organization, partner: partner_2, issued_at: now)
+      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