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

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

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

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

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

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

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),
     ]

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

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

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

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")

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)
 

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

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"

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

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

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

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

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

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
       ],

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

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