Merge pull request #1978 from alphagov/add-job-to-schedule-weekly-deliveries

Add job to schedule weekly deliveries

Author: stephencdaly

app/jobs/schedule_daily_batch_deliveries_job.rb

@@ -10,7 +10,7 @@ def perform
     date = Time.zone.yesterday
     batch_begin_at = date.in_time_zone(TimeZoneUtils.submission_time_zone).beginning_of_day
 
-    DailySubmissionBatchSelector.batches(date).each do |batch|
+    BatchSubmissionsSelector.daily_batches(date).each do |batch|
       existing_deliveries = batch.submissions.first.deliveries.daily
       if existing_deliveries.any?
         Rails.logger.warn("Daily batch delivery already exists for batch - skipping", {

app/jobs/schedule_weekly_batch_deliveries_job.rb

@@ -0,0 +1,38 @@
+class ScheduleWeeklyBatchDeliveriesJob < ApplicationJob
+  # If we change the queue for this job, ensure we add a new alert in CloudWatch for failed executions
+  queue_as :submissions
+
+  def perform
+    CloudWatchService.record_job_started_metric(self.class.name)
+    CurrentJobLoggingAttributes.job_class = self.class.name
+    CurrentJobLoggingAttributes.job_id = job_id
+
+    batch_begin_at = 1.week.ago.in_time_zone(TimeZoneUtils.submission_time_zone).beginning_of_week(:monday)
+
+    BatchSubmissionsSelector.weekly_batches(batch_begin_at).each do |batch|
+      existing_deliveries = batch.submissions.first.deliveries.weekly
+      if existing_deliveries.any?
+        Rails.logger.warn("Weekly batch delivery already exists for batch - skipping", {
+          form_id: batch.form_id, mode: batch.mode, batch_begin_at:, delivery_id: existing_deliveries.first.id
+        })
+        next
+      end
+
+      delivery = Delivery.create!(
+        delivery_schedule: :weekly,
+        submissions: batch.submissions,
+        batch_begin_at:,
+      )
+
+      send_batch_job = SendSubmissionBatchJob.perform_later(delivery:)
+
+      Rails.logger.info("Scheduled SendSubmissionBatchJob to send weekly submission batch", {
+        form_id: batch.form_id,
+        mode: batch.mode,
+        batch_begin_date: batch_begin_at.to_date,
+        send_submission_batch_job_id: send_batch_job.job_id,
+        delivery_id: delivery.id,
+      })
+    end
+  end
+end

app/lib/batch_submissions_selector.rb

@@ -0,0 +1,53 @@
+class BatchSubmissionsSelector
+  Batch = Data.define(:form_id, :mode, :submissions)
+
+  class << self
+    def daily_batches(date)
+      Enumerator.new do |yielder|
+        form_ids_and_modes_with_send_daily_submission_batch(date).each do |form_id, mode|
+          submissions = Submission.for_form_and_mode(form_id, mode).on_day(date).order(created_at: :desc)
+
+          # If the send_daily_submission_batch is true for the most recent submission, include all submissions on that
+          # day in the batch. If it is false do not return a batch for any of the submissions on that day.
+          next unless submissions.any? && submissions.first.form_document["send_daily_submission_batch"] == true
+
+          yielder << Batch.new(form_id, mode, submissions)
+        end
+      end
+    end
+
+    def weekly_batches(time_in_week)
+      Enumerator.new do |yielder|
+        form_ids_and_modes_with_send_weekly_submission_batch(time_in_week).each do |form_id, mode|
+          submissions = Submission.for_form_and_mode(form_id, mode).in_week(time_in_week).order(created_at: :desc)
+
+          # If the send_weekly_submission_batch is true for the most recent submission, include all submissions in that
+          # week in the batch. If it is false do not return a batch for any of the submissions in that week.
+          next unless submissions.any? && submissions.first.form_document["send_weekly_submission_batch"] == true
+
+          yielder << Batch.new(form_id, mode, submissions)
+        end
+      end
+    end
+
+  private
+
+    def form_ids_and_modes_with_send_daily_submission_batch(date)
+      # Get all form_ids and modes that have at least one submission on the date with send_daily_submission_batch
+      # set to true.
+      Submission.on_day(date)
+                .where("(form_document->>'send_daily_submission_batch')::boolean = true")
+                .distinct
+                .pluck(:form_id, :mode)
+    end
+
+    def form_ids_and_modes_with_send_weekly_submission_batch(begin_at)
+      # Get all form_ids and modes that have at least one submission in the week with send_weekly_submission_batch
+      # set to true.
+      Submission.in_week(begin_at)
+                .where("(form_document->>'send_weekly_submission_batch')::boolean = true")
+                .distinct
+                .pluck(:form_id, :mode)
+    end
+  end
+end

app/lib/daily_submission_batch_selector.rb

@@ -11,6 +11,11 @@ class Submission < ApplicationRecord
     where(created_at: range)
   }
 
+  scope :in_week, lambda { |time_in_week|
+    range = time_in_week.in_time_zone(TimeZoneUtils.submission_time_zone).all_week(:monday)
+    where(created_at: range)
+  }
+
   scope :ordered_by_form_version_and_date, lambda {
     order(Arel.sql("(form_document->>'updated_at')::timestamptz ASC, created_at ASC"))
   }

app/models/submission.rb

@@ -14,6 +14,9 @@ production:
   schedule_daily_batch_deliveries_job:
     class: ScheduleDailyBatchDeliveriesJob
     schedule: every day at 2am Europe/London
+  schedule_weekly_batch_deliveries_job:
+    class: ScheduleWeeklyBatchDeliveriesJob
+    schedule: every Monday at 2:15am Europe/London
 development:
   delete_submissions_job:
     class: DeleteSubmissionsJob
@@ -30,3 +33,6 @@ development:
   schedule_daily_batch_deliveries_job:
     class: ScheduleDailyBatchDeliveriesJob
     schedule: every day at 2am Europe/London
+  schedule_weekly_batch_deliveries_job:
+    class: ScheduleWeeklyBatchDeliveriesJob
+    schedule: every Monday at 2:15am Europe/London

config/recurring.yml

@@ -11,8 +11,8 @@
   let(:other_form_submissions) { create_list(:submission, 1, form_id: other_form_id, mode: "preview-draft") }
   let!(:batches) do
     [
-      DailySubmissionBatchSelector::Batch.new(101, "form", form_submissions),
-      DailySubmissionBatchSelector::Batch.new(201, "preview-draft", other_form_submissions),
+      BatchSubmissionsSelector::Batch.new(101, "form", form_submissions),
+      BatchSubmissionsSelector::Batch.new(201, "preview-draft", other_form_submissions),
     ]
   end
 
@@ -23,7 +23,7 @@
   end
 
   before do
-    allow(DailySubmissionBatchSelector).to receive(:batches).and_return(batches.to_enum)
+    allow(BatchSubmissionsSelector).to receive(:daily_batches).and_return(batches.to_enum)
   end
 
   context "when Deliveries do not already exist for batches" do
@@ -32,7 +32,7 @@
     end
 
     it "calls the selector with yesterday's date" do
-      expect(DailySubmissionBatchSelector).to have_received(:batches).with(Time.zone.yesterday)
+      expect(BatchSubmissionsSelector).to have_received(:daily_batches).with(Time.zone.yesterday)
     end
 
     it "creates a delivery record per batch job" do

spec/jobs/schedule_daily_batch_deliveries_job_spec.rb

@@ -0,0 +1,132 @@
+require "rails_helper"
+
+RSpec.describe ScheduleWeeklyBatchDeliveriesJob do
+  include ActiveSupport::Testing::TimeHelpers
+  include ActiveJob::TestHelper
+
+  let(:travel_time) { Time.utc(2025, 5, 20, 2, 0, 0) }
+  let(:form_id) { 101 }
+  let(:other_form_id) { 201 }
+  let(:form_submissions) { create_list(:submission, 2, form_id: form_id, mode: "form") }
+  let(:other_form_submissions) { create_list(:submission, 1, form_id: other_form_id, mode: "preview-draft") }
+  let!(:batches) do
+    [
+      BatchSubmissionsSelector::Batch.new(101, "form", form_submissions),
+      BatchSubmissionsSelector::Batch.new(201, "preview-draft", other_form_submissions),
+    ]
+  end
+
+  around do |example|
+    travel_to travel_time do
+      example.run
+    end
+  end
+
+  before do
+    allow(BatchSubmissionsSelector).to receive(:weekly_batches).and_return(batches.to_enum)
+  end
+
+  context "when Deliveries do not already exist for batches" do
+    before do
+      described_class.perform_now
+    end
+
+    it "calls the selector passing in the start time of the previous week" do
+      expect(BatchSubmissionsSelector).to have_received(:weekly_batches).with(Time.utc(2025, 5, 11, 23, 0, 0))
+    end
+
+    it "creates a delivery record per batch job" do
+      expect(Delivery.weekly.count).to eq(2)
+      expect(Delivery.first.submissions.map(&:id)).to match_array(form_submissions.map(&:id))
+      expect(Delivery.second.submissions.map(&:id)).to match_array(other_form_submissions.map(&:id))
+    end
+
+    it "enqueues a SendSubmissionBatchJob per batch" do
+      expect(ActiveJob::Base.queue_adapter.enqueued_jobs.size).to eq(2)
+    end
+
+    it "enqueues the jobs with the correct args" do
+      enqueued_args = ActiveJob::Base.queue_adapter.enqueued_jobs.map { |j| j[:args].first }
+      expect(enqueued_args.first).to include("delivery" => hash_including("_aj_globalid"))
+      expect(locate_delivery(enqueued_args.first)).to eq(Delivery.first)
+
+      expect(enqueued_args.second).to include("delivery" => hash_including("_aj_globalid"))
+      expect(locate_delivery(enqueued_args.second)).to eq(Delivery.second)
+    end
+
+    describe "setting batch_begin_at" do
+      context "when the week for the batch is the week the clocks go forwards" do
+        let(:travel_time) { Time.utc(2025, 3, 31, 2, 0, 0) }
+
+        it "sets the batch_begin_at to the beginning of the week in GMT" do
+          expect(Delivery.first.batch_begin_at).to eq(Time.utc(2025, 3, 24, 0, 0, 0))
+        end
+      end
+
+      context "when the week for the batch is the week after the clocks have gone forwards" do
+        let(:travel_time) { Time.utc(2025, 4, 7, 2, 0, 0) }
+
+        it "sets the batch_begin_at to the beginning of the week in BST" do
+          expect(Delivery.first.batch_begin_at).to eq(Time.utc(2025, 3, 30, 23, 0, 0))
+        end
+      end
+
+      context "when the week for the batch is the week the clocks go back" do
+        let(:travel_time) { Time.zone.local(2025, 10, 27, 2, 0, 0) }
+
+        it "sets the batch_begin_at to the beginning of the week in BST" do
+          expect(Delivery.first.batch_begin_at).to eq(Time.utc(2025, 10, 19, 23, 0, 0))
+        end
+      end
+
+      context "when the week for the batch is the week after the clocks have gone back" do
+        let(:travel_time) { Time.utc(2025, 11, 3, 2, 0, 0) }
+
+        it "sets the batch_begin_at to the beginning of the week in GMT" do
+          expect(Delivery.first.batch_begin_at).to eq(Time.utc(2025, 10, 27, 0, 0, 0))
+        end
+      end
+    end
+  end
+
+  context "when a Delivery already exists for a batch" do
+    let!(:existing_delivery) { create(:delivery, delivery_schedule: :weekly, submissions: form_submissions) }
+
+    it "logs that the delivery will be skipped" do
+      expect(Rails.logger).to receive(:warn).with(
+        "Weekly batch delivery already exists for batch - skipping",
+        hash_including(
+          form_id: form_id,
+          mode: "form",
+          batch_begin_at: Time.utc(2025, 5, 11, 23, 0, 0),
+          delivery_id: existing_delivery.id,
+        ),
+      )
+
+      described_class.perform_now
+    end
+
+    it "only creates a delivery for the batch without an existing delivery" do
+      expect {
+        described_class.perform_now
+      }.to change(Delivery, :count).by(1)
+
+      expect(Delivery.last.submissions.map(&:id)).to match_array(other_form_submissions.map(&:id))
+    end
+
+    it "only schedules a job for the batch without an existing delivery" do
+      expect {
+        described_class.perform_now
+      }.to change { ActiveJob::Base.queue_adapter.enqueued_jobs.size }.by(1)
+
+      enqueued_args = ActiveJob::Base.queue_adapter.enqueued_jobs.map { |j| j[:args].first }
+      expect(enqueued_args.first).to include("delivery" => hash_including("_aj_globalid"))
+      expect(locate_delivery(enqueued_args.first)).to eq(Delivery.last)
+    end
+  end
+
+  def locate_delivery(enqueued_args)
+    gid_string = enqueued_args.dig("delivery", "_aj_globalid")
+    GlobalID::Locator.locate(gid_string)
+  end
+end