Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/jobs/schedule_daily_batch_deliveries_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand Down
38 changes: 38 additions & 0 deletions app/jobs/schedule_weekly_batch_deliveries_job.rb
Original file line number Diff line number Diff line change
@@ -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
53 changes: 53 additions & 0 deletions app/lib/batch_submissions_selector.rb
Original file line number Diff line number Diff line change
@@ -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
30 changes: 0 additions & 30 deletions app/lib/daily_submission_batch_selector.rb

This file was deleted.

5 changes: 5 additions & 0 deletions app/models/submission.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
Expand Down
6 changes: 6 additions & 0 deletions config/recurring.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
8 changes: 4 additions & 4 deletions spec/jobs/schedule_daily_batch_deliveries_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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
Expand Down
132 changes: 132 additions & 0 deletions spec/jobs/schedule_weekly_batch_deliveries_job_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Loading
Loading