From e16fae8810c2e9be27ab94566692a929cc7c4ea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Meny?= Date: Tue, 11 Jan 2022 14:13:50 +0000 Subject: [PATCH] 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 | 53 ++++++++++--------- .../validations/property_validations.rb | 8 ++- config/forms/2021_2022.json | 2 +- 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/fixtures/complete_case_log.json | 2 +- spec/models/case_log_spec.rb | 5 ++ spec/requests/case_log_controller_spec.rb | 11 ++-- 10 files changed, 52 insertions(+), 37 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index a1cd948c4..042759733 100644 --- a/app/models/case_log.rb +++ b/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 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/config/forms/2021_2022.json b/config/forms/2021_2022.json index ab28d23e9..8c70609fd 100644 --- a/config/forms/2021_2022.json +++ b/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" diff --git a/spec/factories/case_log.rb b/spec/factories/case_log.rb index b67e2aa1f..8a133549b 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/fixtures/complete_case_log.json b/spec/fixtures/complete_case_log.json index 1ed4d5206..785ddf9eb 100644 --- a/spec/fixtures/complete_case_log.json +++ b/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", diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index d0bb4c718..98cce6750 100644 --- a/spec/models/case_log_spec.rb +++ b/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" }) diff --git a/spec/requests/case_log_controller_spec.rb b/spec/requests/case_log_controller_spec.rb index ac5da57fb..e27a57f41 100644 --- a/spec/requests/case_log_controller_spec.rb +++ b/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 = "Manchester" @@ -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