7101 - Fix Prosopite N+1 harness and clear datatable + request specs

app/controllers/all_casa_admins/dashboard_controller.rb

@@ -3,5 +3,7 @@ class AllCasaAdmins::DashboardController < AllCasaAdminsController
 
   def show
     @organizations = CasaOrg.all
+    @user_counts = CasaOrg.user_count_by_org_id
+    @case_contacts_counts = CasaOrg.case_contacts_count_by_org_id
   end
 end

app/controllers/case_contacts/case_contacts_new_design_controller.rb

@@ -16,6 +16,8 @@ def index
 
     scope = filter_case_contacts(policy_scope(current_organization.case_contacts))
       .includes(:casa_case, :contact_types, :contact_topics, :followups, :creator, contact_topic_answers: :contact_topic)
+      # CaseContactPolicy#same_org? reads both of these for every row's edit/destroy permissions.
+      .preload(:casa_org, :creator_casa_org)
     order = Arel.sql("case_contacts.#{@sort} #{@direction} NULLS LAST, case_contacts.id DESC")
     @pagy, @case_contacts = pagy(scope.order(order))
 

app/controllers/case_contacts/form_controller.rb

@@ -106,14 +106,20 @@ def set_active_nav
 
   def set_case_contact
     @case_contact = CaseContact
-      .includes(:creator, :contact_topic_answers)
+      # contact_topic_answers' contact_topic too: saving the nested answers validates each
+      # belongs_to :contact_topic, which is satisfied from the loaded record instead of a query
+      # per answer.
+      .includes(:creator, contact_topic_answers: :contact_topic)
       .find(params[:case_contact_id])
   end
 
   def prepare_form
     @casa_cases = get_casa_cases
     contact_types = get_contact_types.decorate
     @grouped_contact_types = group_contact_types_by_name(contact_types)
+    # Resolved once for the whole form: both the checkbox list and the multi-select show a
+    # "last logged" hint per contact type.
+    @last_logged_at_by_contact_type = CaseContact.last_logged_at_by_contact_type(@casa_cases.map(&:id))
     @contact_topics = get_contact_topics
     # No pre-built blank answer: the Notes checklist lists every topic and creates an answer
     # only when a topic is checked (contact-topics controller). A seeded blank row would just

app/controllers/case_court_reports_controller.rb

@@ -69,12 +69,15 @@ def set_casa_case
     @casa_case = CasaCase.find_by(case_number: params[:id], casa_org: current_user.casa_org)
   end
 
+  # The case picker renders CasaCaseDecorator#court_report_select_option for every case -- for an
+  # admin that is every active case in the chapter -- which reads the assigned volunteers and, via
+  # court_report_default_start_date, the case's court dates.
   def assigned_cases
     @assigned_cases = if current_user.volunteer?
       CasaCase.actively_assigned_to(current_user)
     else
       current_user.casa_org.casa_cases.active
-    end
+    end.includes(:assigned_volunteers, :court_dates)
   end
 
   def generate_report_to_string(casa_case, time_range)

app/controllers/notifications_controller.rb

@@ -15,7 +15,8 @@ def index
     # the date-split helpers.
     hidden_ids = notifications.reject { |notification| notification.event&.renderable? }.map(&:id)
     @notifications = hidden_ids.any? ? notifications.where.not(id: hidden_ids) : notifications
-    @patch_notes = PatchNote.notes_available_for_user(current_user)
+    # includes(:patch_note_type) because the view groups the notes by type name.
+    @patch_notes = PatchNote.notes_available_for_user(current_user).includes(:patch_note_type)
   end
 
   def mark_as_read

app/datatables/volunteer_datatable.rb

@@ -10,6 +10,9 @@ class VolunteerDatatable < ApplicationDatatable
     hours_spent_in_days
   ]
 
+  # Window for the "Hours (30 days)" column.
+  HOURS_SPENT_IN_DAYS = 30
+
   # Server-side entry point for the migrated (bespoke Pagy) index. Reuses the same
   # filter/search/order SQL as the DataTables JSON path; the controller maps plain GET
   # params into the DataTables param shape. Preloads languages for the extra-languages column.
@@ -28,7 +31,13 @@ def index_count
   private
 
   def data
-    records.map do |volunteer|
+    volunteers = records.to_a
+    preload_languages(volunteers)
+    active_case_counts = active_case_counts_for(volunteers)
+    contacted_case_counts = contacted_case_counts_for(volunteers)
+    minutes_spent = minutes_spent_for(volunteers)
+
+    volunteers.map do |volunteer|
       {
         active: volunteer.active?,
         casa_cases: volunteer.casa_cases.map { |cc| {id: cc.id, case_number: cc.case_number} },
@@ -37,18 +46,75 @@ def data
         email: volunteer.email,
         has_transition_aged_youth_cases: volunteer.has_transition_aged_youth_cases?,
         id: volunteer.id,
-        made_contact_with_all_cases_in_days: volunteer.made_contact_with_all_cases_in_days?,
+        made_contact_with_all_cases_in_days:
+          made_contact_with_all_cases?(volunteer, active_case_counts, contacted_case_counts),
         most_recent_attempt: {
           case_id: volunteer.most_recent_attempt_case_id,
           occurred_at: I18n.l(volunteer.most_recent_attempt_occurred_at, format: :full, default: nil)
         },
         supervisor: {id: volunteer.supervisor_id, name: NamePresentation.strip_honorific(volunteer.supervisor_name)},
-        hours_spent_in_days: volunteer.hours_spent_in_days(30),
+        hours_spent_in_days: Volunteer.format_hours_and_minutes(minutes_spent.fetch(volunteer.id, 0)),
         extra_languages: volunteer.languages&.map { |lang| {id: lang.id, name: lang.name} }
       }
     end
   end
 
+  # Preloaded against the already-loaded page rather than via raw_records.includes(:languages):
+  # filtered_records can carry SELECT aliases, DISTINCT and an ORDER BY on an alias (the
+  # extra_languages filter), and an includes on that relation makes Rails build an id-lookup
+  # query that repeats the alias in its own SELECT list, which Postgres rejects.
+  def preload_languages(volunteers)
+    ActiveRecord::Associations::Preloader.new(records: volunteers, associations: :languages).call
+  end
+
+  # Mirrors Volunteer#made_contact_with_all_cases_in_days? using the page-wide counts below.
+  def made_contact_with_all_cases?(volunteer, active_case_counts, contacted_case_counts)
+    active_cases = active_case_counts.fetch(volunteer.id, 0)
+    return true if active_cases.zero?
+
+    contacted_case_counts.fetch(volunteer.id, 0) == active_cases
+  end
+
+  # The three aggregates below mirror Volunteer#made_contact_with_all_cases_in_days? and
+  # Volunteer#hours_spent_in_days, but each runs a single grouped query for the whole page
+  # instead of two or three queries per row.
+  def active_case_counts_for(volunteers)
+    actively_assigned(volunteers).group(:volunteer_id).count
+  end
+
+  # NOTE: deliberately NOT distinct, to match Volunteer#cases_where_contact_made_in_days, which
+  # counts contact rows rather than cases. That makes a volunteer with more contacts than active
+  # cases look like they have not reached everyone -- pre-existing behaviour, preserved here so
+  # this stays a performance change only.
+  def contacted_case_counts_for(volunteers)
+    actively_assigned(volunteers)
+      .joins(casa_case: :case_contacts)
+      .where(case_contacts: {contact_made: true, occurred_at: contact_made_cutoff..})
+      .group(:volunteer_id)
+      .count
+  end
+
+  def minutes_spent_for(volunteers)
+    actively_assigned(volunteers)
+      .joins(casa_case: :case_contacts)
+      .where(case_contacts: {contact_made: true, occurred_at: HOURS_SPENT_IN_DAYS.days.ago.to_date..})
+      .group(:volunteer_id)
+      .sum(:duration_minutes)
+  end
+
+  # Volunteer#actively_assigned_and_active_cases, expressed from the assignment side so it can be
+  # grouped by volunteer.
+  def actively_assigned(volunteers)
+    CaseAssignment
+      .active
+      .joins(:casa_case)
+      .where(volunteer_id: volunteers.map(&:id), casa_cases: {active: true})
+  end
+
+  def contact_made_cutoff
+    Volunteer::CONTACT_MADE_IN_DAYS_NUM.days.ago.to_date
+  end
+
   def filtered_records
     extra_languages_filter do
       raw_records

app/decorators/contact_type_decorator.rb

@@ -2,7 +2,9 @@ class ContactTypeDecorator < Draper::Decorator
   include ActionView::Helpers::DateHelper
   delegate_all
 
-  def hash_for_multi_select_with_cases(casa_case_ids)
+  # last_logged_at_by_type is the CaseContact.last_logged_at_by_contact_type lookup. Pass it when
+  # rendering a list of contact types; without it every option runs its own recency query.
+  def hash_for_multi_select_with_cases(casa_case_ids, last_logged_at_by_type = nil)
     casa_case_ids = [] if casa_case_ids.nil?
 
     {
@@ -13,19 +15,23 @@ def hash_for_multi_select_with_cases(casa_case_ids)
       # subtext instead of a bare "never" beside every option. `.to_s`, not the bare nil: the option
       # template substitutes this through TomSelect's escape(), which turns nil into the literal
       # string "null".
-      subtext: last_logged_hint_with_cases(casa_case_ids).to_s
+      subtext: last_logged_hint_with_cases(casa_case_ids, last_logged_at_by_type).to_s
     }
   end
 
   # Labeled recency hint. Returns nil when this type has never been logged for the case(s) so the
   # caller can omit the line rather than show a bare "never". Used by both the contact-type
   # checkboxes and the multi-select options -- they had diverged, which is how "never" survived on
   # casa_cases#edit after being removed elsewhere.
-  def last_logged_hint_with_cases(casa_case_ids)
-    last_contact = last_contact_with_cases(casa_case_ids)
-    return if last_contact&.occurred_at.blank?
+  def last_logged_hint_with_cases(casa_case_ids, last_logged_at_by_type = nil)
+    occurred_at = if last_logged_at_by_type
+      last_logged_at_by_type[object.id]
+    else
+      last_contact_with_cases(casa_case_ids)&.occurred_at
+    end
+    return if occurred_at.blank?
 
-    "Last logged #{time_ago_in_words(last_contact.occurred_at)} ago"
+    "Last logged #{time_ago_in_words(occurred_at)} ago"
   end
 
   private

app/models/casa_case.rb

@@ -148,11 +148,19 @@ def latest_court_report
     court_reports.order("created_at").last
   end
 
+  # Both of these are called per case when a list of cases is rendered (the court-report case
+  # picker, the missing-data report). Filtering in Ruby when court_dates is already loaded lets an
+  # includes(:court_dates) actually take effect -- a scoped where/order on the association always
+  # issues its own query and silently defeats the preload.
   def next_court_date
+    return court_dates.select { |court_date| court_date.date >= Date.today }.min_by(&:date) if court_dates.loaded?
+
     court_dates.where("date >= ?", Date.today).order(:date).first
   end
 
   def most_recent_past_court_date
+    return court_dates.select { |court_date| court_date.date < Date.today }.max_by(&:date) if court_dates.loaded?
+
     court_dates.where("date < ?", Date.today).order(:date).last
   end
 

app/models/casa_org.rb

@@ -65,6 +65,22 @@ def case_contacts_count
     case_contacts.count
   end
 
+  # Per-org totals for the all-CASA dashboard, which lists every organisation: #case_contacts_count
+  # and #user_count each cost a query per row when called in that loop. COALESCE mirrors
+  # #case_contacts_count, which attributes a contact to its case's org, falling back to the
+  # creator's org for org-less (draft) contacts. "users" is the alias Rails gives the :creator
+  # join here -- there is only one users join in this relation.
+  def self.case_contacts_count_by_org_id
+    CaseContact
+      .left_joins(:casa_case, :creator)
+      .group(Arel.sql("COALESCE(casa_cases.casa_org_id, users.casa_org_id)"))
+      .count
+  end
+
+  def self.user_count_by_org_id
+    User.group(:casa_org_id).count
+  end
+
   def org_logo
     if logo.attached?
       Rails.application.routes.url_helpers.rails_blob_path(logo, only_path: true)

app/models/case_contact.rb

@@ -306,9 +306,23 @@ def has_casa_case_transitioned
     casa_case.in_transition_age?
   end
 
+  # {contact_type_id => most recent occurred_at} for the given cases, in one query. The contact
+  # form renders a "last logged" hint for every contact type in the chapter, which otherwise costs
+  # a query per type.
+  def self.last_logged_at_by_contact_type(casa_case_ids)
+    joins(:contact_types)
+      .where(casa_case_id: casa_case_ids)
+      .group("contact_types.id")
+      .maximum(:occurred_at)
+  end
+
   def contact_groups_with_types
     hash = Hash.new { |h, k| h[k] = [] }
-    contact_types.includes(:contact_type_group).each do |contact_type|
+    # Only add the includes when the caller has not already preloaded contact_types: calling
+    # .includes on a loaded association throws the preload away and re-queries per case contact,
+    # which is an N+1 on the contacts list (LoadsCaseContacts preloads contact_types).
+    types = contact_types.loaded? ? contact_types : contact_types.includes(:contact_type_group)
+    types.each do |contact_type|
       hash[contact_type.contact_type_group.name] << contact_type.name
     end
     hash

app/models/case_court_report_context.rb

@@ -44,8 +44,16 @@ def case_contacts
   end
 
   def latest_hearing_date
-    latest_hearing_date = @casa_case.most_recent_past_court_date
-    latest_hearing_date.nil? ? "___<LATEST HEARING DATE>____" : I18n.l(latest_hearing_date.date, format: :full, default: nil)
+    most_recent_past_court_date.nil? ? "___<LATEST HEARING DATE>____" : I18n.l(most_recent_past_court_date.date, format: :full, default: nil)
+  end
+
+  # CasaCase#most_recent_past_court_date is a scoped query on the court_dates association, so it
+  # re-runs on every call. Both the date range and the hearing-date label need it, so resolve it
+  # once per report.
+  def most_recent_past_court_date
+    return @most_recent_past_court_date if defined?(@most_recent_past_court_date)
+
+    @most_recent_past_court_date = @casa_case.most_recent_past_court_date
   end
 
   def case_orders(orders)
@@ -57,9 +65,12 @@ def case_orders(orders)
     end
   end
 
+  # includes, not just joins: CaseContactsContactDates walks every row calling #contact_type and
+  # #case_contact, so joining alone still costs two queries per interviewee row.
   def filtered_interviewees
     CaseContactContactType
       .joins(:contact_type, case_contact: :casa_case)
+      .includes(:contact_type, :case_contact)
       .where("case_contacts.casa_case_id": @casa_case.id)
       .where("case_contacts.occurred_at": @date_range)
   end
@@ -150,7 +161,7 @@ def court_topic_answers
   def calculate_date_range(args)
     zone = args[:time_zone] ? ActiveSupport::TimeZone.new(args[:time_zone]) : Time.zone
 
-    start_date = @casa_case.most_recent_past_court_date&.date&.in_time_zone(zone)
+    start_date = most_recent_past_court_date&.date&.in_time_zone(zone)
     start_date = zone.parse(args[:start_date]) if args[:start_date]&.present?
 
     end_date = args[:end_date]&.present? ? zone.parse(args[:end_date]) : nil

app/models/supervisor.rb

@@ -5,8 +5,11 @@ class Supervisor < User
   has_many :active_supervisor_volunteers, -> { where(is_active: true) }, class_name: "SupervisorVolunteer", foreign_key: "supervisor_id"
   has_many :unassigned_supervisor_volunteers, -> { where(is_active: false) }, class_name: "SupervisorVolunteer", foreign_key: "supervisor_id"
 
-  has_many :volunteers, -> { includes(:supervisor_volunteer).order(:display_name) }, through: :active_supervisor_volunteers
-  has_many :volunteers_ever_assigned, -> { includes(:supervisor_volunteer).order(:display_name) }, through: :supervisor_volunteers, source: :volunteer
+  # :supervisor as well as :supervisor_volunteer -- supervisors#edit lists who each volunteer is
+  # currently assigned to. Volunteer#supervisor is its own has_one :through, so preloading the join
+  # row alone does not satisfy it and it costs a query per volunteer.
+  has_many :volunteers, -> { includes(:supervisor_volunteer, :supervisor).order(:display_name) }, through: :active_supervisor_volunteers
+  has_many :volunteers_ever_assigned, -> { includes(:supervisor_volunteer, :supervisor).order(:display_name) }, through: :supervisor_volunteers, source: :volunteer
 
   scope :active, -> { where(active: true) }
 

app/models/volunteer.rb

@@ -117,13 +117,20 @@ def made_contact_with_all_cases_in_days?(num_days = CONTACT_MADE_IN_DAYS_NUM)
     current_contact_cases_count == total_active_case_count
   end
 
+  # Formats a raw minute total as "1h 30m", dropping any zero component. Extracted so that
+  # VolunteerDatatable, which sums the same minutes for a whole page in one grouped query rather
+  # than once per row, renders identical strings.
+  def self.format_hours_and_minutes(minutes)
+    ["#{minutes / 60}h", "#{minutes % 60}m"].select { |str| str =~ /[1-9]/ }.join(" ")
+  end
+
   def hours_spent_in_days(num_days)
     minutes = actively_assigned_and_active_cases
       .includes(:case_contacts)
       .where(case_contacts: {contact_made: true, occurred_at: num_days.days.ago.to_date..})
       .sum(:duration_minutes)
 
-    ["#{minutes / 60}h", "#{minutes % 60}m"].select { |str| str =~ /[1-9]/ }.join(" ")
+    self.class.format_hours_and_minutes(minutes)
   end
 
   # Calendar year to date, matching the learning-hours roster's default period -- and matching what the

app/views/all_casa_admins/dashboard/show.html.erb

@@ -35,8 +35,8 @@
                 <tr>
                   <td class="<%= td %>"><%= link_to organization.name, all_casa_admins_casa_org_path(organization), class: "font-medium text-brand-600 hover:text-brand-700" %></td>
                   <td class="<%= td %> whitespace-nowrap"><%= I18n.l(organization.created_at, format: :full, default: nil) %></td>
-                  <td class="<%= td %>"><%= organization.user_count %></td>
-                  <td class="<%= td %>"><%= organization.case_contacts_count %></td>
+                  <td class="<%= td %>"><%= @user_counts.fetch(organization.id, 0) %></td>
+                  <td class="<%= td %>"><%= @case_contacts_counts.fetch(organization.id, 0) %></td>
                 </tr>
               <% end %>
             </tbody>

app/views/case_contacts/form/_contact_types.html.erb

@@ -3,7 +3,7 @@
   <%= render(Form::MultipleSelectComponent.new(
     form: form,
     name: :contact_type_ids,
-    options: options.decorate.map { |ct| ct.hash_for_multi_select_with_cases(casa_cases&.pluck(:id)) },
+    options: options.decorate.map { |ct| ct.hash_for_multi_select_with_cases(casa_cases&.pluck(:id), @last_logged_at_by_contact_type) },
     selected_items: selected_items,
     render_option_subtext: local_assigns.fetch(:render_option_subtext, true),
     placeholder_term: "contact types",

app/views/case_contacts/form/details.html.erb

@@ -152,7 +152,7 @@
                         <div class="text-sm leading-5">
                           <%= b.label(class: "block font-medium text-slate-700") %>
                           <%# recency hint: only when this type has actually been logged for the case(s) %>
-                          <% last_logged = b.object.last_logged_hint_with_cases(casa_case_ids) %>
+                          <% last_logged = b.object.last_logged_hint_with_cases(casa_case_ids, @last_logged_at_by_contact_type) %>
                           <% if last_logged %>
                             <span class="text-xs text-slate-500"><%= last_logged %></span>
                           <% end %>

app/views/case_groups/_form.html.erb

@@ -20,7 +20,7 @@
             <%= form.label :casa_case_ids, class: "mb-1.5 block text-sm font-medium text-slate-700" do %>Cases <%= req %><% end %>
             <%= form.select(
               :casa_case_ids,
-              current_organization.casa_cases.map { |casa_case| ["#{casa_case.case_number} - #{casa_case.assigned_volunteers.map(&:display_name).join(", ").presence || "Unassigned"}", casa_case.id] },
+              current_organization.casa_cases.includes(:assigned_volunteers).map { |casa_case| ["#{casa_case.case_number} - #{casa_case.assigned_volunteers.map(&:display_name).join(", ").presence || "Unassigned"}", casa_case.id] },
               {include_hidden: false, autocomplete: "off"},
               {multiple: true, data: {"multiple-select-target": "select"}}
             ) %>