From 44862af259f67f6e40c6159db99a7e229dea21c9 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Thu, 29 Feb 2024 12:43:03 +0000 Subject: [PATCH 01/23] feat: mvp commit with address selector from address line 1 and postcode using OS places find endpoint --- .../form/lettings/pages/address_matcher.rb | 18 ++++++ .../form/lettings/pages/address_selection.rb | 17 ++++++ .../address_line1_for_address_matcher.rb | 14 +++++ .../lettings/questions/address_selection.rb | 60 +++++++++++++++++++ .../questions/postcode_for_address_matcher.rb | 25 ++++++++ .../subsections/property_information.rb | 10 +++- app/models/lettings_log.rb | 5 ++ app/models/log.rb | 44 +++++++++++++- app/services/address_client.rb | 52 ++++++++++++++++ app/services/address_data_presenter.rb | 41 +++++++++++++ app/views/form/page.html.erb | 1 + ..._add_address_selection_to_lettings_logs.rb | 5 ++ db/schema.rb | 3 +- 13 files changed, 292 insertions(+), 3 deletions(-) create mode 100644 app/models/form/lettings/pages/address_matcher.rb create mode 100644 app/models/form/lettings/pages/address_selection.rb create mode 100644 app/models/form/lettings/questions/address_line1_for_address_matcher.rb create mode 100644 app/models/form/lettings/questions/address_selection.rb create mode 100644 app/models/form/lettings/questions/postcode_for_address_matcher.rb create mode 100644 app/services/address_client.rb create mode 100644 app/services/address_data_presenter.rb create mode 100644 db/migrate/20240227163853_add_address_selection_to_lettings_logs.rb diff --git a/app/models/form/lettings/pages/address_matcher.rb b/app/models/form/lettings/pages/address_matcher.rb new file mode 100644 index 000000000..cfd24a4d1 --- /dev/null +++ b/app/models/form/lettings/pages/address_matcher.rb @@ -0,0 +1,18 @@ +class Form::Lettings::Pages::AddressMatcher < ::Form::Page + def initialize(id, hsh, subsection) + super + @id = "address_matcher" + @header = "Enter address details" + @depends_on = [ + { "is_supported_housing?" => false, "uprn_known" => 0 }, + { "is_supported_housing?" => false, "uprn_confirmed" => 0 }, + ] + end + + def questions + @questions ||= [ + Form::Lettings::Questions::AddressLine1ForAddressMatcher.new(nil, nil, self), + Form::Lettings::Questions::PostcodeForAddressMatcher.new(nil, nil, self), + ] + end +end diff --git a/app/models/form/lettings/pages/address_selection.rb b/app/models/form/lettings/pages/address_selection.rb new file mode 100644 index 000000000..72c93cd1d --- /dev/null +++ b/app/models/form/lettings/pages/address_selection.rb @@ -0,0 +1,17 @@ +class Form::Lettings::Pages::AddressSelection < ::Form::Page + def initialize(id, hsh, subsection) + super + @id = "address_selection" + @header = "We found some addresses that might be this property" + end + + def questions + @questions ||= [ + Form::Lettings::Questions::AddressSelection.new(nil, nil, self), + ] + end + + def routed_to?(log, _current_user = nil) + log.uprn_known == 0 && log.address_line1.present? && log.postcode_full.present? + end +end diff --git a/app/models/form/lettings/questions/address_line1_for_address_matcher.rb b/app/models/form/lettings/questions/address_line1_for_address_matcher.rb new file mode 100644 index 000000000..601b3f5eb --- /dev/null +++ b/app/models/form/lettings/questions/address_line1_for_address_matcher.rb @@ -0,0 +1,14 @@ +class Form::Lettings::Questions::AddressLine1ForAddressMatcher < ::Form::Question + def initialize(id, hsh, page) + super + @id = "address_line1" + @header = "Address line 1" + @error_label = "Address line 1" + @type = "text" + @plain_label = true + @check_answer_label = "Address line 1" + @disable_clearing_if_not_routed_or_dynamic_answer_options = true + @question_number = 12 + @hide_question_number_on_page = true + end +end diff --git a/app/models/form/lettings/questions/address_selection.rb b/app/models/form/lettings/questions/address_selection.rb new file mode 100644 index 000000000..5c2a80ae1 --- /dev/null +++ b/app/models/form/lettings/questions/address_selection.rb @@ -0,0 +1,60 @@ +class Form::Lettings::Questions::AddressSelection < ::Form::Question + def initialize(id, hsh, page) + super + @id = "address_selection" + @header = "Select the correct address" + @type = "radio" + @check_answer_label = "Select the correct address" + @disable_clearing_if_not_routed_or_dynamic_answer_options = true # have just added this, check if it works! + end + + def answer_options(log = nil, user = nil) + answer_opts = { + # "0" => { "value" => "address 0" }, + # "1" => { "value" => "address 1" }, + # "2" => { "value" => "address 2" }, + # "3" => { "value" => "address 3" }, + # "4" => { "value" => "address 4" }, + # "5" => { "value" => "address 5" }, + # "6" => { "value" => "address 6" }, + # "7" => { "value" => "address 7" }, + # "8" => { "value" => "address 8" }, + # "9" => { "value" => "address 9" }, + } + return answer_opts unless ActiveRecord::Base.connected? + return answer_opts unless log + return answer_opts unless log.address_options + + values = [] + log.address_options.each do |option| + values.append(option) + end + + { + "0" => { "value" => values[0] }, + "1" => { "value" => values[1] }, + "2" => { "value" => values[2] }, + "3" => { "value" => values[3] }, + "4" => { "value" => values[4] }, + "5" => { "value" => values[5] }, + "6" => { "value" => values[6] }, + "7" => { "value" => values[7] }, + "8" => { "value" => values[8] }, + "9" => { "value" => values[9] }, + }.freeze + end + + def displayed_answer_options(log, user = nil) + answer_options(log, user) + end + + def hidden_in_check_answers?(log, _current_user = nil) + log.uprn_known == 1 || log.uprn_confirmed == 1 + end + +private + + def selected_answer_option_is_derived?(_log) + true + end +end diff --git a/app/models/form/lettings/questions/postcode_for_address_matcher.rb b/app/models/form/lettings/questions/postcode_for_address_matcher.rb new file mode 100644 index 000000000..0d8c8ff7c --- /dev/null +++ b/app/models/form/lettings/questions/postcode_for_address_matcher.rb @@ -0,0 +1,25 @@ +class Form::Lettings::Questions::PostcodeForAddressMatcher < ::Form::Question + def initialize(id, hsh, page) + super + @id = "postcode_full" + @header = "Postcode" + @type = "text" + @width = 5 + @inferred_check_answers_value = [{ + "condition" => { + "pcodenk" => 1, + }, + "value" => "Not known", + }] + @inferred_answers = { + "la" => { + "is_la_inferred" => true, + }, + } + @plain_label = true + @check_answer_label = "Postcode" + @disable_clearing_if_not_routed_or_dynamic_answer_options = true + @question_number = 12 + @hide_question_number_on_page = true + end +end diff --git a/app/models/form/lettings/subsections/property_information.rb b/app/models/form/lettings/subsections/property_information.rb index 4600d5e95..edf4f6a58 100644 --- a/app/models/form/lettings/subsections/property_information.rb +++ b/app/models/form/lettings/subsections/property_information.rb @@ -31,7 +31,15 @@ class Form::Lettings::Subsections::PropertyInformation < ::Form::Subsection end def uprn_questions - if form.start_date.year >= 2023 + if form.start_year_after_2024? + [ + Form::Lettings::Pages::Uprn.new(nil, nil, self), + Form::Lettings::Pages::UprnConfirmation.new(nil, nil, self), + Form::Lettings::Pages::AddressMatcher.new(nil, nil, self), + Form::Lettings::Pages::AddressSelection.new(nil, nil, self), + Form::Lettings::Pages::Address.new(nil, nil, self), + ] + elsif form.start_date.year == 2023 [ Form::Lettings::Pages::Uprn.new(nil, nil, self), Form::Lettings::Pages::UprnConfirmation.new(nil, nil, self), diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 5dc998bbb..500416ace 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -34,6 +34,7 @@ class LettingsLog < Log before_validation :reset_previous_location_fields!, unless: :previous_postcode_known? before_validation :set_derived_fields! before_validation :process_uprn_change!, if: :should_process_uprn_change? + before_validation :process_address_change!, if: :should_process_address_change? belongs_to :scheme, optional: true belongs_to :location, optional: true @@ -847,4 +848,8 @@ private def should_process_uprn_change? uprn && startdate && (uprn_changed? || startdate_changed?) && collection_start_year_for_date(startdate) >= 2023 end + + def should_process_address_change? + address_selection && startdate && (address_selection_changed? || startdate_changed?) && form.start_year_after_2024? + end end diff --git a/app/models/log.rb b/app/models/log.rb index e15869634..95a72a981 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -53,7 +53,7 @@ class Log < ApplicationRecord scope :filter_by_owning_organisation, ->(owning_organisation, _user = nil) { where(owning_organisation:) } scope :filter_by_managing_organisation, ->(managing_organisation, _user = nil) { where(managing_organisation:) } - attr_accessor :skip_update_status, :skip_update_uprn_confirmed, :skip_dpo_validation + attr_accessor :skip_update_status, :skip_update_uprn_confirmed, :skip_update_address_selection, :skip_dpo_validation def process_uprn_change! if uprn.present? @@ -66,6 +66,7 @@ class Log < ApplicationRecord self.uprn_known = 1 self.uprn_confirmed = nil unless skip_update_uprn_confirmed + self.address_selection = nil self.address_line1 = presenter.address_line1 self.address_line2 = presenter.address_line2 self.town_or_city = presenter.town_or_city @@ -75,6 +76,47 @@ class Log < ApplicationRecord end end + def process_address_change! + if [address_selection, address_line1, postcode_full].all?(&:present?) + address_string = "#{address_line1}, , , #{postcode_full}" + service = AddressClient.new(address_string) + service.call + + return errors.add(:address_line1, :address_error, message: service.error) if service.error.present? + + presenter = AddressDataPresenter.new(service.result[address_selection]) + + self.uprn_known = 1 + self.uprn_confirmed = 1 + self.address_selection = nil # unless skip_update_address_confirmed + self.uprn = presenter.uprn #skip process uprn change? + self.address_line1 = presenter.address_line1 + self.address_line2 = presenter.address_line2 + self.town_or_city = presenter.town_or_city + self.postcode_full = presenter.postcode + self.county = nil + process_postcode_changes! + end + end + + def address_options + if [address_line1, postcode_full].all?(&:present?) + address_string = "#{address_line1}, , , #{postcode_full}" + service = AddressClient.new(address_string) + service.call + + return errors.add(:address_line1, :address_error, message: service.error) if service.error.present? + + address_options = [] + service.result.first(10).each do |result| + presenter = AddressDataPresenter.new(result) + address_options.append(presenter.address) + end + + @address_options = address_options + end + end + def collection_start_year return @start_year if @start_year diff --git a/app/services/address_client.rb b/app/services/address_client.rb new file mode 100644 index 000000000..7f270bfd1 --- /dev/null +++ b/app/services/address_client.rb @@ -0,0 +1,52 @@ +require "net/http" + +class AddressClient + attr_reader :address + attr_accessor :error + + ADDRESS = "api.os.uk".freeze + PATH = "/search/places/v1/find".freeze + + def initialize(address) + @address = address + end + + def call + unless response.is_a?(Net::HTTPSuccess) && result.present? + @error = "Address is not recognised. Check the address, or enter the UPRN" + end + rescue JSON::ParserError + @error = "Address is not recognised. Check the address, or enter the UPRN" + end + + def result + @result ||= JSON.parse(response.body)["results"]&.map { |address| address["DPA"] } + end + + private + + def http_client + client = Net::HTTP.new(ADDRESS, 443) + client.use_ssl = true + client.verify_mode = OpenSSL::SSL::VERIFY_PEER + client.max_retries = 3 + client.read_timeout = 10 # seconds + client + end + + def endpoint_uri + uri = URI(PATH) + params = { + query: address, + key: ENV["OS_DATA_KEY"], + matchprecision: 3, + maxresults: 10, + } + uri.query = URI.encode_www_form(params) + uri.to_s + end + + def response + @response ||= http_client.request_get(endpoint_uri) + end +end diff --git a/app/services/address_data_presenter.rb b/app/services/address_data_presenter.rb new file mode 100644 index 000000000..3dcced33e --- /dev/null +++ b/app/services/address_data_presenter.rb @@ -0,0 +1,41 @@ +require "net/http" + +class AddressDataPresenter + attr_reader :data + + def initialize(data) + @data = data + end + + def uprn + data["UPRN"] + end + + def address_line1 + [data["BUILDING_NUMBER"], data["BUILDING_NAME"], data["THOROUGHFARE_NAME"]].compact.join(", ") + end + + def address_line2 + data["DEPENDENT_LOCALITY"] + end + + def town_or_city + data["POST_TOWN"] + end + + def postcode + data["POSTCODE"] + end + + def address + data["ADDRESS"] + end + + # def match + # data["MATCH"] + # end + # + # def match_description + # data["MATCH_DESCRIPTION"] + # end +end diff --git a/app/views/form/page.html.erb b/app/views/form/page.html.erb index a16c01c89..9506983f4 100644 --- a/app/views/form/page.html.erb +++ b/app/views/form/page.html.erb @@ -61,6 +61,7 @@ <%= f.hidden_field :page, value: @page.id %> <%= f.hidden_field :interruption_page_id, value: @interruption_page_id %> <%= f.hidden_field :interruption_page_referrer_type, value: @interruption_page_referrer_type %> + <%#= f.hidden_field :address_answer_options, value: @log.address_options %>
<% if !@page.interruption_screen? %> diff --git a/db/migrate/20240227163853_add_address_selection_to_lettings_logs.rb b/db/migrate/20240227163853_add_address_selection_to_lettings_logs.rb new file mode 100644 index 000000000..cea9e3db0 --- /dev/null +++ b/db/migrate/20240227163853_add_address_selection_to_lettings_logs.rb @@ -0,0 +1,5 @@ +class AddAddressSelectionToLettingsLogs < ActiveRecord::Migration[7.0] + def change + add_column :lettings_logs, :address_selection, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index b5b9c9fc0..b80b2fe56 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2024_02_16_163519) do +ActiveRecord::Schema[7.0].define(version: 2024_02_27_163853) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -308,6 +308,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_02_16_163519) do t.integer "nationality_all_group" t.integer "reasonother_value_check" t.integer "accessible_register" + t.integer "address_selection" t.index ["bulk_upload_id"], name: "index_lettings_logs_on_bulk_upload_id" t.index ["created_by_id"], name: "index_lettings_logs_on_created_by_id" t.index ["location_id"], name: "index_lettings_logs_on_location_id" From 6dc81e535f42bf4bb8aa9803f1daff4df95d6cca Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Thu, 29 Feb 2024 16:07:51 +0000 Subject: [PATCH 02/23] feat: build out alternative routes through the address lookup flow --- .../form/lettings/pages/address_fallback.rb | 22 +++++++++++++ .../form/lettings/pages/address_matcher.rb | 2 +- .../form/lettings/pages/address_selection.rb | 9 +++++ app/models/form/lettings/pages/uprn.rb | 12 +++++-- .../address_line1_for_address_matcher.rb | 10 ++++-- .../lettings/questions/address_selection.rb | 25 +++----------- .../questions/postcode_for_address_matcher.rb | 2 +- .../subsections/property_information.rb | 2 +- app/models/log.rb | 33 ++++++++++++------- app/services/address_data_presenter.rb | 2 +- 10 files changed, 79 insertions(+), 40 deletions(-) create mode 100644 app/models/form/lettings/pages/address_fallback.rb diff --git a/app/models/form/lettings/pages/address_fallback.rb b/app/models/form/lettings/pages/address_fallback.rb new file mode 100644 index 000000000..5e932d913 --- /dev/null +++ b/app/models/form/lettings/pages/address_fallback.rb @@ -0,0 +1,22 @@ +class Form::Lettings::Pages::AddressFallback < ::Form::Page + def initialize(id, hsh, subsection) + super + @id = "address" + @header = "Q12 - What is the property's address?" + @depends_on = [ + { "is_supported_housing?" => false, "uprn_known" => nil, "address_selection" => 10 }, + { "is_supported_housing?" => false, "uprn_known" => 0, "address_selection" => 10 }, + { "is_supported_housing?" => false, "uprn_confirmed" => 0, "address_selection" => 10 }, + ] + end + + def questions + @questions ||= [ + Form::Lettings::Questions::AddressLine1.new(nil, nil, self), + Form::Lettings::Questions::AddressLine2.new(nil, nil, self), + Form::Lettings::Questions::TownOrCity.new(nil, nil, self), + Form::Lettings::Questions::County.new(nil, nil, self), + Form::Lettings::Questions::PostcodeForFullAddress.new(nil, nil, self), + ] + end +end diff --git a/app/models/form/lettings/pages/address_matcher.rb b/app/models/form/lettings/pages/address_matcher.rb index cfd24a4d1..4bf7bad41 100644 --- a/app/models/form/lettings/pages/address_matcher.rb +++ b/app/models/form/lettings/pages/address_matcher.rb @@ -2,7 +2,7 @@ class Form::Lettings::Pages::AddressMatcher < ::Form::Page def initialize(id, hsh, subsection) super @id = "address_matcher" - @header = "Enter address details" + @header = "Find address" @depends_on = [ { "is_supported_housing?" => false, "uprn_known" => 0 }, { "is_supported_housing?" => false, "uprn_confirmed" => 0 }, diff --git a/app/models/form/lettings/pages/address_selection.rb b/app/models/form/lettings/pages/address_selection.rb index 72c93cd1d..4e7a8ec74 100644 --- a/app/models/form/lettings/pages/address_selection.rb +++ b/app/models/form/lettings/pages/address_selection.rb @@ -14,4 +14,13 @@ class Form::Lettings::Pages::AddressSelection < ::Form::Page def routed_to?(log, _current_user = nil) log.uprn_known == 0 && log.address_line1.present? && log.postcode_full.present? end + + def skip_text + "Search for address again" + end + + def skip_href(log = nil) + return unless log + "/#{log.model_name.param_key.dasherize}s/#{log.id}/address-matcher" + end end diff --git a/app/models/form/lettings/pages/uprn.rb b/app/models/form/lettings/pages/uprn.rb index eb59e4a24..83ea11507 100644 --- a/app/models/form/lettings/pages/uprn.rb +++ b/app/models/form/lettings/pages/uprn.rb @@ -13,12 +13,20 @@ class Form::Lettings::Pages::Uprn < ::Form::Page end def skip_text - "Enter address instead" + if form.start_year_after_2024? + "Search for address instead" + else + "Enter address instead" + end end def skip_href(log = nil) return unless log - "/#{log.model_name.param_key.dasherize}s/#{log.id}/address" + if form.start_year_after_2024? + "/#{log.model_name.param_key.dasherize}s/#{log.id}/address-matcher" + else + "/#{log.model_name.param_key.dasherize}s/#{log.id}/address" + end end end diff --git a/app/models/form/lettings/questions/address_line1_for_address_matcher.rb b/app/models/form/lettings/questions/address_line1_for_address_matcher.rb index 601b3f5eb..2c807f85b 100644 --- a/app/models/form/lettings/questions/address_line1_for_address_matcher.rb +++ b/app/models/form/lettings/questions/address_line1_for_address_matcher.rb @@ -6,9 +6,15 @@ class Form::Lettings::Questions::AddressLine1ForAddressMatcher < ::Form::Questio @error_label = "Address line 1" @type = "text" @plain_label = true - @check_answer_label = "Address line 1" + @check_answer_label = "Find address" @disable_clearing_if_not_routed_or_dynamic_answer_options = true - @question_number = 12 @hide_question_number_on_page = true end + + def answer_label(log, _current_user = nil) + [ + log.address_line1, + log.postcode_full, + ].select(&:present?).join("\n") + end end diff --git a/app/models/form/lettings/questions/address_selection.rb b/app/models/form/lettings/questions/address_selection.rb index 5c2a80ae1..3f0f5e1c7 100644 --- a/app/models/form/lettings/questions/address_selection.rb +++ b/app/models/form/lettings/questions/address_selection.rb @@ -5,22 +5,11 @@ class Form::Lettings::Questions::AddressSelection < ::Form::Question @header = "Select the correct address" @type = "radio" @check_answer_label = "Select the correct address" - @disable_clearing_if_not_routed_or_dynamic_answer_options = true # have just added this, check if it works! + @disable_clearing_if_not_routed_or_dynamic_answer_options = true end def answer_options(log = nil, user = nil) - answer_opts = { - # "0" => { "value" => "address 0" }, - # "1" => { "value" => "address 1" }, - # "2" => { "value" => "address 2" }, - # "3" => { "value" => "address 3" }, - # "4" => { "value" => "address 4" }, - # "5" => { "value" => "address 5" }, - # "6" => { "value" => "address 6" }, - # "7" => { "value" => "address 7" }, - # "8" => { "value" => "address 8" }, - # "9" => { "value" => "address 9" }, - } + answer_opts = { "10" => { "value" => "The address is not listed" } } return answer_opts unless ActiveRecord::Base.connected? return answer_opts unless log return answer_opts unless log.address_options @@ -41,6 +30,8 @@ class Form::Lettings::Questions::AddressSelection < ::Form::Question "7" => { "value" => values[7] }, "8" => { "value" => values[8] }, "9" => { "value" => values[9] }, + "divider" => { "value" => true }, + "10" => { "value" => "The address is not listed, I want to enter the address manually" }, }.freeze end @@ -49,12 +40,6 @@ class Form::Lettings::Questions::AddressSelection < ::Form::Question end def hidden_in_check_answers?(log, _current_user = nil) - log.uprn_known == 1 || log.uprn_confirmed == 1 - end - -private - - def selected_answer_option_is_derived?(_log) - true + (log.uprn_known == 1 || log.uprn_confirmed == 1) end end diff --git a/app/models/form/lettings/questions/postcode_for_address_matcher.rb b/app/models/form/lettings/questions/postcode_for_address_matcher.rb index 0d8c8ff7c..989b976c7 100644 --- a/app/models/form/lettings/questions/postcode_for_address_matcher.rb +++ b/app/models/form/lettings/questions/postcode_for_address_matcher.rb @@ -19,7 +19,7 @@ class Form::Lettings::Questions::PostcodeForAddressMatcher < ::Form::Question @plain_label = true @check_answer_label = "Postcode" @disable_clearing_if_not_routed_or_dynamic_answer_options = true - @question_number = 12 @hide_question_number_on_page = true + @hidden_in_check_answers = true end end diff --git a/app/models/form/lettings/subsections/property_information.rb b/app/models/form/lettings/subsections/property_information.rb index edf4f6a58..4fdb9c922 100644 --- a/app/models/form/lettings/subsections/property_information.rb +++ b/app/models/form/lettings/subsections/property_information.rb @@ -37,7 +37,7 @@ class Form::Lettings::Subsections::PropertyInformation < ::Form::Subsection Form::Lettings::Pages::UprnConfirmation.new(nil, nil, self), Form::Lettings::Pages::AddressMatcher.new(nil, nil, self), Form::Lettings::Pages::AddressSelection.new(nil, nil, self), - Form::Lettings::Pages::Address.new(nil, nil, self), + Form::Lettings::Pages::AddressFallback.new(nil, nil, self), ] elsif form.start_date.year == 2023 [ diff --git a/app/models/log.rb b/app/models/log.rb index 95a72a981..483ebfaed 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -84,18 +84,27 @@ class Log < ApplicationRecord return errors.add(:address_line1, :address_error, message: service.error) if service.error.present? - presenter = AddressDataPresenter.new(service.result[address_selection]) - - self.uprn_known = 1 - self.uprn_confirmed = 1 - self.address_selection = nil # unless skip_update_address_confirmed - self.uprn = presenter.uprn #skip process uprn change? - self.address_line1 = presenter.address_line1 - self.address_line2 = presenter.address_line2 - self.town_or_city = presenter.town_or_city - self.postcode_full = presenter.postcode - self.county = nil - process_postcode_changes! + if address_selection.between?(0, 9) + presenter = AddressDataPresenter.new(service.result[address_selection]) + + self.uprn_known = 1 + self.uprn_confirmed = 1 + self.address_selection = nil # unless skip_update_address_confirmed + self.uprn = presenter.uprn # skip process uprn change? + self.address_line1 = presenter.address_line1 + self.address_line2 = presenter.address_line2 + self.town_or_city = presenter.town_or_city + self.postcode_full = presenter.postcode + self.county = nil + process_postcode_changes! + elsif address_selection == 10 + self.uprn_known = 0 + self.uprn_confirmed = nil + self.uprn = nil + self.address_line2 = nil + self.town_or_city = nil + self.county = nil + end end end diff --git a/app/services/address_data_presenter.rb b/app/services/address_data_presenter.rb index 3dcced33e..1ce9064f5 100644 --- a/app/services/address_data_presenter.rb +++ b/app/services/address_data_presenter.rb @@ -12,7 +12,7 @@ class AddressDataPresenter end def address_line1 - [data["BUILDING_NUMBER"], data["BUILDING_NAME"], data["THOROUGHFARE_NAME"]].compact.join(", ") + [data["SUB_BUILDING_NAME"], data["BUILDING_NUMBER"], data["BUILDING_NAME"], data["THOROUGHFARE_NAME"]].compact.join(", ") end def address_line2 From fbeed2ffadf9fbb15138dc00392e21b5df3872fc Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Thu, 29 Feb 2024 16:11:53 +0000 Subject: [PATCH 03/23] refactor: lint --- app/models/form/lettings/pages/address_selection.rb | 3 ++- app/models/form/lettings/questions/address_selection.rb | 2 +- app/services/address_client.rb | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/models/form/lettings/pages/address_selection.rb b/app/models/form/lettings/pages/address_selection.rb index 4e7a8ec74..850680308 100644 --- a/app/models/form/lettings/pages/address_selection.rb +++ b/app/models/form/lettings/pages/address_selection.rb @@ -12,7 +12,7 @@ class Form::Lettings::Pages::AddressSelection < ::Form::Page end def routed_to?(log, _current_user = nil) - log.uprn_known == 0 && log.address_line1.present? && log.postcode_full.present? + log.uprn_known.zero? && log.address_line1.present? && log.postcode_full.present? end def skip_text @@ -21,6 +21,7 @@ class Form::Lettings::Pages::AddressSelection < ::Form::Page def skip_href(log = nil) return unless log + "/#{log.model_name.param_key.dasherize}s/#{log.id}/address-matcher" end end diff --git a/app/models/form/lettings/questions/address_selection.rb b/app/models/form/lettings/questions/address_selection.rb index 3f0f5e1c7..c28a69e99 100644 --- a/app/models/form/lettings/questions/address_selection.rb +++ b/app/models/form/lettings/questions/address_selection.rb @@ -8,7 +8,7 @@ class Form::Lettings::Questions::AddressSelection < ::Form::Question @disable_clearing_if_not_routed_or_dynamic_answer_options = true end - def answer_options(log = nil, user = nil) + def answer_options(log = nil, _user = nil) answer_opts = { "10" => { "value" => "The address is not listed" } } return answer_opts unless ActiveRecord::Base.connected? return answer_opts unless log diff --git a/app/services/address_client.rb b/app/services/address_client.rb index 7f270bfd1..9bc827f20 100644 --- a/app/services/address_client.rb +++ b/app/services/address_client.rb @@ -23,7 +23,7 @@ class AddressClient @result ||= JSON.parse(response.body)["results"]&.map { |address| address["DPA"] } end - private +private def http_client client = Net::HTTP.new(ADDRESS, 443) From 687e0705fb236cd8e7fa9dab4499c0d072f18d55 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Thu, 29 Feb 2024 17:20:49 +0000 Subject: [PATCH 04/23] refactor: remove commented lines --- app/services/address_data_presenter.rb | 8 -------- app/views/form/page.html.erb | 1 - 2 files changed, 9 deletions(-) diff --git a/app/services/address_data_presenter.rb b/app/services/address_data_presenter.rb index 1ce9064f5..442017019 100644 --- a/app/services/address_data_presenter.rb +++ b/app/services/address_data_presenter.rb @@ -30,12 +30,4 @@ class AddressDataPresenter def address data["ADDRESS"] end - - # def match - # data["MATCH"] - # end - # - # def match_description - # data["MATCH_DESCRIPTION"] - # end end diff --git a/app/views/form/page.html.erb b/app/views/form/page.html.erb index 9506983f4..a16c01c89 100644 --- a/app/views/form/page.html.erb +++ b/app/views/form/page.html.erb @@ -61,7 +61,6 @@ <%= f.hidden_field :page, value: @page.id %> <%= f.hidden_field :interruption_page_id, value: @interruption_page_id %> <%= f.hidden_field :interruption_page_referrer_type, value: @interruption_page_referrer_type %> - <%#= f.hidden_field :address_answer_options, value: @log.address_options %>
<% if !@page.interruption_screen? %> From 7b71467cda2adbb944b7bb330d4d10bd91ffa263 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Fri, 1 Mar 2024 09:29:07 +0000 Subject: [PATCH 05/23] feat: make nil safe --- app/models/form/lettings/pages/address_selection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/form/lettings/pages/address_selection.rb b/app/models/form/lettings/pages/address_selection.rb index 850680308..962246df0 100644 --- a/app/models/form/lettings/pages/address_selection.rb +++ b/app/models/form/lettings/pages/address_selection.rb @@ -12,7 +12,7 @@ class Form::Lettings::Pages::AddressSelection < ::Form::Page end def routed_to?(log, _current_user = nil) - log.uprn_known.zero? && log.address_line1.present? && log.postcode_full.present? + log.uprn_known.present? && log.uprn_known.zero? && log.address_line1.present? && log.postcode_full.present? end def skip_text From 2288cbf634d5989217272bbdca68fd02afcbc478 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Mar 2024 09:34:54 +0000 Subject: [PATCH 06/23] feat: test address client --- app/services/address_client.rb | 1 - spec/services/address_client_spec.rb | 68 ++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 spec/services/address_client_spec.rb diff --git a/app/services/address_client.rb b/app/services/address_client.rb index 9bc827f20..02fad05c3 100644 --- a/app/services/address_client.rb +++ b/app/services/address_client.rb @@ -39,7 +39,6 @@ private params = { query: address, key: ENV["OS_DATA_KEY"], - matchprecision: 3, maxresults: 10, } uri.query = URI.encode_www_form(params) diff --git a/spec/services/address_client_spec.rb b/spec/services/address_client_spec.rb new file mode 100644 index 000000000..3f15b5a6a --- /dev/null +++ b/spec/services/address_client_spec.rb @@ -0,0 +1,68 @@ +require "rails_helper" + +describe AddressClient do + let(:client) { described_class.new("123") } + + let(:valid_response) do + { results: [{ DPA: { UPRN: "12345" } }] }.to_json + end + + def stub_api_request(body:, status: 200) + stub_request(:get, "https://api.os.uk/search/places/v1/find?key=OS_DATA_KEY&maxresults=10&query=123") + .to_return(status:, body:, headers: {}) + end + + describe "call" do + context "when json parse error" do + before do + stub_api_request(body: "{", status: 200) + + client.call + end + + it "returns error" do + expect(client.error).to eq("Address is not recognised. Check the address, or enter the UPRN") + end + end + + context "when http error" do + before do + stub_api_request(body: valid_response, status: 500) + + client.call + end + + it "returns error" do + expect(client.error).to eq("Address is not recognised. Check the address, or enter the UPRN") + end + end + + context "when results empty" do + before do + stub_api_request(body: {}.to_json) + + client.call + end + + it "returns error" do + expect(client.error).to eq("Address is not recognised. Check the address, or enter the UPRN") + end + end + + context "with results" do + before do + stub_api_request(body: valid_response) + + client.call + end + + it "returns result" do + expect(client.result).to eq([{ "UPRN" => "12345" }]) + end + + it "returns no error" do + expect(client.error).to be_nil + end + end + end +end From 5997403fae955f0b4ed69a9c126d87d4ab69e698 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Mar 2024 11:03:09 +0000 Subject: [PATCH 07/23] feat: store address input separately --- .../form/lettings/pages/address_selection.rb | 2 +- .../questions/address_line1_for_address_matcher.rb | 2 +- .../questions/postcode_for_address_matcher.rb | 2 +- app/models/log.rb | 12 ++++++------ db/schema.rb | 14 ++++++++------ 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/app/models/form/lettings/pages/address_selection.rb b/app/models/form/lettings/pages/address_selection.rb index 962246df0..40a48c27b 100644 --- a/app/models/form/lettings/pages/address_selection.rb +++ b/app/models/form/lettings/pages/address_selection.rb @@ -12,7 +12,7 @@ class Form::Lettings::Pages::AddressSelection < ::Form::Page end def routed_to?(log, _current_user = nil) - log.uprn_known.present? && log.uprn_known.zero? && log.address_line1.present? && log.postcode_full.present? + log.uprn_known.present? && log.uprn_known.zero? && log.address_line1_input.present? && log.postcode_full_input.present? end def skip_text diff --git a/app/models/form/lettings/questions/address_line1_for_address_matcher.rb b/app/models/form/lettings/questions/address_line1_for_address_matcher.rb index 2c807f85b..fa9dbf98b 100644 --- a/app/models/form/lettings/questions/address_line1_for_address_matcher.rb +++ b/app/models/form/lettings/questions/address_line1_for_address_matcher.rb @@ -1,7 +1,7 @@ class Form::Lettings::Questions::AddressLine1ForAddressMatcher < ::Form::Question def initialize(id, hsh, page) super - @id = "address_line1" + @id = "address_line1_input" @header = "Address line 1" @error_label = "Address line 1" @type = "text" diff --git a/app/models/form/lettings/questions/postcode_for_address_matcher.rb b/app/models/form/lettings/questions/postcode_for_address_matcher.rb index 989b976c7..81387c233 100644 --- a/app/models/form/lettings/questions/postcode_for_address_matcher.rb +++ b/app/models/form/lettings/questions/postcode_for_address_matcher.rb @@ -1,7 +1,7 @@ class Form::Lettings::Questions::PostcodeForAddressMatcher < ::Form::Question def initialize(id, hsh, page) super - @id = "postcode_full" + @id = "postcode_full_input" @header = "Postcode" @type = "text" @width = 5 diff --git a/app/models/log.rb b/app/models/log.rb index 483ebfaed..1012f60ee 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -77,12 +77,12 @@ class Log < ApplicationRecord end def process_address_change! - if [address_selection, address_line1, postcode_full].all?(&:present?) - address_string = "#{address_line1}, , , #{postcode_full}" + if [address_selection, address_line1_input, postcode_full_input].all?(&:present?) + address_string = "#{address_line1_input}, , , #{postcode_full_input}" service = AddressClient.new(address_string) service.call - return errors.add(:address_line1, :address_error, message: service.error) if service.error.present? + return errors.add(:address_line1_input, :address_error, message: service.error) if service.error.present? if address_selection.between?(0, 9) presenter = AddressDataPresenter.new(service.result[address_selection]) @@ -109,12 +109,12 @@ class Log < ApplicationRecord end def address_options - if [address_line1, postcode_full].all?(&:present?) - address_string = "#{address_line1}, , , #{postcode_full}" + if [address_line1_input, postcode_full_input].all?(&:present?) + address_string = "#{address_line1_input}, , , #{postcode_full_input}" service = AddressClient.new(address_string) service.call - return errors.add(:address_line1, :address_error, message: service.error) if service.error.present? + return errors.add(:address_line1_input, :address_error, message: service.error) if service.error.present? address_options = [] service.result.first(10).each do |result| diff --git a/db/schema.rb b/db/schema.rb index b80b2fe56..0600c096f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2024_02_27_163853) do +ActiveRecord::Schema[7.0].define(version: 2024_03_04_103216) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -193,14 +193,14 @@ ActiveRecord::Schema[7.0].define(version: 2024_02_27_163853) do t.integer "hb" t.integer "hbrentshortfall" t.integer "property_relet" - t.datetime "mrcdate" + t.datetime "mrcdate", precision: nil t.integer "incref" - t.datetime "startdate" + t.datetime "startdate", precision: nil t.integer "armedforces" t.integer "first_time_property_let_as_social_housing" t.integer "unitletas" t.integer "builtype" - t.datetime "voiddate" + t.datetime "voiddate", precision: nil t.bigint "owning_organisation_id" t.bigint "managing_organisation_id" t.integer "renttype" @@ -309,6 +309,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_02_27_163853) do t.integer "reasonother_value_check" t.integer "accessible_register" t.integer "address_selection" + t.string "address_line1_input" + t.string "postcode_full_input" t.index ["bulk_upload_id"], name: "index_lettings_logs_on_bulk_upload_id" t.index ["created_by_id"], name: "index_lettings_logs_on_created_by_id" t.index ["location_id"], name: "index_lettings_logs_on_location_id" @@ -713,8 +715,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_02_27_163853) do t.string "name" t.bigint "organisation_id" t.integer "sign_in_count", default: 0, null: false - t.datetime "current_sign_in_at" - t.datetime "last_sign_in_at" + t.datetime "current_sign_in_at", precision: nil + t.datetime "last_sign_in_at", precision: nil t.string "current_sign_in_ip" t.string "last_sign_in_ip" t.integer "role" From 107143d1e48351aee149135e7368f753739be13d Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Mar 2024 11:06:45 +0000 Subject: [PATCH 08/23] feat: add migration --- ...240304103216_add_address_input_fields_to_lettings_log.rb | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 db/migrate/20240304103216_add_address_input_fields_to_lettings_log.rb diff --git a/db/migrate/20240304103216_add_address_input_fields_to_lettings_log.rb b/db/migrate/20240304103216_add_address_input_fields_to_lettings_log.rb new file mode 100644 index 000000000..e0fe83397 --- /dev/null +++ b/db/migrate/20240304103216_add_address_input_fields_to_lettings_log.rb @@ -0,0 +1,6 @@ +class AddAddressInputFieldsToLettingsLog < ActiveRecord::Migration[7.0] + def change + add_column :lettings_logs, :address_line1_input, :string + add_column :lettings_logs, :postcode_full_input, :string + end +end From 0c7b1738288eecdac5068280cccc540f2785be53 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Mar 2024 11:09:24 +0000 Subject: [PATCH 09/23] feat: display inputs in answer label --- .../lettings/questions/address_line1_for_address_matcher.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/form/lettings/questions/address_line1_for_address_matcher.rb b/app/models/form/lettings/questions/address_line1_for_address_matcher.rb index fa9dbf98b..b2247a8f0 100644 --- a/app/models/form/lettings/questions/address_line1_for_address_matcher.rb +++ b/app/models/form/lettings/questions/address_line1_for_address_matcher.rb @@ -13,8 +13,8 @@ class Form::Lettings::Questions::AddressLine1ForAddressMatcher < ::Form::Questio def answer_label(log, _current_user = nil) [ - log.address_line1, - log.postcode_full, + log.address_line1_input, + log.postcode_full_input, ].select(&:present?).join("\n") end end From dcc4a733340e04a9e17da6702977ba545e41bebd Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Mar 2024 11:16:48 +0000 Subject: [PATCH 10/23] feat: update data presenter and test --- app/services/address_data_presenter.rb | 22 +++++- spec/services/address_data_presenter_spec.rb | 72 ++++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 spec/services/address_data_presenter_spec.rb diff --git a/app/services/address_data_presenter.rb b/app/services/address_data_presenter.rb index 442017019..0a4201cc1 100644 --- a/app/services/address_data_presenter.rb +++ b/app/services/address_data_presenter.rb @@ -12,15 +12,31 @@ class AddressDataPresenter end def address_line1 - [data["SUB_BUILDING_NAME"], data["BUILDING_NUMBER"], data["BUILDING_NAME"], data["THOROUGHFARE_NAME"]].compact.join(", ") + data.values_at( + "PO_BOX_NUMBER", + "ORGANISATION_NAME", + "DEPARTMENT_NAME", + "SUB_BUILDING_NAME", + "BUILDING_NAME", + "BUILDING_NUMBER", + "DEPENDENT_THOROUGHFARE_NAME", + "THOROUGHFARE_NAME", + ).compact + .join(", ") + .titleize end def address_line2 - data["DEPENDENT_LOCALITY"] + data.values_at( + "DOUBLE_DEPENDENT_LOCALITY", "DEPENDENT_LOCALITY" + ).compact + .join(", ") + .titleize + .presence end def town_or_city - data["POST_TOWN"] + data["POST_TOWN"].titleize end def postcode diff --git a/spec/services/address_data_presenter_spec.rb b/spec/services/address_data_presenter_spec.rb new file mode 100644 index 000000000..34bdbf03d --- /dev/null +++ b/spec/services/address_data_presenter_spec.rb @@ -0,0 +1,72 @@ +require "rails_helper" + +describe AddressDataPresenter do + let(:data) do + JSON.parse( + '{ + "UPRN": "UPRN", + "UDPRN": "UDPRN", + "ADDRESS": "full address", + "SUB_BUILDING_NAME": "0", + "BUILDING_NAME": "building name", + "THOROUGHFARE_NAME": "thoroughfare", + "POST_TOWN": "posttown", + "POSTCODE": "postcode", + "STATUS": "APPROVED", + "DOUBLE_DEPENDENT_LOCALITY": "double dependent locality", + "DEPENDENT_LOCALITY": "dependent locality", + "CLASSIFICATION_CODE": "classification code", + "LOCAL_CUSTODIAN_CODE_DESCRIPTION": "LONDON BOROUGH OF HARINGEY", + "BLPU_STATE_CODE": "2", + "BLPU_STATE_CODE_DESCRIPTION": "In use", + "LAST_UPDATE_DATE": "31/07/2020", + "ENTRY_DATE": "30/01/2015", + "BLPU_STATE_DATE": "30/01/2015", + "LANGUAGE": "EN", + "MATCH_DESCRIPTION": "EXACT" + }', + ) + end + + let(:presenter) { described_class.new(data) } + + describe "#postcode" do + it "returns postcode" do + expect(presenter.postcode).to eq("postcode") + end + end + + describe "#address_line1" do + it "returns address_line1" do + expect(presenter.address_line1).to eq("0, Building Name, Thoroughfare") + end + end + + describe "#address_line2" do + it "returns address_line2" do + expect(presenter.address_line2).to eq("Double Dependent Locality, Dependent Locality") + end + end + + describe "#town_or_city" do + it "returns town_or_city" do + expect(presenter.town_or_city).to eq("Posttown") + end + end + + describe "#address" do + it "returns address" do + expect(presenter.address).to eq("full address") + end + end + + context "when address_line2 fields are missing" do + let(:data) { {} } + + describe "#address_line2" do + it "returns nil" do + expect(presenter.address_line2).to be_nil + end + end + end +end From aafda17e3eb2c5c3eba0d91fc06a125a7d1351ca Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Mar 2024 11:19:54 +0000 Subject: [PATCH 11/23] feat: revert precicions schema update --- db/schema.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 0600c096f..3fae42050 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -193,14 +193,14 @@ ActiveRecord::Schema[7.0].define(version: 2024_03_04_103216) do t.integer "hb" t.integer "hbrentshortfall" t.integer "property_relet" - t.datetime "mrcdate", precision: nil + t.datetime "mrcdate" t.integer "incref" - t.datetime "startdate", precision: nil + t.datetime "startdate" t.integer "armedforces" t.integer "first_time_property_let_as_social_housing" t.integer "unitletas" t.integer "builtype" - t.datetime "voiddate", precision: nil + t.datetime "voiddate" t.bigint "owning_organisation_id" t.bigint "managing_organisation_id" t.integer "renttype" @@ -715,8 +715,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_03_04_103216) do t.string "name" t.bigint "organisation_id" t.integer "sign_in_count", default: 0, null: false - t.datetime "current_sign_in_at", precision: nil - t.datetime "last_sign_in_at", precision: nil + t.datetime "current_sign_in_at" + t.datetime "last_sign_in_at" t.string "current_sign_in_ip" t.string "last_sign_in_ip" t.integer "role" From 40b25c664d09bc0e9967729404681232117b1ac9 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Mar 2024 11:19:54 +0000 Subject: [PATCH 12/23] feat: revert precision schema update --- db/schema.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 0600c096f..3fae42050 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -193,14 +193,14 @@ ActiveRecord::Schema[7.0].define(version: 2024_03_04_103216) do t.integer "hb" t.integer "hbrentshortfall" t.integer "property_relet" - t.datetime "mrcdate", precision: nil + t.datetime "mrcdate" t.integer "incref" - t.datetime "startdate", precision: nil + t.datetime "startdate" t.integer "armedforces" t.integer "first_time_property_let_as_social_housing" t.integer "unitletas" t.integer "builtype" - t.datetime "voiddate", precision: nil + t.datetime "voiddate" t.bigint "owning_organisation_id" t.bigint "managing_organisation_id" t.integer "renttype" @@ -715,8 +715,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_03_04_103216) do t.string "name" t.bigint "organisation_id" t.integer "sign_in_count", default: 0, null: false - t.datetime "current_sign_in_at", precision: nil - t.datetime "last_sign_in_at", precision: nil + t.datetime "current_sign_in_at" + t.datetime "last_sign_in_at" t.string "current_sign_in_ip" t.string "last_sign_in_ip" t.integer "role" From b8259c68fc1b4ffeb7bf39a4b0888b5f457fcc18 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Mar 2024 12:00:32 +0000 Subject: [PATCH 13/23] feat: migrate new fields to sales logs --- ...add_address_lookup_fields_to_sales_logs.rb | 7 ++++ db/schema.rb | 5 ++- spec/models/form/lettings/pages/uprn_spec.rb | 40 +++++++++++++++---- .../subsections/property_information_spec.rb | 2 + 4 files changed, 45 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20240304115940_add_address_lookup_fields_to_sales_logs.rb diff --git a/db/migrate/20240304115940_add_address_lookup_fields_to_sales_logs.rb b/db/migrate/20240304115940_add_address_lookup_fields_to_sales_logs.rb new file mode 100644 index 000000000..69c654295 --- /dev/null +++ b/db/migrate/20240304115940_add_address_lookup_fields_to_sales_logs.rb @@ -0,0 +1,7 @@ +class AddAddressLookupFieldsToSalesLogs < ActiveRecord::Migration[7.0] + def change + add_column :sales_logs, :address_selection, :integer + add_column :sales_logs, :address_line1_input, :string + add_column :sales_logs, :postcode_full_input, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 3fae42050..b107b985e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2024_03_04_103216) do +ActiveRecord::Schema[7.0].define(version: 2024_03_04_115940) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -664,6 +664,9 @@ ActiveRecord::Schema[7.0].define(version: 2024_03_04_103216) do t.integer "nationality_all_group" t.integer "nationality_all_buyer2" t.integer "nationality_all_buyer2_group" + t.integer "address_selection" + t.string "address_line1_input" + t.string "postcode_full_input" t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id" t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id" t.index ["managing_organisation_id"], name: "index_sales_logs_on_managing_organisation_id" diff --git a/spec/models/form/lettings/pages/uprn_spec.rb b/spec/models/form/lettings/pages/uprn_spec.rb index 009f7b392..5894bdbcd 100644 --- a/spec/models/form/lettings/pages/uprn_spec.rb +++ b/spec/models/form/lettings/pages/uprn_spec.rb @@ -6,6 +6,12 @@ RSpec.describe Form::Lettings::Pages::Uprn, type: :model do let(:page_id) { nil } let(:page_definition) { nil } let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form) } + + before do + allow(form).to receive(:start_year_after_2024?).and_return(false) + allow(subsection).to receive(:form).and_return(form) + end it "has correct subsection" do expect(page.subsection).to eq(subsection) @@ -31,10 +37,6 @@ RSpec.describe Form::Lettings::Pages::Uprn, type: :model do expect(page.depends_on).to eq([{ "is_supported_housing?" => false }]) end - it "has correct skip_text" do - expect(page.skip_text).to eq("Enter address instead") - end - describe "has correct skip_href" do context "when log is nil" do it "is nil" do @@ -45,10 +47,32 @@ RSpec.describe Form::Lettings::Pages::Uprn, type: :model do context "when log is present" do let(:log) { create(:lettings_log) } - it "points to address page" do - expect(page.skip_href(log)).to eq( - "/lettings-logs/#{log.id}/address", - ) + context "with 2023/24 form" do + it "points to address page" do + expect(page.skip_href(log)).to eq( + "/lettings-logs/#{log.id}/address", + ) + end + + it "has correct skip_text" do + expect(page.skip_text).to eq("Enter address instead") + end + end + + context "with 2024/25 form" do + before do + allow(form).to receive(:start_year_after_2024?).and_return(true) + end + + it "points to address search page" do + expect(page.skip_href(log)).to eq( + "/lettings-logs/#{log.id}/address-matcher", + ) + end + + it "has correct skip_text" do + expect(page.skip_text).to eq("Search for address instead") + end end end end diff --git a/spec/models/form/lettings/subsections/property_information_spec.rb b/spec/models/form/lettings/subsections/property_information_spec.rb index 0471db588..fd1bb4ef6 100644 --- a/spec/models/form/lettings/subsections/property_information_spec.rb +++ b/spec/models/form/lettings/subsections/property_information_spec.rb @@ -91,6 +91,8 @@ RSpec.describe Form::Lettings::Subsections::PropertyInformation, type: :model do %w[ uprn uprn_confirmation + address_matcher + address_selection address property_local_authority local_authority_min_rent_value_check From 779e528ed0ee8d817e301292068eaedad2a63db8 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Mar 2024 15:56:56 +0000 Subject: [PATCH 14/23] feat: update shared log examples tests --- app/models/log.rb | 2 +- spec/shared/shared_log_examples.rb | 62 ++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/app/models/log.rb b/app/models/log.rb index 1012f60ee..17f372db0 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -82,7 +82,7 @@ class Log < ApplicationRecord service = AddressClient.new(address_string) service.call - return errors.add(:address_line1_input, :address_error, message: service.error) if service.error.present? + return errors.add(:address_selection, :address_error, message: service.error) if service.error.present? if address_selection.between?(0, 9) presenter = AddressDataPresenter.new(service.result[address_selection]) diff --git a/spec/shared/shared_log_examples.rb b/spec/shared/shared_log_examples.rb index 354a8b1ee..7202544ba 100644 --- a/spec/shared/shared_log_examples.rb +++ b/spec/shared/shared_log_examples.rb @@ -106,5 +106,67 @@ RSpec.shared_examples "shared log examples" do |log_type| end end end + + describe "#process_address_change!" do + context "when address_line1_input and postcode_full_input set to a value" do + let(:log) do + log = build( + log_type, + address_selection: 0, + address_line1: "Address line 1", + postcode_full: "AA1 1AA", + county: "county", + ) + log.save!(validate: false) + log + end + + it "updates log fields" do + log.address_line1_input = "New address line 1" + log.postcode_full_input = "BB2 2BB" + + allow_any_instance_of(AddressClient).to receive(:call) + allow_any_instance_of(AddressClient).to receive(:result).and_return([{ + "UPRN" => "UPRN", + "UDPRN" => "UDPRN", + "ADDRESS" => "full address", + "SUB_BUILDING_NAME" => "0", + "BUILDING_NAME" => "building name", + "THOROUGHFARE_NAME" => "thoroughfare", + "POST_TOWN" => "posttown", + "POSTCODE" => "postcode", + }]) + + expect { log.process_address_change! }.to change(log, :address_line1).from("Address line 1").to("0, Building Name, Thoroughfare") + .and change(log, :town_or_city).from(nil).to("Posttown") + .and change(log, :postcode_full).from("AA1 1AA").to("POSTCODE") + .and change(log, :uprn_confirmed).from(nil).to(1) + .and change(log, :uprn).from(nil).to("UPRN") + .and change(log, :uprn_known).from(nil).to(1) + .and change(log, :address_selection).from(0).to(nil) + .and change(log, :county).from("county").to(nil) + end + end + + context "when address inputs are nil" do + let(:log) { create(log_type, address_selection: nil, address_line1_input: nil, postcode_full_input: nil) } + + it "does not update log" do + expect { log.process_address_change! }.not_to change(log, :attributes) + end + end + + context "when service errors" do + let(:log) { build(log_type, :in_progress, address_selection: 0, address_line1_input: "123", postcode_full_input: "AA1 1AA") } + let(:error_message) { "error" } + + it "adds error to log" do + allow_any_instance_of(AddressClient).to receive(:call) + allow_any_instance_of(AddressClient).to receive(:error).and_return(error_message) + + expect { log.process_address_change! }.to change { log.errors[:address_selection] }.from([]).to([error_message]) + end + end + end end # rubocop:enable RSpec/AnyInstance From d61e83bdf7dc968a95361fd1e99c1c6573b09555 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Mar 2024 16:54:14 +0000 Subject: [PATCH 15/23] feat: use -1 for not listed address for extensibility --- app/models/form/lettings/questions/address_selection.rb | 2 +- app/models/log.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/form/lettings/questions/address_selection.rb b/app/models/form/lettings/questions/address_selection.rb index c28a69e99..09fb6a475 100644 --- a/app/models/form/lettings/questions/address_selection.rb +++ b/app/models/form/lettings/questions/address_selection.rb @@ -31,7 +31,7 @@ class Form::Lettings::Questions::AddressSelection < ::Form::Question "8" => { "value" => values[8] }, "9" => { "value" => values[9] }, "divider" => { "value" => true }, - "10" => { "value" => "The address is not listed, I want to enter the address manually" }, + "-1" => { "value" => "The address is not listed, I want to enter the address manually" }, }.freeze end diff --git a/app/models/log.rb b/app/models/log.rb index 17f372db0..b0c32e0e2 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -97,7 +97,7 @@ class Log < ApplicationRecord self.postcode_full = presenter.postcode self.county = nil process_postcode_changes! - elsif address_selection == 10 + elsif address_selection == -1 self.uprn_known = 0 self.uprn_confirmed = nil self.uprn = nil From 1afee2c66fa89058b55b272e61c800823086e38b Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Mar 2024 17:30:18 +0000 Subject: [PATCH 16/23] feat: add service error to address_selection --- app/models/log.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/log.rb b/app/models/log.rb index b0c32e0e2..d06227d5d 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -114,7 +114,7 @@ class Log < ApplicationRecord service = AddressClient.new(address_string) service.call - return errors.add(:address_line1_input, :address_error, message: service.error) if service.error.present? + return errors.add(:address_selection, :address_error, message: service.error) if service.error.present? address_options = [] service.result.first(10).each do |result| @@ -122,7 +122,7 @@ class Log < ApplicationRecord address_options.append(presenter.address) end - @address_options = address_options + address_options end end From 80861fe2d853179caa3267106374878778f1da92 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 5 Mar 2024 10:47:30 +0000 Subject: [PATCH 17/23] feat: handle case when no addresses are found --- .../form/lettings/pages/address_fallback.rb | 9 ++++-- .../form/lettings/pages/address_selection.rb | 3 +- .../lettings/questions/address_selection.rb | 32 +++++++------------ app/models/log.rb | 10 ++++-- 4 files changed, 26 insertions(+), 28 deletions(-) diff --git a/app/models/form/lettings/pages/address_fallback.rb b/app/models/form/lettings/pages/address_fallback.rb index 5e932d913..40f23aff1 100644 --- a/app/models/form/lettings/pages/address_fallback.rb +++ b/app/models/form/lettings/pages/address_fallback.rb @@ -4,9 +4,12 @@ class Form::Lettings::Pages::AddressFallback < ::Form::Page @id = "address" @header = "Q12 - What is the property's address?" @depends_on = [ - { "is_supported_housing?" => false, "uprn_known" => nil, "address_selection" => 10 }, - { "is_supported_housing?" => false, "uprn_known" => 0, "address_selection" => 10 }, - { "is_supported_housing?" => false, "uprn_confirmed" => 0, "address_selection" => 10 }, + { "is_supported_housing?" => false, "uprn_known" => nil, "address_selection" => -1 }, + { "is_supported_housing?" => false, "uprn_known" => 0, "address_selection" => -1 }, + { "is_supported_housing?" => false, "uprn_confirmed" => 0, "address_selection" => -1 }, + { "is_supported_housing?" => false, "uprn_known" => nil, "address_options_present?" => false }, + { "is_supported_housing?" => false, "uprn_known" => 0, "address_options_present?" => false }, + { "is_supported_housing?" => false, "uprn_confirmed" => 0, "address_options_present?" => false }, ] end diff --git a/app/models/form/lettings/pages/address_selection.rb b/app/models/form/lettings/pages/address_selection.rb index 40a48c27b..3ba14e7b8 100644 --- a/app/models/form/lettings/pages/address_selection.rb +++ b/app/models/form/lettings/pages/address_selection.rb @@ -3,6 +3,7 @@ class Form::Lettings::Pages::AddressSelection < ::Form::Page super @id = "address_selection" @header = "We found some addresses that might be this property" + @depends_on = [{ "address_options_present?" => true }] end def questions @@ -12,7 +13,7 @@ class Form::Lettings::Pages::AddressSelection < ::Form::Page end def routed_to?(log, _current_user = nil) - log.uprn_known.present? && log.uprn_known.zero? && log.address_line1_input.present? && log.postcode_full_input.present? + log.uprn_known.present? && log.uprn_known.zero? && log.address_line1_input.present? && log.postcode_full_input.present? && (1..10).cover?(log.address_options.count) end def skip_text diff --git a/app/models/form/lettings/questions/address_selection.rb b/app/models/form/lettings/questions/address_selection.rb index 09fb6a475..2b6ac4099 100644 --- a/app/models/form/lettings/questions/address_selection.rb +++ b/app/models/form/lettings/questions/address_selection.rb @@ -9,30 +9,20 @@ class Form::Lettings::Questions::AddressSelection < ::Form::Question end def answer_options(log = nil, _user = nil) - answer_opts = { "10" => { "value" => "The address is not listed" } } + answer_opts = { "-1" => { "value" => "The address is not listed, I want to enter the address manually" } } return answer_opts unless ActiveRecord::Base.connected? - return answer_opts unless log - return answer_opts unless log.address_options + return answer_opts unless log&.address_options + return answer_opts if log.errors.of_kind?(:address_selection, :address_error) - values = [] - log.address_options.each do |option| - values.append(option) + answer_opts = {} + + (0...[log.address_options.count, 10].min).each do |i| + answer_opts[i.to_s] = { "value" => log.address_options[i] } end - { - "0" => { "value" => values[0] }, - "1" => { "value" => values[1] }, - "2" => { "value" => values[2] }, - "3" => { "value" => values[3] }, - "4" => { "value" => values[4] }, - "5" => { "value" => values[5] }, - "6" => { "value" => values[6] }, - "7" => { "value" => values[7] }, - "8" => { "value" => values[8] }, - "9" => { "value" => values[9] }, - "divider" => { "value" => true }, - "-1" => { "value" => "The address is not listed, I want to enter the address manually" }, - }.freeze + answer_opts["divider"] = { "value" => true } + answer_opts["-1"] = { "value" => "The address is not listed, I want to enter the address manually" } + answer_opts end def displayed_answer_options(log, user = nil) @@ -40,6 +30,6 @@ class Form::Lettings::Questions::AddressSelection < ::Form::Question end def hidden_in_check_answers?(log, _current_user = nil) - (log.uprn_known == 1 || log.uprn_confirmed == 1) + (log.uprn_known == 1 || log.uprn_confirmed == 1) || !(1..10).cover?(log.address_options.count) end end diff --git a/app/models/log.rb b/app/models/log.rb index d06227d5d..5f17167c6 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -55,6 +55,8 @@ class Log < ApplicationRecord attr_accessor :skip_update_status, :skip_update_uprn_confirmed, :skip_update_address_selection, :skip_dpo_validation + delegate :present?, to: :address_options, prefix: true + def process_uprn_change! if uprn.present? service = UprnClient.new(uprn) @@ -109,6 +111,8 @@ class Log < ApplicationRecord end def address_options + return @address_options if @address_options + if [address_line1_input, postcode_full_input].all?(&:present?) address_string = "#{address_line1_input}, , , #{postcode_full_input}" service = AddressClient.new(address_string) @@ -116,13 +120,13 @@ class Log < ApplicationRecord return errors.add(:address_selection, :address_error, message: service.error) if service.error.present? - address_options = [] + address_opts = [] service.result.first(10).each do |result| presenter = AddressDataPresenter.new(result) - address_options.append(presenter.address) + address_opts.append(presenter.address) end - address_options + @address_options = address_opts end end From 8c8f01d346f8fa00ead6248ab5b39176d13caef0 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 5 Mar 2024 11:26:07 +0000 Subject: [PATCH 18/23] feat: allow re-entering different uprns --- .../lettings/questions/uprn_confirmation.rb | 18 +++++++++++++----- app/models/log.rb | 4 ++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/app/models/form/lettings/questions/uprn_confirmation.rb b/app/models/form/lettings/questions/uprn_confirmation.rb index 5b7bbd535..1c31485b6 100644 --- a/app/models/form/lettings/questions/uprn_confirmation.rb +++ b/app/models/form/lettings/questions/uprn_confirmation.rb @@ -4,14 +4,22 @@ class Form::Lettings::Questions::UprnConfirmation < ::Form::Question @id = "uprn_confirmed" @header = "Is this the property address?" @type = "radio" - @answer_options = ANSWER_OPTIONS @check_answer_label = "Is this the right address?" end - ANSWER_OPTIONS = { - "1" => { "value" => "Yes" }, - "0" => { "value" => "No, I want to enter the address manually" }, - }.freeze + def answer_options + if form.start_year_after_2024? + { + "1" => { "value" => "Yes" }, + "0" => { "value" => "No, I want to search for the address instead" }, + }.freeze + else + { + "1" => { "value" => "Yes" }, + "0" => { "value" => "No, I want to enter the address manually" }, + }.freeze + end + end def notification_banner(log = nil) return unless log&.uprn diff --git a/app/models/log.rb b/app/models/log.rb index 5f17167c6..4c966d175 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -68,7 +68,7 @@ class Log < ApplicationRecord self.uprn_known = 1 self.uprn_confirmed = nil unless skip_update_uprn_confirmed - self.address_selection = nil + self.address_selection = nil # unless skip_update_address_confirmed self.address_line1 = presenter.address_line1 self.address_line2 = presenter.address_line2 self.town_or_city = presenter.town_or_city @@ -90,7 +90,7 @@ class Log < ApplicationRecord presenter = AddressDataPresenter.new(service.result[address_selection]) self.uprn_known = 1 - self.uprn_confirmed = 1 + self.uprn_confirmed = nil # unless skip_update_uprn_confirmed self.address_selection = nil # unless skip_update_address_confirmed self.uprn = presenter.uprn # skip process uprn change? self.address_line1 = presenter.address_line1 From 99f5ae007feafd0faff29eb98413eab3ab96833c Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 5 Mar 2024 12:13:55 +0000 Subject: [PATCH 19/23] feat: improve error handling and don't accept "no match" precisions --- app/models/form/lettings/pages/address_selection.rb | 2 +- app/models/form/lettings/questions/address_selection.rb | 1 - app/models/log.rb | 2 +- app/services/address_client.rb | 1 + 4 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/form/lettings/pages/address_selection.rb b/app/models/form/lettings/pages/address_selection.rb index 3ba14e7b8..291486593 100644 --- a/app/models/form/lettings/pages/address_selection.rb +++ b/app/models/form/lettings/pages/address_selection.rb @@ -13,7 +13,7 @@ class Form::Lettings::Pages::AddressSelection < ::Form::Page end def routed_to?(log, _current_user = nil) - log.uprn_known.present? && log.uprn_known.zero? && log.address_line1_input.present? && log.postcode_full_input.present? && (1..10).cover?(log.address_options.count) + log.uprn_known.present? && log.uprn_known.zero? && log.address_line1_input.present? && log.postcode_full_input.present? && (1..10).cover?(log.address_options&.count) end def skip_text diff --git a/app/models/form/lettings/questions/address_selection.rb b/app/models/form/lettings/questions/address_selection.rb index 2b6ac4099..acf8ef968 100644 --- a/app/models/form/lettings/questions/address_selection.rb +++ b/app/models/form/lettings/questions/address_selection.rb @@ -12,7 +12,6 @@ class Form::Lettings::Questions::AddressSelection < ::Form::Question answer_opts = { "-1" => { "value" => "The address is not listed, I want to enter the address manually" } } return answer_opts unless ActiveRecord::Base.connected? return answer_opts unless log&.address_options - return answer_opts if log.errors.of_kind?(:address_selection, :address_error) answer_opts = {} diff --git a/app/models/log.rb b/app/models/log.rb index 4c966d175..10b766e39 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -118,7 +118,7 @@ class Log < ApplicationRecord service = AddressClient.new(address_string) service.call - return errors.add(:address_selection, :address_error, message: service.error) if service.error.present? + return nil if service.error.present? address_opts = [] service.result.first(10).each do |result| diff --git a/app/services/address_client.rb b/app/services/address_client.rb index 02fad05c3..2f0ded738 100644 --- a/app/services/address_client.rb +++ b/app/services/address_client.rb @@ -40,6 +40,7 @@ private query: address, key: ENV["OS_DATA_KEY"], maxresults: 10, + minmatch: 0.6, } uri.query = URI.encode_www_form(params) uri.to_s From 5961938de1d6e7e8d0fb72efd37c0cb35db79187 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 5 Mar 2024 12:28:00 +0000 Subject: [PATCH 20/23] feat: add page tests --- .../form/lettings/pages/address_matcher.rb | 2 +- .../lettings/pages/address_fallback_spec.rb | 40 +++++++++++++++++ .../lettings/pages/address_matcher_spec.rb | 33 ++++++++++++++ .../lettings/pages/address_selection_spec.rb | 44 +++++++++++++++++++ 4 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 spec/models/form/lettings/pages/address_fallback_spec.rb create mode 100644 spec/models/form/lettings/pages/address_matcher_spec.rb create mode 100644 spec/models/form/lettings/pages/address_selection_spec.rb diff --git a/app/models/form/lettings/pages/address_matcher.rb b/app/models/form/lettings/pages/address_matcher.rb index 4bf7bad41..5c6f04478 100644 --- a/app/models/form/lettings/pages/address_matcher.rb +++ b/app/models/form/lettings/pages/address_matcher.rb @@ -2,7 +2,7 @@ class Form::Lettings::Pages::AddressMatcher < ::Form::Page def initialize(id, hsh, subsection) super @id = "address_matcher" - @header = "Find address" + @header = "Find an address" @depends_on = [ { "is_supported_housing?" => false, "uprn_known" => 0 }, { "is_supported_housing?" => false, "uprn_confirmed" => 0 }, diff --git a/spec/models/form/lettings/pages/address_fallback_spec.rb b/spec/models/form/lettings/pages/address_fallback_spec.rb new file mode 100644 index 000000000..803049cc7 --- /dev/null +++ b/spec/models/form/lettings/pages/address_fallback_spec.rb @@ -0,0 +1,40 @@ +require "rails_helper" + +RSpec.describe Form::Lettings::Pages::AddressFallback, type: :model do + subject(:page) { described_class.new(page_id, page_definition, subsection) } + + let(:page_id) { nil } + let(:page_definition) { nil } + let(:subsection) { instance_double(Form::Subsection) } + + it "has correct subsection" do + expect(page.subsection).to eq(subsection) + end + + it "has correct questions" do + expect(page.questions.map(&:id)).to eq(%w[address_line1 address_line2 town_or_city county postcode_full]) + end + + it "has the correct id" do + expect(page.id).to eq("address") + end + + it "has the correct header" do + expect(page.header).to eq("Q12 - What is the property's address?") + end + + it "has the correct description" do + expect(page.description).to be_nil + end + + it "has correct depends_on" do + expect(page.depends_on).to eq([ + { "is_supported_housing?" => false, "uprn_known" => nil, "address_selection" => -1 }, + { "is_supported_housing?" => false, "uprn_known" => 0, "address_selection" => -1 }, + { "is_supported_housing?" => false, "uprn_confirmed" => 0, "address_selection" => -1 }, + { "is_supported_housing?" => false, "uprn_known" => nil, "address_options_present?" => false }, + { "is_supported_housing?" => false, "uprn_known" => 0, "address_options_present?" => false }, + { "is_supported_housing?" => false, "uprn_confirmed" => 0, "address_options_present?" => false }, + ]) + end +end diff --git a/spec/models/form/lettings/pages/address_matcher_spec.rb b/spec/models/form/lettings/pages/address_matcher_spec.rb new file mode 100644 index 000000000..44a14a98f --- /dev/null +++ b/spec/models/form/lettings/pages/address_matcher_spec.rb @@ -0,0 +1,33 @@ +require "rails_helper" + +RSpec.describe Form::Lettings::Pages::AddressMatcher, type: :model do + subject(:page) { described_class.new(page_id, page_definition, subsection) } + + let(:page_id) { nil } + let(:page_definition) { nil } + let(:subsection) { instance_double(Form::Subsection) } + + it "has correct subsection" do + expect(page.subsection).to eq(subsection) + end + + it "has correct questions" do + expect(page.questions.map(&:id)).to eq(%w[address_line1_input postcode_full_input]) + end + + it "has the correct id" do + expect(page.id).to eq("address_matcher") + end + + it "has the correct header" do + expect(page.header).to eq("Find an address") + end + + it "has the correct description" do + expect(page.description).to be_nil + end + + it "has correct depends_on" do + expect(page.depends_on).to eq([{ "is_supported_housing?" => false, "uprn_known" => 0 }, { "is_supported_housing?" => false, "uprn_confirmed" => 0 }]) + end +end diff --git a/spec/models/form/lettings/pages/address_selection_spec.rb b/spec/models/form/lettings/pages/address_selection_spec.rb new file mode 100644 index 000000000..97c9a295f --- /dev/null +++ b/spec/models/form/lettings/pages/address_selection_spec.rb @@ -0,0 +1,44 @@ +require "rails_helper" + +RSpec.describe Form::Lettings::Pages::AddressSelection, type: :model do + subject(:page) { described_class.new(page_id, page_definition, subsection) } + + let(:page_id) { nil } + let(:page_definition) { nil } + let(:subsection) { instance_double(Form::Subsection) } + let(:log) { create(:lettings_log) } + + it "has correct subsection" do + expect(page.subsection).to eq(subsection) + end + + it "has correct questions" do + expect(page.questions.map(&:id)).to eq(%w[address_selection]) + end + + it "has the correct id" do + expect(page.id).to eq("address_selection") + end + + it "has the correct header" do + expect(page.header).to eq("We found some addresses that might be this property") + end + + it "has the correct description" do + expect(page.description).to be_nil + end + + it "has the correct skip text" do + "Search for address again" + end + + it "has the correct skip_href" do + expect(page.skip_href(log)).to eq( + "/lettings-logs/#{log.id}/address-matcher", + ) + end + + it "has correct depends_on" do + expect(page.depends_on).to eq([{ "address_options_present?" => true }]) + end +end From 6bbb9af40851ac84ff2c502040da47d9dec01925 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 5 Mar 2024 16:07:33 +0000 Subject: [PATCH 21/23] feat: confirm uprn on address update --- app/models/log.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/log.rb b/app/models/log.rb index 10b766e39..d06591b31 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -90,7 +90,7 @@ class Log < ApplicationRecord presenter = AddressDataPresenter.new(service.result[address_selection]) self.uprn_known = 1 - self.uprn_confirmed = nil # unless skip_update_uprn_confirmed + self.uprn_confirmed = 1 # unless skip_update_uprn_confirmed self.address_selection = nil # unless skip_update_address_confirmed self.uprn = presenter.uprn # skip process uprn change? self.address_line1 = presenter.address_line1 @@ -118,7 +118,7 @@ class Log < ApplicationRecord service = AddressClient.new(address_string) service.call - return nil if service.error.present? + return nil if service.result.blank? || service.error.present? address_opts = [] service.result.first(10).each do |result| From 2fffaf8e78fdad76e5a529e3221484a4e18d1c67 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 5 Mar 2024 16:09:39 +0000 Subject: [PATCH 22/23] feat: add question tests --- .../lettings/questions/address_selection.rb | 2 +- .../questions/postcode_for_address_matcher.rb | 12 --- .../address_line1_for_address_matcher_spec.rb | 62 +++++++++++ .../questions/address_selection_spec.rb | 100 ++++++++++++++++++ .../postcode_for_address_matcher_spec.rb | 62 +++++++++++ 5 files changed, 225 insertions(+), 13 deletions(-) create mode 100644 spec/models/form/lettings/questions/address_line1_for_address_matcher_spec.rb create mode 100644 spec/models/form/lettings/questions/address_selection_spec.rb create mode 100644 spec/models/form/lettings/questions/postcode_for_address_matcher_spec.rb diff --git a/app/models/form/lettings/questions/address_selection.rb b/app/models/form/lettings/questions/address_selection.rb index acf8ef968..943dd24ac 100644 --- a/app/models/form/lettings/questions/address_selection.rb +++ b/app/models/form/lettings/questions/address_selection.rb @@ -29,6 +29,6 @@ class Form::Lettings::Questions::AddressSelection < ::Form::Question end def hidden_in_check_answers?(log, _current_user = nil) - (log.uprn_known == 1 || log.uprn_confirmed == 1) || !(1..10).cover?(log.address_options.count) + (log.uprn_known == 1 || log.uprn_confirmed == 1) || !(1..10).cover?(log.address_options&.count) end end diff --git a/app/models/form/lettings/questions/postcode_for_address_matcher.rb b/app/models/form/lettings/questions/postcode_for_address_matcher.rb index 81387c233..2cac3ce92 100644 --- a/app/models/form/lettings/questions/postcode_for_address_matcher.rb +++ b/app/models/form/lettings/questions/postcode_for_address_matcher.rb @@ -5,19 +5,7 @@ class Form::Lettings::Questions::PostcodeForAddressMatcher < ::Form::Question @header = "Postcode" @type = "text" @width = 5 - @inferred_check_answers_value = [{ - "condition" => { - "pcodenk" => 1, - }, - "value" => "Not known", - }] - @inferred_answers = { - "la" => { - "is_la_inferred" => true, - }, - } @plain_label = true - @check_answer_label = "Postcode" @disable_clearing_if_not_routed_or_dynamic_answer_options = true @hide_question_number_on_page = true @hidden_in_check_answers = true diff --git a/spec/models/form/lettings/questions/address_line1_for_address_matcher_spec.rb b/spec/models/form/lettings/questions/address_line1_for_address_matcher_spec.rb new file mode 100644 index 000000000..7c0baa66e --- /dev/null +++ b/spec/models/form/lettings/questions/address_line1_for_address_matcher_spec.rb @@ -0,0 +1,62 @@ +require "rails_helper" + +RSpec.describe Form::Lettings::Questions::AddressLine1ForAddressMatcher, type: :model do + subject(:question) { described_class.new(question_id, question_definition, page) } + + let(:question_id) { nil } + let(:question_definition) { nil } + let(:page) { instance_double(Form::Page) } + let(:log) { create(:lettings_log, :in_progress, address_line1_input: "Address line 1", postcode_full_input: "AA1 1AA") } + + it "has correct page" do + expect(question.page).to eq(page) + end + + it "has the correct id" do + expect(question.id).to eq("address_line1_input") + end + + it "has the correct header" do + expect(question.header).to eq("Address line 1") + end + + it "has the correct error label" do + expect(question.error_label).to eq("Address line 1") + end + + it "has the correct check_answer_label" do + expect(question.check_answer_label).to eq("Find address") + end + + it "has the correct question_number" do + expect(question.question_number).to eq(nil) + end + + it "has the correct type" do + expect(question.type).to eq("text") + end + + it "is not marked as derived" do + expect(question.derived?).to be false + end + + it "has the correct hint" do + expect(question.hint_text).to be_nil + end + + it "has the correct answer label" do + expect(question.answer_label(log)).to eq("Address line 1\nAA1 1AA") + end + + it "has the correct inferred check answers value" do + expect(question.inferred_check_answers_value).to be_nil + end + + it "has the correct check_answers_card_number" do + expect(question.check_answers_card_number).to be_nil + end + + it "has the correct disable_clearing_if_not_routed_or_dynamic_answer_options value" do + expect(question.disable_clearing_if_not_routed_or_dynamic_answer_options).to eq(true) + end +end diff --git a/spec/models/form/lettings/questions/address_selection_spec.rb b/spec/models/form/lettings/questions/address_selection_spec.rb new file mode 100644 index 000000000..6669f5c46 --- /dev/null +++ b/spec/models/form/lettings/questions/address_selection_spec.rb @@ -0,0 +1,100 @@ +require "rails_helper" + +RSpec.describe Form::Lettings::Questions::AddressSelection, type: :model do + subject(:question) { described_class.new(question_id, question_definition, page) } + + let(:question_id) { nil } + let(:question_definition) { nil } + let(:page) { instance_double(Form::Page) } + let(:log) { create(:lettings_log, :in_progress, address_line1_input: "Address line 1", postcode_full_input: "AA1 1AA") } + + before do + allow_any_instance_of(AddressClient).to receive(:call) + allow_any_instance_of(AddressClient).to receive(:result).and_return([{ + "UPRN" => "UPRN", + "UDPRN" => "UDPRN", + "ADDRESS" => "full address", + "SUB_BUILDING_NAME" => "0", + "BUILDING_NAME" => "building name", + "THOROUGHFARE_NAME" => "thoroughfare", + "POST_TOWN" => "posttown", + "POSTCODE" => "postcode", + }]) + end + + it "has correct page" do + expect(question.page).to eq(page) + end + + it "has the correct id" do + expect(question.id).to eq("address_selection") + end + + it "has the correct header" do + expect(question.header).to eq("Select the correct address") + end + + it "has the correct check_answer_label" do + expect(question.check_answer_label).to eq("Select the correct address") + end + + it "has the correct question_number" do + expect(question.question_number).to eq(nil) + end + + it "has the correct type" do + expect(question.type).to eq("radio") + end + + it "is not marked as derived" do + expect(question.derived?).to be false + end + + it "has the correct hint" do + expect(question.hint_text).to be_nil + end + + it "has the correct answer options" do + stub_request(:get, /api.os.uk/) + .to_return(status: 200, body: "", headers: {}) + + expect(question.answer_options(log)).to eq({ "-1" => { "value" => "The address is not listed, I want to enter the address manually" }, "0" => { "value" => "full address" }, "divider" => { "value" => true } }) + end + + it "has the correct displayed answer options" do + stub_request(:get, /api.os.uk/) + .to_return(status: 200, body: "", headers: {}) + + expect(question.displayed_answer_options(log)).to eq({ "-1" => { "value" => "The address is not listed, I want to enter the address manually" }, "0" => { "value" => "full address" }, "divider" => { "value" => true } }) + end + + it "has the correct inferred check answers value" do + expect(question.inferred_check_answers_value).to be_nil + end + + it "has the correct check_answers_card_number" do + expect(question.check_answers_card_number).to be_nil + end + + context "when the log has address options" do + it "has the correct hidden_in_check_answers?" do + stub_request(:get, /api.os.uk/) + .to_return(status: 200, body: '{"results": {"0": "address_0", "1": "address_1", "2": "address_2"}}', headers: {}) + + expect(question.hidden_in_check_answers?(log)).to eq(false) + end + end + + context "when the log does not have address options" do + before do + allow_any_instance_of(AddressClient).to receive(:result).and_return(nil) + end + + it "has the correct hidden_in_check_answers?" do + stub_request(:get, /api.os.uk/) + .to_return(status: 200, body: "", headers: {}) + + expect(question.hidden_in_check_answers?(log)).to eq(true) + end + end +end diff --git a/spec/models/form/lettings/questions/postcode_for_address_matcher_spec.rb b/spec/models/form/lettings/questions/postcode_for_address_matcher_spec.rb new file mode 100644 index 000000000..9d12cd59c --- /dev/null +++ b/spec/models/form/lettings/questions/postcode_for_address_matcher_spec.rb @@ -0,0 +1,62 @@ +require "rails_helper" + +RSpec.describe Form::Lettings::Questions::PostcodeForAddressMatcher, type: :model do + subject(:question) { described_class.new(question_id, question_definition, page) } + + let(:question_id) { nil } + let(:question_definition) { nil } + let(:page) { instance_double(Form::Page) } + let(:log) { create(:lettings_log, :in_progress, address_line1_input: "Address line 1", postcode_full_input: "AA1 1AA") } + + it "has correct page" do + expect(question.page).to eq(page) + end + + it "has the correct id" do + expect(question.id).to eq("postcode_full_input") + end + + it "has the correct header" do + expect(question.header).to eq("Postcode") + end + + it "has the correct check_answer_label" do + expect(question.check_answer_label).to eq("Postcode") + end + + it "has the correct question_number" do + expect(question.question_number).to eq(nil) + end + + it "has the correct type" do + expect(question.type).to eq("text") + end + + it "is not marked as derived" do + expect(question.derived?).to be false + end + + it "has the correct hint" do + expect(question.hint_text).to be_nil + end + + it "has the correct answer label" do + expect(question.answer_label(log)).to eq("AA1 1AA") + end + + it "has the correct inferred check answers value" do + expect(question.inferred_check_answers_value).to be_nil + end + + it "has the correct inferred_answers value" do + expect(question.inferred_answers).to be_nil + end + + it "has the correct check_answers_card_number" do + expect(question.check_answers_card_number).to be_nil + end + + it "has the correct disable_clearing_if_not_routed_or_dynamic_answer_options value" do + expect(question.disable_clearing_if_not_routed_or_dynamic_answer_options).to eq(true) + end +end From 57395be99bbc95e25c676937eee8ac2c1c001a9c Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 5 Mar 2024 16:17:16 +0000 Subject: [PATCH 23/23] feat: don't set minimum match for initial search --- app/services/address_client.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/services/address_client.rb b/app/services/address_client.rb index 2f0ded738..02fad05c3 100644 --- a/app/services/address_client.rb +++ b/app/services/address_client.rb @@ -40,7 +40,6 @@ private query: address, key: ENV["OS_DATA_KEY"], maxresults: 10, - minmatch: 0.6, } uri.query = URI.encode_www_form(params) uri.to_s