Fix tenancy validations (#296)

* Add compound tenancy validations to both fields

* Don't validate tenancy length if not answered yet

* Name subject

app/models/validations/tenancy_validations.rb

@@ -8,11 +8,16 @@ module Validations::TenancyValidations
     is_ast = record.tenancy == "Assured Shorthold"
     conditions = [
       { condition: !(is_secure || is_ast) && is_present, error: I18n.t("validations.tenancy.length.fixed_term_not_required") },
-      { condition: is_ast && !is_in_range,  error: I18n.t("validations.tenancy.length.shorthold") },
+      { condition: (is_ast && !is_in_range) && is_present, error: I18n.t("validations.tenancy.length.shorthold") },
       { condition: is_secure && (!is_in_range && is_present), error: I18n.t("validations.tenancy.length.secure") },
     ]
 
-    conditions.each { |condition| condition[:condition] ? (record.errors.add :tenancylength, condition[:error]) : nil }
+    conditions.each do |condition|
+      next unless condition[:condition]
+
+      record.errors.add :tenancylength, condition[:error]
+      record.errors.add :tenancy, condition[:error]
+    end
   end
 
   def validate_other_tenancy_type(record)

spec/models/case_log_spec.rb

@@ -357,7 +357,7 @@ RSpec.describe CaseLog do
       end
     end
 
-    context "when validaiting fixed term tenancy" do
+    context "when validating fixed term tenancy" do
       it "Must not be completed if Type of main tenancy is not responded with either Secure or Assured shorthold " do
         expect {
           described_class.create!(tenancy: "Other",
@@ -366,59 +366,6 @@ RSpec.describe CaseLog do
                                   managing_organisation: managing_organisation)
         }.to raise_error(ActiveRecord::RecordInvalid)
       end
-
-      it "Must be completed and between 2 and 99 if type of tenancy is Assured shorthold" do
-        expect {
-          described_class.create!(tenancy: "Assured Shorthold",
-                                  tenancylength: 1,
-                                  owning_organisation: owning_organisation,
-                                  managing_organisation: managing_organisation)
-        }.to raise_error(ActiveRecord::RecordInvalid)
-
-        expect {
-          described_class.create!(tenancy: "Assured Shorthold",
-                                  tenancylength: nil,
-                                  owning_organisation: owning_organisation,
-                                  managing_organisation: managing_organisation)
-        }.to raise_error(ActiveRecord::RecordInvalid)
-
-        expect {
-          described_class.create!(tenancy: "Assured Shorthold",
-                                  tenancylength: 2,
-                                  owning_organisation: owning_organisation,
-                                  managing_organisation: managing_organisation)
-        }.not_to raise_error
-      end
-
-      it "Must be empty or between 2 and 99 if type of tenancy is Secure" do
-        expect {
-          described_class.create!(tenancy: "Secure (including flexible)",
-                                  tenancylength: 1,
-                                  owning_organisation: owning_organisation,
-                                  managing_organisation: managing_organisation)
-        }.to raise_error(ActiveRecord::RecordInvalid)
-
-        expect {
-          described_class.create!(tenancy: "Secure (including flexible)",
-                                  tenancylength: 100,
-                                  owning_organisation: owning_organisation,
-                                  managing_organisation: managing_organisation)
-        }.to raise_error(ActiveRecord::RecordInvalid)
-
-        expect {
-          described_class.create!(tenancy: "Secure (including flexible)",
-                                  tenancylength: nil,
-                                  owning_organisation: owning_organisation,
-                                  managing_organisation: managing_organisation)
-        }.not_to raise_error
-
-        expect {
-          described_class.create!(tenancy: "Secure (including flexible)",
-                                  tenancylength: 2,
-                                  owning_organisation: owning_organisation,
-                                  managing_organisation: managing_organisation)
-        }.not_to raise_error
-      end
     end
 
     context "when validating armed forces is active" do

spec/models/validations/tenancy_validations_spec.rb

@@ -0,0 +1,112 @@
+require "rails_helper"
+
+RSpec.describe Validations::TenancyValidations do
+  subject(:tenancy_validator) { validator_class.new }
+
+  let(:validator_class) { Class.new { include Validations::TenancyValidations } }
+  let(:record) { FactoryBot.create(:case_log) }
+
+  describe "#validate_fixed_term_tenancy" do
+    context "when fixed term tenancy" do
+      context "when type of tenancy is not assured or assured shorthold" do
+        let(:expected_error) { I18n.t("validations.tenancy.length.fixed_term_not_required") }
+
+        it "tenancy length should not be present" do
+          record.tenancy = "Other"
+          record.tenancylength = 10
+          tenancy_validator.validate_fixed_term_tenancy(record)
+          expect(record.errors["tenancylength"]).to include(match(expected_error))
+          expect(record.errors["tenancy"]).to include(match(expected_error))
+        end
+      end
+
+      context "when type of tenancy is assured shorthold" do
+        let(:expected_error) { I18n.t("validations.tenancy.length.shorthold") }
+
+        context "when tenancy length is greater than 1" do
+          it "adds an error" do
+            record.tenancy = "Assured Shorthold"
+            record.tenancylength = 1
+            tenancy_validator.validate_fixed_term_tenancy(record)
+            expect(record.errors["tenancylength"]).to include(match(expected_error))
+            expect(record.errors["tenancy"]).to include(match(expected_error))
+          end
+        end
+
+        context "when tenancy length is less than 100" do
+          it "adds an error" do
+            record.tenancy = "Assured Shorthold"
+            record.tenancylength = 100
+            tenancy_validator.validate_fixed_term_tenancy(record)
+            expect(record.errors["tenancylength"]).to include(match(expected_error))
+            expect(record.errors["tenancy"]).to include(match(expected_error))
+          end
+        end
+
+        context "when tenancy length is between 2-99" do
+          it "does not add an error" do
+            record.tenancy = "Assured Shorthold"
+            record.tenancylength = 3
+            tenancy_validator.validate_fixed_term_tenancy(record)
+            expect(record.errors["tenancylength"]).to be_empty
+            expect(record.errors["tenancy"]).to be_empty
+          end
+        end
+
+        context "when tenancy length has not been answered" do
+          it "does not add an error" do
+            record.tenancy = "Assured Shorthold"
+            record.tenancylength = nil
+            tenancy_validator.validate_fixed_term_tenancy(record)
+            expect(record.errors["tenancylength"]).to be_empty
+            expect(record.errors["tenancy"]).to be_empty
+          end
+        end
+      end
+
+      context "when type of tenancy is secure" do
+        let(:expected_error) { I18n.t("validations.tenancy.length.secure") }
+
+        context "when tenancy length is greater than 1" do
+          it "adds an error" do
+            record.tenancy = "Secure (including flexible)"
+            record.tenancylength = 1
+            tenancy_validator.validate_fixed_term_tenancy(record)
+            expect(record.errors["tenancylength"]).to include(match(expected_error))
+            expect(record.errors["tenancy"]).to include(match(expected_error))
+          end
+        end
+
+        context "when tenancy length is less than 100" do
+          it "adds an error" do
+            record.tenancy = "Secure (including flexible)"
+            record.tenancylength = 100
+            tenancy_validator.validate_fixed_term_tenancy(record)
+            expect(record.errors["tenancylength"]).to include(match(expected_error))
+            expect(record.errors["tenancy"]).to include(match(expected_error))
+          end
+        end
+
+        context "when tenancy length is between 2-99" do
+          it "does not add an error" do
+            record.tenancy = "Secure (including flexible)"
+            record.tenancylength = 3
+            tenancy_validator.validate_fixed_term_tenancy(record)
+            expect(record.errors["tenancylength"]).to be_empty
+            expect(record.errors["tenancy"]).to be_empty
+          end
+        end
+
+        context "when tenancy length has not been answered" do
+          it "does not add an error" do
+            record.tenancy = "Secure (including flexible)"
+            record.tenancylength = nil
+            tenancy_validator.validate_fixed_term_tenancy(record)
+            expect(record.errors["tenancylength"]).to be_empty
+            expect(record.errors["tenancy"]).to be_empty
+          end
+        end
+      end
+    end
+  end
+end