diff --git a/app/models/eligible_fe_providers_importer.rb b/app/models/eligible_fe_providers_importer.rb index 5028e13e15..fdbf66582e 100644 --- a/app/models/eligible_fe_providers_importer.rb +++ b/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 diff --git a/lib/csv_importer/base.rb b/lib/csv_importer/base.rb index dd0b593675..95621a1d81 100644 --- a/lib/csv_importer/base.rb +++ b/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 diff --git a/spec/factories/file_uploads.rb b/spec/factories/file_uploads.rb index 3735794d53..04744f613e 100644 --- a/spec/factories/file_uploads.rb +++ b/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 diff --git a/spec/jobs/import_census_job_spec.rb b/spec/jobs/import_census_job_spec.rb index 54b735b9a9..ad32b48fe4 100644 --- a/spec/jobs/import_census_job_spec.rb +++ b/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) diff --git a/spec/models/claim_spec.rb b/spec/models/claim_spec.rb index a6962a42ff..e63f7660eb 100644 --- a/spec/models/claim_spec.rb +++ b/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) } diff --git a/spec/models/eligible_fe_providers_importer_spec.rb b/spec/models/eligible_fe_providers_importer_spec.rb index e9fbebf634..14fe3a1b3b 100644 --- a/spec/models/eligible_fe_providers_importer_spec.rb +++ b/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