From e6ba3c1683fd6c98dae5eee28f340782d9f72c7e Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 18 Mar 2026 14:44:50 +0000 Subject: [PATCH 1/6] Rename DailySubmissionBatchSelector to BatchSubmissionsSelector Rename this class so we can use it for getting submissions for both daily and weekly batch deliveries. --- app/jobs/schedule_daily_batch_deliveries_job.rb | 2 +- ...on_batch_selector.rb => batch_submissions_selector.rb} | 2 +- spec/jobs/schedule_daily_batch_deliveries_job_spec.rb | 8 ++++---- ...elector_spec.rb => batch_submissions_selector_spec.rb} | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) rename app/lib/{daily_submission_batch_selector.rb => batch_submissions_selector.rb} (97%) rename spec/lib/{daily_submission_batch_selector_spec.rb => batch_submissions_selector_spec.rb} (99%) diff --git a/app/jobs/schedule_daily_batch_deliveries_job.rb b/app/jobs/schedule_daily_batch_deliveries_job.rb index 545d9112b..a2aed1c26 100644 --- a/app/jobs/schedule_daily_batch_deliveries_job.rb +++ b/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.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", { diff --git a/app/lib/daily_submission_batch_selector.rb b/app/lib/batch_submissions_selector.rb similarity index 97% rename from app/lib/daily_submission_batch_selector.rb rename to app/lib/batch_submissions_selector.rb index 7c075fccc..cb3246257 100644 --- a/app/lib/daily_submission_batch_selector.rb +++ b/app/lib/batch_submissions_selector.rb @@ -1,4 +1,4 @@ -class DailySubmissionBatchSelector +class BatchSubmissionsSelector Batch = Data.define(:form_id, :mode, :submissions) class << self diff --git a/spec/jobs/schedule_daily_batch_deliveries_job_spec.rb b/spec/jobs/schedule_daily_batch_deliveries_job_spec.rb index b49d0331e..a4e058a46 100644 --- a/spec/jobs/schedule_daily_batch_deliveries_job_spec.rb +++ b/spec/jobs/schedule_daily_batch_deliveries_job_spec.rb @@ -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(: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(:batches).with(Time.zone.yesterday) end it "creates a delivery record per batch job" do diff --git a/spec/lib/daily_submission_batch_selector_spec.rb b/spec/lib/batch_submissions_selector_spec.rb similarity index 99% rename from spec/lib/daily_submission_batch_selector_spec.rb rename to spec/lib/batch_submissions_selector_spec.rb index dde2c7b68..bcda9550b 100644 --- a/spec/lib/daily_submission_batch_selector_spec.rb +++ b/spec/lib/batch_submissions_selector_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -RSpec.describe DailySubmissionBatchSelector do +RSpec.describe BatchSubmissionsSelector do let(:form_document_with_batch_enabled) { create(:v2_form_document, send_daily_submission_batch: true) } let(:form_document_with_batch_disabled) { create(:v2_form_document, send_daily_submission_batch: false) } From 1eafc25ad036ee787c08eb7d7e2df91ec2c595e2 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 18 Mar 2026 15:16:02 +0000 Subject: [PATCH 2/6] Rename BatchSubmissionsSelector#batches to daily_batches We will add a separate method to get weekly batches, so rename the existing method. --- .../schedule_daily_batch_deliveries_job.rb | 2 +- app/lib/batch_submissions_selector.rb | 2 +- ...chedule_daily_batch_deliveries_job_spec.rb | 4 +-- spec/lib/batch_submissions_selector_spec.rb | 26 +++++++++---------- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/app/jobs/schedule_daily_batch_deliveries_job.rb b/app/jobs/schedule_daily_batch_deliveries_job.rb index a2aed1c26..8262ee796 100644 --- a/app/jobs/schedule_daily_batch_deliveries_job.rb +++ b/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 - BatchSubmissionsSelector.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", { diff --git a/app/lib/batch_submissions_selector.rb b/app/lib/batch_submissions_selector.rb index cb3246257..8ee978df9 100644 --- a/app/lib/batch_submissions_selector.rb +++ b/app/lib/batch_submissions_selector.rb @@ -2,7 +2,7 @@ class BatchSubmissionsSelector Batch = Data.define(:form_id, :mode, :submissions) class << self - def batches(date) + 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) diff --git a/spec/jobs/schedule_daily_batch_deliveries_job_spec.rb b/spec/jobs/schedule_daily_batch_deliveries_job_spec.rb index a4e058a46..71a553036 100644 --- a/spec/jobs/schedule_daily_batch_deliveries_job_spec.rb +++ b/spec/jobs/schedule_daily_batch_deliveries_job_spec.rb @@ -23,7 +23,7 @@ end before do - allow(BatchSubmissionsSelector).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(BatchSubmissionsSelector).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 diff --git a/spec/lib/batch_submissions_selector_spec.rb b/spec/lib/batch_submissions_selector_spec.rb index bcda9550b..40f77f424 100644 --- a/spec/lib/batch_submissions_selector_spec.rb +++ b/spec/lib/batch_submissions_selector_spec.rb @@ -7,11 +7,11 @@ let(:date) { Time.zone.local(2022, 12, 1) } let(:form_id) { 101 } - describe ".batches" do - subject(:batches) { described_class.batches(date) } + describe ".daily_batches" do + subject(:daily_batches) { described_class.daily_batches(date) } it "returns an enumerator" do - expect(batches).to be_an(Enumerator) + expect(daily_batches).to be_an(Enumerator) end context "when send_daily_submissions_batch is enabled for the form document" do @@ -36,15 +36,15 @@ end it "includes only forms/modes with submissions on the date and their submissions" do - expect(batches.map(&:to_h)).to contain_exactly( + expect(daily_batches.map(&:to_h)).to contain_exactly( a_hash_including(form_id: form_id, mode: "form"), a_hash_including(form_id: form_id, mode: "preview-draft"), ) end it "includes only submissions on the day in the batches" do - expect(batches.to_a[0].submissions.pluck(:reference)).to contain_exactly(form_submission.reference) - expect(batches.to_a[1].submissions.pluck(:reference)).to contain_exactly(preview_draft_submission.reference) + expect(daily_batches.to_a[0].submissions.pluck(:reference)).to contain_exactly(form_submission.reference) + expect(daily_batches.to_a[1].submissions.pluck(:reference)).to contain_exactly(preview_draft_submission.reference) end end @@ -69,15 +69,15 @@ end it "includes only forms/modes with submissions on the date" do - expect(batches.map(&:to_h)).to contain_exactly( + expect(daily_batches.map(&:to_h)).to contain_exactly( a_hash_including(form_id: form_id, mode: "form"), a_hash_including(form_id: form_id, mode: "preview-draft"), ) end it "includes only submissions on the day in the batches" do - expect(batches.to_a[0].submissions.pluck(:reference)).to contain_exactly(form_submission.reference) - expect(batches.to_a[1].submissions.pluck(:reference)).to contain_exactly(preview_draft_submission.reference) + expect(daily_batches.to_a[0].submissions.pluck(:reference)).to contain_exactly(form_submission.reference) + expect(daily_batches.to_a[1].submissions.pluck(:reference)).to contain_exactly(preview_draft_submission.reference) end end end @@ -93,13 +93,13 @@ end it "includes a batch for the form and mode" do - expect(batches.map(&:to_h)).to contain_exactly( + expect(daily_batches.map(&:to_h)).to contain_exactly( a_hash_including(form_id: form_id, mode: "form"), ) end it "includes all the submissions in the batch" do - submissions = batches.first.submissions + submissions = daily_batches.first.submissions expect(submissions.pluck(:reference)).to contain_exactly(latest_submission.reference, earlier_submission.reference) end end @@ -110,7 +110,7 @@ end it "does not include a batch for the form and mode" do - expect(batches.to_a).to be_empty + expect(daily_batches.to_a).to be_empty end end @@ -121,7 +121,7 @@ end it "does not include a batch for the form and mode" do - expect(batches.to_a).to be_empty + expect(daily_batches.to_a).to be_empty end end end From ca3a9017d2133c1d206df62e6f89063776751701 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 18 Mar 2026 16:20:20 +0000 Subject: [PATCH 3/6] Add scope to get submissions created in a week Use the `.all_week` method to get the date range for the week, ensuring it is in the London timezone. For now, a week for batching will always start at the beginning of the day on a Monday. If this later becomes configurable, we will need to update this implementation. --- app/models/submission.rb | 5 +++ spec/models/submission_spec.rb | 60 ++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/app/models/submission.rb b/app/models/submission.rb index 3120c448a..7ad08edce 100644 --- a/app/models/submission.rb +++ b/app/models/submission.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")) } diff --git a/spec/models/submission_spec.rb b/spec/models/submission_spec.rb index 8bc129a18..1969a9bce 100644 --- a/spec/models/submission_spec.rb +++ b/spec/models/submission_spec.rb @@ -85,6 +85,66 @@ end end + describe ".in_week" do + context "when the date is around the start of BST" do + let!(:gmt_monday_submission) { create(:submission, created_at: Time.utc(2025, 3, 17, 0, 0, 0)) } + let!(:gmt_sunday_submission) { create(:submission, created_at: Time.utc(2025, 3, 23, 23, 59, 59)) } + + let!(:clock_change_week_monday_submission) { create(:submission, created_at: Time.utc(2025, 3, 24, 0, 0, 0)) } + let!(:clock_change_week_sunday_submission) { create(:submission, created_at: Time.utc(2025, 3, 30, 22, 59, 59)) } + + let!(:bst_monday_submission) { create(:submission, created_at: Time.utc(2025, 3, 30, 23, 0, 0)) } + let!(:bst_sunday_submission) { create(:submission, created_at: Time.utc(2025, 4, 6, 22, 59, 59)) } + + it "returns the submissions for the week before the clocks change" do + submissions = described_class.in_week(Time.utc(2025, 3, 17, 0, 0, 0)) + expect(submissions.size).to eq(2) + expect(submissions).to contain_exactly(gmt_monday_submission, gmt_sunday_submission) + end + + it "returns the submissions for the week of the clocks change" do + submissions = described_class.in_week(Time.utc(2025, 3, 24, 0, 0, 0)) + expect(submissions.size).to eq(2) + expect(submissions).to contain_exactly(clock_change_week_monday_submission, clock_change_week_sunday_submission) + end + + it "returns the submissions for the week after the clocks change" do + submissions = described_class.in_week(Time.utc(2025, 3, 30, 23, 0, 0)) + expect(submissions.size).to eq(2) + expect(submissions).to contain_exactly(bst_monday_submission, bst_sunday_submission) + end + end + + context "when the date is around the end of BST" do + let!(:bst_monday_submission) { create(:submission, created_at: Time.utc(2025, 10, 12, 23, 0, 0)) } + let!(:bst_sunday_submission) { create(:submission, created_at: Time.utc(2025, 10, 19, 22, 59, 59)) } + + let!(:clock_change_week_monday_submission) { create(:submission, created_at: Time.utc(2025, 10, 19, 23, 0, 0)) } + let!(:clock_change_week_sunday_submission) { create(:submission, created_at: Time.utc(2025, 10, 26, 23, 59, 59)) } + + let!(:gmt_monday_submission) { create(:submission, created_at: Time.utc(2025, 10, 27, 0, 0, 0)) } + let!(:gmt_sunday_submission) { create(:submission, created_at: Time.utc(2025, 11, 2, 23, 59, 59)) } + + it "returns the submissions for the week before the clocks change" do + submissions = described_class.in_week(Time.utc(2025, 10, 13, 23, 0, 0)) + expect(submissions.size).to eq(2) + expect(submissions).to contain_exactly(bst_monday_submission, bst_sunday_submission) + end + + it "returns the submissions for the week of the clocks change" do + submissions = described_class.in_week(Time.utc(2025, 10, 20, 23, 0, 0)) + expect(submissions.size).to eq(2) + expect(submissions).to contain_exactly(clock_change_week_monday_submission, clock_change_week_sunday_submission) + end + + it "returns the submissions for the week after the clocks change" do + submissions = described_class.in_week(Time.utc(2025, 10, 27, 0, 0, 0)) + expect(submissions.size).to eq(2) + expect(submissions).to contain_exactly(gmt_monday_submission, gmt_sunday_submission) + end + end + end + describe ".ordered_by_form_version_and_date" do let(:first_form_version) { create :v2_form_document, updated_at: Time.utc(2022, 6, 1, 12, 0, 0) } let(:second_form_version) { create :v2_form_document, updated_at: Time.utc(2022, 12, 1, 12, 0, 0) } From fa3707f6f42c7fc84a207b358cc17541f3e59643 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 18 Mar 2026 16:27:59 +0000 Subject: [PATCH 4/6] Add method to get all weekly batches for a given date Add a method, that given a date, will return batches of submissions per form and mode where the form has `send_weekly_submission_batch` set to true for the final submission in the week. Include all submissions for the week in the batch, even if they were submitted for an older version of the form that did not have `send_weekly_submission_batch` set to true. --- app/lib/batch_submissions_selector.rb | 23 +++ spec/lib/batch_submissions_selector_spec.rb | 147 ++++++++++++++++++-- 2 files changed, 159 insertions(+), 11 deletions(-) diff --git a/app/lib/batch_submissions_selector.rb b/app/lib/batch_submissions_selector.rb index 8ee978df9..99efa7268 100644 --- a/app/lib/batch_submissions_selector.rb +++ b/app/lib/batch_submissions_selector.rb @@ -16,6 +16,20 @@ def daily_batches(date) 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) @@ -26,5 +40,14 @@ def form_ids_and_modes_with_send_daily_submission_batch(date) .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 diff --git a/spec/lib/batch_submissions_selector_spec.rb b/spec/lib/batch_submissions_selector_spec.rb index 40f77f424..f76abfc1d 100644 --- a/spec/lib/batch_submissions_selector_spec.rb +++ b/spec/lib/batch_submissions_selector_spec.rb @@ -1,20 +1,20 @@ require "rails_helper" RSpec.describe BatchSubmissionsSelector do - let(:form_document_with_batch_enabled) { create(:v2_form_document, send_daily_submission_batch: true) } - let(:form_document_with_batch_disabled) { create(:v2_form_document, send_daily_submission_batch: false) } - - let(:date) { Time.zone.local(2022, 12, 1) } let(:form_id) { 101 } describe ".daily_batches" do subject(:daily_batches) { described_class.daily_batches(date) } + let(:date) { Time.zone.local(2022, 12, 1) } + let(:form_document_with_batch_enabled) { create(:v2_form_document, send_daily_submission_batch: true) } + let(:form_document_with_batch_disabled) { create(:v2_form_document, send_daily_submission_batch: false) } + it "returns an enumerator" do expect(daily_batches).to be_an(Enumerator) end - context "when send_daily_submissions_batch is enabled for the form document" do + context "when send_daily_submission_batch is enabled for the form document" do context "when the date is during BST" do let(:date) { Time.zone.local(2022, 6, 1) } @@ -35,7 +35,7 @@ create(:submission, form_id: form_id, mode: "form", reference: "OMITTED4", created_at: Time.utc(2022, 6, 1, 23, 0, 0), form_document: form_document_with_batch_enabled) end - it "includes only forms/modes with submissions on the date and their submissions" do + it "includes only forms/modes with submissions on the date" do expect(daily_batches.map(&:to_h)).to contain_exactly( a_hash_including(form_id: form_id, mode: "form"), a_hash_including(form_id: form_id, mode: "preview-draft"), @@ -82,9 +82,7 @@ end end - context "when send_daily_submissions_batch is enabled part-way through the day for the form document" do - let(:other_form_id) { 102 } - + context "when send_daily_submission_batch is enabled part-way through the day for the form document" do let!(:latest_submission) do create(:submission, form_id: form_id, mode: "form", reference: "INCLUDED1", created_at: Time.utc(2022, 12, 1, 10, 0, 0), form_document: form_document_with_batch_enabled) end @@ -104,7 +102,7 @@ end end - context "when send_daily_submissions_batch is disabled for the form document" do + context "when send_daily_submission_batch is disabled for the form document" do before do create(:submission, form_id: form_id, mode: "form", created_at: Time.utc(2022, 12, 1, 10, 0, 0), form_document: form_document_with_batch_disabled) end @@ -114,7 +112,7 @@ end end - context "when send_daily_submissions_batch is disabled part-way through the day for the form document" do + context "when send_daily_submission_batch is disabled part-way through the day for the form document" do before do create(:submission, form_id: form_id, mode: "form", created_at: Time.utc(2022, 12, 1, 10, 0, 0), form_document: form_document_with_batch_disabled) create(:submission, form_id: form_id, mode: "form", created_at: Time.utc(2022, 12, 1, 9, 0, 0), form_document: form_document_with_batch_enabled) @@ -125,4 +123,131 @@ end end end + + describe ".weekly_batches" do + subject(:weekly_batches) { described_class.weekly_batches(date) } + + let(:date) { Time.zone.local(2025, 5, 19) } + let(:form_document_with_batch_enabled) { create(:v2_form_document, send_weekly_submission_batch: true) } + let(:form_document_with_batch_disabled) { create(:v2_form_document, send_weekly_submission_batch: false) } + + it "returns an enumerator" do + expect(weekly_batches).to be_an(Enumerator) + end + + context "when send_weekly_submission_batch is enabled for the form document" do + context "when the week is during BST" do + let(:date) { Time.zone.local(2025, 5, 19) } + + let!(:form_submission) do + create(:submission, form_id: form_id, mode: "form", reference: "INCLUDED1", created_at: Time.utc(2025, 5, 18, 23, 0, 0), form_document: form_document_with_batch_enabled) + end + let!(:preview_draft_submission) do + create(:submission, form_id: form_id, mode: "preview-draft", reference: "INCLUDED2", created_at: Time.utc(2025, 5, 25, 22, 59, 59), form_document: form_document_with_batch_enabled) + end + + before do + # create form/mode combinations that only have submissions outside the BST week + create(:submission, form_id: form_id, mode: "preview-archived", reference: "OMITTED1", created_at: Time.utc(2025, 5, 18, 22, 59, 59), form_document: form_document_with_batch_enabled) + create(:submission, form_id: 102, mode: "form", reference: "OMITTED2", created_at: Time.utc(2025, 5, 25, 23, 0, 0), form_document: form_document_with_batch_enabled) + + # create submissions for the form/mode included in a batch outside the BST week to ensure they are excluded + create(:submission, form_id: form_id, mode: "form", reference: "OMITTED3", created_at: Time.utc(2025, 5, 18, 22, 59, 59), form_document: form_document_with_batch_enabled) + create(:submission, form_id: form_id, mode: "form", reference: "OMITTED4", created_at: Time.utc(2025, 5, 25, 23, 0, 0), form_document: form_document_with_batch_enabled) + end + + it "includes only forms/modes with submissions in the week" do + expect(weekly_batches.map(&:to_h)).to contain_exactly( + a_hash_including(form_id: form_id, mode: "form"), + a_hash_including(form_id: form_id, mode: "preview-draft"), + ) + end + + it "includes only submissions in the week in the batches" do + expect(weekly_batches.to_a[0].submissions.pluck(:reference)).to contain_exactly(form_submission.reference) + expect(weekly_batches.to_a[1].submissions.pluck(:reference)).to contain_exactly(preview_draft_submission.reference) + end + end + + context "when the week is not in BST" do + let(:date) { Time.zone.local(2025, 11, 3) } + + let!(:form_submission) do + create(:submission, form_id: form_id, mode: "form", reference: "INCLUDED1", created_at: Time.utc(2025, 11, 3, 0, 0, 0), form_document: form_document_with_batch_enabled) + end + let!(:preview_draft_submission) do + create(:submission, form_id: form_id, mode: "preview-draft", reference: "INCLUDED2", created_at: Time.utc(2025, 11, 9, 23, 59, 59), form_document: form_document_with_batch_enabled) + end + + before do + # create form/mode combinations that only have submissions outside the week + create(:submission, form_id: form_id, mode: "preview-archived", reference: "OMITTED1", created_at: Time.utc(2025, 11, 2, 23, 59, 59), form_document: form_document_with_batch_enabled) + create(:submission, form_id: 102, mode: "form", reference: "OMITTED2", created_at: Time.utc(2025, 11, 10, 0, 0, 0), form_document: form_document_with_batch_enabled) + + # create submissions for the form/mode included in a batch outside the week to ensure they are excluded + create(:submission, form_id: form_id, mode: "form", reference: "OMITTED3", created_at: Time.utc(2025, 11, 2, 23, 59, 59), form_document: form_document_with_batch_enabled) + create(:submission, form_id: form_id, mode: "form", reference: "OMITTED4", created_at: Time.utc(2025, 11, 10, 0, 0, 0), form_document: form_document_with_batch_enabled) + end + + it "includes only forms/modes with submissions in the week" do + expect(weekly_batches.map(&:to_h)).to contain_exactly( + a_hash_including(form_id: form_id, mode: "form"), + a_hash_including(form_id: form_id, mode: "preview-draft"), + ) + end + + it "includes only submissions in the week in the batches" do + expect(weekly_batches.to_a[0].submissions.pluck(:reference)).to contain_exactly(form_submission.reference) + expect(weekly_batches.to_a[1].submissions.pluck(:reference)).to contain_exactly(preview_draft_submission.reference) + end + end + end + + context "when send_weekly_submission_batch is enabled part-way through the week for the form document" do + let(:date) { Time.zone.local(2025, 11, 3) } + + let!(:latest_submission) do + create(:submission, form_id: form_id, mode: "form", reference: "INCLUDED1", created_at: Time.utc(2025, 11, 5, 0, 0, 0), form_document: form_document_with_batch_enabled) + end + let!(:earlier_submission) do + create(:submission, form_id: form_id, mode: "form", reference: "INCLUDED2", created_at: Time.utc(2025, 11, 4, 0, 0, 0), form_document: form_document_with_batch_disabled) + end + + it "includes a batch for the form and mode" do + expect(weekly_batches.map(&:to_h)).to contain_exactly( + a_hash_including(form_id: form_id, mode: "form"), + ) + end + + it "includes all the submissions in the batch" do + submissions = weekly_batches.first.submissions + expect(submissions.pluck(:reference)).to contain_exactly(latest_submission.reference, earlier_submission.reference) + end + end + + context "when send_weekly_submission_batch is disabled for the form document" do + let(:date) { Time.zone.local(2025, 11, 3) } + + before do + create(:submission, form_id: form_id, mode: "form", created_at: Time.utc(2025, 11, 5, 10, 0, 0), form_document: form_document_with_batch_disabled) + end + + it "does not include a batch for the form and mode" do + expect(weekly_batches.to_a).to be_empty + end + end + + context "when send_weekly_submission_batch is disabled part-way through the week for the form document" do + let(:date) { Time.zone.local(2025, 11, 3) } + + before do + create(:submission, form_id: form_id, mode: "form", created_at: Time.utc(2025, 11, 5, 0, 0, 0), form_document: form_document_with_batch_disabled) + create(:submission, form_id: form_id, mode: "form", created_at: Time.utc(2025, 11, 4, 0, 0, 0), form_document: form_document_with_batch_enabled) + end + + it "does not include a batch for the form and mode" do + expect(weekly_batches.to_a).to be_empty + end + end + end end From d3d8550b0c55d42503d481e3be94aa75def95bd7 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Thu, 19 Mar 2026 11:23:11 +0000 Subject: [PATCH 5/6] Add job to schedule weekly deliveries Add a job that will be run on a recurring schedule that finds all form_ids and modes to send submission batches for the previous week. For each form_id and mode, create a Delivery, associating the submissions with it, and schedule a SendSubmissionBatchJob to send the email. --- .../schedule_weekly_batch_deliveries_job.rb | 38 +++++ ...hedule_weekly_batch_deliveries_job_spec.rb | 132 ++++++++++++++++++ 2 files changed, 170 insertions(+) create mode 100644 app/jobs/schedule_weekly_batch_deliveries_job.rb create mode 100644 spec/jobs/schedule_weekly_batch_deliveries_job_spec.rb diff --git a/app/jobs/schedule_weekly_batch_deliveries_job.rb b/app/jobs/schedule_weekly_batch_deliveries_job.rb new file mode 100644 index 000000000..f8f6c308f --- /dev/null +++ b/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 diff --git a/spec/jobs/schedule_weekly_batch_deliveries_job_spec.rb b/spec/jobs/schedule_weekly_batch_deliveries_job_spec.rb new file mode 100644 index 000000000..d0d222cdb --- /dev/null +++ b/spec/jobs/schedule_weekly_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 From b88eb1c12a67d67afa1b832d035407180f6bb2d5 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Thu, 19 Mar 2026 11:55:14 +0000 Subject: [PATCH 6/6] Add recurring schedule for weekly batch scheduler job Schedule the job to run every Monday at 2:15 AM London time. We tell users that we will run the job shortly after 2:00 AM. We're scheduling it to run a little while after the job to schedule the daily batches has run in order to spread the load on the system. --- config/recurring.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/recurring.yml b/config/recurring.yml index 28986e8d9..d1e7f7c78 100644 --- a/config/recurring.yml +++ b/config/recurring.yml @@ -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