Remove batching from payroll csv

Requirement is to only produce a single payroll file with no limit on
the number of payments in the file.
This commit removes the zip file as we no longer need it and instead
serves the generated csv directly, this lets us simplify the code quite
a bit.

app/controllers/admin/payroll_run_downloads_controller.rb

@@ -15,7 +15,8 @@ def create
   def show
     respond_to do |format|
       format.html
-      format.zip do
+
+      format.csv do
         out = Payroll::PaymentsCsv.new(@payroll_run)
         send_data out.data, type: out.content_type, filename: out.filename
       end

app/models/payroll/payment_csv_row.rb

@@ -6,7 +6,6 @@
 
 module Payroll
   class PaymentCsvRow < SimpleDelegator
-    ROW_SEPARATOR = "\r\n"
     DATE_FORMAT = "%d/%m/%Y"
     UNITED_KINGDOM = "United Kingdom"
     BASIC_RATE_TAX_CODE = "BR"
@@ -19,19 +18,15 @@ class PaymentCsvRow < SimpleDelegator
     RIGHT_TO_WORK_CONFIRM_STATUS = "2"
     TITLE = "Prof."
 
-    def to_s
-      CSV.generate_line(data, row_sep: ROW_SEPARATOR)
-    end
-
-    private
-
-    def data
+    def to_a
       Payroll::PaymentsCsv::FIELDS_WITH_HEADERS.keys.map do |f|
         field = send(f)
         ExcelUtils.escape_formulas(field)
       end
     end
 
+    private
+
     def title
       TITLE
     end

app/models/payroll/payments_csv.rb

@@ -4,6 +4,8 @@ module Payroll
   class PaymentsCsv
     attr_reader :payroll_run
 
+    ROW_SEPARATOR = "\r\n"
+
     FIELDS_WITH_HEADERS = {
       title: "TITLE",
       first_name: "FORENAME",
@@ -44,49 +46,25 @@ def initialize(payroll_run)
     end
 
     def data
-      buffer = ::StringIO.new
-      Zip::OutputStream.write_buffer(buffer) do |file|
-        payroll_run.payments_in_batches.each.with_index(1) do |batch, index|
-          file = write_csv(file, batch, index)
+      CSV.generate(
+        row_sep: ROW_SEPARATOR,
+        write_headers: true,
+        headers: FIELDS_WITH_HEADERS.values
+      ) do |csv|
+        payroll_run.payments.includes(:claims).find_in_batches(batch_size: 1000) do |batch|
+          batch.each do |payment|
+            csv << PaymentCsvRow.new(payment).to_a
+          end
         end
       end
-      buffer.rewind
-      buffer.read
     end
 
     def content_type
-      "application/zip"
+      "text/csv"
     end
 
     def filename
-      "#{filename_base}.zip"
-    end
-
-    private
-
-    def write_csv(file, batch, index)
-      file.put_next_entry(filename_batch(index))
-      file.write(header_row)
-      batch.each do |payment|
-        file.write(Payroll::PaymentCsvRow.new(payment).to_s)
-      end
-      file
-    end
-
-    def filename_batch(index)
-      "#{filename_base}-batch_#{index}.csv"
-    end
-
-    def filename_base
-      "payroll_data_#{payroll_run.created_at.to_date.iso8601}"
-    end
-
-    def header_row
-      CSV.generate_line(csv_headers, row_sep: PaymentCsvRow::ROW_SEPARATOR)
-    end
-
-    def csv_headers
-      FIELDS_WITH_HEADERS.values
+      "payroll_data_#{payroll_run.created_at.to_date.iso8601}.csv"
     end
   end
 end

app/models/payroll_run.rb

@@ -35,10 +35,6 @@ def total_claim_amount_for_policy(policy, filter: :all)
     line_items(policy, filter: filter).sum(&:award_amount)
   end
 
-  def payments_in_batches
-    payments.includes(:claims).find_in_batches(batch_size: MAX_BATCH_SIZE)
-  end
-
   def total_batches
     (payments.count / MAX_BATCH_SIZE.to_f).ceil
   end

app/views/admin/payroll_run_downloads/show.html.erb

@@ -8,13 +8,9 @@
       If the download does not start automatically, click on the link below.
     </p>
 
-    <p class="govuk-body govuk-!-font-weight-bold">
-      NOTE: The generated file will include <%= @payroll_run.total_batches %>
-      <%= "batch".pluralize(@payroll_run.total_batches) %>.
-    </p>
-
   </div>
+
   <div class="govuk-grid-column-full">
-    <%= link_to "Download #{@payroll_run.created_at.strftime("%B")} payroll file", admin_payroll_run_download_path(@payroll_run, format: :zip), class: ["govuk-body", "govuk-link"], data: { "auto-follow-link": "true" } if @payroll_run.download_triggered? %>
+    <%= link_to "Download #{@payroll_run.created_at.strftime("%B")} payroll file", admin_payroll_run_download_path(@payroll_run, format: :csv), class: ["govuk-body", "govuk-link"], data: { "auto-follow-link": "true" } if @payroll_run.download_triggered? %>
   </div>
 </div>

spec/features/admin/admin_payroll_run_download_spec.rb

@@ -14,10 +14,9 @@
 
     click_on "Download #{payroll_run.created_at.strftime("%B")} payroll file"
 
-    expect(page.response_headers["Content-Type"]).to eq("application/zip")
+    expect(page.response_headers["Content-Type"]).to eq("text/csv")
 
-    zip = Zip::InputStream.open(::StringIO.new(body))
-    csv = CSV.parse(zip.get_next_entry.get_input_stream.read, headers: true)
+    csv = CSV.parse(body, headers: true)
     expect(csv.count).to eq(9)
   end
 end

spec/models/payroll/payment_csv_row_spec.rb

@@ -8,8 +8,8 @@
 
   subject { described_class.new(payment) }
 
-  describe "#to_s" do
-    let(:row) { CSV.parse(subject.to_s).first }
+  describe "#to_a" do
+    let(:row) { subject.to_a }
     let(:payment_award_amount) { BigDecimal("1234.56") }
     let(:payment) { create(:payment, award_amount: payment_award_amount, claims: claims) }
 
@@ -247,7 +247,6 @@
     describe "start and end dates" do
       it "returns the first and last dates of the month" do
         travel_to Date.parse "20 February 2019" do
-          row = CSV.parse(subject.to_s).first
           expect(row[7]).to eq "01/02/2019"
           expect(row[8]).to eq "28/02/2019"
         end
@@ -267,9 +266,5 @@
 
       expect(row[Payroll::PaymentsCsv::FIELDS_WITH_HEADERS.find_index { |k, _| k == :address_line_2 }]).to eq("\\#{claims.first.address_line_2}")
     end
-
-    it "outputs a CRLF line feed" do
-      expect(subject.to_s).to end_with("\r\n")
-    end
   end
 end

spec/models/payroll/payments_csv_spec.rb

@@ -4,76 +4,49 @@
   subject(:generator) { described_class.new(payroll_run) }
 
   let(:payroll_run) do
-    create(:payroll_run, claims_counts: {Policies::StudentLoans => 1, Policies::EarlyCareerPayments => 2}, created_at: creation_date)
-  end
-  let(:creation_date) { "2023-07-17" }
-
-  def generate_csv(payments)
-    csv_headers = Payroll::PaymentsCsv::FIELDS_WITH_HEADERS.values
-    csv_header_row = CSV.generate_line(csv_headers).chomp
-
-    csv_payment_rows = payments.map do |payment|
-      Payroll::PaymentCsvRow.new(payment).to_s.chomp
-    end
-
-    [csv_header_row, csv_payment_rows].join("\r\n") + "\r\n"
-  end
-
-  def load_zip(buffer)
-    Zip::File.open_buffer(buffer)
-  end
-
-  def extract(data, index)
-    load_zip(data).zip.flatten[index - 1]
+    create(
+      :payroll_run,
+      claims_counts: {
+        Policies::StudentLoans => 1,
+        Policies::EarlyCareerPayments => 2
+      },
+      created_at: creation_date
+    )
   end
 
-  def extract_csv_name(data, index)
-    extract(data, index).name
-  end
+  let(:creation_date) { "2023-07-17" }
 
-  def extract_csv_content(data, index)
-    extract(data, index).get_input_stream.read
+  let(:rows) do
+    generator.data.split("\r\n").map { |row| row.split(",") }
   end
 
   describe "#data" do
-    subject(:data) { generator.data }
-
-    let(:payments_batch_one) { payroll_run.payments.ordered.first(2) }
-    let(:payments_batch_two) { [payroll_run.payments.ordered.last] }
-
-    let(:csv_batch_one) { generate_csv(payments_batch_one) }
-    let(:csv_batch_two) { generate_csv(payments_batch_two) }
-
-    let(:max_batch_size) { 2 }
+    describe "CSV headers" do
+      subject(:headers) { rows.first }
 
-    before do
-      stub_const("PayrollRun::MAX_BATCH_SIZE", max_batch_size)
+      it { is_expected.to eq(described_class::FIELDS_WITH_HEADERS.values) }
     end
 
-    it "produces a zip file" do
-      expect(load_zip(data)).to be_truthy
-    end
-
-    context "zip file" do
-      it "contains CSVs with the batch number in the filename", :aggregate_failures do
-        expect(extract_csv_name(data, 1)).to eq("payroll_data_#{creation_date}-batch_1.csv")
-        expect(extract_csv_name(data, 2)).to eq("payroll_data_#{creation_date}-batch_2.csv")
-      end
+    describe "CSV content" do
+      subject(:body) { rows[1..] }
 
-      it "contains CSVs with batched payment data", :aggregate_failures do
-        expect(extract_csv_content(data, 1)).to eq(csv_batch_one)
-        expect(extract_csv_content(data, 2)).to eq(csv_batch_two)
+      it do
+        is_expected.to match_array(
+          payroll_run.payments.map do |payment|
+            Payroll::PaymentCsvRow.new(payment).to_a.map(&:to_s)
+          end
+        )
       end
     end
   end
 
   describe "#content_type" do
-    it { expect(generator.content_type).to eq("application/zip") }
+    subject { generator.content_type }
+    it { is_expected.to eq("text/csv") }
   end
 
   describe "#filename" do
-    it "returns a ZIP filename that includes the date of the payroll run" do
-      expect(generator.filename).to eq("payroll_data_#{creation_date}.zip")
-    end
+    subject { generator.filename }
+    it { is_expected.to eq("payroll_data_#{creation_date}.csv") }
   end
 end

spec/models/payroll_run_spec.rb

@@ -159,24 +159,6 @@
     end
   end
 
-  describe "#payments_in_batches" do
-    subject(:batches) { payroll_run.payments_in_batches }
-
-    let(:payroll_run) { create(:payroll_run, claims_counts: {Policies::StudentLoans => 5}) }
-    let(:batch_size) { 2 }
-    let(:expected_batches) { payroll_run.payments.ordered.each_slice(batch_size).to_a }
-
-    before do
-      stub_const("#{described_class}::MAX_BATCH_SIZE", batch_size)
-    end
-
-    it { is_expected.to be_an(Enumerator) }
-
-    it "returns payments in batches" do
-      expect(batches.to_a).to eq(expected_batches)
-    end
-  end
-
   describe "#total_batches" do
     subject(:total) { payroll_run.total_batches }
 

spec/requests/admin/admin_payroll_run_downloads_spec.rb

@@ -53,16 +53,16 @@
 
         get admin_payroll_run_download_path(payroll_run)
 
-        expect(response.body).to include admin_payroll_run_download_path(payroll_run, format: :zip)
+        expect(response.body).to include admin_payroll_run_download_path(payroll_run, format: :csv)
       end
     end
 
     context "when requesting zip" do
       it "allows the payroll run file to be downloaded" do
         payroll_run = create(:payroll_run, downloaded_at: Time.zone.now, downloaded_by: admin)
-        get admin_payroll_run_download_path(payroll_run, format: :zip)
+        get admin_payroll_run_download_path(payroll_run, format: :csv)
 
-        expect(response.headers["Content-Type"]).to eq("application/zip")
+        expect(response.headers["Content-Type"]).to eq("text/csv")
       end
     end
   end