CLDC-886 add validations for discounts (#1214)

* fix rebase conflicts

* fix rebase conflicts

* feat: update validation

* fix rebase conflicts

* fix rebase conflicts

* test: update

* fix rebase conflicts

* fix rebase conflicts

* fix rebase conflicts

* fix rebase conflicts

* fix rebase conflicts

* implement before validation check to ensure that if the user declares no mortgage will be used, mortgage value is set to 0

* fix rebase conflicts

* fix rebase conflicts

* remove comment

* fix rebase conflicts

* Update config/locales/en.yml

Co-authored-by: James Rose <james@jbpr.net>

* alter order of before validation methods on sales log so that derived fields are calculated after invalidated dependent fields are cleared, fix tests broken by this change

* add mortgage to derived variables so that mortgage value is set to 0 if a mortgage is not being used in the sale

* remove hard validation of range for discount question and allow shared validation of numeric questions to handle this case

* amend tests after rebasing

* fix tests after rebasing

* fix test and ensure schema correct after rebase

---------

Co-authored-by: natdeanlewissoftwire <nat.dean-lewis@softwire.com>
Co-authored-by: James Rose <james@jbpr.net>

app/models/derived_variables/sales_log_variables.rb

@@ -15,6 +15,9 @@ module DerivedVariables::SalesLogVariables
     if mscharge_known.present? && mscharge_known.zero?
       self.mscharge = 0
     end
+    if mortgage_not_used?
+      self.mortgage = 0
+    end
     self.pcode1, self.pcode2 = postcode_full.split(" ") if postcode_full.present?
     self.totchild = total_child
     self.totadult = total_adult + total_elder

app/models/form/sales/pages/deposit_and_mortgage_value_check.rb

@@ -0,0 +1,17 @@
+class Form::Sales::Pages::DepositAndMortgageValueCheck < ::Form::Page
+  def initialize(id, hsh, subsection)
+    super
+    @depends_on = [
+      {
+        "mortgage_plus_deposit_less_than_discounted_value?" => true,
+      },
+    ]
+    @informative_text = {}
+  end
+
+  def questions
+    @questions ||= [
+      Form::Sales::Questions::DepositAndMortgageValueCheck.new(nil, nil, self),
+    ]
+  end
+end

app/models/form/sales/questions/deposit_and_mortgage_value_check.rb

@@ -0,0 +1,23 @@
+class Form::Sales::Questions::DepositAndMortgageValueCheck < ::Form::Question
+  def initialize(id, hsh, page)
+    super
+    @id = "deposit_and_mortgage_value_check"
+    @check_answer_label = "Deposit and mortgage against discount confirmation"
+    @header = "Are you sure? Mortgage and deposit usually equal or are more than (value - discount)"
+    @type = "interruption_screen"
+    @answer_options = {
+      "0" => { "value" => "Yes" },
+      "1" => { "value" => "No" },
+    }
+    @hidden_in_check_answers = {
+      "depends_on" => [
+        {
+          "deposit_and_mortgage_value_check" => 0,
+        },
+        {
+          "deposit_and_mortgage_value_check" => 1,
+        },
+      ],
+    }
+  end
+end

app/models/form/sales/subsections/discounted_ownership_scheme.rb

@@ -14,9 +14,11 @@ class Form::Sales::Subsections::DiscountedOwnershipScheme < ::Form::Subsection
       Form::Sales::Pages::AboutPriceNotRtb.new(nil, nil, self),
       Form::Sales::Pages::GrantValueCheck.new(nil, nil, self),
       Form::Sales::Pages::PurchasePrice.new("purchase_price_discounted_ownership", nil, self),
+      Form::Sales::Pages::DepositAndMortgageValueCheck.new("discounted_ownership_deposit_and_mortgage_value_check_after_value_and_discount", nil, self),
       Form::Sales::Pages::Mortgageused.new("mortgage_used_discounted_ownership", nil, self),
       Form::Sales::Pages::MortgageAmount.new("mortgage_amount_discounted_ownership", nil, self),
       Form::Sales::Pages::ExtraBorrowingValueCheck.new("extra_borrowing_mortgage_value_check", nil, self),
+      Form::Sales::Pages::DepositAndMortgageValueCheck.new("discounted_ownership_deposit_and_mortgage_value_check_after_mortgage", nil, self),
       Form::Sales::Pages::MortgageLender.new("mortgage_lender_discounted_ownership", nil, self),
       Form::Sales::Pages::MortgageLenderOther.new("mortgage_lender_other_discounted_ownership", nil, self),
       Form::Sales::Pages::MortgageLength.new("mortgage_length_discounted_ownership", nil, self),
@@ -25,6 +27,7 @@ class Form::Sales::Subsections::DiscountedOwnershipScheme < ::Form::Subsection
       Form::Sales::Pages::AboutDepositWithoutDiscount.new("about_deposit_discounted_ownership", nil, self),
       Form::Sales::Pages::ExtraBorrowingValueCheck.new("extra_borrowing_deposit_value_check", nil, self),
       Form::Sales::Pages::DepositValueCheck.new("discounted_ownership_deposit_value_check", nil, self),
+      Form::Sales::Pages::DepositAndMortgageValueCheck.new("discounted_ownership_deposit_and_mortgage_value_check_after_deposit", nil, self),
       Form::Sales::Pages::LeaseholdCharges.new("leasehold_charges_discounted_ownership", nil, self),
     ]
   end

app/models/log.rb

@@ -108,7 +108,6 @@ private
     return unless form
 
     form.reset_not_routed_questions(self)
-
     reset_created_by!
   end
 

app/models/sales_log.rb

@@ -23,12 +23,12 @@ class SalesLog < Log
   has_paper_trail
 
   validates_with SalesLogValidator
-  before_validation :set_derived_fields!
   before_validation :reset_invalidated_dependent_fields!
   before_validation :process_postcode_changes!, if: :postcode_full_changed?
   before_validation :process_previous_postcode_changes!, if: :ppostcode_full_changed?
   before_validation :reset_location_fields!, unless: :postcode_known?
   before_validation :reset_previous_location_fields!, unless: :previous_postcode_known?
+  before_validation :set_derived_fields!
 
   scope :filter_by_year, ->(year) { where(saledate: Time.zone.local(year.to_i, 4, 1)...Time.zone.local(year.to_i + 1, 4, 1)) }
   scope :search_by, ->(param) { filter_by_id(param) }

app/models/validations/sales/soft_validations.rb

@@ -53,6 +53,13 @@ module Validations::Sales::SoftValidations
     mortgage_value + deposit + cash_discount != value * equity / 100
   end
 
+  def mortgage_plus_deposit_less_than_discounted_value?
+    return unless mortgage && deposit && value && discount
+
+    discounted_value = value * (100 - discount) / 100
+    mortgage + deposit < discounted_value
+  end
+
   def hodate_3_years_or_more_saledate?
     return unless hodate && saledate
 

db/migrate/20230120163049_add_deposit_and_mortgage_value_check_to_sales_logs.rb

@@ -0,0 +1,5 @@
+class AddDepositAndMortgageValueCheckToSalesLogs < ActiveRecord::Migration[7.0]
+  def change
+    add_column :sales_logs, :deposit_and_mortgage_value_check, :integer
+  end
+end

db/schema.rb

@@ -487,9 +487,9 @@ ActiveRecord::Schema[7.0].define(version: 2023_01_26_145529) do
     t.integer "hoyear"
     t.integer "fromprop"
     t.integer "socprevten"
-    t.integer "mortlen"
     t.integer "mortgagelender"
     t.string "mortgagelenderother"
+    t.integer "mortlen"
     t.integer "extrabor"
     t.integer "hhmemb"
     t.integer "totadult"
@@ -502,13 +502,13 @@ ActiveRecord::Schema[7.0].define(version: 2023_01_26_145529) do
     t.boolean "is_la_inferred"
     t.bigint "bulk_upload_id"
     t.integer "retirement_value_check"
-    t.integer "deposit_and_mortgage_value_check"
-    t.integer "grant_value_check"
     t.integer "hodate_check"
     t.integer "extrabor_value_check"
+    t.integer "deposit_and_mortgage_value_check"
+    t.integer "shared_ownership_deposit_value_check"
+    t.integer "grant_value_check"
     t.integer "old_persons_shared_ownership_value_check"
     t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id"
-    t.integer "shared_ownership_deposit_value_check"
     t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id"
     t.index ["owning_organisation_id"], name: "index_sales_logs_on_owning_organisation_id"
     t.index ["updated_by_id"], name: "index_sales_logs_on_updated_by_id"

spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb

@@ -20,9 +20,11 @@ RSpec.describe Form::Sales::Subsections::DiscountedOwnershipScheme, type: :model
         about_price_not_rtb
         grant_value_check
         purchase_price_discounted_ownership
+        discounted_ownership_deposit_and_mortgage_value_check_after_value_and_discount
         mortgage_used_discounted_ownership
         mortgage_amount_discounted_ownership
         extra_borrowing_mortgage_value_check
+        discounted_ownership_deposit_and_mortgage_value_check_after_mortgage
         mortgage_lender_discounted_ownership
         mortgage_lender_other_discounted_ownership
         mortgage_length_discounted_ownership
@@ -31,6 +33,7 @@ RSpec.describe Form::Sales::Subsections::DiscountedOwnershipScheme, type: :model
         about_deposit_discounted_ownership
         extra_borrowing_deposit_value_check
         discounted_ownership_deposit_value_check
+        discounted_ownership_deposit_and_mortgage_value_check_after_deposit
         leasehold_charges_discounted_ownership
       ],
     )

spec/models/form_handler_spec.rb

@@ -52,14 +52,14 @@ RSpec.describe FormHandler do
     it "is able to load a current sales form" do
       form = form_handler.get_form("current_sales")
       expect(form).to be_a(Form)
-      expect(form.pages.count).to eq(194)
+      expect(form.pages.count).to eq(197)
       expect(form.name).to eq("2022_2023_sales")
     end
 
     it "is able to load a previous sales form" do
       form = form_handler.get_form("previous_sales")
       expect(form).to be_a(Form)
-      expect(form.pages.count).to eq(194)
+      expect(form.pages.count).to eq(197)
       expect(form.name).to eq("2021_2022_sales")
     end
   end

spec/models/sales_log_spec.rb

@@ -109,7 +109,7 @@ RSpec.describe SalesLog, type: :model do
     let(:sales_log) { FactoryBot.create(:sales_log, :completed) }
 
     it "correctly derives and saves exday, exmonth and exyear" do
-      sales_log.update!(exdate: Time.gm(2022, 5, 4))
+      sales_log.update!(exdate: Time.gm(2022, 5, 4), saledate: Time.gm(2022, 7, 4), ownershipsch: 1, staircase: 2, resale: 2)
       record_from_db = ActiveRecord::Base.connection.execute("select exday, exmonth, exyear from sales_logs where id=#{sales_log.id}").to_a[0]
       expect(record_from_db["exday"]).to eq(4)
       expect(record_from_db["exmonth"]).to eq(5)
@@ -140,6 +140,14 @@ RSpec.describe SalesLog, type: :model do
       expect(record_from_db["pcode1"]).to eq("W6")
       expect(record_from_db["pcode2"]).to eq("0SP")
     end
+
+    it "derives a mortgage value of 0 when mortgage is not used" do
+      # to avoid log failing validations when mortgage value is removed:
+      new_grant_value = sales_log.grant + sales_log.mortgage
+      sales_log.update!(mortgageused: 2, grant: new_grant_value)
+      record_from_db = ActiveRecord::Base.connection.execute("select mortgage from sales_logs where id=#{sales_log.id}").to_a[0]
+      expect(record_from_db["mortgage"]).to eq(0.0)
+    end
   end
 
   context "when saving addresses" do
@@ -238,39 +246,49 @@ RSpec.describe SalesLog, type: :model do
   end
 
   context "when deriving household variables" do
-    let!(:household_lettings_log) do
+    let!(:sales_log) do
-      described_class.create!({
+      FactoryBot.create(
+        :sales_log,
+        :completed,
         jointpur: 1,
-        hholdcount: 3,
+        hholdcount: 4,
+        details_known_1: 1,
+        details_known_2: 1,
+        details_known_3: 1,
+        details_known_4: 1,
         relat2: "C",
         relat3: "C",
         relat4: "X",
         relat5: "X",
-        age1: 22,
+        relat6: "P",
-        age2: 40,
+        ecstat2: 9,
-        age3: 19,
+        ecstat3: 7,
+        age1: 47,
+        age2: 14,
+        age3: 17,
         age4: 88,
-        age5: 14,
+        age5: 19,
-      })
+        age6: 46,
+      )
     end
 
     it "correctly derives and saves hhmemb" do
-      record_from_db = ActiveRecord::Base.connection.execute("select hhmemb from sales_logs where id=#{household_lettings_log.id}").to_a[0]
+      record_from_db = ActiveRecord::Base.connection.execute("select hhmemb from sales_logs where id=#{sales_log.id}").to_a[0]
-      expect(record_from_db["hhmemb"]).to eq(5)
+      expect(record_from_db["hhmemb"]).to eq(6)
     end
 
     it "correctly derives and saves totchild" do
-      record_from_db = ActiveRecord::Base.connection.execute("select totchild from sales_logs where id=#{household_lettings_log.id}").to_a[0]
+      record_from_db = ActiveRecord::Base.connection.execute("select totchild from sales_logs where id=#{sales_log.id}").to_a[0]
       expect(record_from_db["totchild"]).to eq(2)
     end
 
     it "correctly derives and saves totadult" do
-      record_from_db = ActiveRecord::Base.connection.execute("select totadult from sales_logs where id=#{household_lettings_log.id}").to_a[0]
+      record_from_db = ActiveRecord::Base.connection.execute("select totadult from sales_logs where id=#{sales_log.id}").to_a[0]
-      expect(record_from_db["totadult"]).to eq(3)
+      expect(record_from_db["totadult"]).to eq(4)
     end
 
     it "correctly derives and saves hhtype" do
-      record_from_db = ActiveRecord::Base.connection.execute("select hhtype from sales_logs where id=#{household_lettings_log.id}").to_a[0]
+      record_from_db = ActiveRecord::Base.connection.execute("select hhtype from sales_logs where id=#{sales_log.id}").to_a[0]
       expect(record_from_db["hhtype"]).to eq(9)
     end
   end

spec/models/validations/sales/soft_validations_spec.rb

@@ -201,6 +201,59 @@ RSpec.describe Validations::Sales::SoftValidations do
       end
     end
 
+    context "when validating mortgage and deposit against discounted value" do
+      [
+        {
+          nil_field: "mortgage",
+          value: 500_000,
+          deposit: 10_000,
+          discount: 10,
+        },
+        {
+          nil_field: "value",
+          mortgage: 100_000,
+          deposit: 10_000,
+          discount: 10,
+        },
+        {
+          nil_field: "deposit",
+          value: 500_000,
+          mortgage: 100_000,
+          discount: 10,
+        },
+        {
+          nil_field: "discount",
+          value: 500_000,
+          mortgage: 100_000,
+          deposit: 10_000,
+        },
+      ].each do |test_case|
+        it "returns false if #{test_case[:nil_field]} is not present" do
+          record.value = test_case[:value]
+          record.mortgage = test_case[:mortgage]
+          record.deposit = test_case[:deposit]
+          record.discount = test_case[:discount]
+          expect(record).not_to be_mortgage_plus_deposit_less_than_discounted_value
+        end
+      end
+
+      it "returns false if the deposit and mortgage add up to the discounted value or more" do
+        record.value = 500_000
+        record.discount = 20
+        record.mortgage = 200_000
+        record.deposit = 200_000
+        expect(record).not_to be_mortgage_plus_deposit_less_than_discounted_value
+      end
+
+      it "returns true if the deposit and mortgage add up to less than the discounted value" do
+        record.value = 500_000
+        record.discount = 10
+        record.mortgage = 200_000
+        record.deposit = 200_000
+        expect(record).to be_mortgage_plus_deposit_less_than_discounted_value
+      end
+    end
+
     context "when validating extra borrowing" do
       it "returns false if extrabor not present" do
         record.mortgage = 50_000