CLDC-4461: Hide confidential addresses - fix confidential to non confidential edge case (#3373)

* Fix bug where the UPRN/address questions were not shown after switching from a confidential to a non-confidential scheme

* Improve tests

* Hide address data in CSV download for logs in confidential schemes

* Action PR comments

* Action second round of PR comments

app/models/derived_variables/lettings_log_variables.rb

@@ -185,7 +185,6 @@ module DerivedVariables::LettingsLogVariables
       reset_address_fields!
       self.uprn_selection = nil
       self.postcode_known = nil
-      self.manual_address_entry_selected = nil
     end
 
     clear_gender_description_unless_gender_not_same_as_sex! if form.start_year_2026_or_later?

app/services/csv/lettings_log_csv_service.rb

@@ -280,6 +280,32 @@ module Csv
 
     SCHEME_AND_LOCATION_ATTRIBUTES = %w[scheme_code scheme_service_name scheme_confidential SCHTYPE scheme_registered_under_care_act scheme_owning_organisation_name scheme_primary_client_group scheme_has_other_client_group scheme_secondary_client_group scheme_support_type scheme_intended_stay scheme_created_at location_code location_postcode location_name location_units location_type_of_unit location_mobility_type location_local_authority location_startdate].freeze
 
+    # TODO: (CLDC-4462): delete once address data for logs in confidential schemes is wiped.
+    # Interim measure: logs in a confidential scheme that were created before the
+    # confidential address feature may still hold property address/UPRN data that is no
+    # longer collected. Blank those columns in the download until the data is wiped (the
+    # local authority, derived from the scheme location, is intentionally retained).
+    ADDRESS_FIELDS_HIDDEN_FOR_CONFIDENTIAL_SCHEME = %w[
+      uprn
+      uprn_known
+      uprn_confirmed
+      uprn_selection
+      address_line1
+      address_line2
+      town_or_city
+      county
+      postcode_full
+      postcode_known
+      address_line1_input
+      postcode_full_input
+      address_line1_as_entered
+      address_line2_as_entered
+      town_or_city_as_entered
+      county_as_entered
+      postcode_full_as_entered
+      la_as_entered
+    ].freeze
+
     def lettings_log_attributes
       ordered_questions = FormHandler.instance.ordered_questions_for_year(@year, "lettings")
       soft_validations_attributes = soft_validations_attributes(ordered_questions)
@@ -344,6 +370,8 @@ module Csv
 
     def value(attribute, log)
       attribute = "rent_type" if attribute == "rent_type_detail" # rent_type_detail is the requested column header for rent_type, so as not to confuse with renttype. It can be exported as label or code.
+      return nil if hide_confidential_scheme_address?(attribute, log) # TODO: (CLDC-4462): delete once address data for logs in confidential schemes is wiped.
+
       if CUSTOM_CALL_CHAINS.key? attribute.to_sym
         call_chain = CUSTOM_CALL_CHAINS[attribute.to_sym][@export_type.to_sym]
         call_chain.reduce(log) { |object, next_call| object&.public_send(next_call) }
@@ -376,6 +404,16 @@ module Csv
       end
     end
 
+    # TODO: (CLDC-4462): delete once address data for logs in confidential schemes is wiped.
+    def hide_confidential_scheme_address?(attribute, log)
+      ADDRESS_FIELDS_HIDDEN_FOR_CONFIDENTIAL_SCHEME.include?(attribute) && confidential_scheme_ids.include?(log.scheme_id)
+    end
+
+    # TODO: (CLDC-4462): delete once address data for logs in confidential schemes is wiped.
+    def confidential_scheme_ids
+      @confidential_scheme_ids ||= Scheme.where(sensitive: "Yes").pluck(:id).to_set
+    end
+
     def person_details_not_known?(log, attribute)
       details_known_field = PERSON_DETAILS.find { |key, _value| key == attribute }[1]["details_known_field"]
       log[details_known_field] == 1 # 1 for lettings logs, 2 for sales logs

spec/models/form/lettings/pages/address_fallback_spec.rb

@@ -26,4 +26,77 @@ RSpec.describe Form::Lettings::Pages::AddressFallback, type: :model do
   it "has correct depends_on" do
     expect(page.depends_on).to eq([{ "manual_address_entry_selected" => true, "is_address_asked?" => true }])
   end
+
+  context "when routing to the page" do
+    let(:form) { FormHandler.instance.forms["current_lettings"] }
+    let(:subsection) { instance_double(Form::Subsection, form:, enabled?: true) }
+
+    context "when the log is general needs" do
+      let(:log) { build(:lettings_log, needstype: 1) }
+
+      it "is routed to when the address is being entered manually" do
+        log.manual_address_entry_selected = true
+        expect(page).to be_routed_to(log, nil)
+      end
+
+      it "is not routed to when the address is not being entered manually" do
+        log.manual_address_entry_selected = false
+        expect(page).not_to be_routed_to(log, nil)
+      end
+
+      it "is not routed to when `manual_address_entry_selected` is nil" do
+        log.manual_address_entry_selected = nil
+        expect(page).not_to be_routed_to(log, nil)
+      end
+    end
+
+    context "when the log is supported housing" do
+      let(:log) { build(:lettings_log, needstype: 2) }
+
+      context "and the collection year is 2026 or later" do
+        before do
+          allow(form).to receive(:start_year_2026_or_later?).and_return(true)
+        end
+
+        it "is routed to when the address is being entered manually" do
+          log.manual_address_entry_selected = true
+          expect(page).to be_routed_to(log, nil)
+        end
+
+        it "is not routed to when the address is not being entered manually" do
+          log.manual_address_entry_selected = false
+          expect(page).not_to be_routed_to(log, nil)
+        end
+
+        it "is not routed to when `manual_address_entry_selected` is nil" do
+          log.manual_address_entry_selected = nil
+          expect(page).not_to be_routed_to(log, nil)
+        end
+      end
+
+      context "and the collection year is before 2026" do
+        before do
+          allow(form).to receive(:start_year_2026_or_later?).and_return(false)
+        end
+
+        it "is not routed to, even when the address is being entered manually" do
+          log.manual_address_entry_selected = true
+          expect(page).not_to be_routed_to(log, nil)
+        end
+      end
+
+      context "when the scheme has confidential information" do
+        let(:log) { build(:lettings_log, needstype: 2, scheme: build(:scheme, sensitive: 1)) }
+
+        before do
+          allow(form).to receive(:start_year_2026_or_later?).and_return(true)
+        end
+
+        it "is not routed to, even when the address is being entered manually" do
+          log.manual_address_entry_selected = true
+          expect(page).not_to be_routed_to(log, nil)
+        end
+      end
+    end
+  end
 end

spec/models/form/lettings/pages/address_search_spec.rb

@@ -39,4 +39,77 @@ RSpec.describe Form::Lettings::Pages::AddressSearch, type: :model do
       expect(page.question_number).to eq(16)
     end
   end
+
+  context "when routing to the page" do
+    let(:form) { FormHandler.instance.forms["current_lettings"] }
+    let(:subsection) { instance_double(Form::Subsection, form:, enabled?: true) }
+
+    context "when the log is general needs" do
+      let(:log) { build(:lettings_log, needstype: 1) }
+
+      it "is routed to when the address is not being entered manually" do
+        log.manual_address_entry_selected = false
+        expect(page).to be_routed_to(log, nil)
+      end
+
+      it "is not routed to when the address is being entered manually" do
+        log.manual_address_entry_selected = true
+        expect(page).not_to be_routed_to(log, nil)
+      end
+
+      it "is not routed to when `manual_address_entry_selected` is nil" do
+        log.manual_address_entry_selected = nil
+        expect(page).not_to be_routed_to(log, nil)
+      end
+    end
+
+    context "when the log is supported housing" do
+      let(:log) { build(:lettings_log, needstype: 2) }
+
+      context "and the collection year is 2026 or later" do
+        before do
+          allow(form).to receive(:start_year_2026_or_later?).and_return(true)
+        end
+
+        it "is routed to when the address is not being entered manually" do
+          log.manual_address_entry_selected = false
+          expect(page).to be_routed_to(log, nil)
+        end
+
+        it "is not routed to when the address is being entered manually" do
+          log.manual_address_entry_selected = true
+          expect(page).not_to be_routed_to(log, nil)
+        end
+
+        it "is not routed to when `manual_address_entry_selected` is nil" do
+          log.manual_address_entry_selected = nil
+          expect(page).not_to be_routed_to(log, nil)
+        end
+      end
+
+      context "and the collection year is before 2026" do
+        before do
+          allow(form).to receive(:start_year_2026_or_later?).and_return(false)
+        end
+
+        it "is not routed to, even when the address is not being entered manually" do
+          log.manual_address_entry_selected = false
+          expect(page).not_to be_routed_to(log, nil)
+        end
+      end
+
+      context "when the scheme has confidential information" do
+        let(:log) { build(:lettings_log, needstype: 2, scheme: build(:scheme, sensitive: 1)) }
+
+        before do
+          allow(form).to receive(:start_year_2026_or_later?).and_return(true)
+        end
+
+        it "is not routed to, even when the address is not being entered manually" do
+          log.manual_address_entry_selected = false
+          expect(page).not_to be_routed_to(log, nil)
+        end
+      end
+    end
+  end
 end

spec/models/lettings_log_derived_fields_spec.rb

@@ -1637,8 +1637,12 @@ RSpec.describe LettingsLog, type: :model do
           .and change { log.read_attribute(:county) }.from(county).to(nil)
           .and change { log.read_attribute(:postcode_full) }.from(postcode_full).to(nil)
           .and change { log.read_attribute(:uprn_selection) }.from(uprn_selection).to(nil)
-          .and change { log.read_attribute(:postcode_known) }.from(postcode_known).to(nil)
-          .and change { log.read_attribute(:manual_address_entry_selected) }.from(manual_address_entry_selected).to(nil)
+          .and(change { log.read_attribute(:postcode_known) }.from(postcode_known).to(nil))
+      end
+
+      it "does not reset `manual_address_entry_selected`" do
+        expect { log.set_derived_fields! }
+          .not_to(change { log.read_attribute(:manual_address_entry_selected) })
       end
 
       context "when the log is a new-build first let" do
@@ -1664,8 +1668,12 @@ RSpec.describe LettingsLog, type: :model do
             .and change { log.read_attribute(:county) }.from(county).to(nil)
             .and change { log.read_attribute(:postcode_full) }.from(postcode_full).to(nil)
             .and change { log.read_attribute(:uprn_selection) }.from(uprn_selection).to(nil)
-            .and change { log.read_attribute(:postcode_known) }.from(postcode_known).to(nil)
-            .and change { log.read_attribute(:manual_address_entry_selected) }.from(manual_address_entry_selected).to(nil)
+            .and(change { log.read_attribute(:postcode_known) }.from(postcode_known).to(nil))
+        end
+
+        it "does not reset `manual_address_entry_selected`" do
+          expect { log.set_derived_fields! }
+            .not_to(change { log.read_attribute(:manual_address_entry_selected) })
         end
       end
     end
@@ -1690,6 +1698,32 @@ RSpec.describe LettingsLog, type: :model do
           .to change(log, :manual_address_entry_selected).from(false).to(true)
       end
     end
+
+    context "when a log is changed from a confidential to a non-confidential scheme" do
+      # The confidential logic must leave manual_address_entry_selected in a
+      # routable state (either true or false), otherwise neither address page routes after the
+      # switch and the address question is never shown again.
+      let(:confidential_scheme) { create(:scheme, sensitive: 1) }
+      let(:non_confidential_scheme) { create(:scheme, sensitive: 0) }
+      let(:confidential_location) { create(:location, scheme: confidential_scheme) }
+      let(:non_confidential_location) { create(:location, scheme: non_confidential_scheme) }
+
+      before do
+        log.assign_attributes(manual_address_entry_selected: true)
+        log.scheme = confidential_scheme
+        log.location = confidential_location
+        log.set_derived_fields!
+
+        log.scheme = non_confidential_scheme
+        log.location = non_confidential_location
+        log.set_derived_fields!
+      end
+
+      it "asks the address question again with a routable value for `manual_address_entry_selected` (i.e., not nil)" do
+        expect(log.is_address_asked?).to be true
+        expect(log.manual_address_entry_selected).to be true
+      end
+    end
   end
 
   describe "#infer_at_most_one_relationship!" do

spec/services/bulk_upload/lettings/year2026/row_parser_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe BulkUpload::Lettings::Year2026::RowParser do
   let(:owning_org) { create(:organisation, :with_old_visible_id) }
   let(:managing_org) { create(:organisation, :with_old_visible_id, rent_periods: [4, 1]) }
   # Pinned non-confidential so the address/UPRN validations run as these tests expect;
-  # the confidential-scheme behaviour is covered by its own describe below (sensitive: 1).
+  # the confidential scheme behaviour is covered by its own describe below (sensitive: 1).
   let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org, sensitive: 0) }
   let(:postcode_first_part) { "AA1".freeze }
   let(:postcode_second_part) { "1AA".freeze }
@@ -1887,11 +1887,11 @@ RSpec.describe BulkUpload::Lettings::Year2026::RowParser do
       end
 
       context "when no UPRN or address fields are provided" do
-        let(:attributes) { base_attributes.merge({ field_18: nil, field_19: nil, field_21: nil, field_23: nil, field_24: nil }) }
+        let(:attributes) { base_attributes.merge({ field_18: nil, field_19: nil, field_20: nil, field_21: nil, field_22: nil, field_23: nil, field_24: nil }) }
 
         it "does not require the address or UPRN (no not answered errors)" do
           parser.valid?
-          %i[field_18 field_19 field_21 field_23 field_24].each do |field|
+          %i[field_18 field_19 field_20 field_21 field_22 field_23 field_24].each do |field|
             expect(parser.errors[field]).to be_empty
           end
         end
@@ -1901,7 +1901,9 @@ RSpec.describe BulkUpload::Lettings::Year2026::RowParser do
           log.valid?
           expect(log.read_attribute(:uprn)).to be_nil
           expect(log.read_attribute(:address_line1)).to be_nil
+          expect(log.read_attribute(:address_line2)).to be_nil
           expect(log.read_attribute(:town_or_city)).to be_nil
+          expect(log.read_attribute(:county)).to be_nil
           expect(log.read_attribute(:postcode_full)).to be_nil
         end
 
@@ -1917,7 +1919,9 @@ RSpec.describe BulkUpload::Lettings::Year2026::RowParser do
           base_attributes.merge({
             field_18: "123456789012",
             field_19: "1 Test Street",
+            field_20: "Test District",
             field_21: "Testville",
+            field_22: "Testshire",
             field_23: postcode_first_part,
             field_24: postcode_second_part,
             field_25: "E09000008",
@@ -1929,6 +1933,9 @@ RSpec.describe BulkUpload::Lettings::Year2026::RowParser do
           log.valid?
           expect(log.read_attribute(:uprn)).to be_nil
           expect(log.read_attribute(:address_line1)).to be_nil
+          expect(log.read_attribute(:address_line2)).to be_nil
+          expect(log.read_attribute(:town_or_city)).to be_nil
+          expect(log.read_attribute(:county)).to be_nil
           expect(log.read_attribute(:postcode_full)).to be_nil
         end
 

spec/services/csv/lettings_log_csv_service_spec.rb

@@ -193,6 +193,82 @@ RSpec.describe Csv::LettingsLogCsvService do
       end
     end
 
+    # TODO: (CLDC-4462): delete once address data for logs in confidential schemes is wiped.
+    describe "confidential scheme behaviour" do
+      let(:year) { 2026 }
+      let(:owning_organisation) { create(:organisation) }
+      let(:scheme) { create(:scheme, sensitive: 1, owning_organisation:) }
+      let(:location) { create(:location, scheme:) }
+      # Every hidden field that actually appears as a column in this export.
+      let(:hidden_columns) { described_class::ADDRESS_FIELDS_HIDDEN_FOR_CONFIDENTIAL_SCHEME & attribute_line }
+      let(:log) do
+        create(
+          :lettings_log,
+          :ignore_validation_errors,
+          needstype: 2,
+          owning_organisation:,
+          managing_organisation: owning_organisation,
+          assigned_to: user,
+          scheme:,
+          location:,
+          startdate: Time.zone.local(2026, 5, 1),
+        ).tap do |confidential_log|
+          # Simulate a log created before the confidential address feature that still holds
+          # property address data in the database. Populate every hidden address column so
+          # that the blanking is observable (a nil column would pass the assertion vacuously).
+          confidential_log.update_columns(
+            uprn: "123456789012",
+            uprn_known: 1,
+            uprn_confirmed: 1,
+            uprn_selection: "123456789012",
+            address_line1: "1 Secret Street",
+            address_line2: "Flat 2",
+            town_or_city: "Secretville",
+            county: "Secretshire",
+            postcode_full: "AB1 2CD",
+            postcode_known: 1,
+            address_line1_input: "1 Secret Street input",
+            postcode_full_input: "AB1 2CD",
+            address_line1_as_entered: "1 Secret Street as entered",
+            address_line2_as_entered: "Flat 2 as entered",
+            town_or_city_as_entered: "Secretville as entered",
+            county_as_entered: "Secretshire as entered",
+            postcode_full_as_entered: "AB1 2CD",
+            la_as_entered: "la as entered",
+            la: "E09000003",
+          )
+        end
+      end
+
+      def csv_value(attribute)
+        content_line[attribute_line.index(attribute)]
+      end
+
+      context "when a log's scheme is confidential" do
+        it "blanks every hidden address and UPRN column" do
+          expect(hidden_columns).not_to be_empty
+          hidden_columns.each do |attribute|
+            expect(csv_value(attribute)).to be_nil, "expected the #{attribute} column to be blank for a confidential-scheme log"
+          end
+        end
+
+        it "still exports the local authority" do
+          expect(csv_value("la")).to eq("E09000003")
+        end
+      end
+
+      context "when the scheme is not confidential" do
+        let(:scheme) { create(:scheme, sensitive: 0, owning_organisation:) }
+
+        it "exports every one of those columns as normal" do
+          expect(hidden_columns).not_to be_empty
+          hidden_columns.each do |attribute|
+            expect(csv_value(attribute)).not_to be_nil, "expected the #{attribute} column to be populated for a non-confidential-scheme log"
+          end
+        end
+      end
+    end
+
     describe "the full CSV output" do
       context "when the requested log year is 2026" do
         let(:year) { 2026 }