diff --git a/app/services/bulk_upload/lettings/log_creator.rb b/app/services/bulk_upload/lettings/log_creator.rb index 9dd4a2f66..625d53f43 100644 --- a/app/services/bulk_upload/lettings/log_creator.rb +++ b/app/services/bulk_upload/lettings/log_creator.rb @@ -10,6 +10,8 @@ class BulkUpload::Lettings::LogCreator row_parsers.each do |row_parser| row_parser.valid? + next if row_parser.blank_row? + row_parser.log.blank_invalid_non_setup_fields! row_parser.log.bulk_upload = bulk_upload diff --git a/app/services/bulk_upload/lettings/row_parser.rb b/app/services/bulk_upload/lettings/row_parser.rb index 42e762be1..9d5604587 100644 --- a/app/services/bulk_upload/lettings/row_parser.rb +++ b/app/services/bulk_upload/lettings/row_parser.rb @@ -154,6 +154,8 @@ class BulkUpload::Lettings::RowParser def valid? errors.clear + return true if blank_row? + super log.valid? @@ -166,6 +168,10 @@ class BulkUpload::Lettings::RowParser errors.blank? end + def blank_row? + attribute_set.to_hash.reject { |k, _| %w[bulk_upload].include?(k) }.values.compact.empty? + end + def log @log ||= LettingsLog.new(attributes_for_log) end diff --git a/spec/services/bulk_upload/lettings/log_creator_spec.rb b/spec/services/bulk_upload/lettings/log_creator_spec.rb index fc7d1cdc0..39dfd00fe 100644 --- a/spec/services/bulk_upload/lettings/log_creator_spec.rb +++ b/spec/services/bulk_upload/lettings/log_creator_spec.rb @@ -24,6 +24,23 @@ RSpec.describe BulkUpload::Lettings::LogCreator do end end + context "when a valid csv with several blank rows" do + let(:file) { Tempfile.new } + let(:path) { file.path } + let(:log) { LettingsLog.new } + + before do + file.write(BulkUpload::LogToCsv.new(log:, col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log:, col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log:, col_offset: 0).to_csv_row) + file.rewind + end + + it "ignores them and does not create the logs" do + expect { service.call }.not_to change(LettingsLog, :count) + end + end + context "when a valid csv with row with one invalid non setup field" do let(:file) { Tempfile.new } let(:path) { file.path } diff --git a/spec/services/bulk_upload/lettings/row_parser_spec.rb b/spec/services/bulk_upload/lettings/row_parser_spec.rb index 29ce6efbb..8f609838c 100644 --- a/spec/services/bulk_upload/lettings/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/row_parser_spec.rb @@ -31,6 +31,24 @@ RSpec.describe BulkUpload::Lettings::RowParser do FormHandler.instance.use_fake_forms! end + describe "#blank_row?" do + context "when a new object" do + it "returns true" do + expect(parser).to be_blank_row + end + end + + context "when any field is populated" do + before do + parser.field_1 = "1" + end + + it "returns false" do + expect(parser).not_to be_blank_row + end + end + end + describe "validations" do before do stub_request(:get, /api.postcodes.io/) @@ -40,6 +58,14 @@ RSpec.describe BulkUpload::Lettings::RowParser do end describe "#valid?" do + context "when the row is blank" do + let(:attributes) { { bulk_upload: } } + + it "returns true" do + expect(parser).to be_valid + end + end + context "when calling the method multiple times" do let(:attributes) { { bulk_upload:, field_134: 2 } } @@ -172,7 +198,7 @@ RSpec.describe BulkUpload::Lettings::RowParser do describe "#field_1" do context "when null" do - let(:attributes) { { bulk_upload:, field_1: nil } } + let(:attributes) { { bulk_upload:, field_1: nil, field_4: "1" } } it "returns an error" do expect(parser.errors[:field_1]).to be_present @@ -347,7 +373,7 @@ RSpec.describe BulkUpload::Lettings::RowParser do describe "fields 96, 97, 98 => startdate" do context "when any one of these fields is blank" do - let(:attributes) { { bulk_upload:, field_96: nil, field_97: nil, field_98: nil } } + let(:attributes) { { bulk_upload:, field_1: "1", field_96: nil, field_97: nil, field_98: nil } } it "returns an error" do parser.valid?