From 352dae8efecdbdbd3e46e3e0d5bb76543fd9fbee Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 16 Jan 2023 15:34:00 +0000 Subject: [PATCH] CLDC-1468 Add property postcode questions to sales (#1187) * Add postcode fields * Add postcode page and questions * Add postcode page to the property information subsection * Only display local authority question if the la is not inferred * Fix migration * Move postcode validation * Infer LA fields from postcode * infer partial postcodes --- .../derived_variables/sales_log_variables.rb | 1 + app/models/form/sales/pages/postcode.rb | 13 +++ .../sales/pages/property_local_authority.rb | 3 + app/models/form/sales/questions/postcode.rb | 21 ++++ .../form/sales/questions/postcode_known.rb | 28 +++++ .../sales/subsections/property_information.rb | 1 + app/models/lettings_log.rb | 4 - app/models/log.rb | 4 + app/models/sales_log.rb | 19 ++++ .../validations/property_validations.rb | 8 -- app/models/validations/shared_validations.rb | 8 ++ ...0113125117_add_postcode_fields_to_sales.rb | 11 ++ db/schema.rb | 19 ++-- spec/factories/sales_log.rb | 4 +- spec/models/form/sales/pages/postcode_spec.rb | 33 ++++++ .../pages/property_local_authority_spec.rb | 6 + .../sales/questions/postcode_known_spec.rb | 59 ++++++++++ .../form/sales/questions/postcode_spec.rb | 58 ++++++++++ .../subsections/property_information_spec.rb | 1 + spec/models/form_handler_spec.rb | 4 +- spec/models/sales_log_spec.rb | 103 ++++++++++++++++++ 21 files changed, 386 insertions(+), 22 deletions(-) create mode 100644 app/models/form/sales/pages/postcode.rb create mode 100644 app/models/form/sales/questions/postcode.rb create mode 100644 app/models/form/sales/questions/postcode_known.rb create mode 100644 db/migrate/20230113125117_add_postcode_fields_to_sales.rb create mode 100644 spec/models/form/sales/pages/postcode_spec.rb create mode 100644 spec/models/form/sales/questions/postcode_known_spec.rb create mode 100644 spec/models/form/sales/questions/postcode_spec.rb diff --git a/app/models/derived_variables/sales_log_variables.rb b/app/models/derived_variables/sales_log_variables.rb index 034816090..a527ed3f0 100644 --- a/app/models/derived_variables/sales_log_variables.rb +++ b/app/models/derived_variables/sales_log_variables.rb @@ -15,5 +15,6 @@ module DerivedVariables::SalesLogVariables if mscharge_known.present? && mscharge_known.zero? self.mscharge = 0 end + self.pcode1, self.pcode2 = postcode_full.split(" ") if postcode_full.present? end end diff --git a/app/models/form/sales/pages/postcode.rb b/app/models/form/sales/pages/postcode.rb new file mode 100644 index 000000000..c40a18845 --- /dev/null +++ b/app/models/form/sales/pages/postcode.rb @@ -0,0 +1,13 @@ +class Form::Sales::Pages::Postcode < ::Form::Page + def initialize(id, hsh, subsection) + super + @id = "property_postcode" + end + + def questions + @questions ||= [ + Form::Sales::Questions::PostcodeKnown.new(nil, nil, self), + Form::Sales::Questions::Postcode.new(nil, nil, self), + ] + end +end diff --git a/app/models/form/sales/pages/property_local_authority.rb b/app/models/form/sales/pages/property_local_authority.rb index 0fc05b6d7..7e63b2f6c 100644 --- a/app/models/form/sales/pages/property_local_authority.rb +++ b/app/models/form/sales/pages/property_local_authority.rb @@ -2,6 +2,9 @@ class Form::Sales::Pages::PropertyLocalAuthority < ::Form::Page def initialize(id, hsh, subsection) super @id = "property_local_authority" + @depends_on = [{ + "is_la_inferred" => false, + }] end def questions diff --git a/app/models/form/sales/questions/postcode.rb b/app/models/form/sales/questions/postcode.rb new file mode 100644 index 000000000..bae59637c --- /dev/null +++ b/app/models/form/sales/questions/postcode.rb @@ -0,0 +1,21 @@ +class Form::Sales::Questions::Postcode < ::Form::Question + def initialize(id, hsh, page) + super + @id = "postcode_full" + @check_answer_label = "Property’s postcode" + @header = "Postcode" + @type = "text" + @width = 5 + @inferred_check_answers_value = { + "condition" => { + "pcodenk" => 1, + }, + "value" => "Not known", + } + @inferred_answers = { + "la" => { + "is_la_inferred" => true, + }, + } + end +end diff --git a/app/models/form/sales/questions/postcode_known.rb b/app/models/form/sales/questions/postcode_known.rb new file mode 100644 index 000000000..208f8df22 --- /dev/null +++ b/app/models/form/sales/questions/postcode_known.rb @@ -0,0 +1,28 @@ +class Form::Sales::Questions::PostcodeKnown < ::Form::Question + def initialize(id, hsh, page) + super + @id = "pcodenk" + @check_answer_label = "Property’s postcode" + @header = "Do you know the property’s postcode?" + @type = "radio" + @answer_options = ANSWER_OPTIONS + @conditional_for = { + "postcode_full" => [0], + } + @hidden_in_check_answers = { + "depends_on" => [ + { + "pcodenk" => 0, + }, + { + "pcodenk" => 1, + }, + ], + } + end + + ANSWER_OPTIONS = { + "0" => { "value" => "Yes" }, + "1" => { "value" => "No" }, + }.freeze +end diff --git a/app/models/form/sales/subsections/property_information.rb b/app/models/form/sales/subsections/property_information.rb index 301f72725..c1c150b9f 100644 --- a/app/models/form/sales/subsections/property_information.rb +++ b/app/models/form/sales/subsections/property_information.rb @@ -11,6 +11,7 @@ class Form::Sales::Subsections::PropertyInformation < ::Form::Subsection Form::Sales::Pages::PropertyNumberOfBedrooms.new(nil, nil, self), Form::Sales::Pages::PropertyUnitType.new(nil, nil, self), Form::Sales::Pages::PropertyBuildingType.new(nil, nil, self), + Form::Sales::Pages::Postcode.new(nil, nil, self), Form::Sales::Pages::PropertyLocalAuthority.new(nil, nil, self), Form::Sales::Pages::PropertyWheelchairAccessible.new(nil, nil, self), ] diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 69769b782..2584f95ae 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -608,10 +608,6 @@ private self[la_key] = inferred_la if inferred_la.present? end - def reset_location_fields! - reset_location(is_la_inferred, "la", "is_la_inferred", "postcode_full", 1) - end - def get_has_benefits HAS_BENEFITS_OPTIONS.include?(hb) ? 1 : 0 end diff --git a/app/models/log.rb b/app/models/log.rb index 798b78af7..0d0df6451 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -105,6 +105,10 @@ private string.present? ? string.upcase.gsub(/\s+/, "") : string end + def reset_location_fields! + reset_location(is_la_inferred, "la", "is_la_inferred", "postcode_full", 1) + end + def reset_previous_location_fields! reset_location(is_previous_la_inferred, "prevloc", "is_previous_la_inferred", "ppostcode_full", previous_la_known) end diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index 22f750148..c5c51578b 100644 --- a/app/models/sales_log.rb +++ b/app/models/sales_log.rb @@ -21,7 +21,9 @@ class SalesLog < Log validates_with SalesLogValidator before_validation :set_derived_fields! before_validation :reset_invalidated_dependent_fields! + before_validation :process_postcode_changes!, if: :postcode_full_changed? before_validation :process_previous_postcode_changes!, if: :ppostcode_full_changed? + before_validation :reset_location_fields!, unless: :postcode_known? before_validation :reset_previous_location_fields!, unless: :previous_postcode_known? scope :filter_by_year, ->(year) { where(saledate: Time.zone.local(year.to_i, 4, 1)...Time.zone.local(year.to_i + 1, 4, 1)) } @@ -135,6 +137,18 @@ class SalesLog < Log ppcodenk&.zero? end + def postcode_known? + pcodenk&.zero? + end + + def postcode_full=(postcode) + if postcode + super UKPostcode.parse(postcode).to_s + else + super nil + end + end + def process_postcode(postcode, postcode_known_key, la_inferred_key, la_key) return if postcode.blank? @@ -151,4 +165,9 @@ class SalesLog < Log def mortgage_not_used? mortgageused == 2 end + + def process_postcode_changes! + self.postcode_full = upcase_and_remove_whitespace(postcode_full) + process_postcode(postcode_full, "pcodenk", "is_la_inferred", "la") + end end diff --git a/app/models/validations/property_validations.rb b/app/models/validations/property_validations.rb index c655d8c85..8b0988dd9 100644 --- a/app/models/validations/property_validations.rb +++ b/app/models/validations/property_validations.rb @@ -50,14 +50,6 @@ module Validations::PropertyValidations end end - def validate_property_postcode(record) - postcode = record.postcode_full - if record.postcode_known? && (postcode.blank? || !postcode.match(POSTCODE_REGEXP)) - error_message = I18n.t("validations.postcode") - record.errors.add :postcode_full, error_message - end - end - def validate_shared_housing_rooms(record) if record.beds.present? && record.beds <= 0 record.errors.add :beds, I18n.t("validations.property.beds.non_positive") diff --git a/app/models/validations/shared_validations.rb b/app/models/validations/shared_validations.rb index 75ddaf745..d8d4e2059 100644 --- a/app/models/validations/shared_validations.rb +++ b/app/models/validations/shared_validations.rb @@ -34,6 +34,14 @@ module Validations::SharedValidations end end + def validate_property_postcode(record) + postcode = record.postcode_full + if record.postcode_known? && (postcode.blank? || !postcode.match(POSTCODE_REGEXP)) + error_message = I18n.t("validations.postcode") + record.errors.add :postcode_full, error_message + end + end + def location_during_startdate_validation(record, field) location_inactive_status = inactive_status(record.startdate, record.location) diff --git a/db/migrate/20230113125117_add_postcode_fields_to_sales.rb b/db/migrate/20230113125117_add_postcode_fields_to_sales.rb new file mode 100644 index 000000000..0de137326 --- /dev/null +++ b/db/migrate/20230113125117_add_postcode_fields_to_sales.rb @@ -0,0 +1,11 @@ +class AddPostcodeFieldsToSales < ActiveRecord::Migration[7.0] + def change + change_table :sales_logs, bulk: true do |t| + t.column :pcode1, :string + t.column :pcode2, :string + t.column :pcodenk, :integer + t.column :postcode_full, :string + t.column :is_la_inferred, :boolean + end + end +end diff --git a/db/schema.rb b/db/schema.rb index d248d46fc..1a280f737 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: 2023_01_12_093524) do +ActiveRecord::Schema[7.0].define(version: 2023_01_13_125117) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -434,6 +434,9 @@ ActiveRecord::Schema[7.0].define(version: 2023_01_12_093524) do t.string "relat5" t.string "relat6" t.integer "hb" + t.string "sex4" + t.string "sex5" + t.string "sex6" t.integer "savings_value_check" t.integer "deposit_value_check" t.integer "frombeds" @@ -470,22 +473,24 @@ ActiveRecord::Schema[7.0].define(version: 2023_01_12_093524) do t.integer "hhregres" t.integer "hhregresstill" t.integer "proplen" + t.integer "mscharge_known" + t.decimal "mscharge", precision: 10, scale: 2 t.integer "prevten" t.integer "mortgageused" t.integer "wchair" t.integer "armedforcesspouse" - t.integer "mscharge_known" - t.decimal "mscharge", precision: 10, scale: 2 - t.string "sex4" - t.string "sex5" - t.string "sex6" - t.integer "mortlen" t.datetime "hodate", precision: nil t.integer "hoday" t.integer "homonth" t.integer "hoyear" t.integer "fromprop" t.integer "socprevten" + t.integer "mortlen" + t.string "pcode1" + t.string "pcode2" + t.integer "pcodenk" + t.string "postcode_full" + t.boolean "is_la_inferred" 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" t.index ["owning_organisation_id"], name: "index_sales_logs_on_owning_organisation_id" diff --git a/spec/factories/sales_log.rb b/spec/factories/sales_log.rb index 23dc0c5fe..105008350 100644 --- a/spec/factories/sales_log.rb +++ b/spec/factories/sales_log.rb @@ -57,7 +57,7 @@ FactoryBot.define do income2nk { 0 } income2 { 10_000 } inc2mort { 1 } - la_known { "1" } + la_known { 1 } la { "E09000003" } savingsnk { 1 } prevown { 1 } @@ -96,6 +96,8 @@ FactoryBot.define do mscharge_known { 1 } mscharge { 100 } mortlen { 10 } + pcodenk { 1 } + is_la_inferred { false } end end end diff --git a/spec/models/form/sales/pages/postcode_spec.rb b/spec/models/form/sales/pages/postcode_spec.rb new file mode 100644 index 000000000..485411414 --- /dev/null +++ b/spec/models/form/sales/pages/postcode_spec.rb @@ -0,0 +1,33 @@ +require "rails_helper" + +RSpec.describe Form::Sales::Pages::Postcode, 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[pcodenk postcode_full]) + end + + it "has the correct id" do + expect(page.id).to eq("property_postcode") + end + + it "has the correct header" do + expect(page.header).to be_nil + 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 be_nil + end +end diff --git a/spec/models/form/sales/pages/property_local_authority_spec.rb b/spec/models/form/sales/pages/property_local_authority_spec.rb index 0cf653a80..3c88a0cd6 100644 --- a/spec/models/form/sales/pages/property_local_authority_spec.rb +++ b/spec/models/form/sales/pages/property_local_authority_spec.rb @@ -31,4 +31,10 @@ RSpec.describe Form::Sales::Pages::PropertyLocalAuthority, type: :model do it "has the correct description" do expect(page.description).to be_nil end + + it "has the correct depends_on" do + expect(page.depends_on).to eq([{ + "is_la_inferred" => false, + }]) + end end diff --git a/spec/models/form/sales/questions/postcode_known_spec.rb b/spec/models/form/sales/questions/postcode_known_spec.rb new file mode 100644 index 000000000..b6c404c56 --- /dev/null +++ b/spec/models/form/sales/questions/postcode_known_spec.rb @@ -0,0 +1,59 @@ +require "rails_helper" + +RSpec.describe Form::Sales::Questions::PostcodeKnown, 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) } + + it "has correct page" do + expect(question.page).to eq(page) + end + + it "has the correct id" do + expect(question.id).to eq("pcodenk") + end + + it "has the correct header" do + expect(question.header).to eq("Do you know the property’s postcode?") + end + + it "has the correct check_answer_label" do + expect(question.check_answer_label).to eq("Property’s postcode") + 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 answer_options" do + expect(question.answer_options).to eq({ + "0" => { "value" => "Yes" }, + "1" => { "value" => "No" }, + }) + end + + it "has correct conditional for" do + expect(question.conditional_for).to eq({ + "postcode_full" => [0], + }) + end + + it "has the correct hint" do + expect(question.hint_text).to be_nil + end + + it "has the correct hidden_in_check_answers" do + expect(question.hidden_in_check_answers).to eq({ + "depends_on" => [ + { "pcodenk" => 0 }, + { "pcodenk" => 1 }, + ], + }) + end +end diff --git a/spec/models/form/sales/questions/postcode_spec.rb b/spec/models/form/sales/questions/postcode_spec.rb new file mode 100644 index 000000000..b0d7eefcb --- /dev/null +++ b/spec/models/form/sales/questions/postcode_spec.rb @@ -0,0 +1,58 @@ +require "rails_helper" + +RSpec.describe Form::Sales::Questions::Postcode, 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) } + + 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") + 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("Property’s postcode") + 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 width" do + expect(question.width).to eq(5) + end + + it "has the correct inferred_answers" do + expect(question.inferred_answers).to eq({ + "la" => { + "is_la_inferred" => true, + }, + }) + end + + it "has the correct inferred_check_answers_value" do + expect(question.inferred_check_answers_value).to eq({ + "condition" => { + "pcodenk" => 1, + }, + "value" => "Not known", + }) + end +end diff --git a/spec/models/form/sales/subsections/property_information_spec.rb b/spec/models/form/sales/subsections/property_information_spec.rb index 878c678e1..00b901142 100644 --- a/spec/models/form/sales/subsections/property_information_spec.rb +++ b/spec/models/form/sales/subsections/property_information_spec.rb @@ -17,6 +17,7 @@ RSpec.describe Form::Sales::Subsections::PropertyInformation, type: :model do property_number_of_bedrooms property_unit_type property_building_type + property_postcode property_local_authority property_wheelchair_accessible ], diff --git a/spec/models/form_handler_spec.rb b/spec/models/form_handler_spec.rb index db8c90aaa..4365eedcb 100644 --- a/spec/models/form_handler_spec.rb +++ b/spec/models/form_handler_spec.rb @@ -52,14 +52,14 @@ RSpec.describe FormHandler do it "is able to load a current sales form" do form = form_handler.get_form("current_sales") expect(form).to be_a(Form) - expect(form.pages.count).to eq(137) + expect(form.pages.count).to eq(138) expect(form.name).to eq("2022_2023_sales") end it "is able to load a previous sales form" do form = form_handler.get_form("previous_sales") expect(form).to be_a(Form) - expect(form.pages.count).to eq(137) + expect(form.pages.count).to eq(138) expect(form.name).to eq("2021_2022_sales") end end diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index 8f46db42d..bad72db3b 100644 --- a/spec/models/sales_log_spec.rb +++ b/spec/models/sales_log_spec.rb @@ -127,6 +127,109 @@ RSpec.describe SalesLog, type: :model do record_from_db = ActiveRecord::Base.connection.execute("select deposit from sales_logs where id=#{sales_log.id}").to_a[0] expect(record_from_db["deposit"]).to eq(nil) end + + it "correctly derives and saves pcode1 and pcode1 and pcode2" do + sales_log.update!(postcode_full: "W6 0SP") + record_from_db = ActiveRecord::Base.connection.execute("select pcode1, pcode2 from sales_logs where id=#{sales_log.id}").to_a[0] + expect(record_from_db["pcode1"]).to eq("W6") + expect(record_from_db["pcode2"]).to eq("0SP") + end + end + + context "when saving addresses" do + before do + stub_request(:get, /api.postcodes.io/) + .to_return(status: 200, body: "{\"status\":200,\"result\":{\"admin_district\":\"Manchester\",\"codes\":{\"admin_district\": \"E08000003\"}}}", headers: {}) + end + + def check_postcode_fields(postcode_field) + record_from_db = ActiveRecord::Base.connection.execute("select #{postcode_field} from sales_logs where id=#{address_sales_log.id}").to_a[0] + expect(address_sales_log[postcode_field]).to eq("M1 1AE") + expect(record_from_db[postcode_field]).to eq("M1 1AE") + end + + let!(:address_sales_log) do + FactoryBot.create( + :sales_log, + :completed, + managing_organisation: owning_organisation, + owning_organisation:, + created_by: created_by_user, + pcodenk: 0, + postcode_full: "M1 1AE", + ) + end + + def check_property_postcode_fields + check_postcode_fields("postcode_full") + end + + it "correctly formats previous postcode" do + address_sales_log.update!(postcode_full: "M1 1AE") + check_property_postcode_fields + + address_sales_log.update!(postcode_full: "m1 1ae") + check_property_postcode_fields + + address_sales_log.update!(postcode_full: "m11Ae") + check_property_postcode_fields + + address_sales_log.update!(postcode_full: "m11ae") + check_property_postcode_fields + end + + it "correctly infers la" do + record_from_db = ActiveRecord::Base.connection.execute("select la from sales_logs where id=#{address_sales_log.id}").to_a[0] + expect(address_sales_log.la).to eq("E08000003") + expect(record_from_db["la"]).to eq("E08000003") + end + + it "errors if the property postcode is emptied" do + expect { address_sales_log.update!({ postcode_full: "" }) } + .to raise_error(ActiveRecord::RecordInvalid, /#{I18n.t("validations.postcode")}/) + end + + it "errors if the property postcode is not valid" do + expect { address_sales_log.update!({ postcode_full: "invalid_postcode" }) } + .to raise_error(ActiveRecord::RecordInvalid, /#{I18n.t("validations.postcode")}/) + end + + context "when the local authority lookup times out" do + before do + allow(Timeout).to receive(:timeout).and_raise(Timeout::Error) + end + + it "logs a warning" do + expect(Rails.logger).to receive(:warn).with("Postcodes.io lookup timed out") + address_sales_log.update!({ pcodenk: 1, postcode_full: "M1 1AD" }) + end + end + + it "correctly resets all fields if property postcode not known" do + address_sales_log.update!({ pcodenk: 1 }) + + record_from_db = ActiveRecord::Base.connection.execute("select la, postcode_full from sales_logs where id=#{address_sales_log.id}").to_a[0] + expect(record_from_db["postcode_full"]).to eq(nil) + expect(address_sales_log.la).to eq(nil) + expect(record_from_db["la"]).to eq(nil) + end + + it "changes the LA if property postcode changes from not known to known and provided" do + address_sales_log.update!({ pcodenk: 1 }) + address_sales_log.update!({ la: "E09000033" }) + + record_from_db = ActiveRecord::Base.connection.execute("select la, postcode_full from sales_logs where id=#{address_sales_log.id}").to_a[0] + expect(record_from_db["postcode_full"]).to eq(nil) + expect(address_sales_log.la).to eq("E09000033") + expect(record_from_db["la"]).to eq("E09000033") + + address_sales_log.update!({ pcodenk: 0, postcode_full: "M1 1AD" }) + + record_from_db = ActiveRecord::Base.connection.execute("select la, postcode_full from sales_logs where id=#{address_sales_log.id}").to_a[0] + expect(record_from_db["postcode_full"]).to eq("M1 1AD") + expect(address_sales_log.la).to eq("E08000003") + expect(record_from_db["la"]).to eq("E08000003") + end end context "when saving previous address" do