Merge pull request #1935 from alphagov/rake-task-resend-submission-batch

Updating rake tasks to handle batches of submissions

Author: SamJamCul

app/jobs/send_submission_batch_job.rb

@@ -32,9 +32,9 @@ def perform(delivery:)
 
     message_id = AwsSesSubmissionBatchService.new(submissions_query: submissions, form:, date:, mode:).send_batch
 
+    delivery.new_attempt!
     delivery.update!(
       delivery_reference: message_id,
-      last_attempt_at: Time.zone.now,
     )
 
     CurrentJobLoggingAttributes.delivery_reference = delivery.delivery_reference

lib/tasks/submissions.rake

@@ -14,10 +14,12 @@ namespace :submissions do
 
     deliveries = Delivery.failed.joins(:submissions).where(submissions: { form_id: form_id }).distinct
     Rails.logger.info "Found #{deliveries.length} bounced submission deliveries for form with ID #{form_id}"
-    deliveries.find_each do |delivery|
-      # This will need to be updated when we support batches
+    deliveries.immediate.each do |delivery|
       submission = delivery.submissions.first
-      Rails.logger.info "Submission reference: #{submission.reference}, created_at: #{submission.created_at}, last_attempt_at: #{delivery.last_attempt_at}"
+      Rails.logger.info "Immediate delivery - submission reference: #{submission.reference}, created_at: #{submission.created_at}, last_attempt_at: #{delivery.last_attempt_at}"
+    end
+    deliveries.daily.each do |delivery|
+      Rails.logger.info "Daily batch delivery - delivery_reference: #{delivery.delivery_reference}, created_at: #{delivery.created_at}, last_attempt_at: #{delivery.last_attempt_at}"
     end
   end
 
@@ -42,13 +44,17 @@ namespace :submissions do
 
     bounced_deliveries = Delivery.failed.joins(:submissions).where(submissions: { form_id: form_id }).distinct
 
-    Rails.logger.info "#{bounced_deliveries.length} submission deliveries to retry for form with ID: #{form_id}"
+    Rails.logger.info "#{bounced_deliveries.length} deliveries to retry for form with ID: #{form_id}"
 
     bounced_deliveries.each do |delivery|
-      # This will need to be updated when we support batches
       submission = delivery.submissions.first
-      Rails.logger.info "Retrying submission with reference #{submission.reference} for form with ID: #{form_id}"
-      SendSubmissionJob.perform_later(submission)
+      if delivery.immediate?
+        Rails.logger.info "Retrying submission with reference #{submission.reference} for form with ID: #{form_id}"
+        SendSubmissionJob.perform_later(submission)
+      elsif delivery.daily?
+        Rails.logger.info "Retrying daily batch delivery with delivery_id: #{delivery.id} for date: #{submission.submission_time.to_date} for form with ID: #{form_id}"
+        SendSubmissionBatchJob.perform_later(delivery:)
+      end
     end
   end
 
@@ -166,6 +172,55 @@ namespace :submissions do
     end
   end
 
+  desc "Re-deliver daily submission batches corresponding to submissions between two timestamps for a specific form"
+  task :redeliver_daily_batches_by_date, %i[form_id start_timestamp end_timestamp dry_run] => :environment do |_, args|
+    form_id = args[:form_id]
+    start_timestamp = args[:start_timestamp]
+    end_timestamp = args[:end_timestamp]
+    dry_run_arg = args[:dry_run]
+    dry_run = dry_run_arg == "true"
+
+    usage_message = "usage: rake submissions:redeliver_daily_batches_by_date[<form_id>,<start_timestamp>,<end_timestamp>,<dry_run>]".freeze
+
+    if form_id.blank? || start_timestamp.blank? || end_timestamp.blank? || !dry_run_arg.in?(%w[true false])
+      abort usage_message
+    end
+
+    start_time = Time.zone.parse(start_timestamp)
+    end_time = Time.zone.parse(end_timestamp)
+
+    if start_time.nil? || end_time.nil?
+      abort "Error: Invalid timestamp format. Use ISO 8601 format (e.g. '2024-01-01T00:00:00Z')"
+    end
+
+    if start_time >= end_time
+      abort "Error: Start timestamp must be before end timestamp"
+    end
+
+    deliveries_to_redeliver = Delivery.daily
+                                      .joins(:submissions)
+                                      .where(submissions: { form_id: form_id, created_at: start_time..end_time })
+                                      .distinct
+
+    Rails.logger.info "Time range: #{start_time} to #{end_time}"
+    Rails.logger.info "Dry run mode: #{dry_run ? 'enabled' : 'disabled'}"
+
+    if deliveries_to_redeliver.any?
+      Rails.logger.info "Found #{deliveries_to_redeliver.count} submission batches to re-deliver for form ID: #{form_id}"
+
+      deliveries_to_redeliver.each do |delivery|
+        if dry_run
+          Rails.logger.info "Would re-deliver batch with ID #{delivery.id} and delivery reference #{delivery.delivery_reference}"
+        else
+          Rails.logger.info "Re-delivering batch ID #{delivery.id} and delivery reference #{delivery.delivery_reference}"
+          SendSubmissionBatchJob.perform_later(delivery:)
+        end
+      end
+    else
+      Rails.logger.info "No batches found matching the criteria"
+    end
+  end
+
   namespace :file_answers do
     desc "Generate filename for file upload answers that do not have an original filename stored and schedule the submission to be sent"
     task :fix_missing_original_filenames, %i[reference] => :environment do |_, args|

spec/jobs/send_submission_batch_job_spec.rb

@@ -78,6 +78,16 @@
         expect(delivery.reload.last_attempt_at).to be_within(1.second).of(@job_ran_at)
       end
 
+      context "when the delivery has already been attempted" do
+        let(:delivery) { create(:delivery, delivery_schedule: "daily", submissions:, delivered_at: Time.zone.now - 2.hours, failed_at: Time.zone.now - 1.hour, failure_reason: "bounced") }
+
+        it "updates the resets the delivery details" do
+          expect(delivery.reload.delivered_at).to be_nil
+          expect(delivery.reload.failed_at).to be_nil
+          expect(delivery.reload.failure_reason).to be_nil
+        end
+      end
+
       it "attaches a csv with the expected filename" do
         expect(mail.attachments).not_to be_empty
 

spec/lib/tasks/submissions.rake_spec.rb

@@ -64,6 +64,7 @@
 
     let!(:bounced_submission) { create :submission, :bounced, form_id: }
     let!(:another_bounced_submission) { create :submission, :bounced, form_id: }
+    let!(:bounced_daily_delivery) { create :delivery, :failed, :daily_scheduled_delivery, submissions: [bounced_submission, another_bounced_submission] }
 
     before do
       # create some submissions that won't be matched
@@ -72,9 +73,10 @@
     end
 
     it "logs the bounced submissions" do
-      expect(Rails.logger).to receive(:info).with("Found 2 bounced submission deliveries for form with ID #{form_id}")
-      expect(Rails.logger).to receive(:info).with "Submission reference: #{bounced_submission.reference}, created_at: #{bounced_submission.created_at}, last_attempt_at: #{bounced_submission.single_submission_delivery.last_attempt_at}"
-      expect(Rails.logger).to receive(:info).with "Submission reference: #{another_bounced_submission.reference}, created_at: #{another_bounced_submission.created_at}, last_attempt_at: #{another_bounced_submission.single_submission_delivery.last_attempt_at}"
+      expect(Rails.logger).to receive(:info).with("Found 3 bounced submission deliveries for form with ID #{form_id}")
+      expect(Rails.logger).to receive(:info).with "Immediate delivery - submission reference: #{bounced_submission.reference}, created_at: #{bounced_submission.created_at}, last_attempt_at: #{bounced_submission.single_submission_delivery.last_attempt_at}"
+      expect(Rails.logger).to receive(:info).with "Immediate delivery - submission reference: #{another_bounced_submission.reference}, created_at: #{another_bounced_submission.created_at}, last_attempt_at: #{another_bounced_submission.single_submission_delivery.last_attempt_at}"
+      expect(Rails.logger).to receive(:info).with "Daily batch delivery - delivery_reference: #{bounced_daily_delivery.delivery_reference}, created_at: #{bounced_daily_delivery.created_at}, last_attempt_at: #{bounced_daily_delivery.last_attempt_at}"
       task.invoke(form_id)
     end
   end
@@ -89,6 +91,7 @@
     let(:other_form_id) { 2 }
     let!(:bounced_submission) { create :submission, :bounced, form_id: }
     let!(:pending_submission) { create :submission, :sent, form_id: }
+    let!(:bounced_delivery) { create :delivery, :daily_scheduled_delivery, :failed, submissions: [bounced_submission] }
 
     before do
       create :submission, :sent, form_id: other_form_id
@@ -99,7 +102,7 @@
 
       it "logs how many deliveries to retry" do
         allow(Rails.logger).to receive(:info)
-        expect(Rails.logger).to receive(:info).with("1 submission deliveries to retry for form with ID: #{form_id}")
+        expect(Rails.logger).to receive(:info).with("2 deliveries to retry for form with ID: #{form_id}")
 
         task.invoke(*valid_args)
       end
@@ -108,6 +111,7 @@
         it "logs submissions that are being retried" do
           allow(Rails.logger).to receive(:info)
           expect(Rails.logger).to receive(:info).with("Retrying submission with reference #{bounced_submission.reference} for form with ID: #{form_id}")
+          expect(Rails.logger).to receive(:info).with("Retrying daily batch delivery with delivery_id: #{bounced_delivery.id} for date: #{bounced_submission.submission_time.to_date} for form with ID: #{form_id}")
 
           task.invoke(*valid_args)
         end
@@ -118,6 +122,12 @@
           }.to have_enqueued_job.with(bounced_submission)
         end
 
+        it "enqueues bounced daily deliveries for retrying" do
+          expect {
+            task.invoke(*valid_args)
+          }.to have_enqueued_job.with(delivery: bounced_delivery)
+        end
+
         it "does not enqueue pending submissions for retrying" do
           expect {
             task.invoke(*valid_args)
@@ -254,6 +264,11 @@
       create :delivery, :failed, submissions: [submission], created_at: Time.parse("2024-01-01T18:00:00Z")
     end
 
+    let!(:matching_batch_delivery) do
+      submission = create(:submission, form_id:)
+      create :delivery, :failed, :daily_scheduled_delivery, submissions: [submission], created_at: Time.parse("2024-01-01T12:00:00Z")
+    end
+
     let!(:not_bounced_delivery) do
       submission = create(:submission, form_id:)
       create :delivery, submissions: [submission], created_at: Time.parse("2024-01-01T12:00:00Z")
@@ -291,9 +306,10 @@
 
       it "logs the deliveries to disregard" do
         allow(Rails.logger).to receive(:info)
-        expect(Rails.logger).to receive(:info).with("Found 2 bounced submission deliveries to disregard for form ID #{form_id} in time range: #{Time.zone.parse(start_time)} to #{Time.zone.parse(end_time)}").once
+        expect(Rails.logger).to receive(:info).with("Found 3 bounced submission deliveries to disregard for form ID #{form_id} in time range: #{Time.zone.parse(start_time)} to #{Time.zone.parse(end_time)}").once
         expect(Rails.logger).to receive(:info).with("Disregarded bounce of delivery with delivery_reference #{early_matching_delivery.delivery_reference}")
         expect(Rails.logger).to receive(:info).with("Disregarded bounce of delivery with delivery_reference #{late_matching_delivery.delivery_reference}")
+        expect(Rails.logger).to receive(:info).with("Disregarded bounce of delivery with delivery_reference #{matching_batch_delivery.delivery_reference}")
         task.invoke(*valid_args)
       end
 
@@ -310,6 +326,7 @@
           allow(Rails.logger).to receive(:info)
           expect(Rails.logger).to receive(:info).with("Would disregard bounce of delivery with delivery_reference #{early_matching_delivery.delivery_reference} which was created at #{early_matching_delivery.created_at}")
           expect(Rails.logger).to receive(:info).with("Would disregard bounce of delivery with delivery_reference #{late_matching_delivery.delivery_reference} which was created at #{late_matching_delivery.created_at}")
+          expect(Rails.logger).to receive(:info).with("Would disregard bounce of delivery with delivery_reference #{matching_batch_delivery.delivery_reference} which was created at #{matching_batch_delivery.created_at}")
           task.invoke(*valid_args)
         end
       end
@@ -518,6 +535,164 @@
     end
   end
 
+  describe "submissions:redeliver_daily_batches_by_date" do
+    subject(:task) do
+      Rake::Task["submissions:redeliver_daily_batches_by_date"]
+        .tap(&:reenable)
+    end
+
+    let(:form_id) { 1 }
+    let(:other_form_id) { 2 }
+    let(:start_time) { "2024-01-01T00:00:00Z" }
+    let(:end_time) { "2024-01-02T00:00:00Z" }
+    let(:dry_run) { "false" }
+
+    let!(:batched_submission) do
+      create :submission,
+             :sent,
+             form_id:,
+             created_at: Time.parse("2024-01-01T12:00:00Z"),
+             reference: "ref1"
+    end
+
+    let!(:batch_delivery) do
+      create :delivery,
+             :daily_scheduled_delivery,
+             :failed,
+             submissions: [batched_submission],
+             created_at: Time.parse("2024-01-02T02:00:00Z"),
+             delivery_reference: "batch1"
+    end
+
+    let!(:next_day_batch_delivery) do
+      submission = create :submission,
+                          :sent,
+                          form_id:,
+                          created_at: Time.parse("2024-01-02T12:00:00Z"),
+                          reference: "ref1"
+      create :delivery,
+             :daily_scheduled_delivery,
+             :failed,
+             submissions: [submission],
+             created_at: Time.parse("2024-01-03T02:00:00Z"),
+             delivery_reference: "batch-next-day"
+    end
+
+    let!(:other_form_batch_delivery) do
+      submission = create :submission,
+                          :sent,
+                          form_id: other_form_id,
+                          created_at: Time.parse("2024-01-01T12:00:00Z"),
+                          reference: "ref2"
+      create :delivery,
+             :daily_scheduled_delivery,
+             :failed,
+             submissions: [submission],
+             created_at: Time.parse("2024-01-02T02:00:00Z"),
+             delivery_reference: "batch2"
+    end
+
+    context "with valid arguments" do
+      let(:valid_args) { [form_id, start_time, end_time, dry_run] }
+
+      it "enqueues matching batch delivery for re-delivery" do
+        expect {
+          task.invoke(*valid_args)
+        }.to have_enqueued_job(SendSubmissionBatchJob).with(delivery: batch_delivery)
+      end
+
+      it "does not enqueue a batch delivery outside the time range" do
+        expect {
+          task.invoke(*valid_args)
+        }.not_to have_enqueued_job(SendSubmissionBatchJob).with(delivery: next_day_batch_delivery)
+      end
+
+      it "does not enqueue another form's batch deliveries" do
+        expect {
+          task.invoke(*valid_args)
+        }.not_to have_enqueued_job(SendSubmissionBatchJob).with(delivery: other_form_batch_delivery)
+      end
+
+      context "when dry_run is true" do
+        let(:dry_run) { "true" }
+
+        it "does not enqueue any jobs" do
+          expect {
+            task.invoke(*valid_args)
+          }.not_to have_enqueued_job(SendSubmissionJob)
+        end
+      end
+
+      context "when no submissions took place between the given times" do
+        let(:valid_args) { [form_id, "2025-01-01T00:00:00Z", "2025-01-02T00:00:00Z", dry_run] }
+
+        it "does not enqueue any jobs" do
+          expect {
+            task.invoke(*valid_args)
+          }.not_to have_enqueued_job(SendSubmissionBatchJob)
+        end
+      end
+    end
+
+    context "with invalid arguments" do
+      it "aborts when form_id is missing" do
+        expect {
+          task.invoke("", start_time, end_time, dry_run)
+        }.to raise_error(SystemExit)
+               .and output("usage: rake submissions:redeliver_daily_batches_by_date[<form_id>,<start_timestamp>,<end_timestamp>,<dry_run>]\n").to_stderr
+      end
+
+      it "aborts when start_timestamp is missing" do
+        expect {
+          task.invoke(form_id, "", end_time, dry_run)
+        }.to raise_error(SystemExit)
+               .and output("usage: rake submissions:redeliver_daily_batches_by_date[<form_id>,<start_timestamp>,<end_timestamp>,<dry_run>]\n").to_stderr
+      end
+
+      it "aborts when end_timestamp is missing" do
+        expect {
+          task.invoke(form_id, start_time, "", dry_run)
+        }.to raise_error(SystemExit)
+               .and output("usage: rake submissions:redeliver_daily_batches_by_date[<form_id>,<start_timestamp>,<end_timestamp>,<dry_run>]\n").to_stderr
+      end
+
+      it "aborts when start_timestamp is invalid" do
+        expect {
+          task.invoke(form_id, "invalid-date", end_time, dry_run)
+        }.to raise_error(SystemExit)
+               .and output("Error: Invalid timestamp format. Use ISO 8601 format (e.g. '2024-01-01T00:00:00Z')\n").to_stderr
+      end
+
+      it "aborts when end_timestamp is invalid" do
+        expect {
+          task.invoke(form_id, start_time, "invalid-date", dry_run)
+        }.to raise_error(SystemExit)
+               .and output("Error: Invalid timestamp format. Use ISO 8601 format (e.g. '2024-01-01T00:00:00Z')\n").to_stderr
+      end
+
+      it "aborts when start_timestamp is after end_timestamp" do
+        expect {
+          task.invoke(form_id, "2024-01-02T00:00:00Z", "2024-01-01T00:00:00Z", dry_run)
+        }.to raise_error(SystemExit)
+               .and output("Error: Start timestamp must be before end timestamp\n").to_stderr
+      end
+
+      it "aborts when start_timestamp equals end_timestamp" do
+        expect {
+          task.invoke(form_id, start_time, start_time, dry_run)
+        }.to raise_error(SystemExit)
+               .and output("Error: Start timestamp must be before end timestamp\n").to_stderr
+      end
+
+      it "aborts when dry_run is an invalid value" do
+        expect {
+          task.invoke(form_id, start_time, start_time, "foo")
+        }.to raise_error(SystemExit)
+               .and output("usage: rake submissions:redeliver_daily_batches_by_date[<form_id>,<start_timestamp>,<end_timestamp>,<dry_run>]\n").to_stderr
+      end
+    end
+  end
+
   describe "submissions:file_answers:fix_missing_original_filenames" do
     subject(:task) do
       Rake::Task["submissions:file_answers:fix_missing_original_filenames"]