feat: update tests and functionality

app/services/bulk_upload/lettings/year2023/row_parser.rb

@@ -329,6 +329,15 @@ class BulkUpload::Lettings::Year2023::RowParser
             },
             on: :after_log
 
+
+  validates :field_15,
+            presence: {
+              if: proc { [2, 4, 6, 8, 10, 12].include?(field_5) && log_uses_old_scheme_id? },
+              message: I18n.t("validations.not_answered", question: "management group code"),
+              category: :setup,
+            },
+            on: :after_log
+
   validates :field_16,
             presence: {
               if: proc { [2, 4, 6, 8, 10, 12].include?(field_5) },
@@ -337,6 +346,14 @@ class BulkUpload::Lettings::Year2023::RowParser
             },
             on: :after_log
 
+  validates :field_17,
+            presence: {
+              if: proc { [2, 4, 6, 8, 10, 12].include?(field_5) && log_uses_new_scheme_id? },
+              message: I18n.t("validations.not_answered", question: "location code"),
+              category: :setup,
+            },
+            on: :after_log
+
   validates :field_46, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 1 must be a number or the letter R" }, on: :after_log
   validates :field_52, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 2 must be a number or the letter R" }, allow_blank: true, on: :after_log
   validates :field_56, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 3 must be a number or the letter R" }, allow_blank: true, on: :after_log
@@ -740,7 +757,7 @@ private
   end
 
   def validate_location_exists
-    if scheme && location.nil?
+    if scheme && location_id.present? && location.nil?
       errors.add(location_field, "#{location_or_scheme.capitalize} could not be found with the provided #{scheme_or_management_group} code", category: :setup)
     end
   end
@@ -1274,20 +1291,28 @@ private
     field_16&.start_with?("S")
   end
 
+  def log_uses_old_scheme_id?
+    field_16.present? && !field_16.start_with?("S")
+  end
+
   def scheme_field
-    log_uses_new_scheme_id? ? :field_16 : :field_15
+    return :field_16 if log_uses_new_scheme_id?
+    return :field_15 if log_uses_old_scheme_id?
   end
 
   def scheme_id
-    log_uses_new_scheme_id? ? field_16 : field_15
+    return field_16 if log_uses_new_scheme_id?
+    return field_15 if log_uses_old_scheme_id?
   end
 
   def location_field
-    log_uses_new_scheme_id? ? :field_17 : :field_16
+    return :field_17 if log_uses_new_scheme_id?
+    return :field_16 if log_uses_old_scheme_id?
   end
 
   def location_id
-    log_uses_new_scheme_id? ? field_17 : field_16
+    return field_17 if log_uses_new_scheme_id?
+    return field_16 if log_uses_old_scheme_id?
   end
 
   def scheme_or_management_group

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

@@ -652,164 +652,207 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
       end
     end
 
-    describe "#field_15" do
+    describe "#field_15, field_16, field_17" do # scheme and location fields
-      context "when using New CORE ids" do
-
-      end
-
-      context "when using New CORE ids" do
-
-      end
-    end
-
-    describe "#field_16" do
       context "when nullable not permitted" do
-        let(:attributes) { { bulk_upload:, field_5: "2", field_16: nil } }
+        let(:attributes) { { bulk_upload:, field_5: "2", field_15: nil, field_16: nil, field_17: nil } }
 
         it "cannot be nulled" do
-          expect(parser.errors[:field_16]).to include("You must answer scheme code")
+          expect(parser.errors[:field_15]).to be_blank
+          expect(parser.errors[:field_16]).to eq(["You must answer scheme code"])
+          expect(parser.errors[:field_17]).to be_blank
         end
       end
 
       context "when nullable permitted" do
-        let(:attributes) { { bulk_upload:, field_5: "1", field_16: nil } }
+        let(:attributes) { { bulk_upload:, field_5: "1", field_15: nil, field_16: nil, field_17: nil } }
 
         it "can be nulled" do
           expect(parser.errors[:field_15]).to be_blank
+          expect(parser.errors[:field_16]).to be_blank
+          expect(parser.errors[:field_17]).to be_blank
         end
       end
 
       context "when using New CORE ids" do
+        let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) }
+        let(:location) { create(:location, :with_old_visible_id, scheme:) }
+
         context "when matching scheme cannot be found" do
-          let(:attributes) { { bulk_upload:, field_5: "1", field_16: "S123" } }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_5: "2", field_16: "S123", field_17: location.id } }
 
           it "returns a setup error" do
-            expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to include("The scheme code is not correct")
+            expect(parser.errors[:field_15]).to be_blank
+            expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["The scheme code is not correct"])
+            expect(parser.errors[:field_17]).to be_blank
           end
         end
 
-        context "when scheme belongs to someone else" do
+        context "when missing location" do
-          let(:other_scheme) { create(:scheme, :with_old_visible_id) }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_5: "2", field_16: "S#{scheme.id}", field_17: nil } }
-          let(:attributes) { { bulk_upload:, field_5: "1", field_16: "S#{other_scheme.id}", field_1: owning_org.old_visible_id } }
 
           it "returns a setup error" do
-            expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to include("This scheme code does not belong to your organisation, or any of your stock owners / managing agents")
+            expect(parser.errors[:field_15]).to be_blank
+            expect(parser.errors[:field_16]).to be_blank
+            expect(parser.errors.where(:field_17, category: :setup).map(&:message)).to eq(["You must answer location code"])
           end
         end
 
-        context "when scheme belongs to owning org" do
+        context "when matching location cannot be found" do
-          let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_5: "2", field_16: "S#{scheme.id}", field_17: "123" } }
-          let(:attributes) { { bulk_upload:, field_5: "1", field_16: "S#{scheme.id}", field_1: owning_org.old_visible_id } }
 
-          it "does not return an error" do
+          it "returns a setup error" do
+            expect(parser.errors[:field_15]).to be_blank
             expect(parser.errors[:field_16]).to be_blank
+            expect(parser.errors.where(:field_17, category: :setup).map(&:message)).to eq(["Location could not be found with the provided scheme code"])
           end
         end
 
-        context "when scheme belongs to managing org" do
+        context "when matching location exists" do
-          let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: managing_org) }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_5: "2", field_16: "S#{scheme.id}", field_17: location.id } }
-          let(:attributes) { { bulk_upload:, field_5: "1", field_16: "S#{scheme.id}", field_2: managing_org.old_visible_id } }
 
           it "does not return an error" do
+            expect(parser.errors[:field_15]).to be_blank
             expect(parser.errors[:field_16]).to be_blank
+            expect(parser.errors[:field_17]).to be_blank
           end
         end
-      end
 
-      context "when using Old CORE ids" do
+        context "when location exists but not related" do
-        context "when matching scheme cannot be found" do
+          let(:other_scheme) { create(:scheme, :with_old_visible_id) }
-          let(:attributes) { { bulk_upload:, field_5: "1", field_16: "123" } }
+          let(:other_location) { create(:location, :with_old_visible_id, scheme: other_scheme) }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_5: "2", field_16: "S#{scheme.id}", field_17: other_location.id } }
 
           it "returns a setup error" do
-            expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to include("The scheme code is not correct")
+            expect(parser.errors[:field_15]).to be_blank
+            expect(parser.errors[:field_16]).to be_blank
+            expect(parser.errors.where(:field_17, category: :setup).map(&:message)).to eq(["Location could not be found with the provided scheme code"])
           end
         end
 
         context "when scheme belongs to someone else" do
           let(:other_scheme) { create(:scheme, :with_old_visible_id) }
-          let(:attributes) { { bulk_upload:, field_5: "1", field_16: other_scheme.old_visible_id, field_1: owning_org.old_visible_id } }
+          let(:other_location) { create(:location, :with_old_visible_id, scheme: other_scheme) }
+          let(:attributes) { { bulk_upload:, field_5: "2", field_16: "S#{other_scheme.id}", field_17: other_location.id, field_1: owning_org.old_visible_id } }
 
           it "returns a setup error" do
-            expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to include("This scheme code does not belong to your organisation, or any of your stock owners / managing agents")
+            expect(parser.errors[:field_15]).to be_blank
+            expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["This scheme code does not belong to your organisation, or any of your stock owners / managing agents"])
+            expect(parser.errors[:field_17]).to be_blank
           end
         end
 
         context "when scheme belongs to owning org" do
-          let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) }
+          let(:attributes) { { bulk_upload:, field_5: "2", field_16: "S#{scheme.id}", field_17: location.id, field_1: owning_org.old_visible_id } }
-          let(:attributes) { { bulk_upload:, field_5: "1", field_16: scheme.old_visible_id, field_1: owning_org.old_visible_id } }
 
           it "does not return an error" do
+            expect(parser.errors[:field_15]).to be_blank
             expect(parser.errors[:field_16]).to be_blank
+            expect(parser.errors[:field_17]).to be_blank
           end
         end
 
         context "when scheme belongs to managing org" do
-          let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: managing_org) }
+          let(:managing_org_scheme) { create(:scheme, :with_old_visible_id, owning_organisation: managing_org) }
-          let(:attributes) { { bulk_upload:, field_5: "1", field_16: scheme.old_visible_id, field_2: managing_org.old_visible_id } }
+          let(:managing_org_location) { create(:location, :with_old_visible_id, scheme: managing_org_scheme) }
+          let(:attributes) { { bulk_upload:, field_5: "2", field_16: "S#{managing_org_scheme.id}", field_17: managing_org_location.id, field_2: managing_org.old_visible_id } }
 
           it "does not return an error" do
+            expect(parser.errors[:field_15]).to be_blank
             expect(parser.errors[:field_16]).to be_blank
+            expect(parser.errors[:field_17]).to be_blank
           end
         end
       end
-    end
 
-    describe "#field_17" do
+      context "when using Old CORE ids" do
-      context "when using New CORE ids" do
+        let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) }
-        context "when location does not exist" do
+        let(:location) { create(:location, :with_old_visible_id, scheme:) }
-          let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) }
+
-          let(:attributes) do
+        context "when matching scheme cannot be found" do
-            {
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_5: "2", field_15: "123", field_16: location.old_visible_id } }
-              bulk_upload:,
+
-              field_5: "1",
+          it "returns a setup error" do
-              field_16: scheme.old_visible_id,
+            expect(parser.errors.where(:field_15, category: :setup).map(&:message)).to eq(["The management group code is not correct"])
-              field_17: "dontexist",
+            expect(parser.errors[:field_16]).to be_blank
-              field_1: owning_org.old_visible_id,
+            expect(parser.errors[:field_17]).to be_blank
-            }
           end
+        end
+
+        context "when missing location" do
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_5: "2", field_15: scheme.old_visible_id, field_16: nil } }
 
           it "returns a setup error" do
-            expect(parser.errors.where(:field_17, category: :setup)).to be_present
+            expect(parser.errors[:field_15]).to be_blank
+            expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["You must answer scheme code"])
+            expect(parser.errors[:field_17]).to be_blank
           end
         end
 
-        context "when location exists" do
+        context "when matching location cannot be found" do
-          let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_5: "2", field_15: scheme.old_visible_id, field_16: "123" } }
-          let(:attributes) do
+
-            {
+          it "returns a setup error" do
-              bulk_upload:,
+            expect(parser.errors[:field_15]).to be_blank
-              field_5: "1",
+            expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["Scheme could not be found with the provided management group code"])
-              field_16: scheme.old_visible_id,
+            expect(parser.errors[:field_17]).to be_blank
-              field_17: location.old_visible_id,
-              field_1: owning_org.old_visible_id,
-            }
           end
+        end
+
+        context "when matching location exists" do
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_5: "2", field_15: scheme.old_visible_id, field_16: location.old_visible_id } }
 
           it "does not return an error" do
+            expect(parser.errors[:field_15]).to be_blank
+            expect(parser.errors[:field_16]).to be_blank
             expect(parser.errors[:field_17]).to be_blank
           end
         end
 
         context "when location exists but not related" do
-          let(:location) { create(:scheme, :with_old_visible_id) }
+          let(:other_scheme) { create(:scheme, :with_old_visible_id) }
-          let(:attributes) do
+          let(:other_location) { create(:location, :with_old_visible_id, scheme: other_scheme) }
-            {
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_5: "2", field_15: scheme.old_visible_id, field_16: other_location.old_visible_id } }
-              bulk_upload:,
+
-              field_5: "1",
+          it "returns a setup error" do
-              field_16: scheme.old_visible_id,
+            expect(parser.errors[:field_15]).to be_blank
-              field_17: location.old_visible_id,
+            expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["Scheme could not be found with the provided management group code"])
-              field_1: owning_org.old_visible_id,
+            expect(parser.errors[:field_17]).to be_blank
-            }
           end
+        end
+
+        context "when scheme belongs to someone else" do
+          let(:other_scheme) { create(:scheme, :with_old_visible_id) }
+          let(:other_location) { create(:location, :with_old_visible_id, scheme: other_scheme) }
+          let(:attributes) { { bulk_upload:, field_5: "2", field_15: other_scheme.old_visible_id, field_16: other_location.old_visible_id, field_1: owning_org.old_visible_id } }
 
           it "returns a setup error" do
-            expect(parser.errors.where(:field_17, category: :setup)).to be_present
+            expect(parser.errors.where(:field_15, category: :setup).map(&:message)).to eq(["This management group code does not belong to your organisation, or any of your stock owners / managing agents"])
+            expect(parser.errors[:field_16]).to be_blank
+            expect(parser.errors[:field_17]).to be_blank
           end
         end
-      end
 
-      context "when using Old CORE ids" do
+        context "when scheme belongs to owning org" do
+          let(:attributes) { { bulk_upload:, field_5: "2", field_15: scheme.old_visible_id, field_16: location.old_visible_id, field_1: owning_org.old_visible_id } }
+
+          it "does not return an error" do
+            expect(parser.errors[:field_15]).to be_blank
+            expect(parser.errors[:field_16]).to be_blank
+            expect(parser.errors[:field_17]).to be_blank
+          end
+        end
 
+        context "when scheme belongs to managing org" do
+          let(:managing_org_scheme) { create(:scheme, :with_old_visible_id, owning_organisation: managing_org) }
+          let(:managing_org_location) { create(:location, :with_old_visible_id, scheme: managing_org_scheme) }
+          let(:attributes) { { bulk_upload:, field_5: "2", field_15: managing_org_scheme.old_visible_id, field_16: managing_org_location.old_visible_id, field_2: managing_org.old_visible_id } }
+
+          it "does not return an error" do
+            expect(parser.errors[:field_15]).to be_blank
+            expect(parser.errors[:field_16]).to be_blank
+            expect(parser.errors[:field_17]).to be_blank
+          end
+        end
       end
     end