From 2106232eb1eb8b9096db0b35daeb1db5d61d2f6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Meny?= Date: Mon, 10 Jan 2022 14:56:00 +0000 Subject: [PATCH] CDLC-504: Introduce regexp validation of property postcodes --- app/models/case_log.rb | 52 +++++++++++-------- .../validations/property_validations.rb | 8 ++- spec/factories/case_log.rb | 2 +- .../form/accessible_autocomplete_spec.rb | 2 +- spec/features/form/page_routing_spec.rb | 2 +- spec/features/form/saving_data_spec.rb | 2 +- spec/models/case_log_spec.rb | 5 ++ spec/requests/case_log_controller_spec.rb | 10 ++-- 8 files changed, 49 insertions(+), 34 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 82ca6f582..a90ce60f0 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -35,7 +35,9 @@ class CaseLog < ApplicationRecord default_scope -> { kept } validates_with CaseLogValidator - before_save :update_status! + before_validation :update_status! + before_validation :process_postcode_changes!, if: :property_postcode_changed? + before_validation :reset_location_fields!, unless: :postcode_known? belongs_to :owning_organisation, class_name: "Organisation" belongs_to :managing_organisation, class_name: "Organisation" @@ -162,6 +164,10 @@ class CaseLog < ApplicationRecord status == "in_progress" end + def postcode_known? + postcode_known == "Yes" + end + def weekly_net_income return unless earnings && incfreq @@ -228,26 +234,34 @@ 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 + + # Called when the postcode is not known + 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 } @@ -275,14 +289,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 diff --git a/app/models/validations/property_validations.rb b/app/models/validations/property_validations.rb index 980447e1c..158b237cb 100644 --- a/app/models/validations/property_validations.rb +++ b/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 diff --git a/spec/factories/case_log.rb b/spec/factories/case_log.rb index 8a07b981c..b2724edd9 100644 --- a/spec/factories/case_log.rb +++ b/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 diff --git a/spec/features/form/accessible_autocomplete_spec.rb b/spec/features/form/accessible_autocomplete_spec.rb index 73fb5f252..ef6e5b319 100644 --- a/spec/features/form/accessible_autocomplete_spec.rb +++ b/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 diff --git a/spec/features/form/page_routing_spec.rb b/spec/features/form/page_routing_spec.rb index 5c33f4b8c..8f9d42ad9 100644 --- a/spec/features/form/page_routing_spec.rb +++ b/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 diff --git a/spec/features/form/saving_data_spec.rb b/spec/features/form/saving_data_spec.rb index c4e634909..4dbff2ebb 100644 --- a/spec/features/form/saving_data_spec.rb +++ b/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 diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 3b15a9ce3..6e6607b28 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -1119,6 +1119,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" }) diff --git a/spec/requests/case_log_controller_spec.rb b/spec/requests/case_log_controller_spec.rb index ac5da57fb..8d5759933 100644 --- a/spec/requests/case_log_controller_spec.rb +++ b/spec/requests/case_log_controller_spec.rb @@ -261,7 +261,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 = "Manchester" @@ -282,7 +282,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 +300,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 +340,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 +358,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