diff --git a/app/helpers/duplicate_logs_helper.rb b/app/helpers/duplicate_logs_helper.rb index 077845857..92490c10f 100644 --- a/app/helpers/duplicate_logs_helper.rb +++ b/app/helpers/duplicate_logs_helper.rb @@ -55,16 +55,18 @@ module DuplicateLogsHelper duplicate_sets_count > 1 ? "Review these #{duplicate_sets_count} sets of logs" : "Review this #{duplicate_sets_count} set of logs" end - def duplicate_log_question_label(question) - if question.id == "uprn" + def duplicate_log_question_label(question, log) + if question.id == "uprn" && !log.form.start_year_2026_or_later? "Postcode (from UPRN)" + elsif question.id == "address_line1" + "#{question.question_number_string} - Address line 1" else get_question_label(question) end end def duplicate_log_answer_label(question, log) - if question.id == "uprn" + if question.id == "uprn" && !log.form.start_year_2026_or_later? postcode_question = log.form.get_question("postcode_full", log) get_answer_label(postcode_question, log) else @@ -73,16 +75,26 @@ module DuplicateLogsHelper end def duplicate_log_extra_value(question, log) - if question.id == "uprn" + if log.form.start_year_2026_or_later? + case question.id + when "uprn" + value = [ + log.address_line1, + log.postcode_full, + ].select(&:present?) + + "\n\n#{value.join("\n")}" + else + question.get_extra_check_answer_value(log) + end + elsif question.id == "uprn" && !log.form.start_year_2026_or_later? postcode_question = log.form.get_question("postcode_full", log) postcode_question.get_extra_check_answer_value(log) - else - question.get_extra_check_answer_value(log) end end def duplicate_log_answer_label_present(question, log, current_user) - if question.id == "uprn" + if question.id == "uprn" && !log.form.start_year_2026_or_later? postcode_question = log.form.get_question("postcode_full", log) postcode_question.answer_label(log, current_user).present? else @@ -91,7 +103,7 @@ module DuplicateLogsHelper end def duplicate_log_inferred_answers(question, log) - if question.id == "uprn" + if question.id == "uprn" && !log.form.start_year_2026_or_later? postcode_question = log.form.get_question("postcode_full", log) postcode_question.get_inferred_answers(log) else diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index f2230c4e5..8be7a9140 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -21,6 +21,7 @@ class LettingsLog < Log include Validations::DateValidations include Validations::FinancialValidations include MoneyFormattingHelper + include CollectionTimeHelper has_paper_trail @@ -41,6 +42,8 @@ class LettingsLog < Log belongs_to :managing_organisation, class_name: "Organisation", optional: true scope :filter_by_year, ->(year) { where(startdate: Time.zone.local(year.to_i, 4, 1)...Time.zone.local(year.to_i + 1, 4, 1)) } + scope :filter_by_year_or_later, ->(year) { where("lettings_logs.startdate >= ?", Time.zone.local(year.to_i, 4, 1)) } + scope :filter_by_year_or_earlier, ->(year) { where("lettings_logs.startdate < ?", Time.zone.local(year.to_i + 1, 4, 1)) } scope :filter_by_years_or_nil, lambda { |years, _user = nil| first_year = years.shift query = filter_by_year(first_year) @@ -85,9 +88,15 @@ class LettingsLog < Log scope :age1_answered, -> { where.not(age1: nil).or(where(age1_known: 1)) } scope :tcharge_answered, -> { where.not(tcharge: nil).or(where(household_charge: 1)).or(where(is_carehome: 1)) } scope :chcharge_answered, -> { where.not(chcharge: nil).or(where(is_carehome: [nil, 0])) } - scope :location_for_log_answered, ->(log) { where(location_id: log.location_id).or(where(needstype: 1)) } - scope :postcode_for_log_answered, ->(log) { where(postcode_full: log.postcode_full).or(where(needstype: 2)) } - scope :location_answered, -> { where.not(location_id: nil).or(where(needstype: 1)) } + # once 2025 logs are removed this logic can be simplified + # pre 2025, match on location if supported, or address if general needs + # post 2026, match on address only + scope :location_for_log_answered_as, ->(log) { where(location_id: log.location_id).or(where(needstype: 1)).or(filter_by_year_or_later(2026)) } + scope :address_for_log_answered_as, lambda { |log| + where(postcode_full: log.postcode_full).where(address_line1: log.address_line1).where(uprn: log.uprn) + .or(filter_by_year_or_earlier(2025).where(needstype: 2)) + } + scope :location_answered, -> { where.not(location_id: nil).or(where(needstype: 1)).or(filter_by_year_or_later(2026)) } scope :postcode_answered, -> { where.not(postcode_full: nil).or(where(needstype: 2)) } scope :duplicate_logs, lambda { |log| visible @@ -99,26 +108,27 @@ class LettingsLog < Log .age1_answered .tcharge_answered .chcharge_answered - .location_for_log_answered(log) - .postcode_for_log_answered(log) + .location_for_log_answered_as(log) + .address_for_log_answered_as(log) .where(log.slice(*DUPLICATE_LOG_ATTRIBUTES)) } - scope :duplicate_sets, lambda { |assigned_to_id = nil| + scope :duplicate_sets_2025_and_earlier, lambda { |assigned_to_id = nil| scope = visible - .group(*DUPLICATE_LOG_ATTRIBUTES, :postcode_full, :location_id) - .where.not(startdate: nil) - .where.not(sex1: nil) - .where.not(ecstat1: nil) - .where.not(needstype: nil) - .age1_answered - .tcharge_answered - .chcharge_answered - .location_answered - .postcode_answered - .having( - "COUNT(*) > 1", - ) + .filter_by_year_or_earlier(2025) + .group(*DUPLICATE_LOG_ATTRIBUTES, :postcode_full, :location_id, :uprn, :address_line1) + .where.not(startdate: nil) + .where.not(sex1: nil) + .where.not(ecstat1: nil) + .where.not(needstype: nil) + .age1_answered + .tcharge_answered + .chcharge_answered + .location_answered + .postcode_answered + .having( + "COUNT(*) > 1", + ) if assigned_to_id scope = scope.having("MAX(CASE WHEN assigned_to_id = ? THEN 1 ELSE 0 END) >= 1", assigned_to_id) @@ -126,6 +136,34 @@ class LettingsLog < Log scope.pluck("ARRAY_AGG(id)") } + scope :duplicate_sets_2026_and_later, lambda { |assigned_to_id = nil| + scope = visible + .filter_by_year_or_later(2026) + # separate function as location needs to be fully ignored in 2026 + .group(*DUPLICATE_LOG_ATTRIBUTES, :postcode_full, :uprn, :address_line1) + .where.not(startdate: nil) + .where.not(sex1: nil) + .where.not(ecstat1: nil) + .where.not(needstype: nil) + .age1_answered + .tcharge_answered + .chcharge_answered + .location_answered + .postcode_answered + .having( + "COUNT(*) > 1", + ) + + if assigned_to_id + scope = scope.having("MAX(CASE WHEN assigned_to_id = ? THEN 1 ELSE 0 END) >= 1", assigned_to_id) + end + scope.pluck("ARRAY_AGG(id)") + } + + scope :duplicate_sets, lambda { |assigned_to_id = nil| + duplicate_sets_2025_and_earlier(assigned_to_id) + duplicate_sets_2026_and_later(assigned_to_id) + } + scope :with_illness_without_type, lambda { where(illness: 1, illness_type_1: false, @@ -150,7 +188,7 @@ class LettingsLog < Log HAS_BENEFITS_OPTIONS = [1, 6, 8, 7].freeze NUM_OF_WEEKS_FROM_PERIOD = { 2 => 26, 3 => 13, 4 => 12, 5 => 50, 6 => 49, 7 => 48, 8 => 47, 9 => 46, 11 => 51, 1 => 52, 10 => 53 }.freeze SUFFIX_FROM_PERIOD = { 2 => "every 2 weeks", 3 => "every 4 weeks", 4 => "every month" }.freeze - DUPLICATE_LOG_ATTRIBUTES = %w[owning_organisation_id tenancycode startdate age1_known age1 sex1 ecstat1 tcharge household_charge chcharge].freeze + DUPLICATE_LOG_ATTRIBUTES = %w[owning_organisation_id tenancycode startdate age1_known age1 sex1 sexrab1 ecstat1 tcharge household_charge chcharge].freeze RENT_TYPE = { social_rent: 0, affordable_rent: 1, @@ -190,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 @@ -687,11 +724,13 @@ class LettingsLog < Log ["owning_organisation_id", "startdate", "tenancycode", - uprn.blank? ? "postcode_full" : "uprn", + form.start_year_2026_or_later? ? "address_line1" : nil, + "postcode_full", + "uprn", "scheme_id", - "location_id", + form.start_year_2026_or_later? ? nil : "location_id", "age1", - "sex1", + form.start_year_2026_or_later? ? "sexrab1" : "sex1", "ecstat1", household_charge == 1 ? "household_charge" : nil, "tcharge", diff --git a/app/services/bulk_upload/lettings/year2026/row_parser.rb b/app/services/bulk_upload/lettings/year2026/row_parser.rb index 02484aafd..fb74081a2 100644 --- a/app/services/bulk_upload/lettings/year2026/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2026/row_parser.rb @@ -584,13 +584,13 @@ class BulkUpload::Lettings::Year2026::RowParser "field_1", # owning org "field_8", # startdate "field_9", # startdate - "field_10", # startdate - "field_13", # tenancycode - !general_needs? ? :field_6.to_s : nil, # location # TODO: CLDC-4119: remove location from hash - !supported_housing? ? "field_23" : nil, # postcode # TODO: CLDC-4119: add postcode to hash for supported housing - !supported_housing? ? "field_24" : nil, # postcode # TODO: CLDC-4119: add postcode to hash for supported housing - "field_42", # age1 - "field_43", # sex1 + "field_10", # startdate + "field_13", # tenancycode + "field_18", # uprn + "field_19", # address_line1 + "field_23", # postcode first half + "field_24", # postcode second half + "field_42", # age1 "field_130", # sexrab1 "field_46", # ecstat1 ) @@ -741,12 +741,12 @@ private "startdate", "age1", "sexrab1", - "sex1", "ecstat1", "owning_organisation", "tcharge", - !supported_housing? ? "postcode_full" : nil, # TODO: CLDC-4119: add postcode to duplicate check fields for supported housing - !general_needs? ? "location" : nil, # TODO: CLDC-4119: remove location from duplicate check fields + "postcode_full", + "address_line1", + "uprn", "tenancycode", log.chcharge.present? ? "chcharge" : nil, ].compact @@ -1025,11 +1025,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/app/views/duplicate_logs/_duplicate_log_check_answers.erb b/app/views/duplicate_logs/_duplicate_log_check_answers.erb index 91deb10c8..8dbe7056f 100644 --- a/app/views/duplicate_logs/_duplicate_log_check_answers.erb +++ b/app/views/duplicate_logs/_duplicate_log_check_answers.erb @@ -3,7 +3,7 @@ <%= govuk_summary_list do |summary_list| %> <% log.duplicate_check_questions(current_user).each do |question| %> <% summary_list.with_row do |row| %> - <% row.with_key { duplicate_log_question_label(question) } %> + <% row.with_key { duplicate_log_question_label(question, log) } %> <% row.with_value do %> <%= simple_format( diff --git a/lib/tasks/handle_unpended_logs.rake b/lib/tasks/handle_unpended_logs.rake index d3837ee06..7e2c07862 100644 --- a/lib/tasks/handle_unpended_logs.rake +++ b/lib/tasks/handle_unpended_logs.rake @@ -6,7 +6,7 @@ task :handle_unpended_logs, %i[perform_updates] => :environment do |_task, args| query = "SELECT \"versions\".* FROM \"versions\" WHERE \"versions\".\"item_type\" = 'LettingsLog' AND whodunnit is null AND ((object_changes like '%status:\n- 3\n- 1%') OR (object_changes like '%status:\n- 3\n- 2%'))" results = pg.execute(query) - duplicate_log_attributes = %w[owning_organisation_id tenancycode startdate age1_known age1 sex1 ecstat1 tcharge household_charge chcharge] + duplicate_log_attributes = %w[owning_organisation_id tenancycode startdate age1_known age1 sex1 sexrab1 ecstat1 tcharge household_charge chcharge] seen = [].to_set @@ -47,8 +47,8 @@ task :handle_unpended_logs, %i[perform_updates] => :environment do |_task, args| .age1_answered .tcharge_answered .chcharge_answered - .location_for_log_answered(log) - .postcode_for_log_answered(log) + .location_for_log_answered_as(log) + .address_for_log_answered_as(log) .where(log.slice(*duplicate_log_attributes)) duplicate_count = duplicates.length diff --git a/spec/factories/lettings_log.rb b/spec/factories/lettings_log.rb index 02744e87c..426c882a3 100644 --- a/spec/factories/lettings_log.rb +++ b/spec/factories/lettings_log.rb @@ -35,6 +35,7 @@ FactoryBot.define do setup_completed status { 1 } tenancycode { "same tenancy code" } + address_line1 { "same address line 1" } postcode_full { "A1 1AA" } uprn_known { 0 } declaration { 1 } 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 a85bdebbb..b26f97813 100644 --- a/spec/services/bulk_upload/lettings/year2026/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2026/row_parser_spec.rb @@ -330,7 +330,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) @@ -352,8 +352,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! @@ -375,7 +375,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 @@ -384,141 +386,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