Rework batch CSV generator for multiple versions

The CSV generator now takes a submission query, and creates a set of
CSVs based on how many different versions it can pick out. If two
submissions have different versions but would still have the same CSV
headers, then they will be part of the same CSV.

Co-authored-by: Stephen Daly <stephen.daly@digital.cabinet-office.gov.uk>

Author: SamJamCul

app/jobs/send_submission_batch_job.rb

@@ -18,7 +18,7 @@ def perform(form_id:, mode_string:, date:, delivery:)
     mode = Mode.new(mode_string)
     set_submission_batch_logging_attributes(form:, mode:)
 
-    message_id = AwsSesSubmissionBatchService.new(submissions:, form:, date:, mode:).send_batch
+    message_id = AwsSesSubmissionBatchService.new(submissions_query: submissions, form:, date:, mode:).send_batch
 
     delivery.update!(
       delivery_reference: message_id,

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
 

app/models/submission.rb

@@ -42,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

app/services/aws_ses_submission_batch_service.rb

@@ -1,6 +1,6 @@
 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
@@ -24,11 +24,11 @@ def send_batch
   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

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

spec/lib/csv_generator_spec.rb

@@ -1,17 +1,23 @@
 require "rails_helper"
 
 RSpec.describe CsvGenerator do
-  let(:form_document) { build(:v2_form_document, form_id: 42, available_languages:) }
-  let(:submission) { build(:submission, form_document:, created_at: timestamp, reference: submission_reference, mode:, submission_locale:) }
-  let(:page) { build :page }
-  let(:text_question) { build :text, :with_answer, question_text: "What is the meaning of life?" }
-  let(:name_question) { build :first_middle_last_name_question, question_text: "What is your name?" }
-  let(:file_question) { build :file, :with_uploaded_file, question_text: "Upload a file", original_filename: "test.txt" }
-  let(:first_step) { build :step, question: text_question }
-  let(:second_step) { build :step, question: name_question }
-  let(:third_step) { build :step, question: file_question }
-  let(:all_steps) { [first_step, second_step, third_step] }
-  let(:journey) { instance_double(Flow::Journey, all_steps:, completed_file_upload_questions: [file_question]) }
+  let(:form_document) { build(:v2_form_document, form_id: 42, available_languages:, steps: [text_step, name_step, file_upload_step], start_page: text_step[:id]) }
+  let(:submission) { create(:submission, form_document:, created_at: timestamp, reference: submission_reference, mode:, submission_locale:, answers:) }
+  let(:text_step) { build :v2_question_page_step, :with_text_settings, question_text: "What is the meaning of life?", next_step_id: name_step[:id] }
+  let(:name_step) { build :v2_question_page_step, :with_name_settings, question_text: "What is your name?", next_step_id: file_upload_step[:id] }
+  let(:file_upload_step) { build :v2_question_page_step, :with_file_upload_settings, question_text: "Upload a file" }
+  let(:text_answer) { "blue" }
+  let(:first_name_answer) { "Alice" }
+  let(:last_name_answer) { "Smith" }
+  let(:file_upload_answer) { "file.txt" }
+  let(:answers) do
+    {
+      text_step[:id] => { "text" => text_answer },
+      name_step[:id] => { "first_name" => first_name_answer, "last_name" => last_name_answer },
+      file_upload_step[:id] => { "original_filename" => file_upload_answer },
+    }
+  end
+
   let(:mode) { "form" }
   let(:submission_reference) { Faker::Alphanumeric.alphanumeric(number: 8).upcase }
   let(:submission_locale) { :en }
@@ -20,10 +26,6 @@
     Time.use_zone("London") { Time.zone.local(2022, 9, 14, 8, 0o0, 0o0) }
   end
 
-  before do
-    allow(Flow::Journey).to receive(:new).and_return(journey)
-  end
-
   describe "#generate_submission" do
     subject(:csv) { described_class.generate_submission(submission:, is_s3_submission:) }
 
@@ -38,27 +40,38 @@
         expect(CSV.parse(csv)).to eq(
           [
             ["Reference", "Submitted at", "What is the meaning of life?", "What is your name? - First name", "What is your name? - Last name", "Upload a file"],
-            [submission_reference, "2022-09-14T08:00:00+01:00", text_question.text, name_question.first_name, name_question.last_name, "test_#{submission_reference}.txt"],
+            [submission_reference, "2022-09-14T08:00:00+01:00", text_answer, first_name_answer, last_name_answer, "file_#{submission_reference}.txt"],
           ],
         )
       end
 
       context "when a question is optional and answer is not provided" do
-        let(:text_question) { build :text, question_text: "What is the meaning of life?", is_optional: true, text: nil }
+        let(:text_answer) { "" }
 
         it "generates a CSV including blank column for unanswered optional question" do
           expect(CSV.parse(csv)).to eq(
             [
               ["Reference", "Submitted at", "What is the meaning of life?", "What is your name? - First name", "What is your name? - Last name", "Upload a file"],
-              [submission_reference, "2022-09-14T08:00:00+01:00", "", name_question.first_name, name_question.last_name, "test_#{submission_reference}.txt"],
+              [submission_reference, "2022-09-14T08:00:00+01:00", "", first_name_answer, last_name_answer, "file_#{submission_reference}.txt"],
             ],
           )
         end
       end
 
       context "when there are repeated steps" do
-        let(:name_question_repeated) { build :first_middle_last_name_question, question_text: "What is your name?" }
-        let(:second_step) { build :repeatable_step, questions: [name_question, name_question_repeated] }
+        let(:name_step) { build :v2_question_page_step, :with_name_settings, question_text: "What is your name?", next_step_id: file_upload_step[:id], is_repeatable: true }
+        let(:another_first_name_answer) { "John" }
+        let(:another_last_name_answer) { "Smith" }
+        let(:answers) do
+          {
+            text_step[:id] => { "text" => text_answer },
+            name_step[:id] => [
+              { "first_name" => first_name_answer, "last_name" => last_name_answer },
+              { "first_name" => another_first_name_answer, "last_name" => another_last_name_answer },
+            ],
+            file_upload_step[:id] => { "original_filename" => file_upload_answer },
+          }
+        end
 
         it "generates a CSV with headers containing suffixes for the repeated steps" do
           expect(CSV.parse(csv)).to eq(
@@ -76,12 +89,12 @@
               [
                 submission_reference,
                 "2022-09-14T08:00:00+01:00",
-                text_question.text,
-                name_question.first_name,
-                name_question.last_name,
-                name_question_repeated.first_name,
-                name_question_repeated.last_name,
-                "test_#{submission_reference}.txt",
+                text_answer,
+                first_name_answer,
+                last_name_answer,
+                another_first_name_answer,
+                another_last_name_answer,
+                "file_#{submission_reference}.txt",
               ],
             ],
           )
@@ -95,7 +108,7 @@
           expect(CSV.parse(csv)).to eq(
             [
               ["Reference", "Submitted at", "What is the meaning of life?", "What is your name? - First name", "What is your name? - Last name", "Upload a file", "Language"],
-              [submission_reference, "2022-09-14T08:00:00+01:00", text_question.text, name_question.first_name, name_question.last_name, "test_#{submission_reference}.txt", "en"],
+              [submission_reference, "2022-09-14T08:00:00+01:00", text_answer, first_name_answer, last_name_answer, "file_#{submission_reference}.txt", "en"],
             ],
           )
         end
@@ -109,57 +122,112 @@
         expect(CSV.parse(csv)).to eq(
           [
             ["Reference", "Submitted at", "What is the meaning of life?", "What is your name? - First name", "What is your name? - Last name", "Upload a file"],
-            [submission_reference, "2022-09-14T08:00:00+01:00", text_question.text, name_question.first_name, name_question.last_name, file_question.original_filename],
+            [submission_reference, "2022-09-14T08:00:00+01:00", text_answer, first_name_answer, last_name_answer, file_upload_answer],
           ],
         )
       end
     end
   end
 
   describe "#generate_batched_submissions" do
-    subject(:csv) { described_class.generate_batched_submissions(submissions:, is_s3_submission:) }
+    subject(:csv_list) { described_class.generate_batched_submissions(submissions_query:, is_s3_submission:) }
 
-    let(:submission_2) { build(:submission, form_document:, created_at: timestamp + 1.hour, reference: submission_reference_2, mode:, submission_locale: :cy) }
     let(:submission_reference_2) { Faker::Alphanumeric.alphanumeric(number: 8).upcase }
-    let(:submissions) { [submission, submission_2] }
     let(:is_s3_submission) { false }
+    let(:submissions_query) { Submission.all }
 
-    it "generates a CSV with multiple rows for the submissions" do
-      expect(CSV.parse(csv)).to eq(
-        [
-          ["Reference", "Submitted at", "What is the meaning of life?", "What is your name? - First name", "What is your name? - Last name", "Upload a file"],
-          [submission_reference, "2022-09-14T08:00:00+01:00", text_question.text, name_question.first_name, name_question.last_name, "test_#{submission_reference}.txt"],
-          [submission_reference_2, "2022-09-14T09:00:00+01:00", text_question.text, name_question.first_name, name_question.last_name, "test_#{submission_reference_2}.txt"],
-        ],
-      )
-    end
+    context "when all submissions result in the same CSV headers" do
+      before do
+        submission
+        create(:submission, form_document:, created_at: timestamp + 1.hour, reference: submission_reference_2, mode:, submission_locale: :cy, answers:)
+      end
 
-    context "when the form has multiple available languages" do
-      let(:available_languages) { %w[en cy] }
+      it "returns an array with a single CSV string" do
+        expect(csv_list).to be_a(Array)
+        expect(csv_list.size).to eq(1)
+        expect(csv_list.first).to be_a(String)
+      end
 
-      it "generates a CSV with a language column" do
-        expect(CSV.parse(csv)).to eq(
+      it "generates a CSV with multiple rows for the submissions" do
+        csv = CSV.parse(csv_list.first)
+        expect(csv.size).to eq(3) # header row + 2 submissions
+        expect(csv).to eq(
           [
-            ["Reference", "Submitted at", "What is the meaning of life?", "What is your name? - First name", "What is your name? - Last name", "Upload a file", "Language"],
-            [submission_reference, "2022-09-14T08:00:00+01:00", text_question.text, name_question.first_name, name_question.last_name, "test_#{submission_reference}.txt", "en"],
-            [submission_reference_2, "2022-09-14T09:00:00+01:00", text_question.text, name_question.first_name, name_question.last_name, "test_#{submission_reference_2}.txt", "cy"],
+            ["Reference", "Submitted at", "What is the meaning of life?", "What is your name? - First name", "What is your name? - Last name", "Upload a file"],
+            [submission_reference, "2022-09-14T08:00:00+01:00", text_answer, first_name_answer, last_name_answer, "file_#{submission_reference}.txt"],
+            [submission_reference_2, "2022-09-14T09:00:00+01:00", text_answer, first_name_answer, last_name_answer, "file_#{submission_reference_2}.txt"],
           ],
         )
       end
+
+      context "when the form has multiple available languages" do
+        let(:available_languages) { %w[en cy] }
+
+        it "generates a CSV with a language column" do
+          expect(CSV.parse(csv_list.first)).to eq(
+            [
+              ["Reference", "Submitted at", "What is the meaning of life?", "What is your name? - First name", "What is your name? - Last name", "Upload a file", "Language"],
+              [submission_reference, "2022-09-14T08:00:00+01:00", text_answer, first_name_answer, last_name_answer, "file_#{submission_reference}.txt", "en"],
+              [submission_reference_2, "2022-09-14T09:00:00+01:00", text_answer, first_name_answer, last_name_answer, "file_#{submission_reference_2}.txt", "cy"],
+            ],
+          )
+        end
+      end
+
+      context "when the CSV is being sent to an S3 bucket" do
+        let(:is_s3_submission) { true }
+
+        it "generates a CSV without including the submission reference in the filename for the file upload question" do
+          expect(CSV.parse(csv_list.first)).to eq(
+            [
+              ["Reference", "Submitted at", "What is the meaning of life?", "What is your name? - First name", "What is your name? - Last name", "Upload a file"],
+              [submission_reference, "2022-09-14T08:00:00+01:00", text_answer, first_name_answer, last_name_answer, file_upload_answer],
+              [submission_reference_2, "2022-09-14T09:00:00+01:00", text_answer, first_name_answer, last_name_answer, file_upload_answer],
+            ],
+          )
+        end
+      end
     end
 
-    context "when the CSV is being sent to an S3 bucket" do
-      let(:is_s3_submission) { true }
+    context "when all submissions result in different CSV headers" do
+      let(:form_document) do
+        build(
+          :v2_form_document,
+          steps: [text_step, name_step, file_upload_step],
+          start_page: text_step[:id],
+          updated_at: Time.utc(2022, 9, 14, 7, 0, 0).iso8601(3),
+        )
+      end
 
-      it "generates a CSV without including the submission reference in the filename for the file upload question" do
-        expect(CSV.parse(csv)).to eq(
-          [
-            ["Reference", "Submitted at", "What is the meaning of life?", "What is your name? - First name", "What is your name? - Last name", "Upload a file"],
-            [submission_reference, "2022-09-14T08:00:00+01:00", text_question.text, name_question.first_name, name_question.last_name, file_question.original_filename],
-            [submission_reference_2, "2022-09-14T09:00:00+01:00", text_question.text, name_question.first_name, name_question.last_name, file_question.original_filename],
-          ],
+      let(:form_document_same_steps) do
+        build(
+          :v2_form_document,
+          steps: [text_step, name_step, file_upload_step],
+          start_page: text_step[:id],
+          updated_at: Time.utc(2022, 9, 15, 7, 0, 0).iso8601(3),
         )
       end
+
+      let(:form_document_different_steps) do
+        build(
+          :v2_form_document,
+          steps: [name_step, file_upload_step],
+          start_page: name_step[:id],
+          updated_at: Time.utc(2022, 9, 16, 7, 0, 0).iso8601(3),
+        )
+      end
+
+      before do
+        create_list(:submission, 2, form_document:, answers:)
+        create_list(:submission, 3, form_document: form_document_same_steps, answers:)
+        create_list(:submission, 4, form_document: form_document_different_steps, answers:)
+      end
+
+      it "creates a CSV for each incompatible set of form versions" do
+        expect(csv_list.count).to eq(2)
+        expect(CSV.parse(csv_list[0]).count).to eq(6)
+        expect(CSV.parse(csv_list[1]).count).to eq(5)
+      end
     end
   end
 end