Merge pull request #1901 from alphagov/add-batched-submissions-job

SamJamCul · web-flow · commit 5812bedfea08 · 2026-02-19T14:41:17.000Z
Add job to send a batch of submissions for a form
diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb
@@ -7,4 +7,12 @@ def set_submission_logging_attributes(submission)
     CurrentJobLoggingAttributes.submission_reference = submission.reference
     CurrentJobLoggingAttributes.preview = submission.preview?
   end
+
+  def set_submission_batch_logging_attributes(form:, mode:)
+    CurrentJobLoggingAttributes.job_class = self.class.name
+    CurrentJobLoggingAttributes.job_id = job_id
+    CurrentJobLoggingAttributes.form_id = form.id
+    CurrentJobLoggingAttributes.form_name = form.name
+    CurrentJobLoggingAttributes.preview = mode.preview?
+  end
 end
diff --git a/app/jobs/send_submission_batch_job.rb b/app/jobs/send_submission_batch_job.rb
@@ -0,0 +1,51 @@
+class SendSubmissionBatchJob < ApplicationJob
+  queue_as :submissions
+
+  # this translates to approximately 4.5 hours of retrying in total
+  TOTAL_ATTEMPTS = 10
+
+  retry_on Aws::SESV2::Errors::ServiceError, wait: :polynomially_longer, attempts: TOTAL_ATTEMPTS
+
+  def perform(form_id:, mode_string:, date:, delivery:)
+    submissions = Submission.for_daily_batch(form_id, date, mode_string)
+
+    if submissions.empty?
+      Rails.logger.info("No submissions to batch for form_id: #{form_id}, mode: #{mode_string}, date: #{date}")
+      return
+    end
+
+    form = submissions.first.form
+    mode = Mode.new(mode_string)
+    set_submission_batch_logging_attributes(form:, mode:)
+
+    if form.submission_email.blank?
+      if mode.preview?
+        Rails.logger.info "Skipping sending batch for preview submissions, as the submission email address has not been set"
+        return
+      else
+        raise StandardError, "Form id: #{form.id} is missing a submission email address"
+      end
+    end
+
+    message_id = AwsSesSubmissionBatchService.new(submissions_query: submissions, form:, date:, mode:).send_batch
+
+    delivery.update!(
+      delivery_reference: message_id,
+      last_attempt_at: Time.zone.now,
+      submissions: submissions,
+    )
+
+    EventLogger.log_form_event("daily_batch_email_sent", {
+      mode:,
+      batch_date: date,
+      number_of_submissions: submissions.count,
+    })
+
+    submissions.each do |submission|
+      EventLogger.log_form_event("included_in_daily_batch_email", {
+        submission_reference: submission.reference,
+        batch_date: date,
+      })
+    end
+  end
+end
diff --git a/app/lib/csv_generator.rb b/app/lib/csv_generator.rb
@@ -9,12 +9,22 @@ def generate_submission(submission:, is_s3_submission:)
       end
     end
 
-    def generate_batched_submissions(submissions:, is_s3_submission:)
-      CSV.generate do |csv|
-        csv << headers(submissions.first, is_s3_submission)
-        submissions.each do |submission|
-          csv << values_for_submission(submission, is_s3_submission)
+    def generate_batched_submissions(submissions_query:, is_s3_submission:)
+      sorted_submissions = submissions_query.ordered_by_form_version_and_date
+
+      rows_by_version = []
+
+      sorted_submissions.each do |submission|
+        current_headers = rows_by_version.last&.first
+        unless current_headers == headers(submission, is_s3_submission)
+          # Start a new CSV if the headers are different to the previous submission
+          rows_by_version << [headers(submission, is_s3_submission)]
         end
+        rows_by_version.last << values_for_submission(submission, is_s3_submission)
+      end
+
+      rows_by_version.map do |rows|
+        CSV.generate { |csv| rows.each { |line| csv << line } }
       end
     end
 
diff --git a/app/models/submission.rb b/app/models/submission.rb
@@ -1,9 +1,18 @@
 class Submission < ApplicationRecord
-  include TimeZoneUtils
-
   has_many :submission_deliveries, dependent: :destroy
   has_many :deliveries, through: :submission_deliveries
 
+  scope :for_daily_batch, lambda { |form_id, date, mode|
+    start_time = date.in_time_zone(TimeZoneUtils.submission_time_zone).beginning_of_day
+    end_time = start_time.end_of_day
+
+    where(form_id:, created_at: start_time..end_time, mode: mode)
+  }
+
+  scope :ordered_by_form_version_and_date, lambda {
+    order(Arel.sql("(form_document->>'updated_at')::timestamptz ASC, created_at ASC"))
+  }
+
   delegate :preview?, to: :mode_object
 
   encrypts :answers
@@ -17,7 +26,7 @@ def form
   end
 
   def submission_time
-    created_at.in_time_zone(submission_time_zone)
+    created_at.in_time_zone(TimeZoneUtils.submission_time_zone)
   end
 
   def payment_url
@@ -33,25 +42,6 @@ def self.sent?(reference)
     submission&.single_submission_delivery&.delivery_reference&.present?
   end
 
-  def self.group_by_form_version(submissions)
-    submission_by_version = {}
-    last_version = nil
-
-    # For forms that have the same updated_at timestamp, we know they will be identical. If two forms have different
-    # updated_at timestamps, we check to see if their steps are the same. If they are, we group those forms' submissions
-    # together.
-    submissions.group_by { |submission| submission.form.updated_at }.sort.to_h.each do |updated_at, submissions|
-      if last_version && last_version.steps == submissions.first.form.steps
-        submission_by_version[last_version.updated_at].push(*submissions)
-      else
-        submission_by_version[updated_at] = submissions
-        last_version = submissions.first.form
-      end
-    end
-
-    submission_by_version
-  end
-
 private
 
   def mode_object
diff --git a/app/services/aws_ses_submission_batch_service.rb b/app/services/aws_ses_submission_batch_service.rb
@@ -1,34 +1,19 @@
 class AwsSesSubmissionBatchService
-  def initialize(submissions:, form:, date:, mode:)
-    @submissions = submissions
+  def initialize(submissions_query:, form:, date:, mode:)
+    @submissions_query = submissions_query
     @form = form
     @date = date
     @mode = mode
   end
 
   def send_batch
-    if @form.submission_email.blank?
-      if @mode.preview?
-        Rails.logger.info "Skipping sending batch for preview submissions, as the submission email address has not been set"
-        return
-      else
-        raise StandardError, "Form id: #{@form.id} is missing a submission email address"
-      end
-    end
-
-    deliver_batch_email
-  end
-
-private
-
-  def deliver_batch_email
     files = {}
 
-    submissions_by_version = Submission.group_by_form_version(@submissions)
-    submissions_by_version.each_value.with_index(1) do |submissions, version_number|
-      form_version = submissions_by_version.size > 1 ? version_number : nil
-      filename = SubmissionFilenameGenerator.batch_csv_filename(form_name: @form.name, date: @date, mode: @mode, form_version: form_version)
-      files[filename] = CsvGenerator.generate_batched_submissions(submissions: submissions, is_s3_submission: false)
+    csvs = CsvGenerator.generate_batched_submissions(submissions_query: @submissions_query, is_s3_submission: false)
+    csvs.each.with_index(1) do |csv, index|
+      csv_version = csvs.size > 1 ? index : nil
+      filename = SubmissionFilenameGenerator.batch_csv_filename(form_name: @form.name, date: @date, mode: @mode, form_version: csv_version)
+      files[filename] = csv
     end
 
     mail = AwsSesSubmissionBatchMailer.batch_submission_email(form: @form, date: @date, mode: @mode, files:).deliver_now
diff --git a/db/migrate/20260218110428_add_index_on_created_at_and_form_id_and_mode_to_submissions.rb b/db/migrate/20260218110428_add_index_on_created_at_and_form_id_and_mode_to_submissions.rb
@@ -0,0 +1,5 @@
+class AddIndexOnCreatedAtAndFormIdAndModeToSubmissions < ActiveRecord::Migration[8.1]
+  def change
+    add_index :submissions, %i[created_at form_id mode]
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
diff --git a/lib/time_zone_utils.rb b/lib/time_zone_utils.rb
@@ -1,5 +1,5 @@
 module TimeZoneUtils
-  def submission_time_zone
+  def self.submission_time_zone
     Rails.configuration.x.submission.time_zone || "UTC"
   end
 end
diff --git a/spec/factories/submissions.rb b/spec/factories/submissions.rb
@@ -14,7 +14,7 @@
         },
       }
     end
-    mode { is_preview ? "preview-live" : "live" }
+    mode { is_preview ? "preview-live" : "form" }
     form_document { build :v2_form_document, form_id: }
     submission_locale { :en }
 
diff --git a/spec/factories/v2_form_document.rb b/spec/factories/v2_form_document.rb
@@ -32,7 +32,7 @@
       end
 
       steps do
-        Array.new(steps_count) { attributes_for(:v2_step) }
+        Array.new(steps_count) { attributes_for(:v2_question_page_step) }
       end
 
       question_section_completed { true }
diff --git a/spec/factories/v2_step.rb b/spec/factories/v2_step.rb
@@ -86,6 +86,25 @@
         end
       end
 
+      trait :with_name_settings do
+        transient do
+          input_type { "first_and_last_name" }
+          title_needed { "false" }
+        end
+
+        answer_type { "name" }
+        answer_settings do
+          DataStruct.new(
+            input_type:,
+            title_needed:,
+          )
+        end
+      end
+
+      trait :with_file_upload_settings do
+        answer_type { "file" }
+      end
+
       trait :with_repeatable do
         is_repeatable { true }
       end
diff --git a/spec/jobs/send_submission_batch_job_spec.rb b/spec/jobs/send_submission_batch_job_spec.rb
@@ -0,0 +1,110 @@
+require "rails_helper"
+
+# rubocop:disable RSpec/InstanceVariable
+RSpec.describe SendSubmissionBatchJob, type: :job do
+  include ActiveJob::TestHelper
+
+  let(:mode_string) { "form" }
+  let(:date) { Date.new(2022, 12, 14) }
+  let(:delivery) { create(:delivery, delivery_schedule: "daily") }
+
+  let(:form_document) { create(:v2_form_document, :with_steps, name: "My Form", submission_email:) }
+  let(:submission_email) { "to@example.com" }
+  let(:form_id) { form_document.form_id }
+  let(:submissions) { [] }
+
+  before do
+    submissions
+    ActionMailer::Base.deliveries.clear
+    described_class.perform_later(form_id:, mode_string:, date:, delivery:)
+  end
+
+  context "when there are no submissions" do
+    before do
+      perform_enqueued_jobs
+    end
+
+    it "does not send an email" do
+      expect(ActionMailer::Base.deliveries).to be_empty
+    end
+
+    it "does not update the delivery" do
+      expect(delivery.reload.last_attempt_at).to be_nil
+    end
+  end
+
+  context "when there are submissions" do
+    let(:submissions_to_include) do
+      create_list(
+        :submission,
+        3,
+        form_document:,
+        form_id:,
+        mode: mode_string,
+        created_at: date.beginning_of_day + 1.hour,
+      )
+    end
+    let(:submission_not_on_date) do
+      create(:submission, form_document:, form_id:, mode: mode_string, created_at: date.beginning_of_day - 1.day)
+    end
+    let(:preview_submission) do
+      create(:submission, :preview, form_document:, form_id:, created_at: date.beginning_of_day + 1.hour)
+    end
+    let(:submissions) { [submissions_to_include, submission_not_on_date, preview_submission] }
+
+    context "when the form does not have a submission email address" do
+      let(:submission_email) { nil }
+
+      it "raises an error" do
+        expect {
+          perform_enqueued_jobs
+        }.to raise_error(StandardError, "Form id: #{form_id} is missing a submission email address")
+      end
+
+      context "when the mode is preview" do
+        let(:mode_string) { "preview-live" }
+
+        it "does not call the submission batch service" do
+          perform_enqueued_jobs
+          expect(AwsSesSubmissionBatchService).not_to receive(:new)
+        end
+      end
+    end
+
+    context "when the form has a submission email address" do
+      before do
+        @job_ran_at = Time.zone.now
+        perform_enqueued_jobs
+      end
+
+      it "sends an email" do
+        expect(ActionMailer::Base.deliveries.count).to eq(1)
+
+        mail = ActionMailer::Base.deliveries.last
+        expect(mail.to).to include(form_document.submission_email)
+      end
+
+      it "updates the delivery" do
+        mail = ActionMailer::Base.deliveries.last
+        expect(delivery.reload.delivery_reference).to eq(mail.message_id)
+        expect(delivery.reload.last_attempt_at).to be_within(1.second).of(@job_ran_at)
+      end
+
+      it "attaches a csv with the expected filename" do
+        mail = ActionMailer::Base.deliveries.last
+        expect(mail.attachments).not_to be_empty
+
+        filenames = mail.attachments.map(&:filename)
+        expect(filenames).to contain_exactly("govuk_forms_my_form_2022-12-14.csv")
+      end
+
+      it "attaches a csv containing header plus one line per submission" do
+        mail = ActionMailer::Base.deliveries.last
+
+        csv_content = mail.attachments.first.decoded
+        expect(csv_content.lines.count).to eq(submissions_to_include.count + 1)
+      end
+    end
+  end
+end
+# rubocop:enable RSpec/InstanceVariable
diff --git a/spec/lib/csv_generator_spec.rb b/spec/lib/csv_generator_spec.rb
diff --git a/spec/models/submission_spec.rb b/spec/models/submission_spec.rb
diff --git a/spec/services/aws_ses_submission_batch_service_spec.rb b/spec/services/aws_ses_submission_batch_service_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@`
`14`	`14`	`},`
`15`	`15`	`}`
`16`	`16`	`end`
`17`		`- mode { is_preview ? "preview-live" : "live" }`
	`17`	`+ mode { is_preview ? "preview-live" : "form" }`
`18`	`18`	`form_document { build :v2_form_document, form_id: }`
`19`	`19`	`submission_locale { :en }`
`20`	`20`