Merge 98705a0c2a into c54204af53

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)
+  def duplicate_log_question_label(question, log)
-    if question.id == "uprn"
+    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

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)) }
+  # once 2025 logs are removed this logic can be simplified
-  scope :postcode_for_log_answered, ->(log) { where(postcode_full: log.postcode_full).or(where(needstype: 2)) }
+  # pre 2025, match on location if supported, or address if general needs
-  scope :location_answered, -> { where.not(location_id: nil).or(where(needstype: 1)) }
+  # 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)
+      .location_for_log_answered_as(log)
-      .postcode_for_log_answered(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)
+      .filter_by_year_or_earlier(2025)
-    .where.not(startdate: nil)
+      .group(*DUPLICATE_LOG_ATTRIBUTES, :postcode_full, :location_id, :uprn, :address_line1)
-    .where.not(sex1: nil)
+      .where.not(startdate: nil)
-    .where.not(ecstat1: nil)
+      .where.not(sex1: nil)
-    .where.not(needstype: nil)
+      .where.not(ecstat1: nil)
-    .age1_answered
+      .where.not(needstype: nil)
-    .tcharge_answered
+      .age1_answered
-    .chcharge_answered
+      .tcharge_answered
-    .location_answered
+      .chcharge_answered
-    .postcode_answered
+      .location_answered
-    .having(
+      .postcode_answered
-      "COUNT(*) > 1",
+      .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",

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_10",  # startdate
-      "field_13", # tenancycode
+      "field_13",  # tenancycode
-      !general_needs? ? :field_6.to_s : nil, # location # TODO: CLDC-4119: remove location from hash
+      "field_18",  # uprn
-      !supported_housing? ? "field_23" : nil,  # postcode # TODO: CLDC-4119: add postcode to hash for supported housing
+      "field_19",  # address_line1
-      !supported_housing? ? "field_24" : nil,  # postcode # TODO: CLDC-4119: add postcode to hash for supported housing
+      "field_23",  # postcode first half
-      "field_42", # age1
+      "field_24",  # postcode second half
-      "field_43",  # sex1
+      "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
+      "postcode_full",
-      !general_needs? ? "location" : nil, # TODO: CLDC-4119: remove location from duplicate check fields
+      "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_23, error_message) # postcode_full
-      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_24, error_message) # postcode_full
-      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_25, error_message) # la
-      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_42, error_message) # age1
       errors.add(:field_130, error_message) # sexrab1
       errors.add(:field_43, error_message) # sex1

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(

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)
+                              .location_for_log_answered_as(log)
-                              .postcode_for_log_answered(log)
+                              .address_for_log_answered_as(log)
                               .where(log.slice(*duplicate_log_attributes))
 
       duplicate_count = duplicates.length

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 }

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.
+          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, field_36: 3, field_122: 0 }) }
+            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)
+                expect(parser.errors[field]).to include(error_message), "field: #{field}"
-              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)
               end
 
-              it "is not a valid row" do
+              expect(parser.errors[:field_6]).not_to include(error_message)
-                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
             end
           end