refactor: response to PR comments, fixing beds.negative validation

app/models/la_rent_range.rb

@@ -1,2 +1,3 @@
 class LaRentRange < ApplicationRecord
+  MAX_BEDS = 4
 end

app/models/lettings_log.rb

@@ -435,22 +435,19 @@ class LettingsLog < Log
     Csv::LettingsLogCsvService.new(user).to_csv
   end
 
-  def validation_beds
-    max_beds_in_rent_ranges = 4
-    if needstype == 2
-      0
-    else
-      beds.nil? ? nil : [beds, max_beds_in_rent_ranges].min
-    end
+  def beds_for_la_rent_range
+    return 0 if is_supported_housing?
+
+    beds.nil? ? nil : [beds, LaRentRange::MAX_BEDS].min
   end
 
   def soft_min_for_period
-    soft_min = LaRentRange.find_by(start_year: collection_start_year, la:, beds: validation_beds, lettype:).soft_min
+    soft_min = LaRentRange.find_by(start_year: collection_start_year, la:, beds: beds_for_la_rent_range, lettype:).soft_min
     "#{soft_value_for_period(soft_min)} #{SUFFIX_FROM_PERIOD[period].presence || 'every week'}"
   end
 
   def soft_max_for_period
-    soft_max = LaRentRange.find_by(start_year: collection_start_year, la:, beds: validation_beds, lettype:).soft_max
+    soft_max = LaRentRange.find_by(start_year: collection_start_year, la:, beds: beds_for_la_rent_range, lettype:).soft_max
     "#{soft_value_for_period(soft_max)} #{SUFFIX_FROM_PERIOD[period].presence || 'every week'}"
   end
 

app/models/validations/financial_validations.rb

@@ -182,14 +182,7 @@ private
 
     collection_year = record.collection_start_year
 
-    max_beds_in_rent_ranges = 4
-    validation_beds = if record.needstype == 2
-                        0
-                      else
-                        record.beds.nil? ? nil : [record.beds, max_beds_in_rent_ranges].min
-                      end
-
-    rent_range = LaRentRange.find_by(start_year: collection_year, la: record.la, beds: validation_beds, lettype: record.lettype)
+    rent_range = LaRentRange.find_by(start_year: collection_year, la: record.la, beds: record.beds_for_la_rent_range, lettype: record.lettype)
 
     if rent_range.present? && !weekly_value_in_range(record, "brent", rent_range.hard_min, rent_range.hard_max) && record.brent.present? && record.period.present?
       if record.weekly_value(record["brent"]) < rent_range.hard_min
@@ -202,7 +195,7 @@ private
         record.errors.add :rent_type, I18n.t("validations.financial.brent.rent_type.below_hard_min")
         record.errors.add :needstype, I18n.t("validations.financial.brent.needstype.below_hard_min")
         record.errors.add :period, I18n.t("validations.financial.brent.period.below_hard_min")
-      elsif !(record.beds.present? && record.beds > max_beds_in_rent_ranges)
+      elsif record.beds.blank? || record.beds < LaRentRange::MAX_BEDS
         record.errors.add :brent, I18n.t("validations.financial.brent.above_hard_max")
         record.errors.add :beds, I18n.t("validations.financial.brent.beds.above_hard_max")
         record.errors.add :la, I18n.t("validations.financial.brent.la.above_hard_max")

app/models/validations/property_validations.rb

@@ -55,8 +55,8 @@ module Validations::PropertyValidations
   end
 
   def validate_shared_housing_rooms(record)
-    if record.beds.present? && record.beds.negative?
-      record.errors.add :beds, I18n.t("validations.property.beds.negative")
+    if record.beds.present? && record.beds <= 0
+      record.errors.add :beds, I18n.t("validations.property.beds.non_positive")
     end
 
     unless record.unittype_gn.nil?

app/models/validations/soft_validations.rb

@@ -28,19 +28,18 @@ module Validations::SoftValidations
   def rent_in_soft_min_range?
     return unless brent && weekly_value(brent) && startdate
 
-    rent_range = LaRentRange.find_by(start_year: collection_start_year, la:, beds: validation_beds, lettype: get_lettype)
+    rent_range = LaRentRange.find_by(start_year: collection_start_year, la:, beds: beds_for_la_rent_range, lettype: get_lettype)
     rent_range.present? && weekly_value(brent).between?(rent_range.hard_min, rent_range.soft_min)
   end
 
   def rent_in_soft_max_range?
     return unless brent && weekly_value(brent) && startdate
 
-    rent_range = LaRentRange.find_by(start_year: collection_start_year, la:, beds: validation_beds, lettype: get_lettype)
-    max_beds_in_rent_ranges = 4
-    if beds.present? && beds > max_beds_in_rent_ranges
-      rent_range.present? && weekly_value(brent) > rent_range.soft_max
-    else
-      rent_range.present? && weekly_value(brent).between?(rent_range.soft_max, rent_range.hard_max)
+    rent_range = LaRentRange.find_by(start_year: collection_start_year, la:, beds: beds_for_la_rent_range, lettype: get_lettype)
+    if beds.present? && rent_range.present? && beds > LaRentRange::MAX_BEDS
+      weekly_value(brent) > rent_range.soft_max
+    elsif rent_range.present?
+      weekly_value(brent).between?(rent_range.soft_max, rent_range.hard_max)
     end
   end
 

config/locales/en.yml

@@ -146,7 +146,7 @@ en:
         one_seven_bedroom_shared: "A shared house must have 1 to 7 bedrooms"
         one_three_bedroom_single_tenant_shared: "A shared house with fewer than two tenants must have 1 to 3 bedrooms"
       beds:
-        negative: "Number of bedrooms has to be greater than 0"
+        non_positive: "Number of bedrooms has to be greater than 0"
         over_max: "Number of bedrooms cannot be more than 12"
 
     financial:

spec/models/validations/property_validations_spec.rb

@@ -134,7 +134,7 @@ RSpec.describe Validations::PropertyValidations do
       it "adds an error" do
         record.beds = -4
         property_validator.validate_shared_housing_rooms(record)
-        expect(record.errors["beds"]).to include(I18n.t("validations.property.beds.negative"))
+        expect(record.errors["beds"]).to include(I18n.t("validations.property.beds.non_positive"))
       end
     end