feat: update scheme and location lookup to search for exact correct combination of org, scheme and location (#2016)

app/models/scheme.rb

@@ -163,13 +163,15 @@ class Scheme < ApplicationRecord
 
   enum arrangement_type: ARRANGEMENT_TYPE, _suffix: true
 
-  def self.find_by_id_on_multiple_fields(id)
-    return if id.nil?
+  def self.find_by_id_on_multiple_fields(scheme_id, location_id)
+    return if scheme_id.nil?
 
-    if id.start_with?("S")
-      where(id: id[1..]).first
+    if scheme_id.start_with?("S")
+      where(id: scheme_id[1..]).first
+    elsif location_id.present?
+      joins(:locations).where("schemes.old_visible_id = ? AND locations.old_visible_id = ?", scheme_id.to_s, location_id.to_s).first || where(old_visible_id: scheme_id).first
     else
-      where(old_visible_id: id).first
+      where(old_visible_id: scheme_id).first
     end
   end
 

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

@@ -354,12 +354,10 @@ class BulkUpload::Lettings::Year2022::RowParser
   validate :validate_managing_org_exists, on: :after_log
   validate :validate_managing_org_related, on: :after_log
 
-  validate :validate_scheme_related, on: :after_log
-  validate :validate_scheme_exists, on: :after_log
+  validate :validate_related_scheme_exists, on: :after_log
   validate :validate_scheme_data_given, on: :after_log
 
-  validate :validate_location_related, on: :after_log
-  validate :validate_location_exists, on: :after_log
+  validate :validate_related_location_exists, on: :after_log
   validate :validate_location_data_given, on: :after_log
 
   validate :validate_created_by_exists, on: :after_log
@@ -530,53 +528,36 @@ private
     ].compact
   end
 
-  def validate_location_related
-    return if scheme.blank? || location.blank?
-
-    unless location.scheme == scheme
-      block_log_creation!
-      errors.add(:field_5, "Scheme code must relate to a location that is owned by owning organisation or managing organisation")
-    end
-  end
-
   def location
     return if scheme.nil?
 
     @location ||= scheme.locations.find_by_id_on_multiple_fields(field_5)
   end
 
-  def validate_location_exists
+  def validate_related_location_exists
     if scheme && field_5.present? && location.nil?
-      errors.add(:field_5, "Location could not be found with the provided scheme code", category: :setup)
+      block_log_creation!
+      errors.add(:field_5, "Scheme code must relate to a scheme that is owned by the owning organisation or managing organisation", category: :setup)
     end
   end
 
   def validate_location_data_given
     if bulk_upload.supported_housing? && field_5.blank?
+      block_log_creation!
       errors.add(:field_5, I18n.t("validations.not_answered", question: "scheme code"), category: :setup)
     end
   end
 
-  def validate_scheme_related
-    return unless field_4.present? && scheme.present?
-
-    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
+  def validate_related_scheme_exists
+    if field_4.present? && owning_organisation.present? && managing_organisation.present? && scheme.nil?
       block_log_creation!
-      errors.add(:field_4, "This 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 field_4.present? && scheme.nil?
-      errors.add(:field_4, "The management group code is not correct", category: :setup)
+      errors.add(:field_4, "This management group code does not belong to the owning organisation or managing organisation", category: :setup)
     end
   end
 
   def validate_scheme_data_given
     if bulk_upload.supported_housing? && field_4.blank?
+      block_log_creation!
       errors.add(:field_4, I18n.t("validations.not_answered", question: "management group code"), category: :setup)
     end
   end
@@ -1488,6 +1469,8 @@ private
   end
 
   def scheme
-    @scheme ||= Scheme.find_by_id_on_multiple_fields(field_4)
+    return if field_4.nil? || owning_organisation.nil? || managing_organisation.nil?
+
+    @scheme ||= Scheme.where(id: (owning_organisation.owned_schemes + managing_organisation.owned_schemes).map(&:id)).find_by_id_on_multiple_fields(field_4, field_5)
   end
 end

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

@@ -387,12 +387,10 @@ class BulkUpload::Lettings::Year2023::RowParser
   validate :validate_managing_org_exists, on: :after_log
   validate :validate_managing_org_related, on: :after_log
 
-  validate :validate_scheme_related, on: :after_log
-  validate :validate_scheme_exists, on: :after_log
+  validate :validate_related_scheme_exists, on: :after_log
   validate :validate_scheme_data_given, on: :after_log
 
-  validate :validate_location_related, on: :after_log
-  validate :validate_location_exists, on: :after_log
+  validate :validate_related_location_exists, on: :after_log
   validate :validate_location_data_given, on: :after_log
 
   validate :validate_created_by_exists, on: :after_log
@@ -746,47 +744,30 @@ private
     end
   end
 
-  def validate_location_related
-    return if scheme.blank? || location.blank?
-
-    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
+  def validate_related_location_exists
     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)
+      block_log_creation!
+      errors.add(location_field, "#{location_or_scheme.capitalize} code must relate to a #{location_or_scheme} that is owned by the owning organisation or managing organisation", category: :setup)
     end
   end
 
   def validate_location_data_given
     if supported_housing? && location_id.blank? && location_field.present?
+      block_log_creation!
       errors.add(location_field, I18n.t("validations.not_answered", question: "#{location_or_scheme} code"), category: "setup")
     end
   end
 
-  def validate_scheme_related
-    return unless scheme_id.present? && scheme.present?
-
-    owned_by_owning_org = owning_organisation && scheme.owning_organisation == owning_organisation
-    owned_by_managing_org = managing_organisation && scheme.owning_organisation == managing_organisation
-
-    if !(owned_by_owning_org || owned_by_managing_org) && scheme_field.present?
+  def validate_related_scheme_exists
+    if scheme_id.present? && scheme_field.present? && owning_organisation.present? && managing_organisation.present? && scheme.nil?
       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_field.present? && scheme.nil?
-      errors.add(scheme_field, "The #{scheme_or_management_group} code is not correct", category: :setup)
+      errors.add(scheme_field, "This #{scheme_or_management_group} code does not belong to the owning organisation or managing organisation", category: :setup)
     end
   end
 
   def validate_scheme_data_given
     if supported_housing? && scheme_field.present? && scheme_id.blank?
+      block_log_creation!
       errors.add(scheme_field, I18n.t("validations.not_answered", question: "#{scheme_or_management_group} code"), category: "setup")
     end
   end
@@ -1276,9 +1257,9 @@ private
   end
 
   def scheme
-    return if field_16.nil?
+    return if scheme_id.nil? || owning_organisation.nil? || managing_organisation.nil?
 
-    @scheme ||= Scheme.find_by_id_on_multiple_fields(scheme_id)
+    @scheme ||= Scheme.where(id: (owning_organisation.owned_schemes + managing_organisation.owned_schemes).map(&:id)).find_by_id_on_multiple_fields(scheme_id, location_id)
   end
 
   def location

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

@@ -269,7 +269,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
         context "when a supported housing log already exists in the db" do
           let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) }
           let(:location) { create(:location, :with_old_visible_id, scheme:) }
-          let(:attributes) { valid_attributes.merge({ field_4: scheme.old_visible_id, field_5: location.old_visible_id, field_111: owning_org.old_visible_id }) }
+          let(:attributes) { valid_attributes.merge({ field_4: scheme.old_visible_id, field_5: location.old_visible_id, field_111: owning_org.old_visible_id, field_113: owning_org.old_visible_id }) }
 
           before do
             parser.bulk_upload.needstype = "2"
@@ -639,7 +639,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
       end
 
       context "when matching scheme cannot be found" do
-        let(:attributes) { { bulk_upload:, field_1: "1", field_4: "123" } }
+        let(:attributes) { { bulk_upload:, field_1: "1", field_4: "123", field_111: owning_org.old_visible_id, field_113: owning_org.old_visible_id } }
 
         it "returns an error" do
           expect(parser.errors.where(:field_4, category: :setup)).to be_present
@@ -648,16 +648,16 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
 
       context "when scheme belongs to someone else" do
         let(:other_scheme) { create(:scheme, :with_old_visible_id) }
-        let(:attributes) { { bulk_upload:, field_1: "1", field_4: other_scheme.old_visible_id, field_111: owning_org.old_visible_id } }
+        let(:attributes) { { bulk_upload:, field_1: "1", field_4: other_scheme.old_visible_id, field_111: owning_org.old_visible_id, field_113: owning_org.old_visible_id } }
 
         it "returns an error" do
-          expect(parser.errors.where(:field_4, category: :setup).map(&:message)).to include("This management group code does not belong to your organisation, or any of your stock owners / managing agents")
+          expect(parser.errors.where(:field_4, category: :setup).map(&:message)).to include("This management group code does not belong to the owning organisation or managing organisation")
         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_1: "1", field_4: scheme.old_visible_id, field_111: owning_org.old_visible_id } }
+        let(:attributes) { { bulk_upload:, field_1: "1", field_4: scheme.old_visible_id, field_111: owning_org.old_visible_id, field_113: owning_org.old_visible_id } }
 
         it "does not return an error" do
           expect(parser.errors[:field_4]).to be_blank
@@ -693,6 +693,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
             field_4: scheme.old_visible_id,
             field_5: "dontexist",
             field_111: owning_org.old_visible_id,
+            field_113: owning_org.old_visible_id,
           }
         end
 
@@ -710,6 +711,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
             field_4: scheme.old_visible_id,
             field_5: location.old_visible_id,
             field_111: owning_org.old_visible_id,
+            field_113: owning_org.old_visible_id,
           }
         end
 
@@ -719,7 +721,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
       end
 
       context "when location exists but not related" do
-        let(:location) { create(:scheme, :with_old_visible_id) }
+        let(:location) { create(:location, :with_old_visible_id) }
         let(:attributes) do
           {
             bulk_upload:,
@@ -727,11 +729,12 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
             field_4: scheme.old_visible_id,
             field_5: location.old_visible_id,
             field_111: owning_org.old_visible_id,
+            field_113: owning_org.old_visible_id,
           }
         end
 
         it "returns as setup error" do
-          expect(parser.errors.where(:field_5, category: :setup).map(&:message)).to eql(["Location could not be found with the provided scheme code"])
+          expect(parser.errors.where(:field_5, category: :setup).map(&:message)).to eql(["Scheme code must relate to a scheme that is owned by the owning organisation or managing organisation"])
         end
       end
     end
@@ -880,7 +883,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
 
     describe "#field_78" do # referral
       context "when 3 ie PRP nominated by LA and owning org is LA" do
-        let(:attributes) { { bulk_upload:, field_78: "3", field_111: owning_org.old_visible_id } }
+        let(:attributes) { { bulk_upload:, field_78: "3", field_111: owning_org.old_visible_id, field_113: owning_org.old_visible_id } }
 
         it "is not permitted" do
           expect(parser.errors[:field_78]).to be_present
@@ -888,7 +891,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
       end
 
       context "when 4 ie referred by LA and is general needs and owning org is LA" do
-        let(:attributes) { { bulk_upload:, field_78: "4", field_111: owning_org.old_visible_id.to_s } }
+        let(:attributes) { { bulk_upload:, field_78: "4", field_111: owning_org.old_visible_id, field_113: owning_org.old_visible_id } }
 
         it "is not permitted" do
           expect(parser.errors[:field_78]).to be_present
@@ -898,7 +901,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
       context "when 4 ie referred by LA and is general needs and owning org is PRP" do
         let(:owning_org) { create(:organisation, :prp, :with_old_visible_id) }
 
-        let(:attributes) { { bulk_upload:, field_78: "4", field_111: owning_org.old_visible_id.to_s } }
+        let(:attributes) { { bulk_upload:, field_78: "4", field_111: owning_org.old_visible_id, field_113: owning_org.old_visible_id } }
 
         it "is permitted" do
           expect(parser.errors[:field_78]).to be_blank
@@ -1364,7 +1367,15 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
 
     describe "#location" do
       context "when lookup is via new core id" do
-        let(:attributes) { { bulk_upload:, field_4: scheme.old_visible_id, field_5: location.id, field_111: owning_org } }
+        let(:attributes) { { bulk_upload:, field_4: "S#{scheme.id}", field_5: location.id, field_111: "ORG#{owning_org.id}", field_113: "ORG#{owning_org.id}" } }
+
+        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_4: scheme.old_visible_id, field_5: location.old_visible_id, field_111: owning_org.old_visible_id, field_113: owning_org.old_visible_id } }
 
         it "assigns the correct location" do
           expect(parser.log.location).to eql(location)
@@ -1373,8 +1384,16 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
     end
 
     describe "#scheme" do
-      context "when lookup is via id prefixed with S" do
-        let(:attributes) { { bulk_upload:, field_4: "S#{scheme.id}", field_111: owning_org } }
+      context "when lookup is via new core id" do
+        let(:attributes) { { bulk_upload:, field_4: "S#{scheme.id}", field_5: location.id, field_113: "ORG#{owning_org.id}", field_111: "ORG#{owning_org.id}" } }
+
+        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_4: scheme.old_visible_id, field_5: location.old_visible_id, field_111: owning_org.old_visible_id, field_113: owning_org.old_visible_id } }
 
         it "assigns the correct scheme" do
           expect(parser.log.scheme).to eql(scheme)

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

@@ -862,17 +862,17 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
         let(:location) { create(:location, :with_old_visible_id, scheme:) }
 
         context "when matching scheme cannot be found" do
-          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S123", field_17: location.id } }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S123", field_17: location.id } }
 
           it "returns a setup error" do
             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.where(:field_16, category: :setup).map(&:message)).to eq(["This scheme code does not belong to the owning organisation or managing organisation"])
             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_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: nil } }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: nil } }
 
           it "returns a setup error" do
             expect(parser.errors[:field_15]).to be_blank
@@ -882,17 +882,17 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
         end
 
         context "when matching location cannot be found" do
-          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: "123" } }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: "123" } }
 
           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"])
+            expect(parser.errors.where(:field_17, category: :setup).map(&:message)).to eq(["Location code must relate to a location that is owned by the owning organisation or managing organisation"])
           end
         end
 
         context "when matching location exists" do
-          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: location.id } }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: location.id } }
 
           it "does not return an error" do
             expect(parser.errors[:field_15]).to be_blank
@@ -904,29 +904,29 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
         context "when location exists but not related" 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_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: other_location.id } }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: other_location.id } }
 
           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"])
+            expect(parser.errors.where(:field_17, category: :setup).map(&:message)).to eq(["Location code must relate to a location that is owned by the owning organisation or managing organisation"])
           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_4: "2", field_5: "2", field_16: "S#{other_scheme.id}", field_17: other_location.id, field_1: owning_org.old_visible_id } }
+          let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_16: "S#{other_scheme.id}", field_17: other_location.id, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id } }
 
           it "returns a setup error" do
             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.where(:field_16, category: :setup).map(&:message)).to eq(["This scheme code does not belong to the owning organisation or managing organisation"])
             expect(parser.errors[:field_17]).to be_blank
           end
         end
 
         context "when scheme belongs to owning org" do
-          let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: location.id, field_1: owning_org.old_visible_id } }
+          let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: location.id, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id } }
 
           it "does not return an error" do
             expect(parser.errors[:field_15]).to be_blank
@@ -953,17 +953,17 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
         let(:location) { create(:location, :with_old_visible_id, scheme:) }
 
         context "when matching scheme cannot be found" do
-          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_15: "123", field_16: location.old_visible_id } }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_4: "2", field_5: "2", field_15: "123", field_16: location.old_visible_id } }
 
           it "returns a setup error" do
-            expect(parser.errors.where(:field_15, category: :setup).map(&:message)).to eq(["The management group code is not correct"])
+            expect(parser.errors.where(:field_15, category: :setup).map(&:message)).to eq(["This management group code does not belong to the owning organisation or managing organisation"])
             expect(parser.errors[:field_16]).to be_blank
             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_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: nil } }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: nil } }
 
           it "returns a setup error" do
             expect(parser.errors[:field_15]).to be_blank
@@ -973,17 +973,17 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
         end
 
         context "when matching location cannot be found" do
-          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: "123" } }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: "123" } }
 
           it "returns a setup error" do
             expect(parser.errors[:field_15]).to be_blank
-            expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["Scheme could not be found with the provided management group code"])
+            expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["Scheme code must relate to a scheme that is owned by the owning organisation or managing organisation"])
             expect(parser.errors[:field_17]).to be_blank
           end
         end
 
         context "when matching location exists" do
-          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: location.old_visible_id } }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_4: "2", 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
@@ -995,11 +995,11 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
         context "when location exists but not related" 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_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: other_location.old_visible_id } }
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: other_location.old_visible_id } }
 
           it "returns a setup error" do
             expect(parser.errors[:field_15]).to be_blank
-            expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["Scheme could not be found with the provided management group code"])
+            expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["Scheme code must relate to a scheme that is owned by the owning organisation or managing organisation"])
             expect(parser.errors[:field_17]).to be_blank
           end
         end
@@ -1007,17 +1007,17 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
         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_4: "2", field_5: "2", field_15: other_scheme.old_visible_id, field_16: other_location.old_visible_id, field_1: owning_org.old_visible_id } }
+          let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_15: other_scheme.old_visible_id, field_16: other_location.old_visible_id, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id } }
 
           it "returns a setup error" do
-            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.where(:field_15, category: :setup).map(&:message)).to eq(["This management group code does not belong to the owning organisation or managing organisation"])
             expect(parser.errors[:field_16]).to be_blank
             expect(parser.errors[:field_17]).to be_blank
           end
         end
 
         context "when scheme belongs to owning org" do
-          let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: location.old_visible_id, field_1: owning_org.old_visible_id } }
+          let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: location.old_visible_id, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id } }
 
           it "does not return an error" do
             expect(parser.errors[:field_15]).to be_blank
@@ -1204,7 +1204,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
 
     describe "#field_119" do # referral
       context "when 3 ie PRP nominated by LA and owning org is LA" do
-        let(:attributes) { { bulk_upload:, field_119: "3", field_1: owning_org.old_visible_id } }
+        let(:attributes) { { bulk_upload:, field_119: "3", field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id } }
 
         it "is not permitted" do
           expect(parser.errors[:field_119]).to be_present
@@ -1212,7 +1212,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
       end
 
       context "when 4 ie referred by LA and is general needs and owning org is LA" do
-        let(:attributes) { { bulk_upload:, field_119: "4", field_1: owning_org.old_visible_id.to_s, field_4: "1" } }
+        let(:attributes) { { bulk_upload:, field_119: "4", field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_4: "1" } }
 
         it "is not permitted" do
           expect(parser.errors[:field_119]).to be_present
@@ -1222,7 +1222,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
       context "when 4 ie referred by LA and is general needs and owning org is PRP" do
         let(:owning_org) { create(:organisation, :prp, :with_old_visible_id) }
 
-        let(:attributes) { { bulk_upload:, field_119: "4", field_1: owning_org.old_visible_id.to_s } }
+        let(:attributes) { { bulk_upload:, field_119: "4", field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id } }
 
         it "is permitted" do
           expect(parser.errors[:field_119]).to be_blank
@@ -1327,7 +1327,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
       context "when org is not stock owning" do
         let(:owning_org) { create(:organisation, :with_old_visible_id, :does_not_own_stock) }
 
-        let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id } }
+        let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id } }
 
         it "is not permitted as setup error" do
           setup_errors = parser.errors.select { |e| e.options[:category] == :setup }
@@ -1415,7 +1415,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
 
     describe "#field_6" do # renewal
       context "when blank" do
-        let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_6: "" } }
+        let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_6: "" } }
 
         it "has setup errors on the field" do
           expect(parser.errors.where(:field_6, category: :setup).map(&:message)).to eql(["You must answer property renewal"])
@@ -1710,7 +1710,7 @@ 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: "S#{scheme.id}", field_17: location.id, field_1: owning_org } }
+        let(:attributes) { { bulk_upload:, field_16: "S#{scheme.id}", field_17: location.id, field_1: "ORG#{owning_org.id}", field_2: "ORG#{owning_org.id}" } }
 
         it "assigns the correct location" do
           expect(parser.log.location).to eql(location)
@@ -1718,7 +1718,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
       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 } }
+        let(:attributes) { { bulk_upload:, field_15: scheme.old_visible_id, field_16: location.old_visible_id, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id } }
 
         it "assigns the correct location" do
           expect(parser.log.location).to eql(location)
@@ -1728,7 +1728,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
 
     describe "#scheme" do
       context "when lookup is via new core id" do
-        let(:attributes) { { bulk_upload:, field_16: "S#{scheme.id}", field_1: owning_org } }
+        let(:attributes) { { bulk_upload:, field_16: "S#{scheme.id}", field_1: "ORG#{owning_org.id}", field_2: "ORG#{owning_org.id}" } }
 
         it "assigns the correct scheme" do
           expect(parser.log.scheme).to eql(scheme)
@@ -1736,7 +1736,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
       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 } }
+        let(:attributes) { { bulk_upload:, field_15: scheme.old_visible_id, field_16: location.old_visible_id, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id } }
 
         it "assigns the correct scheme" do
           expect(parser.log.scheme).to eql(scheme)