CDLC-504: Introduce regexp validation of property postcodes (#207)

Introduce regexp validation of property postcodes
Only changes the status after validation
Clean up logging and fix failing test on minimal earning

app/models/case_log.rb

@@ -36,6 +36,9 @@ class CaseLog < ApplicationRecord
 
   validates_with CaseLogValidator
   before_save :update_status!
+  before_validation :process_postcode_changes!, if: :property_postcode_changed?
+  before_validation :reset_location_fields!, unless: :postcode_known?
+  before_validation :set_derived_fields!
 
   belongs_to :owning_organisation, class_name: "Organisation"
   belongs_to :managing_organisation, class_name: "Organisation"
@@ -168,6 +171,10 @@ class CaseLog < ApplicationRecord
     status == "in_progress"
   end
 
+  def postcode_known?
+    postcode_known == "Yes"
+  end
+
   def weekly_net_income
     return unless earnings && incfreq
 
@@ -203,10 +210,9 @@ private
                   else
                     "in_progress"
                   end
-    set_derived_fields
   end
 
-  def set_derived_fields
+  def set_derived_fields!
     if previous_postcode.present?
       self.ppostc1 = UKPostcode.parse(previous_postcode).outcode
       self.ppostc2 = UKPostcode.parse(previous_postcode).incode
@@ -234,26 +240,33 @@ private
     self.hhmemb = other_hhmemb + 1 if other_hhmemb.present?
     self.renttype = RENT_TYPE_MAPPING[rent_type]
     self.lettype = "#{renttype} #{needstype} #{owning_organisation['Org type']}" if renttype.present? && needstype.present? && owning_organisation["Org type"].present?
-    self.is_la_inferred = false if is_la_inferred.nil?
-    if property_postcode.blank? || postcode_known == "No"
-      reset_location_fields
-    else
-      inferred_la = get_inferred_la(property_postcode)
-      self.is_la_inferred = inferred_la.present?
-      self.la = inferred_la if inferred_la.present?
-    end
-    if property_postcode.present?
-      self.postcode = UKPostcode.parse(property_postcode).outcode
-      self.postcod2 = UKPostcode.parse(property_postcode).incode
-    else
-      self.postcode = self.postcod2 = nil
-    end
     self.totchild = get_totchild
     self.totelder = get_totelder
     self.totadult = get_totadult
     self.tcharge = brent.to_i + scharge.to_i + pscharge.to_i + supcharg.to_i
   end
 
+  def process_postcode_changes!
+    return if property_postcode.blank?
+
+    self.postcode_known = "Yes"
+    inferred_la = get_inferred_la(property_postcode)
+    self.is_la_inferred = inferred_la.present?
+    self.la = inferred_la if inferred_la.present?
+    self.postcode = UKPostcode.parse(property_postcode).outcode
+    self.postcod2 = UKPostcode.parse(property_postcode).incode
+  end
+
+  def reset_location_fields!
+    if is_la_inferred == true
+      self.la = nil
+    end
+    self.is_la_inferred = false
+    self.property_postcode = nil
+    self.postcode = nil
+    self.postcod2 = nil
+  end
+
   def get_totelder
     ages = [age1, age2, age3, age4, age5, age6, age7, age8]
     ages.count { |x| !x.nil? && x >= 60 }
@@ -281,14 +294,6 @@ private
     end
   end
 
-  def reset_location_fields
-    if is_la_inferred == true
-      self.la = nil
-    end
-    self.is_la_inferred = false
-    self.property_postcode = nil
-  end
-
   def all_fields_completed?
     mandatory_fields.none? { |_key, val| val.nil? }
   end

app/models/validations/property_validations.rb

@@ -3,6 +3,8 @@ module Validations::PropertyValidations
   # or 'validate_' to run on submit as well
   include Constants::CaseLog
 
+  POSTCODE_REGEXP = /^(([A-Z]{1,2}[0-9][A-Z0-9]?|ASCN|STHL|TDCU|BBND|[BFS]IQQ|PCRN|TKCA) ?[0-9][A-Z]{2}|BFPO ?[0-9]{1,4}|(KY[0-9]|MSR|VG|AI)[ -]?[0-9]{4}|[A-Z]{2} ?[0-9]{2}|GE ?CX|GIR ?0A{2}|SAN ?TA1)$/
+
   def validate_property_number_of_times_relet(record)
     if record.offered && !/^[1-9]$|^0[1-9]$|^1[0-9]$|^20$/.match?(record.offered.to_s)
       record.errors.add :offered, "Property number of times relet must be between 0 and 20"
@@ -35,8 +37,10 @@ module Validations::PropertyValidations
   end
 
   def validate_property_postcode(record)
-    if record.postcode_known == "Yes" && record.property_postcode.blank?
-      record.errors.add :property_postcode, "Enter a postcode in the correct format, for example AA1 1AA"
+    postcode = record.property_postcode
+    if record.postcode_known == "Yes" && (postcode.blank? || !postcode.match(POSTCODE_REGEXP))
+      error_message = "Enter a postcode in the correct format, for example AA1 1AA"
+      record.errors.add :property_postcode, error_message
     end
   end
 end

config/forms/2021_2022.json

@@ -2517,7 +2517,7 @@
               "description": "",
               "questions": {
                 "previous_postcode": {
-                  "check_answer_label": "Postcode of previous accomodation if the household has moved from settled accommodation",
+                  "check_answer_label": "Postcode of previous accommodation if the household has moved from settled accommodation",
                   "header": "Postcode for the previous accommodation",
                   "hint_text": "If the household has moved from settled accommodation immediately prior to being re-housed",
                   "type": "text"

spec/factories/case_log.rb

@@ -14,7 +14,7 @@ FactoryBot.define do
     trait :in_progress do
       status { 1 }
       tenant_code { "TH356" }
-      property_postcode { "P0 5ST" }
+      property_postcode { "PO5 3TE" }
       previous_postcode { "SW2 6HI" }
       age1 { "17" }
     end

spec/features/form/accessible_autocomplete_spec.rb

@@ -39,7 +39,7 @@ RSpec.describe "Accessible Automcomplete" do
   end
 
   it "has the correct option selected if one has been saved" do
-    case_log.update!(property_postcode: nil, la: "Oxford")
+    case_log.update!(postcode_known: "No", la: "Oxford")
     visit("/logs/#{case_log.id}/accessible-select")
     expect(page).to have_select("case-log-la-field", selected: %w[Oxford])
   end

spec/features/form/page_routing_spec.rb

@@ -51,7 +51,7 @@ RSpec.describe "Form Page Routing" do
   context "inferred answers routing", js: true do
     it "shows question if the answer could not be inferred" do
       visit("/logs/#{id}/property-postcode")
-      fill_in("case-log-property-postcode-field", with: "P0 5ST")
+      fill_in("case-log-property-postcode-field", with: "PO5 3TE")
       click_button("Save and continue")
       expect(page).to have_current_path("/logs/#{id}/do-you-know-the-local-authority")
     end

spec/features/form/saving_data_spec.rb

@@ -75,7 +75,7 @@ RSpec.describe "Form Saving Data" do
 
   it "displays number answers in inputs if they are already saved" do
     visit("/logs/#{id}/property-postcode")
-    expect(page).to have_field("case-log-property-postcode-field", with: "P0 5ST")
+    expect(page).to have_field("case-log-property-postcode-field", with: "PO5 3TE")
   end
 
   it "displays text answers in inputs if they are already saved" do

spec/fixtures/complete_case_log.json

@@ -75,7 +75,7 @@
     "offered": 2,
     "wchair": "Yes",
     "net_income_known": "Yes – the household has a weekly income",
-    "earnings": 0,
+    "earnings": 150,
     "benefits": "Some",
     "hb": "Universal Credit with housing element, but not Housing Benefit",
     "period": "Fortnightly",

spec/models/case_log_spec.rb

@@ -1121,6 +1121,11 @@ RSpec.describe Form, type: :model do
           .to raise_error(ActiveRecord::RecordInvalid, /Enter a postcode in the correct format/)
       end
 
+      it "errors if the property postcode is not valid" do
+        expect { address_case_log.update!({ property_postcode: "invalid_postcode" }) }
+          .to raise_error(ActiveRecord::RecordInvalid, /Enter a postcode in the correct format/)
+      end
+
       it "correctly resets all fields if property postcode not known" do
         address_case_log.update!({ postcode_known: "No" })
 

spec/requests/case_log_controller_spec.rb

@@ -99,6 +99,7 @@ RSpec.describe CaseLogsController, type: :request do
 
       it "marks the record as completed" do
         json_response = JSON.parse(response.body)
+
         expect(json_response).not_to have_key("errors")
         expect(json_response["status"]).to eq(completed)
       end
@@ -261,7 +262,7 @@ RSpec.describe CaseLogsController, type: :request do
                                      owning_organisation: organisation,
                                      managing_organisation: organisation,
                                      postcode_known: "Yes",
-                                     property_postcode: "P0 0ST")
+                                     property_postcode: "PO5 3TE")
         id = case_log.id
         get "/logs/#{id}/property-information/check-answers"
         expected_inferred_answer = "<span class=\"govuk-!-font-weight-regular app-!-colour-muted\">Manchester</span>"
@@ -282,7 +283,7 @@ RSpec.describe CaseLogsController, type: :request do
 
   describe "PATCH" do
     let(:case_log) do
-      FactoryBot.create(:case_log, :in_progress, tenant_code: "Old Value", property_postcode: "Old Value")
+      FactoryBot.create(:case_log, :in_progress, tenant_code: "Old Value", property_postcode: "M1 1AE")
     end
     let(:params) do
       { tenant_code: "New Value" }
@@ -300,7 +301,7 @@ RSpec.describe CaseLogsController, type: :request do
     it "updates the case log with the given fields and keeps original values where none are passed" do
       case_log.reload
       expect(case_log.tenant_code).to eq("New Value")
-      expect(case_log.property_postcode).to eq("Old Value")
+      expect(case_log.property_postcode).to eq("M1 1AE")
     end
 
     context "invalid case log id" do
@@ -340,7 +341,7 @@ RSpec.describe CaseLogsController, type: :request do
   # what actually happens to an ActiveRecord object and what we're doing here, but either is allowed.
   describe "PUT" do
     let(:case_log) do
-      FactoryBot.create(:case_log, :in_progress, tenant_code: "Old Value", property_postcode: "Old Value")
+      FactoryBot.create(:case_log, :in_progress, tenant_code: "Old Value", property_postcode: "SW1A 2AA")
     end
     let(:params) do
       { tenant_code: "New Value" }
@@ -358,7 +359,7 @@ RSpec.describe CaseLogsController, type: :request do
     it "updates the case log with the given fields and keeps original values where none are passed" do
       case_log.reload
       expect(case_log.tenant_code).to eq("New Value")
-      expect(case_log.property_postcode).to eq("Old Value")
+      expect(case_log.property_postcode).to eq("SW1A 2AA")
     end
 
     context "invalid case log id" do