diff --git a/app/services/bulk_upload/lettings/year2023/row_parser.rb b/app/services/bulk_upload/lettings/year2023/row_parser.rb index 6a12f8545..9bd8eb8d2 100644 --- a/app/services/bulk_upload/lettings/year2023/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2023/row_parser.rb @@ -749,20 +749,20 @@ private def validate_location_related return if scheme.blank? || location.blank? - unless location.scheme == scheme + if location.scheme != scheme && location_field.present? block_log_creation! errors.add(location_field, "#{scheme_or_management_group.capitalize} code must relate to a #{location_or_scheme} that is owned by owning organisation or managing organisation", category: :setup) end end def validate_location_exists - if scheme && location_id.present? && location.nil? + if scheme && location_id.present? && location.nil? && location_field.present? errors.add(location_field, "#{location_or_scheme.capitalize} could not be found with the provided #{scheme_or_management_group} code", category: :setup) end end def validate_location_data_given - if supported_housing? && location_id.blank? + if supported_housing? && location_id.blank? && location_field.present? errors.add(location_field, I18n.t("validations.not_answered", question: "#{location_or_scheme} code"), category: "setup") end end @@ -773,20 +773,20 @@ private owned_by_owning_org = owning_organisation && scheme.owning_organisation == owning_organisation owned_by_managing_org = managing_organisation && scheme.owning_organisation == managing_organisation - unless owned_by_owning_org || owned_by_managing_org + if !(owned_by_owning_org || owned_by_managing_org) && scheme_field.present? block_log_creation! errors.add(scheme_field, "This #{scheme_or_management_group} code does not belong to your organisation, or any of your stock owners / managing agents", category: :setup) end end def validate_scheme_exists - if scheme_id.present? && scheme.nil? + if scheme_id.present? && scheme_field.present? && scheme.nil? errors.add(scheme_field, "The #{scheme_or_management_group} code is not correct", category: :setup) end end def validate_scheme_data_given - if supported_housing? && scheme_id.blank? + if supported_housing? && scheme_field.present? && scheme_id.blank? errors.add(scheme_field, I18n.t("validations.not_answered", question: "#{scheme_or_management_group} code"), category: "setup") end end @@ -868,7 +868,7 @@ private errors.add(:field_8, error_message) # startdate errors.add(:field_9, error_message) # startdate errors.add(:field_13, error_message) # tenancycode - errors.add(location_field, error_message) if field_4 != 1 # location + errors.add(location_field, error_message) if field_4 != 1 && location_field.present?# location errors.add(:field_23, error_message) if field_4 != 2 # postcode_full errors.add(:field_24, error_message) if field_4 != 2 # postcode_full errors.add(:field_25, error_message) if field_4 != 2 # la 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 d4d62f6cf..cf2fc3c42 100644 --- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb @@ -342,106 +342,214 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end end - context "when a supported housing log with chcharges already exists in the db" do - let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } - let(:attributes) do - valid_attributes.merge({ field_16: scheme.old_visible_id, - field_4: "2", - field_5: "2", - field_17: location.old_visible_id, - field_1: owning_org.old_visible_id, - field_125: 0, - field_44: 4, - field_127: "88" }) - end + context "with old core scheme and location ids" do + context "when a supported housing log with chcharges already exists in the db" do + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } + let(:attributes) do + valid_attributes.merge({ field_15: scheme.old_visible_id, + field_4: "2", + field_5: "2", + field_16: location.old_visible_id, + field_1: owning_org.old_visible_id, + field_125: 0, + field_44: 4, + field_127: "88" }) + end - before do - parser.log.save! - parser.instance_variable_set(:@valid, nil) - end + before do + parser.log.save! + parser.instance_variable_set(:@valid, nil) + end - it "is not a valid row" do - expect(parser).not_to be_valid + it "is not a valid row" do + expect(parser).not_to be_valid + end + + it "adds an error to all the fields used to determine duplicates" do + parser.valid? + + error_message = "This is a duplicate log" + + [ + :field_1, # owning_organisation + :field_7, # startdate + :field_8, # startdate + :field_9, # startdate + :field_13, # tenancycode + :field_16, # location + :field_46, # age1 + :field_47, # sex1 + :field_50, # ecstat1 + :field_127, # chcharge + :field_125, # household_charge + ].each do |field| + expect(parser.errors[field]).to include(error_message) + end + + expect(parser.errors[:field_23]).not_to include(error_message) + expect(parser.errors[:field_24]).not_to include(error_message) + expect(parser.errors[:field_25]).not_to include(error_message) + end end - it "adds an error to all the fields used to determine duplicates" do - parser.valid? + context "when a supported housing log different chcharges already exists in the db" do + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } + let(:attributes) do + valid_attributes.merge({ field_15: scheme.old_visible_id, + field_4: "2", + field_5: "2", + field_16: location.old_visible_id, + field_1: owning_org.old_visible_id, + field_125: 0, + field_44: 4, + field_127: "88" }) + end + let(:attributes_too) do + valid_attributes.merge({ field_15: scheme.old_visible_id, + field_4: "2", + field_5: "2", + field_16: location.old_visible_id, + field_1: owning_org.old_visible_id, + field_125: 0, + field_44: 4, + field_127: "98" }) + end + let(:parser_too) { described_class.new(attributes_too) } - error_message = "This is a duplicate log" + before do + parser.log.save! + parser.instance_variable_set(:@valid, nil) + end - [ - :field_1, # owning_organisation - :field_7, # startdate - :field_8, # startdate - :field_9, # startdate - :field_13, # tenancycode - :field_17, # location - :field_46, # age1 - :field_47, # sex1 - :field_50, # ecstat1 - :field_127, # chcharge - :field_125, # household_charge - ].each do |field| - expect(parser.errors[field]).to include(error_message) + it "is a valid row" do + expect(parser_too).to be_valid end - expect(parser.errors[:field_23]).not_to include(error_message) - expect(parser.errors[:field_24]).not_to include(error_message) - expect(parser.errors[:field_25]).not_to include(error_message) + it "does not add an error to all the fields used to determine duplicates" do + parser_too.valid? + + error_message = "This is a duplicate log" + + [ + :field_1, # owning_organisation + :field_7, # startdate + :field_8, # startdate + :field_9, # startdate + :field_13, # tenancycode + :field_16, # location + :field_46, # age1 + :field_47, # sex1 + :field_50, # ecstat1 + :field_132, # tcharge + ].each do |field| + expect(parser_too.errors[field]).not_to include(error_message) + end + end end end - context "when a supported housing log different chcharges already exists in the db" do - let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } - let(:attributes) do - valid_attributes.merge({ field_16: scheme.old_visible_id, - field_4: "2", - field_5: "2", - field_17: location.old_visible_id, - field_1: owning_org.old_visible_id, - field_125: 0, - field_44: 4, - field_127: "88" }) - end - let(:attributes_too) do - valid_attributes.merge({ field_16: scheme.old_visible_id, - field_4: "2", - field_5: "2", - field_17: location.old_visible_id, - field_1: owning_org.old_visible_id, - field_125: 0, - field_44: 4, - field_127: "98" }) - end - let(:parser_too) { described_class.new(attributes_too) } + context "with new core scheme and location ids" do + context "when a supported housing log with chcharges already exists in the db" do + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } + let(:attributes) do + valid_attributes.merge({ field_16: "S#{scheme.id}", + field_4: "2", + field_5: "2", + field_17: location.id, + field_1: owning_org.old_visible_id, + field_125: 0, + field_44: 4, + field_127: "88" }) + end - before do - parser.log.save! - parser.instance_variable_set(:@valid, nil) - end + before do + parser.log.save! + parser.instance_variable_set(:@valid, nil) + end - it "is a valid row" do - expect(parser_too).to be_valid + it "is not a valid row" do + expect(parser).not_to be_valid + end + + it "adds an error to all the fields used to determine duplicates" do + parser.valid? + + error_message = "This is a duplicate log" + + [ + :field_1, # owning_organisation + :field_7, # startdate + :field_8, # startdate + :field_9, # startdate + :field_13, # tenancycode + :field_17, # location + :field_46, # age1 + :field_47, # sex1 + :field_50, # ecstat1 + :field_127, # chcharge + :field_125, # household_charge + ].each do |field| + expect(parser.errors[field]).to include(error_message) + end + + expect(parser.errors[:field_23]).not_to include(error_message) + expect(parser.errors[:field_24]).not_to include(error_message) + expect(parser.errors[:field_25]).not_to include(error_message) + end end - it "does not add an error to all the fields used to determine duplicates" do - parser_too.valid? + context "when a supported housing log different chcharges already exists in the db" do + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } + let(:attributes) do + valid_attributes.merge({ field_16: "S#{scheme.id}", + field_4: "2", + field_5: "2", + field_17: location.id, + field_1: owning_org.old_visible_id, + field_125: 0, + field_44: 4, + field_127: "88" }) + end + let(:attributes_too) do + valid_attributes.merge({ field_16: "S#{scheme.id}", + field_4: "2", + field_5: "2", + field_17: location.id, + field_1: owning_org.old_visible_id, + field_125: 0, + field_44: 4, + field_127: "98" }) + end + let(:parser_too) { described_class.new(attributes_too) } - error_message = "This is a duplicate log" + before do + parser.log.save! + parser.instance_variable_set(:@valid, nil) + end - [ - :field_1, # owning_organisation - :field_7, # startdate - :field_8, # startdate - :field_9, # startdate - :field_13, # tenancycode - :field_17, # location - :field_46, # age1 - :field_47, # sex1 - :field_50, # ecstat1 - :field_132, # tcharge - ].each do |field| - expect(parser_too.errors[field]).not_to include(error_message) + it "is a valid row" do + expect(parser_too).to be_valid + end + + it "does not add an error to all the fields used to determine duplicates" do + parser_too.valid? + + error_message = "This is a duplicate log" + + [ + :field_1, # owning_organisation + :field_7, # startdate + :field_8, # startdate + :field_9, # startdate + :field_13, # tenancycode + :field_17, # location + :field_46, # age1 + :field_47, # sex1 + :field_50, # ecstat1 + :field_132, # tcharge + ].each do |field| + expect(parser_too.errors[field]).not_to include(error_message) + end end end end @@ -582,7 +690,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do describe "#field_5" do context "when null" do - let(:attributes) { { bulk_upload:, field_5: nil } } + let(:attributes) { { bulk_upload:, field_5: nil, field_15: "1" } } it "returns an error" do expect(parser.errors[:field_5]).to be_present @@ -1508,7 +1616,15 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do describe "#location" do context "when lookup is via new core id" do - let(:attributes) { { bulk_upload:, field_16: scheme.old_visible_id, field_17: location.id, field_1: owning_org } } + let(:attributes) { { bulk_upload:, field_16: "S#{scheme.id}", field_17: location.id, field_1: owning_org } } + + it "assigns the correct location" do + expect(parser.log.location).to eql(location) + end + end + + context "when lookup is via old core id" do + let(:attributes) { { bulk_upload:, field_15: scheme.old_visible_id, field_16: location.old_visible_id, field_1: owning_org } } it "assigns the correct location" do expect(parser.log.location).to eql(location) @@ -1517,13 +1633,21 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end describe "#scheme" do - context "when lookup is via id prefixed with S" do + context "when lookup is via new core id" do let(:attributes) { { bulk_upload:, field_16: "S#{scheme.id}", field_1: owning_org } } it "assigns the correct scheme" do expect(parser.log.scheme).to eql(scheme) end end + + context "when lookup is via old core id" do + let(:attributes) { { bulk_upload:, field_15: scheme.old_visible_id, field_16: location.old_visible_id, field_1: owning_org } } + + it "assigns the correct scheme" do + expect(parser.log.scheme).to eql(scheme) + end + end end describe "#owning_organisation" do