From ad9e524a050564e08d69079436282a88e33e02aa Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Mon, 16 Feb 2026 12:09:31 +0000 Subject: [PATCH] CLDC-4119: Update duplicate log errors we no longer need a lot of the tests here as it's all postcode based --- app/models/lettings_log.rb | 1 - .../lettings/year2026/row_parser.rb | 8 +- .../lettings/year2026/row_parser_spec.rb | 145 +----------------- 3 files changed, 11 insertions(+), 143 deletions(-) diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 741bc6024..8be7a9140 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -228,7 +228,6 @@ class LettingsLog < Log location.linked_local_authorities.active(form.start_date).first&.code || location.location_code end - # TODO: CLDC-4119: Beware! This method may cause issues when testing supported housing log duplicate detection after postcode is added, as it can return `location.postcode` instead of the actual `postcode_full` stored on the log record (`super`). If this happens, investigate why it isn't returning `super`, as it should when `form.start_year_2026_or_later? && super`. def postcode_full return super unless location return super if form.start_year_2026_or_later? && super diff --git a/app/services/bulk_upload/lettings/year2026/row_parser.rb b/app/services/bulk_upload/lettings/year2026/row_parser.rb index 73d0a7d72..17c83545f 100644 --- a/app/services/bulk_upload/lettings/year2026/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2026/row_parser.rb @@ -993,11 +993,9 @@ private errors.add(:field_9, error_message) # startdate errors.add(:field_10, error_message) # startdate errors.add(:field_13, error_message) # tenancycode - errors.add(:field_6, error_message) if !general_needs? && :field_6.present? # location # TODO: CLDC-4119: remove location from error fields - errors.add(:field_5, error_message) if !general_needs? && :field_6.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 # TODO: CLDC-4119: add postcode to error fields for supported housing - errors.add(:field_24, error_message) unless supported_housing? # postcode_full # TODO: CLDC-4119: add postcode to error fields for supported housing - errors.add(:field_25, error_message) unless supported_housing? # la # TODO: CLDC-4119: add LA to error fields for supported housing + errors.add(:field_23, error_message) # postcode_full + errors.add(:field_24, error_message) # postcode_full + errors.add(:field_25, error_message) # la errors.add(:field_42, error_message) # age1 errors.add(:field_130, error_message) # sexrab1 errors.add(:field_43, error_message) # sex1 diff --git a/spec/services/bulk_upload/lettings/year2026/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2026/row_parser_spec.rb index bbfee1bd5..5e9325469 100644 --- a/spec/services/bulk_upload/lettings/year2026/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2026/row_parser_spec.rb @@ -314,7 +314,7 @@ RSpec.describe BulkUpload::Lettings::Year2026::RowParser do :field_126, # pscharge :field_127, # supcharg ].each do |field| - expect(parser.errors[field]).to include(error_message) + expect(parser.errors[field]).to include(error_message), "field: #{field}" end expect(parser.errors[:field_6]).not_to include(error_message) @@ -336,8 +336,8 @@ RSpec.describe BulkUpload::Lettings::Year2026::RowParser do end end - context "when a supported housing log already exists in the db" do # TODO: CLDC-4119: Beware! The `postcode_full` method in the `LettingsLog` class may cause issues with these supported housing log duplicate detection tests after postcode is added. See comment on the `postcode_full` method for details. - let(:attributes) { valid_attributes.merge({ field_4: "2", field_5: "S#{scheme.id}", field_6: location.old_visible_id, field_36: 3, field_122: 0 }) } + context "when a supported housing log already exists in the db" do + let(:attributes) { valid_attributes.merge({ field_4: "2", field_5: "S#{scheme.id}", field_6: location.old_visible_id }) } before do parser.log.save! @@ -359,7 +359,9 @@ RSpec.describe BulkUpload::Lettings::Year2026::RowParser do :field_9, # startdate :field_10, # startdate :field_13, # tenancycode - :field_6, # location + :field_23, # postcode_full + :field_24, # postcode_full + :field_25, # la :field_42, # age1 :field_43, # sex1 :field_46, # ecstat1 @@ -368,141 +370,10 @@ RSpec.describe BulkUpload::Lettings::Year2026::RowParser do :field_126, # pscharge :field_127, # supcharg ].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 "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_5: "123" } } - - 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 = I18n.t("validations.lettings.2026.bulk_upload.duplicate") - - [ - :field_1, # owning_organisation - :field_8, # startdate - :field_9, # startdate - :field_10, # startdate - :field_13, # tenancycode - :field_6, # location - :field_42, # age1 - :field_43, # sex1 - :field_46, # ecstat1 - :field_124, # brent - :field_125, # scharge - :field_126, # pscharge - :field_127, # supcharg - ].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 - end - - 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_5: "S123" } } - - before do - parser.log.save! - parser.instance_variable_set(:@valid, nil) + expect(parser.errors[field]).to include(error_message), "field: #{field}" 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 = I18n.t("validations.lettings.2026.bulk_upload.duplicate") - - [ - :field_1, # owning_organisation - :field_8, # startdate - :field_9, # startdate - :field_10, # startdate - :field_13, # tenancycode - :field_6, # location - :field_42, # age1 - :field_43, # sex1 - :field_46, # ecstat1 - ].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 already exists in the db (2)" do - let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } - let(:attributes) do - valid_attributes.merge({ field_5: "S#{scheme.id}", - field_4: "2", - field_11: "2", - field_6: location.id, - field_1: owning_org.old_visible_id, - field_122: 0, - field_36: 4 }) - 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 = I18n.t("validations.lettings.2026.bulk_upload.duplicate") - - [ - :field_1, # owning_organisation - :field_8, # startdate - :field_9, # startdate - :field_10, # startdate - :field_13, # tenancycode - :field_6, # location - :field_42, # age1 - :field_43, # sex1 - :field_46, # ecstat1 - :field_122, # 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 + expect(parser.errors[:field_6]).not_to include(error_message) end end