From e566ff09a09c19df708722c5e8ed0f928a850f10 Mon Sep 17 00:00:00 2001 From: Arthur Campbell <51094020+arfacamble@users.noreply.github.com> Date: Fri, 21 Jun 2024 17:12:49 +0100 Subject: [PATCH] refactor property validations specs to avoid breaking on collection year change (#2478) --- .../validations/property_validations.rb | 28 +- .../validations/property_validations_spec.rb | 280 ++++++++++-------- 2 files changed, 168 insertions(+), 140 deletions(-) diff --git a/app/models/validations/property_validations.rb b/app/models/validations/property_validations.rb index ba8318282..52afecb49 100644 --- a/app/models/validations/property_validations.rb +++ b/app/models/validations/property_validations.rb @@ -33,20 +33,20 @@ module Validations::PropertyValidations end def validate_shared_housing_rooms(record) - unless record.unittype_gn.nil? - if record.is_bedsit? && record.beds != 1 && record.beds.present? && !record.form.start_year_after_2024? - record.errors.add :unittype_gn, I18n.t("validations.property.unittype_gn.one_bedroom_bedsit") - record.errors.add :beds, I18n.t("validations.property.unittype_gn.one_bedroom_bedsit") - end - - if record.hhmemb == 1 && record.is_shared_housing? && - !record.beds.to_i.between?(1, 3) && record.beds.present? - record.errors.add :unittype_gn, I18n.t("validations.property.unittype_gn.one_three_bedroom_single_tenant_shared") - record.errors.add :beds, :one_three_bedroom_single_tenant_shared, message: I18n.t("validations.property.unittype_gn.one_three_bedroom_single_tenant_shared") - elsif record.is_shared_housing? && record.beds.present? && !record.beds.to_i.between?(1, 7) - record.errors.add :unittype_gn, I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared") - record.errors.add :beds, I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared") - end + return unless record.unittype_gn + + if record.is_bedsit? && record.beds != 1 && record.beds.present? && !record.form.start_year_after_2024? + record.errors.add :unittype_gn, I18n.t("validations.property.unittype_gn.one_bedroom_bedsit") + record.errors.add :beds, I18n.t("validations.property.unittype_gn.one_bedroom_bedsit") + end + + if record.hhmemb == 1 && record.is_shared_housing? && + !record.beds.to_i.between?(1, 3) && record.beds.present? + record.errors.add :unittype_gn, I18n.t("validations.property.unittype_gn.one_three_bedroom_single_tenant_shared") + record.errors.add :beds, :one_three_bedroom_single_tenant_shared, message: I18n.t("validations.property.unittype_gn.one_three_bedroom_single_tenant_shared") + elsif record.is_shared_housing? && record.beds.present? && !record.beds.to_i.between?(1, 7) + record.errors.add :unittype_gn, I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared") + record.errors.add :beds, I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared") end end diff --git a/spec/models/validations/property_validations_spec.rb b/spec/models/validations/property_validations_spec.rb index ae41d4f03..dcbcae337 100644 --- a/spec/models/validations/property_validations_spec.rb +++ b/spec/models/validations/property_validations_spec.rb @@ -4,57 +4,85 @@ RSpec.describe Validations::PropertyValidations do subject(:property_validator) { property_validator_class.new } let(:property_validator_class) { Class.new { include Validations::PropertyValidations } } - let(:record) { FactoryBot.create(:lettings_log, startdate: Time.zone.local(2024, 3, 3)) } + let(:log) { build(:lettings_log) } describe "#validate_shared_housing_rooms" do context "when number of bedrooms has not been answered" do it "does not add an error" do - record.beds = nil - record.unittype_gn = 2 - property_validator.validate_shared_housing_rooms(record) - expect(record.errors).to be_empty + log.beds = nil + log.unittype_gn = 2 + property_validator.validate_shared_housing_rooms(log) + expect(log.errors).to be_empty end end context "when unit type is shared and number of bedrooms has not been answered" do it "does not add an error" do - record.beds = nil - record.unittype_gn = 10 - property_validator.validate_shared_housing_rooms(record) - expect(record.errors).to be_empty + log.beds = nil + log.unittype_gn = 10 + property_validator.validate_shared_housing_rooms(log) + expect(log.errors).to be_empty end end context "when unit type has not been answered" do it "does not add an error" do - record.beds = 2 - record.unittype_gn = nil - property_validator.validate_shared_housing_rooms(record) - expect(record.errors).to be_empty + log.beds = 2 + log.unittype_gn = nil + property_validator.validate_shared_housing_rooms(log) + expect(log.errors).to be_empty end end context "when a bedsit has more than 1 bedroom" do - let(:expected_error) { I18n.t("validations.property.unittype_gn.one_bedroom_bedsit") } + before do + log.beds = 2 + log.unittype_gn = 2 + end - it "adds an error" do - record.beds = 2 - record.unittype_gn = 2 - property_validator.validate_shared_housing_rooms(record) - expect(record.errors["unittype_gn"]).to include(match(expected_error)) - expect(record.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_bedroom_bedsit")) + context "and the log is for 24/25 or later" do + it "does not add an error" do + property_validator.validate_shared_housing_rooms(log) + + expect(log.errors).to be_empty + end + end + + context "and the log is from before 24/25" do + it "adds an error" do + allow(log.form).to receive(:start_year_after_2024?).and_return false + + property_validator.validate_shared_housing_rooms(log) + + expect(log.errors["unittype_gn"]).to include(I18n.t("validations.property.unittype_gn.one_bedroom_bedsit")) + expect(log.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_bedroom_bedsit")) + end end end context "when a bedsit has less than 1 bedroom" do - let(:expected_error) { I18n.t("validations.property.unittype_gn.one_bedroom_bedsit") } + before do + log.beds = 0 + log.unittype_gn = 2 + end - it "adds an error" do - record.beds = 0 - record.unittype_gn = 2 - property_validator.validate_shared_housing_rooms(record) - expect(record.errors["unittype_gn"]).to include(match(expected_error)) - expect(record.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_bedroom_bedsit")) + context "and the log is for 24/25 or later" do + it "does not add an error" do + property_validator.validate_shared_housing_rooms(log) + + expect(log.errors).to be_empty + end + end + + context "and the log is from before 24/25" do + it "adds an error" do + allow(log.form).to receive(:start_year_after_2024?).and_return false + + property_validator.validate_shared_housing_rooms(log) + + expect(log.errors["unittype_gn"]).to include(I18n.t("validations.property.unittype_gn.one_bedroom_bedsit")) + expect(log.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_bedroom_bedsit")) + end end end @@ -62,12 +90,12 @@ RSpec.describe Validations::PropertyValidations do let(:expected_error) { I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared") } it "adds an error if the number of bedrooms is not between 1 and 7" do - record.beds = 8 - record.unittype_gn = 9 - record.hhmemb = 3 - property_validator.validate_shared_housing_rooms(record) - expect(record.errors["unittype_gn"]).to include(match(expected_error)) - expect(record.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared")) + log.beds = 8 + log.unittype_gn = 9 + log.hhmemb = 3 + property_validator.validate_shared_housing_rooms(log) + expect(log.errors["unittype_gn"]).to include(match(expected_error)) + expect(log.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared")) end end @@ -75,12 +103,12 @@ RSpec.describe Validations::PropertyValidations do let(:expected_error) { I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared") } it "adds an error if the number of bedrooms is not between 1 and 7" do - record.beds = 0 - record.unittype_gn = 9 - record.hhmemb = 3 - property_validator.validate_shared_housing_rooms(record) - expect(record.errors["unittype_gn"]).to include(match(expected_error)) - expect(record.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared")) + log.beds = 0 + log.unittype_gn = 9 + log.hhmemb = 3 + property_validator.validate_shared_housing_rooms(log) + expect(log.errors["unittype_gn"]).to include(match(expected_error)) + expect(log.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared")) end end @@ -88,12 +116,12 @@ RSpec.describe Validations::PropertyValidations do let(:expected_error) { I18n.t("validations.property.unittype_gn.one_three_bedroom_single_tenant_shared") } it "adds an error" do - record.beds = 4 - record.unittype_gn = 9 - record.hhmemb = 1 - property_validator.validate_shared_housing_rooms(record) - expect(record.errors["unittype_gn"]).to include(match(expected_error)) - expect(record.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_three_bedroom_single_tenant_shared")) + log.beds = 4 + log.unittype_gn = 9 + log.hhmemb = 1 + property_validator.validate_shared_housing_rooms(log) + expect(log.errors["unittype_gn"]).to include(match(expected_error)) + expect(log.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_three_bedroom_single_tenant_shared")) end end end @@ -101,32 +129,32 @@ RSpec.describe Validations::PropertyValidations do describe "#validate_unitletas" do context "when the property has not been let before" do it "validates that no previous let type is provided" do - record.first_time_property_let_as_social_housing = 1 - record.unitletas = 0 - property_validator.validate_unitletas(record) - expect(record.errors["unitletas"]) + log.first_time_property_let_as_social_housing = 1 + log.unitletas = 0 + property_validator.validate_unitletas(log) + expect(log.errors["unitletas"]) .to include(match I18n.t("validations.property.rsnvac.previous_let_social")) - record.unitletas = 1 - property_validator.validate_unitletas(record) - expect(record.errors["unitletas"]) + log.unitletas = 1 + property_validator.validate_unitletas(log) + expect(log.errors["unitletas"]) .to include(match I18n.t("validations.property.rsnvac.previous_let_social")) - record.unitletas = 2 - property_validator.validate_unitletas(record) - expect(record.errors["unitletas"]) + log.unitletas = 2 + property_validator.validate_unitletas(log) + expect(log.errors["unitletas"]) .to include(match I18n.t("validations.property.rsnvac.previous_let_social")) - record.unitletas = 3 - property_validator.validate_unitletas(record) - expect(record.errors["unitletas"]) + log.unitletas = 3 + property_validator.validate_unitletas(log) + expect(log.errors["unitletas"]) .to include(match I18n.t("validations.property.rsnvac.previous_let_social")) end end context "when the property has been let previously" do it "expects to have a previous let type" do - record.first_time_property_let_as_social_housing = 0 - record.unitletas = 0 - property_validator.validate_unitletas(record) - expect(record.errors["unitletas"]).to be_empty + log.first_time_property_let_as_social_housing = 0 + log.unitletas = 0 + property_validator.validate_unitletas(log) + expect(log.errors["unitletas"]).to be_empty end end end @@ -134,64 +162,64 @@ RSpec.describe Validations::PropertyValidations do describe "validate_rsnvac" do context "when the property has not been let before" do it "validates that it has a first let reason for vacancy" do - record.first_time_property_let_as_social_housing = 1 - record.rsnvac = 6 - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]) + log.first_time_property_let_as_social_housing = 1 + log.rsnvac = 6 + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]) .to include(match I18n.t("validations.property.rsnvac.first_let_social")) end it "expects to have a first let reason for vacancy" do - record.first_time_property_let_as_social_housing = 1 - record.rsnvac = 15 - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]).to be_empty - record.rsnvac = 16 - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]).to be_empty - record.rsnvac = 17 - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]).to be_empty + log.first_time_property_let_as_social_housing = 1 + log.rsnvac = 15 + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]).to be_empty + log.rsnvac = 16 + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]).to be_empty + log.rsnvac = 17 + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]).to be_empty end end context "when the property has been let as social housing before" do it "validates that the reason for vacancy is not a first let as social housing reason" do - record.first_time_property_let_as_social_housing = 0 - record.rsnvac = 15 - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]) + log.first_time_property_let_as_social_housing = 0 + log.rsnvac = 15 + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]) .to include(match I18n.t("validations.property.rsnvac.first_let_not_social")) - record.rsnvac = 16 - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]) + log.rsnvac = 16 + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]) .to include(match I18n.t("validations.property.rsnvac.first_let_not_social")) - record.rsnvac = 17 - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]) + log.rsnvac = 17 + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]) .to include(match I18n.t("validations.property.rsnvac.first_let_not_social")) end it "expects the reason for vacancy to be a first let as social housing reason" do - record.first_time_property_let_as_social_housing = 1 - record.rsnvac = 15 - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]).to be_empty - record.rsnvac = 16 - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]).to be_empty - record.rsnvac = 17 - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]).to be_empty + log.first_time_property_let_as_social_housing = 1 + log.rsnvac = 15 + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]).to be_empty + log.rsnvac = 16 + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]).to be_empty + log.rsnvac = 17 + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]).to be_empty end context "when the letting is not a renewal" do it "validates that the reason for vacancy is not renewal" do - record.first_time_property_let_as_social_housing = 0 - record.renewal = 0 - record.rsnvac = 14 - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]) + log.first_time_property_let_as_social_housing = 0 + log.renewal = 0 + log.rsnvac = 14 + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]) .to include(match I18n.t("validations.property.rsnvac.not_a_renewal")) end end @@ -205,20 +233,20 @@ RSpec.describe Validations::PropertyValidations do it "validates that the property is not being relet to tenant who occupied as temporary" do non_temporary_previous_tenancies.each do |prevten| - record.rsnvac = 9 - record.prevten = prevten - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]) + log.rsnvac = 9 + log.prevten = prevten + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]) .to include(match I18n.t("validations.property.rsnvac.non_temp_accommodation")) end end it "validates that the letting source is not a referral" do referral_sources.each do |src| - record.rsnvac = 9 - record.referral = src - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]) + log.rsnvac = 9 + log.referral = src + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]) .to include(match I18n.t("validations.property.rsnvac.referral_invalid")) end end @@ -226,17 +254,17 @@ RSpec.describe Validations::PropertyValidations do context "when the previous tenancy was temporary" do it "expects that the property can be relet to a tenant who previously occupied it as temporary" do - record.prevten = 0 - record.rsnvac = 2 - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]).to be_empty + log.prevten = 0 + log.rsnvac = 2 + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]).to be_empty end it "expects that the letting source can be a referral" do - record.prevten = 0 - record.referral = 2 - property_validator.validate_rsnvac(record) - expect(record.errors["rsnvac"]).to be_empty + log.prevten = 0 + log.referral = 2 + property_validator.validate_rsnvac(log) + expect(log.errors["rsnvac"]).to be_empty end end end @@ -244,29 +272,29 @@ RSpec.describe Validations::PropertyValidations do describe "#validate_uprn" do context "when within length limit but alphanumeric" do - let(:record) { build(:sales_log, uprn: "123abc") } + let(:log) { build(:sales_log, uprn: "123abc") } it "adds an error" do - property_validator.validate_uprn(record) - expect(record.errors.added?(:uprn, "UPRN must be 12 digits or less")).to be true + property_validator.validate_uprn(log) + expect(log.errors.added?(:uprn, "UPRN must be 12 digits or less")).to be true end end context "when over the length limit" do - let(:record) { build(:sales_log, uprn: "1234567890123") } + let(:log) { build(:sales_log, uprn: "1234567890123") } it "adds an error" do - property_validator.validate_uprn(record) - expect(record.errors.added?(:uprn, "UPRN must be 12 digits or less")).to be true + property_validator.validate_uprn(log) + expect(log.errors.added?(:uprn, "UPRN must be 12 digits or less")).to be true end end context "when within the limit and only numeric" do - let(:record) { build(:sales_log, uprn: "123456789012") } + let(:log) { build(:sales_log, uprn: "123456789012") } it "does not add an error" do - property_validator.validate_uprn(record) - expect(record.errors).not_to be_present + property_validator.validate_uprn(log) + expect(log.errors).not_to be_present end end end