feat: update tests and don't add errors to scheme/location fields unless determined

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

spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb

@@ -342,13 +342,120 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
             end
           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_16: scheme.old_visible_id,
+                valid_attributes.merge({ field_15: scheme.old_visible_id,
                                          field_4: "2",
                                          field_5: "2",
-                                       field_17: location.old_visible_id,
+                                         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
+
+              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
+
+            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) }
+
+              before do
+                parser.log.save!
+                parser.instance_variable_set(:@valid, nil)
+              end
+
+              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_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 "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,
@@ -394,20 +501,20 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
             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,
+                valid_attributes.merge({ field_16: "S#{scheme.id}",
                                          field_4: "2",
                                          field_5: "2",
-                                       field_17: location.old_visible_id,
+                                         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: scheme.old_visible_id,
+                valid_attributes.merge({ field_16: "S#{scheme.id}",
                                          field_4: "2",
                                          field_5: "2",
-                                       field_17: location.old_visible_id,
+                                         field_17: location.id,
                                          field_1: owning_org.old_visible_id,
                                          field_125: 0,
                                          field_44: 4,
@@ -445,6 +552,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
                 end
               end
             end
+          end
 
           context "when a hidden log already exists in db" do
             before do
@@ -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