From 64b90fef102ef8a31af231e47b88c66557a1c6f1 Mon Sep 17 00:00:00 2001 From: SamSeed-Softwire <63662292+SamSeed-Softwire@users.noreply.github.com> Date: Tue, 25 Apr 2023 14:02:11 +0100 Subject: [PATCH] Fix: ignore bulk upload rows which contain whitespace only (#1587) * fix: ignore BU rows which contain whitespace only * test: ensure row as a whole is not blank in 'blank field' tests --- .../lettings/year2022/row_parser.rb | 1 + .../lettings/year2023/row_parser.rb | 1 + .../lettings/year2022/row_parser_spec.rb | 24 +++++++++++----- .../lettings/year2023/row_parser_spec.rb | 28 +++++++++++++------ 4 files changed, 38 insertions(+), 16 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2022/row_parser.rb b/app/services/bulk_upload/lettings/year2022/row_parser.rb index 650c3e896..5b88ebdbe 100644 --- a/app/services/bulk_upload/lettings/year2022/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2022/row_parser.rb @@ -368,6 +368,7 @@ class BulkUpload::Lettings::Year2022::RowParser .to_hash .reject { |k, _| %w[bulk_upload block_log_creation].include?(k) } .values + .reject(&:blank?) .compact .empty? end diff --git a/app/services/bulk_upload/lettings/year2023/row_parser.rb b/app/services/bulk_upload/lettings/year2023/row_parser.rb index 24b42a608..93b6d6ade 100644 --- a/app/services/bulk_upload/lettings/year2023/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2023/row_parser.rb @@ -373,6 +373,7 @@ class BulkUpload::Lettings::Year2023::RowParser .to_hash .reject { |k, _| %w[bulk_upload block_log_creation field_blank].include?(k) } .values + .reject(&:blank?) .compact .empty? end diff --git a/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb index f092b141e..43a4ad822 100644 --- a/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb @@ -165,7 +165,17 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do end end - context "when any field is populated" do + context "when the only populated fields are whitespace" do + before do + parser.field_18 = " " + end + + it "returns true" do + expect(parser).to be_blank_row + end + end + + context "when any field is populated with something other than whitespace" do before do parser.field_1 = "1" end @@ -765,7 +775,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do describe "#field_112" do # username for created_by context "when blank" do - let(:attributes) { { bulk_upload:, field_112: "" } } + let(:attributes) { { bulk_upload:, field_112: "", field_4: 1 } } it "is permitted" do expect(parser.errors[:field_112]).to be_blank @@ -817,7 +827,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do describe "#field_113" do # managing org context "when blank" do - let(:attributes) { { bulk_upload:, field_113: "" } } + let(:attributes) { { bulk_upload:, field_113: "", field_4: 1 } } it "is not permitted as setup error" do setup_errors = parser.errors.select { |e| e.options[:category] == :setup } @@ -909,7 +919,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do ].each do |known, age, field| describe "##{known} and ##{age}" do context "when #{field} is blank" do - let(:attributes) { { bulk_upload:, field.to_s => nil } } + let(:attributes) { { bulk_upload:, field.to_s => nil, field_4: 1 } } it "sets ##{known} 1" do expect(parser.log.public_send(known)).to be(1) @@ -1224,7 +1234,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do end context "when no" do - let(:attributes) { { bulk_upload:, hash[:field] => "" } } + let(:attributes) { { bulk_upload:, hash[:field] => "", field_4: 1 } } it "sets value from correct mapping" do expect(parser.log.public_send(hash[:attribute])).to be_nil @@ -1374,7 +1384,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do end context "when mrcdate not given" do - let(:attributes) { { bulk_upload:, field_92: "", field_93: "", field_94: "" } } + let(:attributes) { { bulk_upload:, field_92: "", field_93: "", field_94: "", field_4: 1 } } it "sets #majorrepairs to 0" do expect(parser.log.majorrepairs).to eq(0) @@ -1442,7 +1452,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do end context "when not known" do - let(:attributes) { { bulk_upload:, field_62: "" } } + let(:attributes) { { bulk_upload:, field_62: "", field_4: 1 } } it "sets to 0" do expect(parser.log.previous_la_known).to eq(0) diff --git a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb index 89abce90e..17d6aa50f 100644 --- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb @@ -48,7 +48,17 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end end - context "when any field is populated" do + context "when the only populated fields are whitespace" do + before do + parser.field_18 = " " + end + + it "returns true" do + expect(parser).to be_blank_row + end + end + + context "when any field is populated with something other than whitespace" do before do parser.field_1 = "1" end @@ -263,7 +273,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do describe "#field_3" do # created_by context "when blank" do - let(:attributes) { { bulk_upload:, field_3: "" } } + let(:attributes) { { bulk_upload:, field_3: "", field_4: 1 } } it "is permitted" do expect(parser.errors[:field_3]).to be_blank @@ -685,7 +695,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do describe "#field_1" do # owning org context "when blank" do - let(:attributes) { { bulk_upload:, field_1: "" } } + let(:attributes) { { bulk_upload:, field_1: "", field_4: 1 } } it "is not permitted as setup error" do setup_errors = parser.errors.select { |e| e.options[:category] == :setup } @@ -747,7 +757,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do describe "#field_2" do # managing org context "when blank" do - let(:attributes) { { bulk_upload:, field_2: "" } } + let(:attributes) { { bulk_upload:, field_2: "", field_4: 1 } } it "is not permitted as setup error" do setup_errors = parser.errors.select { |e| e.options[:category] == :setup } @@ -941,7 +951,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end context "when uprn blank" do - let(:attributes) { { bulk_upload:, field_18: "" } } + let(:attributes) { { bulk_upload:, field_18: "", field_4: 1 } } it "sets to 0" do expect(parser.log.uprn_known).to be(0) @@ -993,7 +1003,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do ].each do |known, age, field| describe "##{known} and ##{age}" do context "when #{field} is blank" do - let(:attributes) { { bulk_upload:, field.to_s => nil } } + let(:attributes) { { bulk_upload:, field.to_s => nil, field_4: 1 } } it "sets ##{known} 1" do expect(parser.log.public_send(known)).to be(1) @@ -1288,7 +1298,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end context "when no" do - let(:attributes) { { bulk_upload:, hash[:field] => "" } } + let(:attributes) { { bulk_upload:, hash[:field] => "", field_4: 1 } } it "sets value from correct mapping" do expect(parser.log.public_send(hash[:attribute])).to be_nil @@ -1438,7 +1448,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end context "when mrcdate not given" do - let(:attributes) { { bulk_upload:, field_36: "", field_37: "", field_38: "" } } + let(:attributes) { { bulk_upload:, field_36: "", field_37: "", field_38: "", field_4: 1 } } it "sets #majorrepairs to 0" do expect(parser.log.majorrepairs).to eq(0) @@ -1506,7 +1516,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end context "when not known" do - let(:attributes) { { bulk_upload:, field_109: "" } } + let(:attributes) { { bulk_upload:, field_109: "", field_4: 1 } } it "sets to 0" do expect(parser.log.previous_la_known).to eq(0)