Cldc 3452 remove dont know option from mortgage used when invalid (#2470)

* initial spec file refactor
remove create for build, extract form to own variable, remove non-valuable tests
run time from 0.29s to0.11s

* refactor to ensure that the questions ownershipsch instance variable and the value of ownershipsch on the log is the same in tests

* delete page test file which is not testing anything meaningful

* add error messages to the I18n yml

* update tests for the validations

* update validation logic

* update tests for the mortgage used question

* update the mortgage used question displayed answer options

* update bulk upload specs to reflect error message updates

* make use of brackets consistent within validation method

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

@@ -10,21 +10,24 @@ class Form::Sales::Questions::Mortgageused < ::Form::Question
     @question_number = QUESTION_NUMBER_FROM_YEAR_AND_OWNERSHIP.fetch(form.start_date.year, QUESTION_NUMBER_FROM_YEAR_AND_OWNERSHIP.max_by { |k, _v| k }.last)[ownershipsch]
   end
 
+  def displayed_answer_options(log, _user = nil)
+    if log.outright_sale? && log.saledate && !form.start_year_after_2024?
+      answer_options_without_dont_know
+    elsif log.stairowned == 100 || log.outright_sale?
+      ANSWER_OPTIONS
+    else
+      answer_options_without_dont_know
+    end
+  end
+
   ANSWER_OPTIONS = {
     "1" => { "value" => "Yes" },
     "2" => { "value" => "No" },
     "3" => { "value" => "Don’t know" },
   }.freeze
 
-  def displayed_answer_options(log, _user = nil)
-    if log.stairowned == 100 || @ownershipsch == 3
-      ANSWER_OPTIONS
-    else
-      {
-        "1" => { "value" => "Yes" },
-        "2" => { "value" => "No" },
-      }
-    end
+  def answer_options_without_dont_know
+    ANSWER_OPTIONS.reject { |key, _v| key == "3" }
   end
 
   QUESTION_NUMBER_FROM_YEAR_AND_OWNERSHIP = {

app/models/validations/sales/sale_information_validations.rb

@@ -147,15 +147,6 @@ module Validations::Sales::SaleInformationValidations
     end
   end
 
-  def validate_mortgage_used_and_stairbought(record)
-    return unless record.stairowned && record.mortgageused
-
-    if !record.stairowned_100? && record.mortgageused == 3
-      record.errors.add :stairowned, I18n.t("validations.sale_information.stairowned.mortgageused_dont_know")
-      record.errors.add :mortgageused, I18n.t("validations.sale_information.stairowned.mortgageused_dont_know")
-    end
-  end
-
   def check_non_staircasing_socialhomebuy_mortgage(record)
     return unless record.cashdis
 
@@ -237,16 +228,22 @@ module Validations::Sales::SaleInformationValidations
   end
 
   def validate_mortgage_used_dont_know(record)
-    return unless record.mortgageused == 3
+    return unless record.mortgage_use_unknown?
 
     if record.discounted_ownership_sale?
-      record.errors.add(:mortgageused, I18n.t("validations.invalid_option", question: "Was a mortgage used for the purchase of this property?"))
+      record.errors.add :mortgageused, I18n.t("validations.invalid_option", question: "was a mortgage used for the purchase of this property?")
     end
     if record.outright_sale? && record.saledate && !record.form.start_year_after_2024?
-      record.errors.add(:mortgageused, I18n.t("validations.invalid_option", question: "Was a mortgage used for the purchase of this property?"))
+      record.errors.add :mortgageused, I18n.t("validations.invalid_option", question: "was a mortgage used for the purchase of this property?")
+      record.errors.add :saledate, I18n.t("validations.financial.mortgage_used.year")
+    end
+    if record.shared_ownership_scheme? && record.is_not_staircasing?
+      record.errors.add :mortgageused, I18n.t("validations.invalid_option", question: "was a mortgage used for the purchase of this property?")
+      record.errors.add :staircase, I18n.t("validations.financial.mortgage_used.staircasing")
     end
-    if record.shared_ownership_scheme? && record.staircase && record.staircase != 1
-      record.errors.add(:mortgageused, I18n.t("validations.invalid_option", question: "Was a mortgage used for the purchase of this property?"))
+    if record.stairowned && !record.stairowned_100?
+      record.errors.add :stairowned, I18n.t("validations.sale_information.stairowned.mortgageused_dont_know")
+      record.errors.add :mortgageused, I18n.t("validations.sale_information.stairowned.mortgageused_dont_know")
     end
   end
 

config/locales/en.yml

@@ -445,6 +445,9 @@ en:
         over_max: "The maximum initial equity stake is %{max_equity}%"
         over_stairowned_minus_stairbought: "The initial equity stake is %{equity}% and the percentage owned in total minus the percentage bought is %{staircase_difference}%. In a staircasing transaction, the equity stake purchased cannot be larger than the percentage the %{buyer_owns} minus the percentage bought."
       mortgage: "Mortgage value cannot be £0 if a mortgage was used for the purchase of this property"
+      mortgage_used:
+        year: "You must answer either ‘yes’ or ‘no’ to the question ‘was a mortgage used’ for the selected year."
+        staircasing: "You must answer either ‘yes’ or ‘no’ to the question ‘was a mortgage used’ for staircasing transactions."
       shared_ownership_deposit: "The %{mortgage_deposit_and_discount_error_fields} added together is %{mortgage_deposit_and_discount_total}. The value times the equity percentage is %{value_times_equity}. These figures should be the same"
 
     household:

spec/models/form/sales/pages/mortgageused_spec.rb

@@ -1,29 +0,0 @@
-require "rails_helper"
-
-RSpec.describe Form::Sales::Pages::Mortgageused, type: :model do
-  subject(:page) { described_class.new(page_id, page_definition, subsection, ownershipsch: 1) }
-
-  let(:page_id) { "mortgage_used" }
-  let(:page_definition) { nil }
-  let(:subsection) { instance_double(Form::Subsection, form: instance_double(Form, start_date: Time.zone.local(2023, 4, 1))) }
-
-  it "has correct subsection" do
-    expect(page.subsection).to eq(subsection)
-  end
-
-  it "has correct questions" do
-    expect(page.questions.map(&:id)).to eq(%w[mortgageused])
-  end
-
-  it "has the correct id" do
-    expect(page.id).to eq("mortgage_used")
-  end
-
-  it "has the correct header" do
-    expect(page.header).to be_nil
-  end
-
-  it "has the correct description" do
-    expect(page.description).to be_nil
-  end
-end

spec/models/form/sales/questions/mortgageused_spec.rb

@@ -1,85 +1,106 @@
 require "rails_helper"
 
 RSpec.describe Form::Sales::Questions::Mortgageused, type: :model do
-  subject(:question) { described_class.new(question_id, question_definition, page, ownershipsch: 1) }
+  subject(:question) { described_class.new(question_id, question_definition, page, ownershipsch:) }
 
+  let(:ownershipsch) { 1 }
   let(:question_id) { nil }
   let(:question_definition) { nil }
-  let(:page) { instance_double(Form::Page, subsection: instance_double(Form::Subsection, form: instance_double(Form, start_date: Time.zone.local(2023, 4, 1)))) }
-  let(:log) { create(:sales_log) }
+  let(:form) { instance_double(Form, start_date: Time.zone.local(2024, 4, 1)) }
+  let(:page) { instance_double(Form::Page, subsection: instance_double(Form::Subsection, form:)) }
+  let(:stairowned) { nil }
+  let(:staircase) { nil }
+  let(:saledate) { Time.zone.today }
+  let(:log) { build(:sales_log, :in_progress, ownershipsch:, stairowned:, staircase:) }
 
-  it "has correct page" do
-    expect(question.page).to eq(page)
+  it "has the correct answer_options" do
+    expect(question.answer_options).to eq({
+      "1" => { "value" => "Yes" },
+      "2" => { "value" => "No" },
+      "3" => { "value" => "Don’t know" },
+    })
   end
 
-  it "has the correct id" do
-    expect(question.id).to eq("mortgageused")
-  end
+  describe "the displayed answer options" do
+    context "when it is a discounted ownership sale" do
+      let(:ownershipsch) { 2 }
 
-  it "has the correct header" do
-    expect(question.header).to eq("Was a mortgage used for the purchase of this property?")
+      it "does not show the don't know option" do
+        expect_the_question_not_to_show_dont_know
       end
-
-  it "has the correct check_answer_label" do
-    expect(question.check_answer_label).to eq("Mortgage used")
     end
 
-  it "has the correct type" do
-    expect(question.type).to eq("radio")
+    context "when it is an outright sale" do
+      let(:ownershipsch) { 3 }
+
+      context "and the saledate is before 24/25" do
+        before do
+          allow(form).to receive(:start_year_after_2024?).and_return false
         end
 
-  it "is not marked as derived" do
-    expect(question.derived?(nil)).to be false
+        it "does not show the don't know option" do
+          expect_the_question_not_to_show_dont_know
+        end
       end
 
-  it "has the correct answer_options" do
-    expect(question.answer_options).to eq({
-      "1" => { "value" => "Yes" },
-      "2" => { "value" => "No" },
-      "3" => { "value" => "Don’t know" },
-    })
+      context "and the saledate is 24/25 or after" do
+        before do
+          allow(form).to receive(:start_year_after_2024?).and_return true
         end
 
-  it "has correct conditional for" do
-    expect(question.conditional_for).to eq(nil)
+        it "shows the don't know option" do
+          expect_the_question_to_show_dont_know
+        end
+      end
     end
 
-  it "has the correct hint" do
-    expect(question.hint_text).to be_nil
+    context "when it is a shared ownership scheme" do
+      let(:ownershipsch) { 1 }
+
+      context "and it is a staircasing transaction" do
+        let(:staircase) { 1 }
+
+        context "and stairowned is less that 100" do
+          let(:stairowned) { 50 }
+
+          it "does not show the don't know option" do
+            expect_the_question_not_to_show_dont_know
+          end
         end
 
-  context "when staircase owned percentage is 100%" do
-    let(:log) { build(:sales_log, stairowned: 100) }
+        context "and stairowned is 100" do
+          let(:stairowned) { 100 }
 
           it "shows the don't know option" do
-      expect(question.displayed_answer_options(log)).to eq({
-        "1" => { "value" => "Yes" },
-        "2" => { "value" => "No" },
-        "3" => { "value" => "Don’t know" },
-      })
+            expect_the_question_to_show_dont_know
+          end
         end
       end
 
-  context "when an outright sale" do
-    subject(:question) { described_class.new(question_id, question_definition, page, ownershipsch: 3) }
+      context "and it is not a staircasing transaction" do
+        let(:staircase) { 2 }
 
-    it "shows the don't know option" do
+        it "does not show the don't know option" do
+          expect_the_question_not_to_show_dont_know
+        end
+      end
+    end
+  end
+
+private
+
+  def expect_the_question_not_to_show_dont_know
     expect(question.displayed_answer_options(log)).to eq({
       "1" => { "value" => "Yes" },
       "2" => { "value" => "No" },
-        "3" => { "value" => "Don’t know" },
     })
   end
-  end
-
-  context "when staircase owned percentage is less than 100%" do
-    let(:log) { build(:sales_log, stairowned: 99) }
 
-    it "shows the don't know option" do
+  def expect_the_question_to_show_dont_know
     expect(question.displayed_answer_options(log)).to eq({
       "1" => { "value" => "Yes" },
       "2" => { "value" => "No" },
+      "3" => { "value" => "Don’t know" },
     })
   end
-  end
 end

spec/models/validations/sales/sale_information_validations_spec.rb

@@ -1344,47 +1344,100 @@ RSpec.describe Validations::Sales::SaleInformationValidations do
     end
   end
 
-  describe "#validate_mortgage_used_and_stairbought" do
-    let(:now) { Time.zone.local(2024, 4, 4) }
+  describe "#validate_mortgage_used_dont_know" do
+    let(:staircase) { nil }
+    let(:stairowned) { nil }
+    let(:saledate) { nil }
+    let(:sales_log) { build(:sales_log, ownershipsch:, mortgageused:, staircase:, stairowned:, saledate:) }
 
     before do
-      Timecop.freeze(now)
-      Singleton.__init__(FormHandler)
+      sale_information_validator.validate_mortgage_used_dont_know(sales_log)
     end
 
-    after do
-      Timecop.return
-      Singleton.__init__(FormHandler)
+    context "when mortgageused is don't know" do
+      let(:mortgageused) { 3 }
+
+      context "and it is a discounted ownership sale" do
+        let(:ownershipsch) { 2 }
+
+        it "adds an error" do
+          expect(sales_log.errors[:mortgageused]).to include "Enter a valid value for was a mortgage used for the purchase of this property?"
+        end
       end
 
-    context "when mortgageused don't know" do
-      let(:record) { build(:sales_log, ownershipsch: 1, type: 9, saledate: now, mortgageused: 3) }
+      context "and it is an outright sale" do
+        let(:ownershipsch) { 3 }
 
-      it "does not add an error if stairowned 100" do
-        record.stairowned = 100
-        sale_information_validator.validate_mortgage_used_and_stairbought(record)
+        context "with a saledate before 24/25" do
+          let(:saledate) { Time.zone.local(2023, 9, 9) }
 
-        expect(record.errors).to be_empty
+          it "adds errors" do
+            expect(sales_log.errors[:mortgageused]).to include "Enter a valid value for was a mortgage used for the purchase of this property?"
+            expect(sales_log.errors[:saledate]).to include "You must answer either ‘yes’ or ‘no’ to the question ‘was a mortgage used’ for the selected year."
+          end
         end
 
-      it "adds an error if stairowned is not 100" do
-        record.stairowned = 90
-        sale_information_validator.validate_mortgage_used_and_stairbought(record)
+        context "with a saledate from 24/25 or after" do
+          let(:saledate) { Time.zone.today }
 
-        expect(record.errors[:stairowned]).to include("The percentage owned has to be 100% if the mortgage used is 'Don’t know'")
-        expect(record.errors[:mortgageused]).to include("The percentage owned has to be 100% if the mortgage used is 'Don’t know'")
+          it "does not add any errors" do
+            expect(sales_log.errors).to be_empty
+          end
         end
       end
 
-    context "when the collection year is before 2024" do
-      let(:record) { build(:sales_log, ownershipsch: 1, type: 9, saledate: now, mortgageused: 3, stairowned: 90) }
-      let(:now) { Time.zone.local(2023, 4, 4) }
+      context "and it is a shared ownership scheme sale" do
+        let(:ownershipsch) { 1 }
 
-      it "adds an error" do
-        sale_information_validator.validate_mortgage_used_and_stairbought(record)
+        context "and a staircasing transaction" do
+          let(:staircase) { 1 }
+
+          context "and stairowned is nil" do
+            let(:stairowned) { nil }
+
+            it "does not add an error" do
+              expect(sales_log.errors).to be_empty
+            end
+          end
 
-        expect(record.errors[:stairowned]).to include("The percentage owned has to be 100% if the mortgage used is 'Don’t know'")
-        expect(record.errors[:mortgageused]).to include("The percentage owned has to be 100% if the mortgage used is 'Don’t know'")
+          context "and stairowned is less than 100" do
+            let(:stairowned) { 50 }
+
+            it "adds errors" do
+              expect(sales_log.errors[:mortgageused]).to include "The percentage owned has to be 100% if the mortgage used is 'Don’t know'"
+              expect(sales_log.errors[:stairowned]).to include "The percentage owned has to be 100% if the mortgage used is 'Don’t know'"
+            end
+          end
+
+          context "and stairowned is 100" do
+            let(:stairowned) { 100 }
+
+            it "does not add an error" do
+              expect(sales_log.errors).to be_empty
+            end
+          end
+        end
+
+        context "and not a staircasing transaction" do
+          let(:staircase) { 2 }
+
+          it "adds errors" do
+            expect(sales_log.errors[:mortgageused]).to include "Enter a valid value for was a mortgage used for the purchase of this property?"
+            expect(sales_log.errors[:staircase]).to include "You must answer either ‘yes’ or ‘no’ to the question ‘was a mortgage used’ for staircasing transactions."
+          end
+        end
+      end
+    end
+
+    context "when mortgageused is not don't know" do
+      let(:mortgageused) { 1 }
+
+      context "and it is a discounted ownership sale" do
+        let(:ownershipsch) { 2 }
+
+        it "does not add an error" do
+          expect(sales_log.errors).to be_empty
+        end
       end
     end
   end

spec/services/bulk_upload/sales/year2023/row_parser_spec.rb

@@ -1075,7 +1075,7 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do
           let(:attributes) { setup_section_params.merge(field_105: "3", field_87: "2", field_88: "50", field_89: nil, field_111: nil) }
 
           it "returns correct errors" do
-            expect(parser.errors[:field_105]).to include("Enter a valid value for Was a mortgage used for the purchase of this property?")
+            expect(parser.errors[:field_105]).to include("Enter a valid value for was a mortgage used for the purchase of this property?")
             parser.log.blank_invalid_non_setup_fields!
             parser.log.save!
             expect(parser.log.mortgageused).to be_nil
@@ -1086,7 +1086,7 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do
           let(:attributes) { setup_section_params.merge(field_105: "3", field_87: "2", field_88: "50", field_89: "100", field_111: nil) }
 
           it "returns correct errors" do
-            expect(parser.errors[:field_105]).to include("Enter a valid value for Was a mortgage used for the purchase of this property?")
+            expect(parser.errors[:field_105]).to include("Enter a valid value for was a mortgage used for the purchase of this property?")
             parser.log.blank_invalid_non_setup_fields!
             parser.log.save!
             expect(parser.log.mortgageused).to be_nil
@@ -1099,7 +1099,7 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do
       let(:attributes) { valid_attributes.merge({ field_7: "2", field_9: "8", field_119: "3" }) }
 
       it "does not allow 3 (don't know) as an option for discounted ownership" do
-        expect(parser.errors[:field_119]).to include("Enter a valid value for Was a mortgage used for the purchase of this property?")
+        expect(parser.errors[:field_119]).to include("Enter a valid value for was a mortgage used for the purchase of this property?")
         parser.log.blank_invalid_non_setup_fields!
         parser.log.save!
         expect(parser.log.mortgageused).to be_nil
@@ -1120,7 +1120,7 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do
       let(:attributes) { valid_attributes.merge({ field_7: "3", field_10: "10", field_128: "3", field_12: "2" }) }
 
       it "does not allow 3 (don't know) as an option for outright sale" do
-        expect(parser.errors[:field_128]).to include("Enter a valid value for Was a mortgage used for the purchase of this property?")
+        expect(parser.errors[:field_128]).to include("Enter a valid value for was a mortgage used for the purchase of this property?")
         parser.log.blank_invalid_non_setup_fields!
         parser.log.save!
         expect(parser.log.mortgageused).to be_nil

spec/services/bulk_upload/sales/year2024/row_parser_spec.rb

@@ -1156,8 +1156,8 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
         let(:attributes) { setup_section_params.merge(field_103: "3", field_86: "1", field_87: "50", field_88: nil, field_109: nil) }
 
         it "does not add errors" do
-          expect(parser.errors[:field_103]).not_to include("The percentage owned has to be 100% if the mortgage used is 'Don’t know'")
-          expect(parser.errors[:field_103]).not_to include("Enter a valid value for Was a mortgage used for the purchase of this property?")
+          expect(parser.errors[:field_103]).to be_empty
+          expect(parser.errors[:field_103]).to be_empty
         end
       end
 
@@ -1166,7 +1166,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
           let(:attributes) { setup_section_params.merge(field_103: "3", field_86: "2", field_87: "50", field_88: nil, field_109: nil) }
 
           it "returns correct errors" do
-            expect(parser.errors[:field_103]).to include("Enter a valid value for Was a mortgage used for the purchase of this property?")
+            expect(parser.errors[:field_103]).to include("Enter a valid value for was a mortgage used for the purchase of this property?")
             parser.log.blank_invalid_non_setup_fields!
             parser.log.save!
             expect(parser.log.mortgageused).to be_nil
@@ -1177,7 +1177,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
           let(:attributes) { setup_section_params.merge(field_103: "3", field_86: "2", field_87: "50", field_88: "100", field_109: nil) }
 
           it "returns correct errors" do
-            expect(parser.errors[:field_103]).to include("Enter a valid value for Was a mortgage used for the purchase of this property?")
+            expect(parser.errors[:field_103]).to include("Enter a valid value for was a mortgage used for the purchase of this property?")
             parser.log.blank_invalid_non_setup_fields!
             parser.log.save!
             expect(parser.log.mortgageused).to be_nil
@@ -1228,7 +1228,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
       let(:attributes) { valid_attributes.merge({ field_8: "2", field_10: "9", field_117: "3" }) }
 
       it "does not allow 3 (don't know) as an option for discounted ownership" do
-        expect(parser.errors[:field_117]).to include("Enter a valid value for Was a mortgage used for the purchase of this property?")
+        expect(parser.errors[:field_117]).to include("Enter a valid value for was a mortgage used for the purchase of this property?")
         parser.log.blank_invalid_non_setup_fields!
         parser.log.save!
         expect(parser.log.mortgageused).to be_nil