Merge pull request #3336 from DFE-Digital/CAPT-1870/remove-1000-payro…

…ll-limitation

Remove batching from payroll csv

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/controllers/admin/payroll_runs_controller.rb

@@ -11,19 +11,6 @@ def index
     def new
       @claims = Claim.payrollable.order(submitted_at: :asc)
 
-      # Due to limitations with the current payroll software we need a temporary
-      # cap on the number of claims that can enter payroll, especially expecting
-      # a high volume of approved claims in the first few months.
-      #
-      # Ideally, we should limit topups as well, as some may be related to claims
-      # paid in a previous payroll run, but we wouldn't have any topups at the beginning.
-      #
-      # TODO: Remove this capping once the payroll software is upgraded.
-      if @claims.size > PayrollRun::MAX_MONTHLY_PAYMENTS
-        flash[:notice] = "The number of payments entering this payrun will be capped to #{PayrollRun::MAX_MONTHLY_PAYMENTS}"
-        @claims = @claims.limit(PayrollRun::MAX_MONTHLY_PAYMENTS)
-      end
-
       @topups = Topup.payrollable
       @total_award_amount = @claims.sum(&:award_amount) + @topups.sum(&:award_amount)
     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

@@ -1,7 +1,4 @@
 class PayrollRun < ApplicationRecord
-  MAX_BATCH_SIZE = 1000
-  MAX_MONTHLY_PAYMENTS = 3000
-
   has_many :payments, dependent: :destroy
   has_many :claims, through: :payments
   has_many :payment_confirmations, dependent: :destroy
@@ -13,7 +10,6 @@ class PayrollRun < ApplicationRecord
   belongs_to :confirmation_report_uploaded_by, class_name: "DfeSignIn::User", optional: true
 
   validate :ensure_no_payroll_run_this_month, on: :create
-  validate :ensure_within_max_monthly_payments, on: :create
 
   scope :this_month, -> { where(created_at: DateTime.now.all_month) }
 
@@ -35,14 +31,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
-
   def total_confirmed_payments
     payments.where.not(confirmation: nil).count
   end
@@ -104,8 +92,4 @@ def line_items(policy, filter: :all)
   def ensure_no_payroll_run_this_month
     errors.add(:base, "There has already been a payroll run for #{Date.today.strftime("%B")}") if PayrollRun.this_month.any?
   end
-
-  def ensure_within_max_monthly_payments
-    errors.add(:base, "This payroll run exceeds #{MAX_MONTHLY_PAYMENTS} payments") if payments.size > MAX_MONTHLY_PAYMENTS
-  end
 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/features/admin/admin_payroll_runs_spec.rb

@@ -59,25 +59,6 @@
     end
   end
 
-  scenario "Limiting the maximum number of claims entering payroll" do
-    click_on "Payroll"
-
-    expected_claims = create_list(:claim, 3, :approved)
-    stubbed_max_payments = stub_const("PayrollRun::MAX_MONTHLY_PAYMENTS", 2)
-
-    month_name = Date.today.strftime("%B")
-    click_on "Run #{month_name} payroll"
-
-    expect(page).to have_content("The number of payments entering this payrun will be capped to #{stubbed_max_payments}")
-
-    click_on "Confirm and submit"
-
-    expect(page).to have_content("Approved claims 2")
-
-    payroll_run = PayrollRun.order(:created_at).last
-    expect(payroll_run.claims).to match_array(expected_claims.first(stubbed_max_payments))
-  end
-
   scenario "Any claims approved in the meantime are not included" do
     click_on "Payroll"
 

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