From 4068d7a2f6ad80558e3a3724efab06be10b0a7e9 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Thu, 12 Oct 2023 15:04:59 +0100 Subject: [PATCH] CLDC-2871 Fix bulk upload scheme lookup bug (#1971) * feat: wip tests * feat: update tests and functionality * refactor: lint * feat: update tests and don't add errors to scheme/location fields unless determined * feat: update duplicate log behaviour and tests * refactor: lint * feat: use needstype helpers * feat: use needstype helpers and test tweak * feat: make tests explicit regarding needstype * feat: make tests explicit regarding needstype --- .../lettings/year2023/row_parser.rb | 112 +++- .../lettings/year2023/row_parser_spec.rb | 598 +++++++++++++----- 2 files changed, 532 insertions(+), 178 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2023/row_parser.rb b/app/services/bulk_upload/lettings/year2023/row_parser.rb index 9e7d8eecf..413e0b10f 100644 --- a/app/services/bulk_upload/lettings/year2023/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2023/row_parser.rb @@ -329,9 +329,26 @@ class BulkUpload::Lettings::Year2023::RowParser }, on: :after_log + validates :field_15, + presence: { + if: proc { supported_housing? && 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) }, + if: proc { supported_housing? }, + message: I18n.t("validations.not_answered", question: "scheme code"), + category: :setup, + }, + on: :after_log + + validates :field_17, + presence: { + if: proc { supported_housing? && log_uses_new_scheme_id? }, + message: I18n.t("validations.not_answered", question: "location code"), category: :setup, }, on: :after_log @@ -464,9 +481,9 @@ class BulkUpload::Lettings::Year2023::RowParser "field_8", # startdate "field_9", # startdate "field_13", # tenancycode - field_4 != 1 ? "field_17" : nil, # location - field_4 != 2 ? "field_23" : nil, # postcode - field_4 != 2 ? "field_24" : nil, # postcode + !general_needs? ? location_field.to_s : nil, # location + !supported_housing? ? "field_23" : nil, # postcode + !supported_housing? ? "field_24" : nil, # postcode "field_46", # age1 "field_47", # sex1 "field_50", # ecstat1 @@ -560,8 +577,8 @@ private "ecstat1", "owning_organisation", "tcharge", - field_4 != 2 ? "postcode_full" : nil, - field_4 != 1 ? "location" : nil, + !supported_housing? ? "postcode_full" : nil, + !general_needs? ? "location" : nil, "tenancycode", log.chcharge.present? ? "chcharge" : nil, ].compact @@ -732,45 +749,45 @@ 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(:field_17, "Scheme code must relate to a location that is owned by owning organisation or managing organisation", category: :setup) + 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 && field_17.present? && location.nil? - errors.add(:field_17, "Location could not be found with the provided scheme code", category: :setup) + 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? && field_17.blank? - errors.add(:field_17, I18n.t("validations.not_answered", question: "scheme code"), category: "setup") + 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 def validate_scheme_related - return unless field_16.present? && scheme.present? + 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 - 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(:field_16, "This management group code does not belong to your organisation, or any of your stock owners / managing agents", category: :setup) + 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 field_16.present? && scheme.nil? - errors.add(:field_16, "The management group code is not correct", category: :setup) + 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? && field_16.blank? - errors.add(:field_16, I18n.t("validations.not_answered", question: "management group code"), category: "setup") + 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 @@ -851,16 +868,17 @@ private errors.add(:field_8, error_message) # startdate errors.add(:field_9, error_message) # startdate errors.add(:field_13, error_message) # tenancycode - errors.add(:field_17, error_message) if field_4 != 1 # 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 + errors.add(location_field, error_message) if !general_needs? && location_field.present? # location + errors.add(:field_16, error_message) if !general_needs? && location_field.blank? # add to Scheme field as unclear whether log uses New or Old CORE ids + errors.add(:field_23, error_message) unless supported_housing? # postcode_full + errors.add(:field_24, error_message) unless supported_housing? # postcode_full + errors.add(:field_25, error_message) unless supported_housing? # la errors.add(:field_46, error_message) # age1 errors.add(:field_47, error_message) # sex1 errors.add(:field_50, error_message) # ecstat1 errors.add(:field_132, error_message) # tcharge errors.add(:field_127, error_message) if log.chcharge.present? # chcharge - errors.add(:field_125, error_message) if bulk_upload.needstype != 1 # household_charge + errors.add(:field_125, error_message) unless general_needs? # household_charge end end @@ -876,8 +894,8 @@ private owning_organisation_id: [:field_1], managing_organisation_id: [:field_2], renewal: [:field_6], - scheme: %i[field_16], - location: %i[field_17], + scheme: [scheme_field], + location: [location_field], created_by: [:field_3], needstype: [:field_4], rent_type: %i[field_5 field_10 field_11], @@ -1258,13 +1276,51 @@ private end def scheme - @scheme ||= Scheme.find_by_id_on_multiple_fields(field_16) + return if field_16.nil? + + @scheme ||= Scheme.find_by_id_on_multiple_fields(scheme_id) end def location return if scheme.nil? - @location ||= scheme.locations.find_by_id_on_multiple_fields(field_17) + @location ||= scheme.locations.find_by_id_on_multiple_fields(location_id) + end + + def log_uses_new_scheme_id? + field_16&.start_with?("S") + end + + def log_uses_old_scheme_id? + field_16.present? && !field_16.start_with?("S") + end + + def scheme_field + return :field_16 if log_uses_new_scheme_id? + return :field_15 if log_uses_old_scheme_id? + end + + def scheme_id + return field_16 if log_uses_new_scheme_id? + return field_15 if log_uses_old_scheme_id? + end + + def location_field + return :field_17 if log_uses_new_scheme_id? + return :field_16 if log_uses_old_scheme_id? + end + + def location_id + return field_17 if log_uses_new_scheme_id? + return field_16 if log_uses_old_scheme_id? + end + + def scheme_or_management_group + log_uses_new_scheme_id? ? "scheme" : "management group" + end + + def location_or_scheme + log_uses_new_scheme_id? ? "location" : "scheme" end def renttype 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 d532c8094..6959dff29 100644 --- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb @@ -20,7 +20,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do field_1: owning_org.old_visible_id, field_2: managing_org.old_visible_id, field_4: "1", - field_5: "2", + field_5: "1", field_6: "2", field_7: now.day.to_s, field_8: now.month.to_s, @@ -327,7 +327,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do :field_8, # startdate :field_9, # startdate :field_13, # tenancycode - :field_17, # location + :field_16, # location :field_46, # age1 :field_47, # sex1 :field_50, # ecstat1 @@ -342,106 +342,290 @@ 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 already exists in the db" do + let(:attributes) { { bulk_upload:, field_4: "2", field_16: "123" } } - before do - parser.log.save! - parser.instance_variable_set(:@valid, nil) + 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_132, # tcharge + ].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 "is not a valid row" do - expect(parser).not_to be_valid + 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 + + 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 already exists in the db" do + let(:attributes) { { bulk_upload:, field_4: "2", field_16: "S123" } } - before do - parser.log.save! - parser.instance_variable_set(:@valid, nil) + 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_17, # location + :field_46, # age1 + :field_47, # sex1 + :field_50, # ecstat1 + :field_132, # tcharge + ].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 "is a valid row" do - expect(parser_too).to be_valid + 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 + + 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 @@ -652,108 +836,206 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end end - describe "#field_16" do + describe "#field_15, field_16, field_17" do # scheme and location fields context "when nullable not permitted" do - let(:attributes) { { bulk_upload:, field_5: "2", field_16: nil } } + let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_15: nil, field_16: nil, field_17: nil } } it "cannot be nulled" do - expect(parser.errors[:field_16]).to be_present + 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_4: "1", 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 matching scheme cannot be found" do - let(:attributes) { { bulk_upload:, field_5: "1", field_16: "123" } } + 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_1: 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.where(:field_16, category: :setup)).to be_present + 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[:field_17]).to be_blank + end 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 } } + 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 } } - it "returns a setup error" do - expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to include("This management group code does not belong to your organisation, or any of your stock owners / managing agents") + 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(["You must answer location code"]) + end 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: "1", field_16: scheme.old_visible_id, field_1: owning_org.old_visible_id } } + 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" } } - it "does not return an error" do - expect(parser.errors[:field_16]).to be_blank + 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 - end - context "when scheme belongs to managing org" do - let(: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 } } + 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 } } - it "does not return an error" do - expect(parser.errors[:field_16]).to be_blank + 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 location does not exist" do - let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) } - let(:attributes) do - { - bulk_upload:, - field_5: "1", - field_16: scheme.old_visible_id, - field_17: "dontexist", - field_1: owning_org.old_visible_id, - } + 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 } } + + 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 - it "returns a setup error" do - expect(parser.errors.where(:field_17, category: :setup)).to be_present + 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 } } + + 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[: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 } } + + 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_4: "2", 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 - context "when location exists" do + context "when using Old CORE ids" do let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) } - let(:attributes) do - { - bulk_upload:, - field_5: "1", - field_16: scheme.old_visible_id, - field_17: location.old_visible_id, - field_1: owning_org.old_visible_id, - } + 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 } } + + 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[:field_16]).to be_blank + expect(parser.errors[:field_17]).to be_blank + end end - it "does not return an error" do - expect(parser.errors[:field_17]).to be_blank + 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 } } + + 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(["You must answer scheme code"]) + expect(parser.errors[:field_17]).to be_blank + end end - end - context "when location exists but not related" do - let(:location) { create(:scheme, :with_old_visible_id) } - let(:attributes) do - { - bulk_upload:, - field_5: "1", - field_16: scheme.old_visible_id, - field_17: location.old_visible_id, - field_1: owning_org.old_visible_id, - } + 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" } } + + 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[:field_17]).to be_blank + end end - it "returns a setup error" do - expect(parser.errors.where(:field_17, category: :setup)).to be_present + 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 } } + + 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(: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 } } + + 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[: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_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 } } + + 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[: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 } } + + 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_4: "2", 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 @@ -1410,7 +1692,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) @@ -1419,13 +1709,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