CLDC-4119: Amend lettings duplicate log check (#3179)

* CLDC-4119: Update row parser duplicate log check

* CLDC-4119: Update main site duplicate logs check

adds sexrab1 to the duplicate log attributes

needs special care for the location and uprn attributes

* CLDC-4119: Update duplicate logs page

shows correct fields for 2025/2026 and for whether manual address or address search

* CLDC-4119: Update duplicate log errors

we no longer need a lot of the tests here as it's all postcode based

* CLDC-4119: Add an address line1 to duplicate log trait

* fixup! CLDC-4119: Update duplicate log errors

* CLDC-4119: Add UPRN to duplicate log trait

* fixup! CLDC-4119: Update main site duplicate logs check

improve comments

* fixup! CLDC-4119: Update duplicate logs page

improve duplicate_logs_helper.rb

* fixup! CLDC-4119: Update main site duplicate logs check

handle sexrab answered properly

* fixup! CLDC-4119: Add UPRN to duplicate log trait

* Revert "fixup! CLDC-4119: Add UPRN to duplicate log trait"

This reverts commit 9ff5f09839.

* Revert "CLDC-4119: Add UPRN to duplicate log trait"

This reverts commit d4d1c00c05.

* fixup! CLDC-4119: Update duplicate log errors

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,21 @@ module DuplicateLogsHelper
   end
 
   def duplicate_log_extra_value(question, log)
-    if question.id == "uprn"
+    case question.id
+    when "uprn"
+      if log.form.start_year_2026_or_later?
+        "\n\n#{[log.address_line1, log.postcode_full].join("\n")}"
+      else
         postcode_question = log.form.get_question("postcode_full", log)
         postcode_question.get_extra_check_answer_value(log)
+      end
     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 +98,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
 
@@ -42,6 +43,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)
@@ -86,28 +89,36 @@ 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
+  # 2025 and before, match on location if supported, or address if general needs
+  # 2026 and after, 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 :sex1_answered, -> { where.not(sex1: nil).filter_by_year_or_earlier(2025).or(where.not(sexrab1: nil).filter_by_year_or_later(2026)) }
   scope :duplicate_logs, lambda { |log|
     visible
       .where.not(id: log.id)
       .where.not(startdate: nil)
-      .where.not(sex1: nil)
+      .sex1_answered
       .where.not(ecstat1: nil)
       .where.not(needstype: nil)
       .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)
+      .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)
@@ -127,6 +138,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(sexrab1: 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,
@@ -151,7 +190,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,
@@ -191,7 +230,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
@@ -698,11 +736,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

@@ -571,9 +571,10 @@ class BulkUpload::Lettings::Year2026::RowParser
       "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_18",  # uprn
+      "field_19",  # address_line1
+      "field_23",  # postcode first half
+      "field_24",  # postcode second half
       "field_41",  # age1
       "field_42",  # sexrab1
       "field_47",  # ecstat1
@@ -728,8 +729,9 @@ private
       "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
@@ -1000,11 +1002,11 @@ 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_18, error_message) # uprn
+      errors.add(:field_19, error_message) # address_line_1
+      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_41, error_message) # age1
       errors.add(:field_42, error_message) # sexrab1
       errors.add(:field_47, error_message) # ecstat1

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
 
@@ -41,14 +41,14 @@ task :handle_unpended_logs, %i[perform_updates] => :environment do |_task, args|
       # This is the normal query for duplicates but without the check that the logs are visible (i.e. not deleted/pending)
       duplicates = LettingsLog.where.not(id: log.id)
                               .where.not(startdate: nil)
-                              .where.not(sex1: nil)
+                              .sex1_answered
                               .where.not(ecstat1: nil)
                               .where.not(needstype: nil)
                               .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

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

@@ -321,10 +321,11 @@ RSpec.describe BulkUpload::Lettings::Year2026::RowParser do
                 :field_144, # pscharge
                 :field_145, # 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)
+              expect(parser.errors[:field_43]).not_to include(error_message)
             end
           end
 
@@ -343,50 +344,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_35: 3, field_140: 0 }) }
-
-            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_41, # age1
-                :field_42, # sexrab1
-                :field_47, # ecstat1
-                :field_142, # brent
-                :field_143, # scharge
-                :field_144, # pscharge
-                :field_145, # 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" } }
+            let(:attributes) { valid_attributes.merge({ field_4: "2", field_5: "S#{scheme.id}", field_6: location.old_visible_id }) }
 
             before do
               parser.log.save!
@@ -408,7 +367,11 @@ RSpec.describe BulkUpload::Lettings::Year2026::RowParser do
                 :field_9, # startdate
                 :field_10, # startdate
                 :field_13, # tenancycode
-                  :field_6, # location
+                :field_18, # uprn
+                :field_19, # address_line_1
+                :field_23, # postcode_full
+                :field_24, # postcode_full
+                :field_25, # la
                 :field_41, # age1
                 :field_42, # sexrab1
                 :field_47, # ecstat1
@@ -417,99 +380,10 @@ RSpec.describe BulkUpload::Lettings::Year2026::RowParser do
                 :field_144, # pscharge
                 :field_145, # 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).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_41, # age1
-                  :field_42, # sexrab1
-                  :field_47, # 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_140: 0,
-                                         field_35: 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_41, # age1
-                  :field_42, # sexrab1
-                  :field_47, # ecstat1
-                  :field_140, # household_charge
-                ].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
+              expect(parser.errors[:field_6]).not_to include(error_message)
             end
           end