refactor property validations specs to avoid breaking on collection year change (#2478)

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?
+    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")
+    if record.is_bedsit? && record.beds != 1 && record.beds.present? && !record.form.start_year_after_2024?
-        record.errors.add :beds, I18n.t("validations.property.unittype_gn.one_bedroom_bedsit")
+      record.errors.add :unittype_gn, I18n.t("validations.property.unittype_gn.one_bedroom_bedsit")
-      end
+      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?
+    if record.hhmemb == 1 && record.is_shared_housing? &&
-        record.errors.add :unittype_gn, I18n.t("validations.property.unittype_gn.one_three_bedroom_single_tenant_shared")
+        !record.beds.to_i.between?(1, 3) && record.beds.present?
-        record.errors.add :beds, :one_three_bedroom_single_tenant_shared, message: I18n.t("validations.property.unittype_gn.one_three_bedroom_single_tenant_shared")
+      record.errors.add :unittype_gn, 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 :beds, :one_three_bedroom_single_tenant_shared, message: I18n.t("validations.property.unittype_gn.one_three_bedroom_single_tenant_shared")
-        record.errors.add :unittype_gn, I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared")
+    elsif record.is_shared_housing? && record.beds.present? && !record.beds.to_i.between?(1, 7)
-        record.errors.add :beds, I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared")
+      record.errors.add :unittype_gn, I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared")
-      end
+      record.errors.add :beds, I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared")
     end
   end
 

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
+        log.beds = nil
-        record.unittype_gn = 2
+        log.unittype_gn = 2
-        property_validator.validate_shared_housing_rooms(record)
+        property_validator.validate_shared_housing_rooms(log)
-        expect(record.errors).to be_empty
+        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
+        log.beds = nil
-        record.unittype_gn = 10
+        log.unittype_gn = 10
-        property_validator.validate_shared_housing_rooms(record)
+        property_validator.validate_shared_housing_rooms(log)
-        expect(record.errors).to be_empty
+        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
+        log.beds = 2
-        record.unittype_gn = nil
+        log.unittype_gn = nil
-        property_validator.validate_shared_housing_rooms(record)
+        property_validator.validate_shared_housing_rooms(log)
-        expect(record.errors).to be_empty
+        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
+      context "and the log is for 24/25 or later" do
-        record.beds = 2
+        it "does not add an error" do
-        record.unittype_gn = 2
+          property_validator.validate_shared_housing_rooms(log)
-        property_validator.validate_shared_housing_rooms(record)
+
-        expect(record.errors["unittype_gn"]).to include(match(expected_error))
+          expect(log.errors).to be_empty
-        expect(record.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_bedroom_bedsit"))
+        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
+      context "and the log is for 24/25 or later" do
-        record.beds = 0
+        it "does not add an error" do
-        record.unittype_gn = 2
+          property_validator.validate_shared_housing_rooms(log)
-        property_validator.validate_shared_housing_rooms(record)
+
-        expect(record.errors["unittype_gn"]).to include(match(expected_error))
+          expect(log.errors).to be_empty
-        expect(record.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_bedroom_bedsit"))
+        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
+        log.beds = 8
-        record.unittype_gn = 9
+        log.unittype_gn = 9
-        record.hhmemb = 3
+        log.hhmemb = 3
-        property_validator.validate_shared_housing_rooms(record)
+        property_validator.validate_shared_housing_rooms(log)
-        expect(record.errors["unittype_gn"]).to include(match(expected_error))
+        expect(log.errors["unittype_gn"]).to include(match(expected_error))
-        expect(record.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared"))
+        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
+        log.beds = 0
-        record.unittype_gn = 9
+        log.unittype_gn = 9
-        record.hhmemb = 3
+        log.hhmemb = 3
-        property_validator.validate_shared_housing_rooms(record)
+        property_validator.validate_shared_housing_rooms(log)
-        expect(record.errors["unittype_gn"]).to include(match(expected_error))
+        expect(log.errors["unittype_gn"]).to include(match(expected_error))
-        expect(record.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared"))
+        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
+        log.beds = 4
-        record.unittype_gn = 9
+        log.unittype_gn = 9
-        record.hhmemb = 1
+        log.hhmemb = 1
-        property_validator.validate_shared_housing_rooms(record)
+        property_validator.validate_shared_housing_rooms(log)
-        expect(record.errors["unittype_gn"]).to include(match(expected_error))
+        expect(log.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"))
+        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
+        log.first_time_property_let_as_social_housing = 1
-        record.unitletas = 0
+        log.unitletas = 0
-        property_validator.validate_unitletas(record)
+        property_validator.validate_unitletas(log)
-        expect(record.errors["unitletas"])
+        expect(log.errors["unitletas"])
           .to include(match I18n.t("validations.property.rsnvac.previous_let_social"))
-        record.unitletas = 1
+        log.unitletas = 1
-        property_validator.validate_unitletas(record)
+        property_validator.validate_unitletas(log)
-        expect(record.errors["unitletas"])
+        expect(log.errors["unitletas"])
           .to include(match I18n.t("validations.property.rsnvac.previous_let_social"))
-        record.unitletas = 2
+        log.unitletas = 2
-        property_validator.validate_unitletas(record)
+        property_validator.validate_unitletas(log)
-        expect(record.errors["unitletas"])
+        expect(log.errors["unitletas"])
           .to include(match I18n.t("validations.property.rsnvac.previous_let_social"))
-        record.unitletas = 3
+        log.unitletas = 3
-        property_validator.validate_unitletas(record)
+        property_validator.validate_unitletas(log)
-        expect(record.errors["unitletas"])
+        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
+        log.first_time_property_let_as_social_housing = 0
-        record.unitletas = 0
+        log.unitletas = 0
-        property_validator.validate_unitletas(record)
+        property_validator.validate_unitletas(log)
-        expect(record.errors["unitletas"]).to be_empty
+        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
+        log.first_time_property_let_as_social_housing = 1
-        record.rsnvac = 6
+        log.rsnvac = 6
-        property_validator.validate_rsnvac(record)
+        property_validator.validate_rsnvac(log)
-        expect(record.errors["rsnvac"])
+        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
+        log.first_time_property_let_as_social_housing = 1
-        record.rsnvac = 15
+        log.rsnvac = 15
-        property_validator.validate_rsnvac(record)
+        property_validator.validate_rsnvac(log)
-        expect(record.errors["rsnvac"]).to be_empty
+        expect(log.errors["rsnvac"]).to be_empty
-        record.rsnvac = 16
+        log.rsnvac = 16
-        property_validator.validate_rsnvac(record)
+        property_validator.validate_rsnvac(log)
-        expect(record.errors["rsnvac"]).to be_empty
+        expect(log.errors["rsnvac"]).to be_empty
-        record.rsnvac = 17
+        log.rsnvac = 17
-        property_validator.validate_rsnvac(record)
+        property_validator.validate_rsnvac(log)
-        expect(record.errors["rsnvac"]).to be_empty
+        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
+        log.first_time_property_let_as_social_housing = 0
-        record.rsnvac = 15
+        log.rsnvac = 15
-        property_validator.validate_rsnvac(record)
+        property_validator.validate_rsnvac(log)
-        expect(record.errors["rsnvac"])
+        expect(log.errors["rsnvac"])
           .to include(match I18n.t("validations.property.rsnvac.first_let_not_social"))
-        record.rsnvac = 16
+        log.rsnvac = 16
-        property_validator.validate_rsnvac(record)
+        property_validator.validate_rsnvac(log)
-        expect(record.errors["rsnvac"])
+        expect(log.errors["rsnvac"])
           .to include(match I18n.t("validations.property.rsnvac.first_let_not_social"))
-        record.rsnvac = 17
+        log.rsnvac = 17
-        property_validator.validate_rsnvac(record)
+        property_validator.validate_rsnvac(log)
-        expect(record.errors["rsnvac"])
+        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
+        log.first_time_property_let_as_social_housing = 1
-        record.rsnvac = 15
+        log.rsnvac = 15
-        property_validator.validate_rsnvac(record)
+        property_validator.validate_rsnvac(log)
-        expect(record.errors["rsnvac"]).to be_empty
+        expect(log.errors["rsnvac"]).to be_empty
-        record.rsnvac = 16
+        log.rsnvac = 16
-        property_validator.validate_rsnvac(record)
+        property_validator.validate_rsnvac(log)
-        expect(record.errors["rsnvac"]).to be_empty
+        expect(log.errors["rsnvac"]).to be_empty
-        record.rsnvac = 17
+        log.rsnvac = 17
-        property_validator.validate_rsnvac(record)
+        property_validator.validate_rsnvac(log)
-        expect(record.errors["rsnvac"]).to be_empty
+        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
+          log.first_time_property_let_as_social_housing = 0
-          record.renewal = 0
+          log.renewal = 0
-          record.rsnvac = 14
+          log.rsnvac = 14
-          property_validator.validate_rsnvac(record)
+          property_validator.validate_rsnvac(log)
-          expect(record.errors["rsnvac"])
+          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
+            log.rsnvac = 9
-            record.prevten = prevten
+            log.prevten = prevten
-            property_validator.validate_rsnvac(record)
+            property_validator.validate_rsnvac(log)
-            expect(record.errors["rsnvac"])
+            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
+            log.rsnvac = 9
-            record.referral = src
+            log.referral = src
-            property_validator.validate_rsnvac(record)
+            property_validator.validate_rsnvac(log)
-            expect(record.errors["rsnvac"])
+            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
+          log.prevten = 0
-          record.rsnvac = 2
+          log.rsnvac = 2
-          property_validator.validate_rsnvac(record)
+          property_validator.validate_rsnvac(log)
-          expect(record.errors["rsnvac"]).to be_empty
+          expect(log.errors["rsnvac"]).to be_empty
         end
 
         it "expects that the letting source can be a referral" do
-          record.prevten = 0
+          log.prevten = 0
-          record.referral = 2
+          log.referral = 2
-          property_validator.validate_rsnvac(record)
+          property_validator.validate_rsnvac(log)
-          expect(record.errors["rsnvac"]).to be_empty
+          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)
+        property_validator.validate_uprn(log)
-        expect(record.errors.added?(:uprn, "UPRN must be 12 digits or less")).to be true
+        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)
+        property_validator.validate_uprn(log)
-        expect(record.errors.added?(:uprn, "UPRN must be 12 digits or less")).to be true
+        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)
+        property_validator.validate_uprn(log)
-        expect(record.errors).not_to be_present
+        expect(log.errors).not_to be_present
       end
     end
   end