diff --git a/app/services/csv/lettings_log_csv_service.rb b/app/services/csv/lettings_log_csv_service.rb index 6521fa52c..146a720d5 100644 --- a/app/services/csv/lettings_log_csv_service.rb +++ b/app/services/csv/lettings_log_csv_service.rb @@ -280,32 +280,6 @@ 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) @@ -370,7 +344,6 @@ 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] @@ -404,16 +377,6 @@ 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 diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb index 779b7be4a..a29589181 100644 --- a/spec/models/lettings_log_spec.rb +++ b/spec/models/lettings_log_spec.rb @@ -467,7 +467,7 @@ RSpec.describe LettingsLog do describe "when changing a log's scheme and hence calling reset_scheme_location!" do context "when there is one valid location and many invalid locations in the new scheme" do - let(:scheme) { create(:scheme) } + let(:scheme) { create(:scheme, sensitive: 0) } let(:invalid_location_1) { create(:location, scheme:, startdate: Time.zone.today + 3.weeks) } let(:valid_location) { create(:location, scheme:, startdate: Time.zone.yesterday) } let(:invalid_location_2) { create(:location, scheme:, startdate: Time.zone.today + 3.weeks) } @@ -479,9 +479,9 @@ RSpec.describe LettingsLog do end context "when there are many valid locations in the new scheme" do - let(:old_scheme) { create(:scheme, owning_organisation:) } + let(:old_scheme) { create(:scheme, sensitive: 0, owning_organisation:) } let(:old_location) { create(:location, scheme: old_scheme) } - let(:new_scheme) { create(:scheme, owning_organisation:) } + let(:new_scheme) { create(:scheme, sensitive: 0, owning_organisation:) } before do create_list(:location, 2, scheme: new_scheme) @@ -508,7 +508,7 @@ RSpec.describe LettingsLog do end context "and a scheme with a single log is selected" do - let(:scheme) { create(:scheme, owning_organisation:) } + let(:scheme) { create(:scheme, sensitive: 0, owning_organisation:) } let!(:location) { create(:location, scheme:) } before do @@ -666,7 +666,7 @@ RSpec.describe LettingsLog do end context "and not renewal" do - let(:scheme) { create(:scheme, owning_organisation:) } + let(:scheme) { create(:scheme, sensitive: 0, owning_organisation:) } let(:location) { create(:location, scheme:, postcode: "M11AE", type_of_unit: 1, mobility_type: "W") } let(:supported_housing_lettings_log) do @@ -1091,7 +1091,7 @@ RSpec.describe LettingsLog do end describe "resetting invalidated fields" do - let(:scheme) { create(:scheme, owning_organisation: assigned_to_user.organisation) } + let(:scheme) { create(:scheme, sensitive: 0, owning_organisation: assigned_to_user.organisation) } let!(:location) { create(:location, location_code: "E07000223", scheme:) } let(:lettings_log) do create( @@ -1181,7 +1181,7 @@ RSpec.describe LettingsLog do let(:organisation_2) { create(:organisation) } context "when the organisation selected doesn't match the scheme set" do - let(:scheme) { create(:scheme, owning_organisation: assigned_to_user.organisation) } + let(:scheme) { create(:scheme, sensitive: 0, owning_organisation: assigned_to_user.organisation) } let(:location) { create_list(:location, 2, scheme:).first } let(:lettings_log) { create(:lettings_log, owning_organisation: nil, needstype: 2, scheme_id: scheme.id, location_id: location.id) } @@ -1194,7 +1194,7 @@ RSpec.describe LettingsLog do end context "when the organisation selected still matches the scheme set" do - let(:scheme) { create(:scheme, owning_organisation: organisation_2) } + let(:scheme) { create(:scheme, sensitive: 0, owning_organisation: organisation_2) } let(:location) { create_list(:location, 2, scheme:).first } let(:lettings_log) { create(:lettings_log, owning_organisation: nil, needstype: 2, scheme_id: scheme.id, location_id: location.id) } @@ -1797,7 +1797,7 @@ RSpec.describe LettingsLog do end context "when there is a duplicate supported housing log" do - let(:scheme) { create(:scheme) } + let(:scheme) { create(:scheme, sensitive: 0) } let(:location) { create(:location, scheme:) } let(:location_2) { create(:location, scheme:) } let(:supported_housing_log) { create(:lettings_log, :duplicate, needstype: 2, location:, scheme:, owning_organisation: organisation) } @@ -1963,7 +1963,7 @@ RSpec.describe LettingsLog do end context "when there is a duplicate supported housing log" do - let(:scheme) { create(:scheme, owning_organisation: organisation) } + let(:scheme) { create(:scheme, sensitive: 0, owning_organisation: organisation) } let(:location) { create(:location, scheme:) } let!(:supported_housing_log) { create(:lettings_log, :duplicate, needstype: 2, location:, scheme:, owning_organisation: organisation) } let!(:duplicate_supported_housing_log) { create(:lettings_log, :duplicate, needstype: 2, location:, scheme:, owning_organisation: organisation) } @@ -2240,5 +2240,104 @@ RSpec.describe LettingsLog do expect(log.scheme_has_confidential_information?).to be true end end + + describe "clearing confidential scheme address data on save" do + let(:owning_organisation) { create(:organisation) } + let(:scheme) { create(:scheme, sensitive: 1, owning_organisation:) } + let(:location) { create(:location, scheme:) } + let(:log) do + create( + :lettings_log, + :ignore_validation_errors, + needstype: 2, + owning_organisation:, + managing_organisation: owning_organisation, + scheme:, + location:, + startdate: Time.zone.local(2026, 5, 1), + ) + end + + it "clears address, UPRN and LA data collected before the confidential address feature" do + 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_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", + address_search_value_check: 1, + la: "E09000003", + is_la_inferred: true, + ) + + log.skip_uprn_lookup = true + log.skip_address_lookup = true + log.valid? + log.save!(validate: false) + log.reload + + expect(log.uprn).to be_nil + expect(log.uprn_known).to be_nil + expect(log.uprn_confirmed).to be_nil + expect(log.uprn_selection).to be_nil + expect(log.address_line1).to be_nil + expect(log.address_line2).to be_nil + expect(log.town_or_city).to be_nil + expect(log.county).to be_nil + expect(log.postcode_known).to be_nil + expect(log.address_line1_input).to be_nil + expect(log.postcode_full_input).to be_nil + expect(log.address_line1_as_entered).to be_nil + expect(log.address_line2_as_entered).to be_nil + expect(log.town_or_city_as_entered).to be_nil + expect(log.county_as_entered).to be_nil + expect(log.postcode_full_as_entered).to be_nil + expect(log.la_as_entered).to be_nil + expect(log.address_search_value_check).to be_nil + # la is deliberately left null in the DB - see LettingsLog#la, which derives it live + # from the scheme location on every read, and reset_and_infer_la! (which only records + # whether that fallback resolves, via is_la_inferred, without persisting the value) + expect(log[:la]).to be_nil + expect(log.la).to eq(location.location_code) + expect(log.is_la_inferred).to be true + end + + it "still infers the local authority from the scheme's location after clearing" do + log.update_columns(la: "E09000003", is_la_inferred: false) + log.valid? + log.save!(validate: false) + expect(log.la).to eq(location.location_code) + expect(log[:la]).to be_nil + expect(log.is_la_inferred).to be true + end + + it "leaves la blank and is_la_inferred false when the location has no resolvable local authority" do + location.update_columns(location_code: nil, location_admin_district: nil, is_la_inferred: false) + log.update_columns(la: "E09000003", is_la_inferred: false) + log.valid? + log.save!(validate: false) + expect(log.la).to be_nil + expect(log.is_la_inferred).to be false + end + + it "does not affect the scheme or location" do + log.update_columns(address_line1: "1 Secret Street") + log.valid? + expect { log.save!(validate: false) }.not_to change(log, :scheme_id) + expect(log.location_id).to eq(location.id) + end + end end # rubocop:enable RSpec/MessageChain diff --git a/spec/services/csv/lettings_log_csv_service_spec.rb b/spec/services/csv/lettings_log_csv_service_spec.rb index 2e52328f4..bc41ee31d 100644 --- a/spec/services/csv/lettings_log_csv_service_spec.rb +++ b/spec/services/csv/lettings_log_csv_service_spec.rb @@ -193,14 +193,31 @@ 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 } + # Address/UPRN columns that have no location-based fallback, so must simply be nil. + let(:blanked_columns) do + %w[uprn + uprn_known + uprn_confirmed + uprn_selection + address_line1 + address_line2 + town_or_city + county + 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] & attribute_line + end let(:log) do create( :lettings_log, @@ -214,8 +231,9 @@ RSpec.describe Csv::LettingsLogCsvService do 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). + # property address data in the database, bypassing callbacks/validations. Saving it + # again (as any ordinary edit of the log would) must clear this legacy data via + # DerivedVariables::LettingsLogVariables#set_derived_fields!. confidential_log.update_columns( uprn: "123456789012", uprn_known: 1, @@ -225,7 +243,6 @@ RSpec.describe Csv::LettingsLogCsvService do 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", @@ -237,6 +254,10 @@ RSpec.describe Csv::LettingsLogCsvService do la_as_entered: "la as entered", la: "E09000003", ) + confidential_log.skip_uprn_lookup = true + confidential_log.skip_address_lookup = true + confidential_log.valid? + confidential_log.save!(validate: false) end end @@ -245,26 +266,16 @@ RSpec.describe Csv::LettingsLogCsvService do 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| + it "blanks every hidden address and UPRN column with no location-based fallback" do + expect(blanked_columns).not_to be_empty + blanked_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 + it "exports the local authority and postcode inferred from the scheme's location, not the log's own (cleared) values" do + expect(csv_value("la")).to eq(location.location_code) + expect(csv_value("postcode_full")).to eq(location.postcode) if attribute_line.include?("postcode_full") end end end