[LUPEYALPHA-1152] Upload CSV with empty rows

app/models/eligible_fe_providers_importer.rb

@@ -19,7 +19,7 @@ def initialize(file, academic_year)
   end
 
   def results_message
-    "Replaced #{deleted_row_count} existing providers with #{rows.count} new providers"
+    "Replaced #{deleted_row_count} existing providers with #{rows_with_data_count} new providers"
   end
 
   private

lib/csv_importer/base.rb

@@ -4,12 +4,13 @@ module CsvImporter
   class Base
     include CsvImporter::Config
 
-    attr_reader :errors, :rows, :deleted_row_count
+    attr_reader :errors, :rows, :deleted_row_count, :skipped_row_count
 
     def initialize(file)
       @errors = []
       @rows = parse_csv_file(file)
       @deleted_row_count = 0
+      @skipped_row_count = 0
 
       check_headers if rows && with_headers?
     end
@@ -21,7 +22,10 @@ def run
         Rails.logger.info "Processing #{target_data_model.to_s.titleize} batch #{i}"
 
         record_hashes = batch_rows.map do |row|
-          next if valid_skip_row_conditions?(row)
+          if empty_row?(row) || valid_skip_row_conditions?(row)
+            @skipped_row_count += 1
+            next
+          end
 
           convert_row_to_hash(row)
         end.compact
@@ -30,8 +34,21 @@ def run
       end
     end
 
+    def rows_with_data_count
+      rows.count - skipped_row_count
+    end
+
     private
 
+    def empty_row?(row)
+      case row
+      when Array
+        row.all? { |cell| cell.blank? }
+      else
+        row.all? { |_, v| v.blank? }
+      end
+    end
+
     def delete_all_scope
       target_data_model
     end

spec/factories/file_uploads.rb

@@ -2,11 +2,37 @@
   factory :file_upload do
     association :uploaded_by, factory: :dfe_signin_user
 
+    transient do
+      row_count { 1 }
+      row do
+        []
+      end
+    end
+
+    trait :school_workforce_census_upload do
+      transient do
+        row do
+          [
+            "1234567",
+            "123456",
+            "",
+            1,
+            "",
+            "",
+            ""
+          ]
+        end
+      end
+    end
+
     body do
-      <<~CSV
-        1234567,1234567,Full time,19,Design and Technlogy - Textiles,DTT,34,
-        ,,,,,,,,,,,,,,,
-      CSV
+      string = ""
+
+      row_count.times do
+        string << (row.join(",") + "\n")
+      end
+
+      string
     end
   end
 end

spec/jobs/import_census_job_spec.rb

@@ -4,7 +4,7 @@
   describe "#perform" do
     context "csv data processes successfully" do
       let(:mail) { AdminMailer.census_csv_processing_success(file_upload.uploaded_by.email) }
-      let(:file_upload) { create(:file_upload) }
+      let(:file_upload) { create(:file_upload, :school_workforce_census_upload) }
 
       it "imports census data, sends success email and deletes the file upload" do
         subject.perform(file_upload.id)
@@ -23,7 +23,7 @@
 
     context "csv data encounters an error" do
       let(:mail) { AdminMailer.census_csv_processing_error(file_upload.uploaded_by.email) }
-      let(:file_upload) { create(:file_upload) }
+      let(:file_upload) { create(:file_upload, :school_workforce_census_upload) }
 
       before do
         allow(SchoolWorkforceCensus).to receive(:insert_all).and_raise(ActiveRecord::RecordInvalid)

spec/models/claim_spec.rb

@@ -834,7 +834,7 @@
   end
 
   describe ".payrollable" do
-    subject { described_class.payrollable }
+    subject { described_class.payrollable.order(:submitted_at) }
 
     let(:payroll_run) { create(:payroll_run, claims_counts: {Policies::StudentLoans => 1}) }
     let!(:submitted_claim) { create(:claim, :submitted) }

spec/models/eligible_fe_providers_importer_spec.rb

@@ -58,6 +58,36 @@ def to_row(hash)
       end
     end
 
+    context "with extra empty rows" do
+      before do
+        file.write correct_headers
+
+        file.write(described_class.mandatory_headers.map { nil }.join(",") + "\n")
+
+        3.times do
+          file.write to_row(attributes_for(:eligible_fe_provider))
+        end
+
+        file.write(described_class.mandatory_headers.map { nil }.join(",") + "\n")
+
+        file.close
+      end
+
+      it "ignores empty rows" do
+        expect { subject.run }.to change { EligibleFeProvider.count }.by(3)
+      end
+
+      it "does not raise errors" do
+        expect { subject.run }.not_to raise_error
+      end
+
+      it "returns correct rows_with_data_count" do
+        subject.run
+
+        expect(subject.rows_with_data_count).to eql(3)
+      end
+    end
+
     context "with valid data" do
       before do
         file.write correct_headers