Split up consent request and reminder jobs (#3328)

thomasleese · web-flow · commit e8b976e57d99 · 2025-04-16T07:07:40.000+01:00
This splits the jobs in to two separate jobs each, one which runs on a
schedule and for each session queues up a separate job which handles the
process of sending the communications for a particular session.

We're finding that the current job is unable to handle a high number of
sessions and runs out of resources. Instead, by queuing a job for each
session we can avoid the individual jobs being too resource hungry and
gives us more flexibility in terms of running the jobs manually.

As part of this change I've also renamed the other jobs for consistency,
specifically jobs which are prefixed with `Enqueue` do nothing other
than queue up other jobs, and jobs prefixed with `Send` do the task of
actually sending out communications.
diff --git a/app/jobs/enqueue_clinic_session_invitations_job.rb b/app/jobs/enqueue_clinic_session_invitations_job.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class ClinicSessionInvitationsJob < ApplicationJob
+class EnqueueClinicSessionInvitationsJob < ApplicationJob
   queue_as :notifications
 
   def perform
@@ -9,7 +9,7 @@ def perform
       .includes(:programmes)
       .joins(:location)
       .merge(Location.clinic)
-      .each do |session|
+      .find_each do |session|
         # We're only inviting patients who don't have a school.
         # Patients who have a school are sent invitations manually by the
         # nurse when they're finished at a school.
diff --git a/app/jobs/enqueue_school_consent_reminders_job.rb b/app/jobs/enqueue_school_consent_reminders_job.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+class EnqueueSchoolConsentRemindersJob < ApplicationJob
+  queue_as :notifications
+
+  def perform
+    sessions =
+      Session.send_consent_reminders.joins(:location).merge(Location.school)
+
+    sessions.find_each do |session|
+      SendSchoolConsentRemindersJob.perform_later(session)
+    end
+  end
+end
diff --git a/app/jobs/enqueue_school_consent_requests_job.rb b/app/jobs/enqueue_school_consent_requests_job.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+class EnqueueSchoolConsentRequestsJob < ApplicationJob
+  queue_as :notifications
+
+  def perform
+    sessions =
+      Session.send_consent_requests.joins(:location).merge(Location.school)
+
+    sessions.find_each do |session|
+      SendSchoolConsentRequestsJob.perform_later(session)
+    end
+  end
+end
diff --git a/app/jobs/enqueue_update_patients_from_pds_job.rb b/app/jobs/enqueue_update_patients_from_pds_job.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class BulkUpdatePatientsFromPDSJob < ApplicationJob
+class EnqueueUpdatePatientsFromPDSJob < ApplicationJob
   include GoodJob::ActiveJobExtensions::Concurrency
 
   queue_as :pds
diff --git a/app/jobs/send_school_consent_reminders_job.rb b/app/jobs/send_school_consent_reminders_job.rb
@@ -1,20 +1,16 @@
 # frozen_string_literal: true
 
-class SchoolConsentRemindersJob < ApplicationJob
+class SendSchoolConsentRemindersJob < ApplicationJob
   queue_as :notifications
 
-  def perform
-    sessions =
-      Session
-        .send_consent_reminders
-        .joins(:location)
-        .includes(:session_dates, :programmes, :patient_sessions, :location)
-        .merge(Location.school)
+  def perform(session)
+    return unless session.school? && session.open_for_consent?
 
-    sessions.find_each(batch_size: 1) do |session|
-      next unless session.open_for_consent?
-
-      session.patient_sessions.each do |patient_session|
+    session
+      .patient_sessions
+      .includes_programmes
+      .includes(patient: %i[consent_notifications consents vaccination_records])
+      .find_each do |patient_session|
         ProgrammeGrouper
           .call(patient_session.programmes)
           .each_value do |programmes|
@@ -39,7 +35,6 @@ def perform
             )
           end
       end
-    end
   end
 
   def should_send_notification?(patient_session:, programmes:)
diff --git a/app/jobs/send_school_consent_requests_job.rb b/app/jobs/send_school_consent_requests_job.rb
@@ -1,20 +1,16 @@
 # frozen_string_literal: true
 
-class SchoolConsentRequestsJob < ApplicationJob
+class SendSchoolConsentRequestsJob < ApplicationJob
   queue_as :notifications
 
-  def perform
-    sessions =
-      Session
-        .send_consent_requests
-        .joins(:location)
-        .includes(:session_dates, :programmes, :patient_sessions, :location)
-        .merge(Location.school)
+  def perform(session)
+    return unless session.school? && session.open_for_consent?
 
-    sessions.find_each(batch_size: 1) do |session|
-      next unless session.open_for_consent?
-
-      session.patient_sessions.each do |patient_session|
+    session
+      .patient_sessions
+      .includes_programmes
+      .includes(patient: %i[consent_notifications consents vaccination_records])
+      .find_each do |patient_session|
         ProgrammeGrouper
           .call(patient_session.programmes)
           .each_value do |programmes|
@@ -30,7 +26,6 @@ def perform
             )
           end
       end
-    end
   end
 
   def should_send_notification?(patient:, programmes:)
diff --git a/app/jobs/send_school_session_reminders_job.rb b/app/jobs/send_school_session_reminders_job.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class SchoolSessionRemindersJob < ApplicationJob
+class SendSchoolSessionRemindersJob < ApplicationJob
   queue_as :notifications
 
   def perform
@@ -16,7 +16,7 @@ def perform
         .merge(Session.has_date(date))
         .notification_not_sent(date)
 
-    patient_sessions.each do |patient_session|
+    patient_sessions.find_each do |patient_session|
       next unless should_send_notification?(patient_session:)
 
       SessionNotification.create_and_send!(
diff --git a/app/jobs/send_vaccination_confirmations_job.rb b/app/jobs/send_vaccination_confirmations_job.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class VaccinationConfirmationsJob < ApplicationJob
+class SendVaccinationConfirmationsJob < ApplicationJob
   include VaccinationMailerConcern
 
   queue_as :notifications
diff --git a/config/environments/production.rb b/config/environments/production.rb
@@ -97,25 +97,20 @@
 
   config.good_job.enable_cron = true
   config.good_job.cron = {
-    bulk_update_patients_from_pds: {
-      cron: "every day at 6:00 and 18:00",
-      class: "BulkUpdatePatientsFromPDSJob",
-      description: "Keep patient details up to date with PDS."
-    },
-    clinic_invitation: {
+    clinic_session_invitations: {
       cron: "every day at 9am",
-      class: "ClinicSessionInvitationsJob",
+      class: "EnqueueClinicSessionInvitationsJob",
       description: "Send school clinic invitation emails to parents"
     },
-    consent_request: {
+    school_consent_requests: {
       cron: "every day at 4pm",
-      class: "SchoolConsentRequestsJob",
+      class: "EnqueueSchoolConsentRequestsJob",
       description:
         "Send school consent request emails to parents for each session"
     },
     consent_reminder: {
-      cron: "every day at 4pm",
-      class: "SchoolConsentRemindersJob",
+      cron: "every day at 9am",
+      class: "SendSchoolConsentReminderJob",
       description:
         "Send school consent reminder emails to parents for each session"
     },
@@ -125,9 +120,9 @@
       description:
         "Invalidate all self-consents and associated triage for the previous day"
     },
-    session_reminder: {
+    school_session_reminders: {
       cron: "every day at 9am",
-      class: "SchoolSessionRemindersJob",
+      class: "SendSchoolSessionRemindersJob",
       description: "Send school session reminder emails to parents"
     },
     remove_import_csv: {
@@ -145,9 +140,14 @@
       class: "TrimActiveRecordSessionsJob",
       description: "Remove ActiveRecord sessions older than 30 days"
     },
+    update_patients_from_pds: {
+      cron: "every day at 6:00 and 18:00",
+      class: "EnqueueUpdatePatientsFromPDSJob",
+      description: "Keep patient details up to date with PDS."
+    },
     vaccination_confirmations: {
       cron: "every day at 7pm",
-      class: "VaccinationConfirmationsJob",
+      class: "SendVaccinationConfirmationsJob",
       description: "Send vaccination confirmation emails to parents"
     }
   }
diff --git a/spec/features/doubles_vaccination_administered_spec.rb b/spec/features/doubles_vaccination_administered_spec.rb
@@ -133,7 +133,7 @@ def then_i_see_the_patient_is_vaccinated_for_td_ipv
   end
 
   def when_vaccination_confirmations_are_sent
-    VaccinationConfirmationsJob.perform_now
+    SendVaccinationConfirmationsJob.perform_now
   end
 
   def then_an_email_is_sent_to_the_parent_confirming_the_vaccinations
diff --git a/spec/features/hpv_vaccination_administered_spec.rb b/spec/features/hpv_vaccination_administered_spec.rb
@@ -224,7 +224,7 @@ def then_i_see_the_patient
   end
 
   def when_vaccination_confirmations_are_sent
-    VaccinationConfirmationsJob.perform_now
+    SendVaccinationConfirmationsJob.perform_now
   end
 
   def then_an_email_is_sent_to_the_parent_confirming_the_vaccination
diff --git a/spec/features/hpv_vaccination_already_had_spec.rb b/spec/features/hpv_vaccination_already_had_spec.rb
@@ -100,7 +100,7 @@ def then_i_see_that_the_status_is_vaccinated
   end
 
   def when_vaccination_confirmations_are_sent
-    VaccinationConfirmationsJob.perform_now
+    SendVaccinationConfirmationsJob.perform_now
   end
 
   def then_an_email_is_sent_saying_the_vaccination_didnt_happen
diff --git a/spec/features/hpv_vaccination_clinic_spec.rb b/spec/features/hpv_vaccination_clinic_spec.rb
@@ -122,7 +122,7 @@ def then_i_see_that_the_status_is_vaccinated
   end
 
   def when_vaccination_confirmations_are_sent
-    VaccinationConfirmationsJob.perform_now
+    SendVaccinationConfirmationsJob.perform_now
   end
 
   def then_an_email_is_sent_to_the_parent_confirming_the_vaccination
diff --git a/spec/features/hpv_vaccination_delayed_spec.rb b/spec/features/hpv_vaccination_delayed_spec.rb
@@ -112,7 +112,7 @@ def then_i_see_the_patient_has_no_outcome_yet
   end
 
   def when_vaccination_confirmations_are_sent
-    VaccinationConfirmationsJob.perform_now
+    SendVaccinationConfirmationsJob.perform_now
   end
 
   def then_an_email_is_sent_to_the_parent_confirming_the_delay
diff --git a/spec/features/hpv_vaccination_offline_spec.rb b/spec/features/hpv_vaccination_offline_spec.rb
@@ -282,7 +282,7 @@ def and_the_clinic_location_is_displayed
   end
 
   def when_vaccination_confirmations_are_sent
-    VaccinationConfirmationsJob.perform_now
+    SendVaccinationConfirmationsJob.perform_now
   end
 
   def then_an_email_is_sent_to_the_parent_confirming_the_vaccination
diff --git a/spec/features/menacwy_vaccination_administered_spec.rb b/spec/features/menacwy_vaccination_administered_spec.rb
@@ -200,7 +200,7 @@ def and_i_see_the_vaccination_details
   end
 
   def when_vaccination_confirmations_are_sent
-    VaccinationConfirmationsJob.perform_now
+    SendVaccinationConfirmationsJob.perform_now
   end
 
   def then_an_email_is_sent_to_the_parent_confirming_the_vaccination
diff --git a/spec/features/scheduled_consent_requests_spec.rb b/spec/features/scheduled_consent_requests_spec.rb
@@ -102,14 +102,16 @@ def when_1_more_day_passes
   end
 
   def then_no_consent_requests_have_been_sent
-    SchoolConsentRequestsJob.perform_now
+    EnqueueSchoolConsentRequestsJob.perform_now
+    perform_enqueued_jobs
 
     expect(email_deliveries).to be_empty
     expect(sms_deliveries).to be_empty
   end
 
   def then_all_four_parents_received_consent_requests
-    SchoolConsentRequestsJob.perform_now
+    EnqueueSchoolConsentRequestsJob.perform_now
+    perform_enqueued_jobs
 
     expect_email_to(
       "parent1.child1@example.com",
diff --git a/spec/features/td_ipv_already_had_spec.rb b/spec/features/td_ipv_already_had_spec.rb
@@ -186,7 +186,8 @@ def and_i_cannot_record_the_patient_as_already_vaccinated
   end
 
   def and_the_consent_requests_are_sent
-    SchoolConsentRequestsJob.perform_now
+    EnqueueSchoolConsentRequestsJob.perform_now
+    perform_enqueued_jobs
   end
 
   def then_the_parent_doesnt_receive_a_consent_request
diff --git a/spec/features/td_ipv_vaccination_administered_spec.rb b/spec/features/td_ipv_vaccination_administered_spec.rb
@@ -201,7 +201,7 @@ def and_i_see_the_vaccination_details
   end
 
   def when_vaccination_confirmations_are_sent
-    VaccinationConfirmationsJob.perform_now
+    SendVaccinationConfirmationsJob.perform_now
   end
 
   def then_an_email_is_sent_to_the_parent_confirming_the_vaccination
diff --git a/spec/jobs/enqueue_clinic_session_invitations_job_spec.rb b/spec/jobs/enqueue_clinic_session_invitations_job_spec.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-describe ClinicSessionInvitationsJob do
+describe EnqueueClinicSessionInvitationsJob do
   subject(:perform_now) { described_class.perform_now }
 
   let(:programmes) { [create(:programme, :hpv)] }
diff --git a/spec/jobs/enqueue_school_consent_reminders_job_spec.rb b/spec/jobs/enqueue_school_consent_reminders_job_spec.rb
@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+describe EnqueueSchoolConsentRemindersJob do
+  subject(:perform_now) { described_class.perform_now }
+
+  let(:programmes) { [create(:programme)] }
+  let(:organisation) { create(:organisation, programmes:) }
+  let(:location) { create(:school, organisation:) }
+
+  let(:dates) { [Date.new(2024, 1, 12), Date.new(2024, 1, 15)] }
+
+  let!(:session) do
+    create(
+      :session,
+      dates:,
+      send_consent_requests_at: dates.first - 3.weeks,
+      days_before_consent_reminders: 7,
+      location:,
+      programmes:,
+      organisation:
+    )
+  end
+
+  around { |example| travel_to(today) { example.run } }
+
+  context "two weeks before the first session" do
+    let(:today) { dates.first - 2.weeks }
+
+    it "doesn't queue any jobs" do
+      expect { perform_now }.not_to have_enqueued_job(
+        SendSchoolConsentRemindersJob
+      )
+    end
+  end
+
+  context "one week before the first session" do
+    let(:today) { dates.first - 1.week }
+
+    it "queues a job for the session" do
+      expect { perform_now }.to have_enqueued_job(
+        SendSchoolConsentRemindersJob
+      ).with(session)
+    end
+
+    context "when location is a generic clinic" do
+      let(:location) { create(:generic_clinic, organisation:) }
+
+      it "doesn't queue any jobs" do
+        expect { perform_now }.not_to have_enqueued_job(
+          SendSchoolConsentRemindersJob
+        )
+      end
+    end
+  end
+
+  context "one week before the second session" do
+    let(:today) { dates.last - 1.week }
+
+    it "queues a job for the session" do
+      expect { perform_now }.to have_enqueued_job(
+        SendSchoolConsentRemindersJob
+      ).with(session)
+    end
+
+    context "when location is a generic clinic" do
+      let(:location) { create(:generic_clinic, organisation:) }
+
+      it "doesn't queue any jobs" do
+        expect { perform_now }.not_to have_enqueued_job(
+          SendSchoolConsentRemindersJob
+        )
+      end
+    end
+  end
+end
diff --git a/spec/jobs/enqueue_school_consent_requests_job_spec.rb b/spec/jobs/enqueue_school_consent_requests_job_spec.rb
diff --git a/spec/jobs/enqueue_update_patients_from_pds_job_spec.rb b/spec/jobs/enqueue_update_patients_from_pds_job_spec.rb
diff --git a/spec/jobs/send_school_consent_reminders_job_spec.rb b/spec/jobs/send_school_consent_reminders_job_spec.rb
diff --git a/spec/jobs/send_school_consent_requests_job_spec.rb b/spec/jobs/send_school_consent_requests_job_spec.rb
diff --git a/spec/jobs/send_school_session_reminders_job_spec.rb b/spec/jobs/send_school_session_reminders_job_spec.rb
diff --git a/spec/jobs/send_vaccination_confirmations_job_spec.rb b/spec/jobs/send_vaccination_confirmations_job_spec.rb
