Compare commits

..

10 Commits

Author SHA1 Message Date
Nat Dean-Lewis 50da65b5be Merge branch 'main' into CLDC-4462-clear-confidential-data-follow-up-actions 3 days ago
Nat Dean-Lewis 21bfc71c7b Merge branch 'main' into CLDC-4462-clear-confidential-data-follow-up-actions 3 days ago
Nat Dean-Lewis be4f02be36
CLDC-4462: Remove unnecessary routing change (#3382) 3 days ago
Nat Dean-Lewis 0004559e7b
CLDC-4462: Clear confidential address data (#3378) 3 days ago
Nat Dean-Lewis 0aeb9e5a1e Merge branch 'CLDC-4462-clear-confidential-address-data' into CLDC-4462-clear-confidential-data-follow-up-actions 3 days ago
Nat Dean-Lewis 1805ff15d6 CLDC-4462: test all fields 3 days ago
Nat Dean-Lewis 1169a2ce0f Merge branch 'CLDC-4462-clear-confidential-address-data' into CLDC-4462-clear-confidential-data-follow-up-actions 3 days ago
Nat Dean-Lewis dbf1e6ece5 Merge branch 'main' into CLDC-4462-clear-confidential-address-data 3 days ago
Nat Dean-Lewis 64c1bc8a6e
CLDC-4513: update rubyzip and patch breaking changes (#3381) 3 days ago
Nat Dean-Lewis 014172cbb6 CLDC-4462: respond to PR comments 3 days ago
  1. 9
      Gemfile.lock
  2. 6
      app/models/derived_variables/lettings_log_variables.rb
  3. 1
      app/models/form/lettings/pages/property_local_authority.rb
  4. 6
      app/models/lettings_log.rb
  5. 2
      app/services/exports/xml_export_service.rb
  6. 21
      lib/tasks/clear_confidential_address_data.rake
  7. 95
      spec/lib/tasks/clear_confidential_address_data_spec.rb
  8. 4
      spec/services/storage/archive_service_spec.rb

9
Gemfile.lock

@ -433,9 +433,12 @@ GEM
actionpack (>= 7.0) actionpack (>= 7.0)
railties (>= 7.0) railties (>= 7.0)
rexml (3.4.4) rexml (3.4.4)
roo (2.10.1) roo (3.0.0)
base64 (~> 0.2)
csv (~> 3)
logger (~> 1)
nokogiri (~> 1) nokogiri (~> 1)
rubyzip (>= 1.3.0, < 3.0.0) rubyzip (>= 3.0.0, < 4.0.0)
rotp (6.3.0) rotp (6.3.0)
rspec-core (3.13.0) rspec-core (3.13.0)
rspec-support (~> 3.13.0) rspec-support (~> 3.13.0)
@ -499,7 +502,7 @@ GEM
faraday-multipart (>= 1) faraday-multipart (>= 1)
ruby-progressbar (1.13.0) ruby-progressbar (1.13.0)
ruby2_keywords (0.0.5) ruby2_keywords (0.0.5)
rubyzip (2.3.2) rubyzip (3.6.0)
securerandom (0.4.1) securerandom (0.4.1)
selenium-webdriver (4.43.0) selenium-webdriver (4.43.0)
base64 (~> 0.2) base64 (~> 0.2)

6
app/models/derived_variables/lettings_log_variables.rb

@ -176,15 +176,11 @@ module DerivedVariables::LettingsLogVariables
if !form.start_year_2026_or_later? && is_supported_housing? if !form.start_year_2026_or_later? && is_supported_housing?
reset_address_fields! reset_address_fields!
elsif form.start_year_2026_or_later? elsif form.start_year_2026_or_later? && location_changed?
if location_changed?
reset_address_fields! reset_address_fields!
self.la = nil self.la = nil
end end
self.is_la_inferred = la.present? if is_supported_housing? && location && self[:la].blank?
end
if scheme_has_confidential_information? if scheme_has_confidential_information?
reset_address_fields! reset_address_fields!
self.uprn_selection = nil self.uprn_selection = nil

1
app/models/form/lettings/pages/property_local_authority.rb

@ -5,7 +5,6 @@ class Form::Lettings::Pages::PropertyLocalAuthority < ::Form::Page
@depends_on = [ @depends_on = [
{ "is_la_inferred" => false, "is_general_needs?" => true, "form.start_year_2025_or_later?" => false, "address_search_given?" => true }, { "is_la_inferred" => false, "is_general_needs?" => true, "form.start_year_2025_or_later?" => false, "address_search_given?" => true },
{ "is_la_inferred" => false, "is_general_needs?" => true, "form.start_year_2025_or_later?" => true }, { "is_la_inferred" => false, "is_general_needs?" => true, "form.start_year_2025_or_later?" => true },
{ "is_la_inferred" => false, "is_supported_housing?" => true, "form.start_year_2026_or_later?" => true },
] ]
end end

6
app/models/lettings_log.rb

@ -31,7 +31,7 @@ class LettingsLog < Log
before_validation :process_postcode_changes!, if: :postcode_full_changed? before_validation :process_postcode_changes!, if: :postcode_full_changed?
before_validation :process_previous_postcode_changes!, if: :ppostcode_full_changed? before_validation :process_previous_postcode_changes!, if: :ppostcode_full_changed?
before_validation :reset_invalidated_dependent_fields! before_validation :reset_invalidated_dependent_fields!
before_validation :reset_location_fields!, unless: :postcode_known_or_la_derived_from_scheme_location? before_validation :reset_location_fields!, unless: :postcode_known?
before_validation :reset_previous_location_fields!, unless: :previous_postcode_known? before_validation :reset_previous_location_fields!, unless: :previous_postcode_known?
before_validation :set_derived_fields! before_validation :set_derived_fields!
before_validation :process_uprn_change!, if: :should_process_uprn_change? before_validation :process_uprn_change!, if: :should_process_uprn_change?
@ -375,10 +375,6 @@ class LettingsLog < Log
postcode_known == 1 postcode_known == 1
end end
def postcode_known_or_la_derived_from_scheme_location?
postcode_known? || (form&.start_year_2026_or_later? && is_supported_housing? && location.present?)
end
def previous_postcode_known? def previous_postcode_known?
# 0: Yes # 0: Yes
ppcodenk&.zero? ppcodenk&.zero?

2
app/services/exports/xml_export_service.rb

@ -48,7 +48,7 @@ module Exports
@logger.info("Creating #{archive} - #{initial_count} resources") @logger.info("Creating #{archive} - #{initial_count} resources")
return {} if initial_count.zero? return {} if initial_count.zero?
zip_file = Zip::File.open_buffer(StringIO.new) zip_file = Zip::File.open_buffer(StringIO.new, create: true)
part_number = 1 part_number = 1
last_processed_marker = nil last_processed_marker = nil

21
lib/tasks/clear_confidential_address_data.rake

@ -41,26 +41,7 @@ task clear_confidential_address_data: :environment do
scope.find_each do |log| scope.find_each do |log|
original_status = log.status original_status = log.status
log.uprn = nil fields_present_scope.each { |field| log[field] = nil }
log.uprn_known = nil
log.uprn_confirmed = nil
log.uprn_selection = nil
log.address_line1 = nil
log.address_line2 = nil
log.town_or_city = nil
log.county = nil
log.postcode_full = nil
log.postcode_known = nil
log.address_line1_input = nil
log.postcode_full_input = nil
log.address_line1_as_entered = nil
log.address_line2_as_entered = nil
log.town_or_city_as_entered = nil
log.county_as_entered = nil
log.postcode_full_as_entered = nil
log.la_as_entered = nil
log.address_search_value_check = nil
log.la = nil
log.is_la_inferred = log.la.present? log.is_la_inferred = log.la.present?
if log.save(validate: false) if log.save(validate: false)

95
spec/lib/tasks/clear_confidential_address_data_spec.rb

@ -16,6 +16,31 @@ RSpec.describe "clear_confidential_address_data" do
let(:non_confidential_scheme) { create(:scheme, sensitive: "No", owning_organisation:) } let(:non_confidential_scheme) { create(:scheme, sensitive: "No", owning_organisation:) }
let(:location) { create(:location, scheme:) } let(:location) { create(:location, scheme:) }
let(:cleared_fields) do
%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
address_search_value_check
la
]
end
def create_log_with_address_data(scheme:, location:, startdate:) def create_log_with_address_data(scheme:, location:, startdate:)
log = create( log = create(
:lettings_log, :lettings_log,
@ -32,9 +57,24 @@ RSpec.describe "clear_confidential_address_data" do
# address/UPRN data collected before the confidential address feature existed. # address/UPRN data collected before the confidential address feature existed.
log.update_columns( log.update_columns(
uprn: "123456789012", uprn: "123456789012",
uprn_known: 1,
uprn_confirmed: 1,
uprn_selection: "123456789012",
address_line1: "1 Secret Street", address_line1: "1 Secret Street",
address_line2: "Flat 2",
town_or_city: "Secretville", town_or_city: "Secretville",
county: "Secretshire",
postcode_full: "SW1A 1AA",
postcode_known: 1, postcode_known: 1,
address_line1_input: "1 Secret Street",
postcode_full_input: "SW1A1AA",
address_line1_as_entered: "1 Secret Street",
address_line2_as_entered: "Flat 2",
town_or_city_as_entered: "Secretville",
county_as_entered: "Secretshire",
postcode_full_as_entered: "SW1A 1AA",
la_as_entered: "E09000003",
address_search_value_check: 1,
la: "E09000003", la: "E09000003",
) )
log log
@ -53,6 +93,15 @@ RSpec.describe "clear_confidential_address_data" do
expect(log.postcode_known).to be_nil expect(log.postcode_known).to be_nil
end end
it "clears every field the task targets" do
task.invoke
log.reload
cleared_fields.each do |field|
expect(log[field]).to be_nil, "expected #{field} to be nil but was #{log[field].inspect}"
end
end
it "keeps the log's status unchanged when the location's LA can be inferred" do it "keeps the log's status unchanged when the location's LA can be inferred" do
task.invoke task.invoke
expect(log.reload.status).to eq("completed") expect(log.reload.status).to eq("completed")
@ -66,6 +115,20 @@ RSpec.describe "clear_confidential_address_data" do
expect(log[:la]).to be_nil expect(log[:la]).to be_nil
expect(log.is_la_inferred).to be true expect(log.is_la_inferred).to be true
end end
it "does not affect the scheme or location associations" do
task.invoke
log.reload
expect(log.scheme_id).to eq(scheme.id)
expect(log.location_id).to eq(location.id)
end
it "is idempotent" do
task.invoke
task.reenable
expect { task.invoke }.not_to(change { log.reload.updated_at })
end
end end
context "when the scheme's location has no resolvable local authority" do context "when the scheme's location has no resolvable local authority" do
@ -88,7 +151,12 @@ RSpec.describe "clear_confidential_address_data" do
it "does not clear the address fields" do it "does not clear the address fields" do
task.invoke task.invoke
expect(log.reload.address_line1).to eq("1 Secret Street") log.reload
expect(log.address_line1).to eq("1 Secret Street")
expect(log.town_or_city).to eq("Secretville")
expect(log.uprn).to eq("123456789012")
expect(log.postcode_known).to eq(1)
end end
end end
@ -97,7 +165,12 @@ RSpec.describe "clear_confidential_address_data" do
it "does not clear the address fields" do it "does not clear the address fields" do
task.invoke task.invoke
expect(log.reload.address_line1).to eq("1 Secret Street") log.reload
expect(log.address_line1).to eq("1 Secret Street")
expect(log.town_or_city).to eq("Secretville")
expect(log.uprn).to eq("123456789012")
expect(log.postcode_known).to eq(1)
end end
end end
@ -119,23 +192,5 @@ RSpec.describe "clear_confidential_address_data" do
expect { task.invoke }.not_to(change { log.reload.updated_at }) expect { task.invoke }.not_to(change { log.reload.updated_at })
end end
end end
it "does not affect the scheme or location associations" do
log = create_log_with_address_data(scheme:, location:, startdate: Time.zone.local(2026, 5, 1))
task.invoke
log.reload
expect(log.scheme_id).to eq(scheme.id)
expect(log.location_id).to eq(location.id)
end
it "is idempotent" do
log = create_log_with_address_data(scheme:, location:, startdate: Time.zone.local(2026, 5, 1))
task.invoke
task.reenable
expect { task.invoke }.not_to(change { log.reload.updated_at })
end
end end
end end

4
spec/services/storage/archive_service_spec.rb

@ -13,7 +13,7 @@ RSpec.describe Storage::ArchiveService do
file file
end end
let(:archive_content) do let(:archive_content) do
zip_file = Zip::File.open_buffer(StringIO.new) zip_file = Zip::File.open_buffer(StringIO.new, create: true)
zip_file.mkdir(compressed_folder) zip_file.mkdir(compressed_folder)
zip_file.add(compressed_filepath, compressed_file) zip_file.add(compressed_filepath, compressed_file)
zip_file.write_buffer zip_file.write_buffer
@ -51,7 +51,7 @@ RSpec.describe Storage::ArchiveService do
it "raises an error if the file exists but is too large" do it "raises an error if the file exists but is too large" do
archive = archive_service.instance_variable_get(:@archive) archive = archive_service.instance_variable_get(:@archive)
allow(archive).to receive(:get_entry).and_return(Zip::Entry.new(nil, "", nil, nil, nil, nil, nil, 100_000_000, nil)) allow(archive).to receive(:get_entry).and_return(Zip::Entry.new(nil, "", size: 100_000_000))
expect { archive_service.get_file_io(compressed_filepath) } expect { archive_service.get_file_io(compressed_filepath) }
.to raise_error(RuntimeError, "File too large to be extracted") .to raise_error(RuntimeError, "File too large to be extracted")

Loading…
Cancel
Save