diff --git a/app/services/bulk_upload/lettings/log_creator.rb b/app/services/bulk_upload/lettings/log_creator.rb index 64d7f6cc7..e6ef290b1 100644 --- a/app/services/bulk_upload/lettings/log_creator.rb +++ b/app/services/bulk_upload/lettings/log_creator.rb @@ -8,8 +8,6 @@ class BulkUpload::Lettings::LogCreator def call row_parsers.each do |row_parser| - next unless row_parser.valid? - row_parser.log.bulk_upload = bulk_upload row_parser.log.save end diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index 43fd17b7d..69857707d 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/app/services/bulk_upload/lettings/validator.rb @@ -169,6 +169,10 @@ class BulkUpload::Lettings::Validator end end + def create_logs? + row_parsers.all?(&:valid?) + end + def self.question_for_field(field) QUESTIONS[field] end diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb index 7e7031c14..c8c8398c2 100644 --- a/app/services/bulk_upload/processor.rb +++ b/app/services/bulk_upload/processor.rb @@ -8,7 +8,7 @@ class BulkUpload::Processor def call download validator.call - create_logs + create_logs if validator.create_logs? ensure downloader.delete_local_file! end diff --git a/spec/fixtures/files/2022_23_lettings_bulk_upload.csv b/spec/fixtures/files/2022_23_lettings_bulk_upload.csv index 4a6b8c843..c20057489 100644 --- a/spec/fixtures/files/2022_23_lettings_bulk_upload.csv +++ b/spec/fixtures/files/2022_23_lettings_bulk_upload.csv @@ -69,5 +69,6 @@ if 87 = 1, then a value must be entered",No,,,Yes,,,,No,,,"If the property is be or 106 = 15 - 17",No,"Only if 1 = 2, 4, 6, 8, 10 or 12",,,,No,,No,"Yes, if 45 = 2, 3 or 6",,"Yes, if 50 = 1","Only if 1 = 1, 3, 5, 7, 9 or 11",No,Yes,,,,,,,,,,Only if 1 = 1 - 4 or 9 - 12.,Only if 1 = 1 - 8.,Only if 130 is not 3,No,No,Yes, Bulk upload format and duplicate check,All lettings,Question removed from 22/23 onwards,,Supported housing only,,Question Removed from 2020/21,,,,,,Duplicate check field,,,,,,,,Duplicate check field,,,,,,,,,,,,,,,Duplicate check field,,,,,,,,,,,,,,,,,,,Question removed from 22/23 onwards,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,Duplicate check field,,,,,,,,,,,Question removed from 22/23 onwards,Duplicate check fields,,,,Duplicate check field,General Needs lettings only,,,All lettings,General Needs lettings only,All lettings,General Needs lettings only,,,Question removed from 2020/21,Duplicate check field, “Username does not exist”. ,,,Question removed from 21/22 onwards,,Supported Housing lettings only.,,,,,,,,,,,,Affordable Rent Lettings only,Intermediate Rent Lettings only,,All lettings,All lettings,, Bulk upload field number,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100,101,102,103,104,105,106,107,108,109,110,111,112,113,114,115,116,117,118,119,120,121,122,123,124,125,126,127,128,129,130,131,132,133,134, +,1,,,,,,123,1,4,,2,55,54,,,,,,,F,,,,,,,,,,,,,,,1,,,,,,,,17,13,2,,2,3,4,,2,7,,,,,,,,,3,,,,2,1,2,1,3,,,,,,,,,16,4,1000,100,100,100,1300,,,,,,,,,,,,13,1,23,,,4,1,1,2,,,,EC1N,2TD,,3,,3,,,,,2,,,,,,,,,,,,,,1,2,2 ,1,,,,,,123,1,2,,6,55,54,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,, ,1,,,,,,123,1,2,,,55,54,,,,,,,"A",,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,13,1,23,,,,,,,,,,,,,123,,123,,,,,,,,,,,,,,,,,,,,,2, diff --git a/spec/services/bulk_upload/lettings/validator_spec.rb b/spec/services/bulk_upload/lettings/validator_spec.rb index 1543774e9..27d52b214 100644 --- a/spec/services/bulk_upload/lettings/validator_spec.rb +++ b/spec/services/bulk_upload/lettings/validator_spec.rb @@ -3,7 +3,9 @@ require "rails_helper" RSpec.describe BulkUpload::Lettings::Validator do subject(:validator) { described_class.new(bulk_upload:, path:) } - let(:bulk_upload) { create(:bulk_upload) } + let(:organisation) { create(:organisation, old_visible_id: "3") } + let(:user) { create(:user, organisation:) } + let(:bulk_upload) { create(:bulk_upload, user:) } let(:path) { file.path } let(:file) { Tempfile.new } @@ -29,18 +31,46 @@ RSpec.describe BulkUpload::Lettings::Validator do context "when incorrect headers" end - context "when a valid csv" do - let(:path) { file_fixture("2022_23_lettings_bulk_upload.csv") } + describe "#call" do + context "when a valid csv" do + let(:path) { file_fixture("2022_23_lettings_bulk_upload.csv") } - it "creates validation errors" do - expect { validator.call }.to change(BulkUploadError, :count) + it "creates validation errors" do + expect { validator.call }.to change(BulkUploadError, :count) + end + + it "create validation error with correct values" do + validator.call + + error = BulkUploadError.first + expect(error.row).to eq("7") + end + end + end + + describe "#should_create_logs?" do + context "when all logs are valid" do + let(:target_path) { file_fixture("2022_23_lettings_bulk_upload.csv") } + + before do + target_array = File.open(target_path).readlines + target_array[0..71].each do |line| + file.write line + end + file.rewind + end + + it "returns truthy" do + expect(validator).to be_create_logs + end end - it "create validation error with correct values" do - validator.call + context "when there is an invalid log" do + let(:path) { file_fixture("2022_23_lettings_bulk_upload.csv") } - error = BulkUploadError.first - expect(error.row).to eq("6") + it "returns falsey" do + expect(validator).not_to be_create_logs + end end end end diff --git a/spec/services/bulk_upload/processor_spec.rb b/spec/services/bulk_upload/processor_spec.rb index 2114fbfd7..aaff12678 100644 --- a/spec/services/bulk_upload/processor_spec.rb +++ b/spec/services/bulk_upload/processor_spec.rb @@ -43,6 +43,14 @@ RSpec.describe BulkUpload::Processor do ) end + let(:mock_validator) do + instance_double( + BulkUpload::Lettings::Validator, + call: nil, + create_logs?: true, + ) + end + let(:mock_creator) do instance_double( BulkUpload::Lettings::LogCreator, @@ -53,6 +61,7 @@ RSpec.describe BulkUpload::Processor do it "creates logs" do allow(BulkUpload::Downloader).to receive(:new).with(bulk_upload:).and_return(mock_downloader) + allow(BulkUpload::Lettings::Validator).to receive(:new).and_return(mock_validator) allow(BulkUpload::Lettings::LogCreator).to receive(:new).with(bulk_upload:, path:).and_return(mock_creator) processor.call