Merge pull request #4344 from nhsuk/optimise-programmes-query

Optimise a number of queries to improve performance

Author: thomasleese

app/components/app_programme_stats_component.rb

@@ -1,21 +1,21 @@
 # frozen_string_literal: true
 
 class AppProgrammeStatsComponent < ViewComponent::Base
-  def initialize(programme, academic_year:, patients:)
+  def initialize(programme, academic_year:, patient_ids:)
     super
 
     @programme = programme
     @academic_year = academic_year
-    @patients = patients
+    @patient_ids = patient_ids
   end
 
-  delegate :count, to: :patients, prefix: true
+  def patients_count = patient_ids.length
 
   def vaccinations_count
     helpers
       .policy_scope(VaccinationRecord)
       .administered
-      .where(patient: patients, programme:)
+      .where(patient_id: patient_ids, programme:)
       .count
   end
 
@@ -25,11 +25,11 @@ def consent_notifications_count
       .has_programme(programme)
       .joins(:session)
       .where(session: { academic_year: })
-      .where(patient: patients)
+      .where(patient_id: patient_ids)
       .count
   end
 
   private
 
-  attr_reader :programme, :academic_year, :patients
+  attr_reader :programme, :academic_year, :patient_ids
 end

app/controllers/programmes/base_controller.rb

@@ -20,13 +20,23 @@ def set_academic_year
     end
   end
 
-  def patients
-    # We do this instead of using `team.patients` as that has a `distinct` on
-    # it which means we cannot apply ordering or grouping.
-    @patients ||=
-      Patient
-        .where(id: current_team.patient_sessions.select(:patient_id).distinct)
-        .appear_in_programmes([@programme], academic_year: @academic_year)
+  def patient_ids
+    @patient_ids ||=
+      PatientSession
+        .distinct
+        .joins(:patient, :session)
+        .where(session_id: session_ids)
+        .appear_in_programmes([@programme])
         .not_archived(team: current_team)
+        .pluck(:patient_id)
+  end
+
+  def session_ids
+    @session_ids ||=
+      current_team
+        .sessions
+        .where(academic_year: @academic_year)
+        .has_programmes([@programme])
+        .pluck(:id)
   end
 end

app/controllers/programmes/overview_controller.rb

@@ -2,7 +2,7 @@
 
 class Programmes::OverviewController < Programmes::BaseController
   before_action :set_consents
-  before_action :set_patients
+  before_action :set_patient_ids
   before_action :set_patient_count_by_year_group
 
   def show
@@ -13,21 +13,21 @@ def show
   def set_consents
     @consents =
       policy_scope(Consent).where(
-        patient: patients,
+        patient_id: patient_ids,
         programme: @programme,
         academic_year: @academic_year
       )
   end
 
-  def set_patients
-    @patients = patients
+  def set_patient_ids
+    @patient_ids = patient_ids
   end
 
   def set_patient_count_by_year_group
     year_groups = current_team.programme_year_groups[@programme]
 
     patient_count_by_birth_academic_year =
-      patients.group(:birth_academic_year).count
+      Patient.where(id: patient_ids).group(:birth_academic_year).count
 
     @patient_count_by_year_group =
       year_groups.index_with do |year_group|

app/controllers/programmes/patients_controller.rb

@@ -9,7 +9,7 @@ def index
     @year_groups = current_team.programme_year_groups[@programme]
 
     scope =
-      patients.includes(
+      Patient.where(id: patient_ids).includes(
         :consent_statuses,
         :triage_statuses,
         :vaccination_statuses,

app/helpers/imports_helper.rb

@@ -3,15 +3,12 @@
 module ImportsHelper
   def import_issues_count
     vaccination_records_with_issues =
-      policy_scope(VaccinationRecord).with_pending_changes.distinct
+      policy_scope(VaccinationRecord).with_pending_changes.distinct.pluck(
+        :patient_id
+      )
 
-    patients_with_issues = policy_scope(Patient).with_pending_changes
+    patients_with_issues = policy_scope(Patient).with_pending_changes.pluck(:id)
 
-    unique_import_issues =
-      (vaccination_records_with_issues + patients_with_issues).uniq do |record|
-        record.is_a?(VaccinationRecord) ? record.patient_id : record.id
-      end
-
-    unique_import_issues.count
+    (vaccination_records_with_issues + patients_with_issues).uniq.length
   end
 end

app/models/patient_session.rb

@@ -93,26 +93,28 @@ class PatientSession < ApplicationRecord
 
   scope :appear_in_programmes,
         ->(programmes) do
+          # Are any of the programmes administered in the session?
+          programme_in_session =
+            SessionProgramme
+              .where(programme: programmes)
+              .where("session_programmes.session_id = sessions.id")
+              .arel
+              .exists
+
           # Is the patient eligible for any of those programmes by year group?
-          location_programme_year_groups =
+          patient_in_administered_year_groups =
             LocationProgrammeYearGroup
-              .where("programme_id = session_programmes.programme_id")
+              .where(programme: programmes)
               .where("location_id = sessions.location_id")
               .where(
                 "year_group = sessions.academic_year " \
                   "- patients.birth_academic_year " \
                   "- #{Integer::AGE_CHILDREN_START_SCHOOL}"
               )
-
-          # Are any of the programmes administered in the session?
-          where(
-            SessionProgramme
-              .where(programme: programmes)
-              .where("session_programmes.session_id = sessions.id")
-              .where(location_programme_year_groups.arel.exists)
               .arel
               .exists
-          )
+
+          where(programme_in_session).where(patient_in_administered_year_groups)
         end
 
   scope :search_by_name,

app/models/session.rb

@@ -18,8 +18,9 @@
 #
 # Indexes
 #
-#  index_sessions_on_location_id              (location_id)
-#  index_sessions_on_team_id_and_location_id  (team_id,location_id)
+#  index_sessions_on_location_id                (location_id)
+#  index_sessions_on_team_id_and_academic_year  (team_id,academic_year)
+#  index_sessions_on_team_id_and_location_id    (team_id,location_id)
 #
 # Foreign Keys
 #
@@ -69,7 +70,6 @@ class Session < ApplicationRecord
             SessionProgramme
               .select("COUNT(session_programmes.id)")
               .where("sessions.id = session_programmes.session_id")
-              .joins(:programme)
               .where(programme: programmes),
             programmes.count
           )

app/views/programmes/overview/show.html.erb

@@ -13,7 +13,7 @@
 
 <%= govuk_button_to "Download vaccination report", programme_reports_path(@programme, @academic_year), secondary: true, class: "nhsuk-u-margin-bottom-5" %>
 
-<%= render AppProgrammeStatsComponent.new(@programme, academic_year: @academic_year, patients: @patients) %>
+<%= render AppProgrammeStatsComponent.new(@programme, academic_year: @academic_year, patient_ids: @patient_ids) %>
 
 <h2 class="nhsuk-heading-m">Cohorts</h2>
 

db/migrate/20250820085332_add_index_on_session_team_and_academic_year.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddIndexOnSessionTeamAndAcademicYear < ActiveRecord::Migration[8.0]
+  disable_ddl_transaction!
+
+  def change
+    add_index :sessions, %i[team_id academic_year], algorithm: :concurrently
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.0].define(version: 2025_08_14_154032) do
+ActiveRecord::Schema[8.0].define(version: 2025_08_20_085332) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_catalog.plpgsql"
   enable_extension "pg_trgm"
@@ -772,6 +772,7 @@
     t.date "send_invitations_at"
     t.boolean "requires_registration", default: true, null: false
     t.index ["location_id"], name: "index_sessions_on_location_id"
+    t.index ["team_id", "academic_year"], name: "index_sessions_on_team_id_and_academic_year"
     t.index ["team_id", "location_id"], name: "index_sessions_on_team_id_and_location_id"
   end