From 81765d857df0950ad6d391a35d5feeb0dec53d15 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Mon, 20 Mar 2023 11:23:15 +0000
Subject: [PATCH 1/4] CLDC-2143 Address import validations (#1425)

* Do not run discounted ownership validation if mortgageused is don't know

* Clear previuos postcode if import hits postcodes_not_matching error

* Remove child income validation for 22/23

* Remove invalid exchange date

* Clear income 1 if it's over allowed non london threshold
---
 .../sales/financial_validations.rb            |  4 +-
 .../sales/sale_information_validations.rb     |  4 +-
 .../imports/sales_logs_import_service.rb      | 21 ++++--
 .../sales/financial_validations_spec.rb       | 10 +++
 .../imports/sales_logs_import_service_spec.rb | 68 ++++++++++++++++---
 5 files changed, 89 insertions(+), 18 deletions(-)

diff --git a/app/models/validations/sales/financial_validations.rb b/app/models/validations/sales/financial_validations.rb
index afb8593a8..faf436afa 100644
--- a/app/models/validations/sales/financial_validations.rb
+++ b/app/models/validations/sales/financial_validations.rb
@@ -9,7 +9,7 @@ module Validations::Sales::FinancialValidations
     if record.london_property? && record.income1 > 90_000
       relevant_fields.each { |field| record.errors.add field, I18n.t("validations.financial.income.over_hard_max_for_london") }
     elsif record.property_not_in_london? && record.income1 > 80_000
-      relevant_fields.each { |field| record.errors.add field, I18n.t("validations.financial.income.over_hard_max_for_outside_london") }
+      relevant_fields.each { |field| record.errors.add field, :over_hard_max_for_outside_london, message: I18n.t("validations.financial.income.over_hard_max_for_outside_london") }
     end
   end
 
@@ -70,7 +70,7 @@ module Validations::Sales::FinancialValidations
   def validate_child_income(record)
     return unless record.income2 && record.ecstat2
 
-    if record.income2.positive? && is_economic_status_child?(record.ecstat2)
+    if record.income2.positive? && is_economic_status_child?(record.ecstat2) && record.form.start_date.year >= 2023
       record.errors.add :ecstat2, I18n.t("validations.financial.income.child_has_income")
       record.errors.add :income2, I18n.t("validations.financial.income.child_has_income")
     end
diff --git a/app/models/validations/sales/sale_information_validations.rb b/app/models/validations/sales/sale_information_validations.rb
index a1deb94f5..ae3bbfe8f 100644
--- a/app/models/validations/sales/sale_information_validations.rb
+++ b/app/models/validations/sales/sale_information_validations.rb
@@ -30,7 +30,7 @@ module Validations::Sales::SaleInformationValidations
     end
 
     if record.saledate - record.exdate >= 1.year
-      record.errors.add :exdate, I18n.t("validations.sale_information.exdate.must_be_less_than_1_year_from_saledate")
+      record.errors.add :exdate, :over_a_year_from_saledate, message: I18n.t("validations.sale_information.exdate.must_be_less_than_1_year_from_saledate")
       record.errors.add :saledate, I18n.t("validations.sale_information.saledate.must_be_less_than_1_year_from_exdate")
     end
   end
@@ -46,7 +46,7 @@ module Validations::Sales::SaleInformationValidations
 
   def validate_discounted_ownership_value(record)
     return unless record.value && record.deposit && record.ownershipsch
-    return unless record.mortgage || record.mortgageused == 2 || record.mortgageused == 3
+    return unless record.mortgage || record.mortgageused == 2
     return unless record.discount || record.grant || record.type == 29
 
     discount_amount = record.discount ? record.value * record.discount / 100 : 0
diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb
index 577e37e5f..c5a49ec9d 100644
--- a/app/services/imports/sales_logs_import_service.rb
+++ b/app/services/imports/sales_logs_import_service.rb
@@ -191,11 +191,22 @@ module Imports
           attributes.delete(error.attribute.to_s)
         end
         @logs_overridden << sales_log.old_id
-        if sales_log.errors.of_kind?(:postcode_full, :postcodes_not_matching)
-          @logger.warn("Log #{sales_log.old_id}: Removing postcode known and previous postcode known as the postcodes are invalid")
-          attributes.delete("pcodenk")
-          attributes.delete("ppcodenk")
-        end
+        save_sales_log(attributes, previous_status)
+      elsif sales_log.errors.of_kind?(:postcode_full, :postcodes_not_matching)
+        @logger.warn("Log #{sales_log.old_id}: Removing previous postcode known and previous postcode as the postcode is invalid")
+        @logs_overridden << sales_log.old_id
+        attributes.delete("ppcodenk")
+        attributes.delete("ppostcode_full")
+        save_sales_log(attributes, previous_status)
+      elsif sales_log.errors.of_kind?(:exdate, :over_a_year_from_saledate)
+        @logger.warn("Log #{sales_log.old_id}: Removing exchange date as the exchange date is invalid")
+        @logs_overridden << sales_log.old_id
+        attributes.delete("exdate")
+        save_sales_log(attributes, previous_status)
+      elsif sales_log.errors.of_kind?(:income1, :over_hard_max_for_outside_london)
+        @logger.warn("Log #{sales_log.old_id}: Removing income1 as the income1 is invalid")
+        @logs_overridden << sales_log.old_id
+        attributes.delete("income1")
         save_sales_log(attributes, previous_status)
       else
         @logger.error("Log #{sales_log.old_id}: Failed to import")
diff --git a/spec/models/validations/sales/financial_validations_spec.rb b/spec/models/validations/sales/financial_validations_spec.rb
index 093c491a4..b60ef5e6a 100644
--- a/spec/models/validations/sales/financial_validations_spec.rb
+++ b/spec/models/validations/sales/financial_validations_spec.rb
@@ -251,6 +251,7 @@ RSpec.describe Validations::Sales::FinancialValidations do
 
     context "when buyer 2 is a child" do
       it "does not add an error if buyer 2 has no income" do
+        record.saledate = Time.zone.local(2023, 4, 3)
         record.ecstat2 = 9
         record.income2 = 0
         financial_validator.validate_child_income(record)
@@ -258,12 +259,21 @@ RSpec.describe Validations::Sales::FinancialValidations do
       end
 
       it "adds errors if buyer 2 has an income" do
+        record.saledate = Time.zone.local(2023, 4, 3)
         record.ecstat2 = 9
         record.income2 = 40_000
         financial_validator.validate_child_income(record)
         expect(record.errors["ecstat2"]).to include(match I18n.t("validations.financial.income.child_has_income"))
         expect(record.errors["income2"]).to include(match I18n.t("validations.financial.income.child_has_income"))
       end
+
+      it "does not add an error if the saledate is before the 23/24 collection window" do
+        record.saledate = Time.zone.local(2022, 4, 3)
+        record.ecstat2 = 9
+        record.income2 = 40_000
+        financial_validator.validate_child_income(record)
+        expect(record.errors).to be_empty
+      end
     end
   end
 
diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb
index 2481cdce7..3276e00b8 100644
--- a/spec/services/imports/sales_logs_import_service_spec.rb
+++ b/spec/services/imports/sales_logs_import_service_spec.rb
@@ -398,17 +398,12 @@ RSpec.describe Imports::SalesLogsImportService do
       let(:sales_log_id) { "discounted_ownership_sales_log" }
 
       before do
-        sales_log_xml.at_xpath("//meta:status").content = "submitted-invalid"
-        sales_log_xml.at_xpath("//xmlns:Q7Postcode").content = "A1 1AA"
-        sales_log_xml.at_xpath("//xmlns:Q14Postcode").content = "A1 2AA"
+        sales_log_xml.at_xpath("//xmlns:Q7Postcode").content = "A1 1AA" # previous postcode
+        sales_log_xml.at_xpath("//xmlns:Q14Postcode").content = "A1 2AA" # postcode
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Removing field postcode_full from log triggering validation: Buyer's last accommodation and discounted ownership postcodes must match/)
-        expect(logger).to receive(:warn).with(/Removing field ppostcode_full from log triggering validation: Buyer's last accommodation and discounted ownership postcodes must match/)
-        expect(logger).to receive(:warn).with(/Removing field postcode_full from log triggering validation: postcodes_not_matching/)
-        expect(logger).to receive(:warn).with(/Removing field ppostcode_full from log triggering validation: postcodes_not_matching/)
-        expect(logger).to receive(:warn).with(/Removing postcode known and previous postcode known as the postcodes are invalid/)
+        expect(logger).to receive(:warn).with(/Removing previous postcode known and previous postcode as the postcode is invalid/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -420,11 +415,66 @@ RSpec.describe Imports::SalesLogsImportService do
         sales_log = SalesLog.find_by(old_id: sales_log_id)
 
         expect(sales_log).not_to be_nil
-        expect(sales_log.postcode_full).to be_nil
+        expect(sales_log.postcode_full).to eq("A1 2AA")
         expect(sales_log.ppostcode_full).to be_nil
       end
     end
 
+    context "and it has an invalid record with invalid contracts exchange date" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:DAY").content = "1"
+        sales_log_xml.at_xpath("//xmlns:MONTH").content = "10"
+        sales_log_xml.at_xpath("//xmlns:YEAR").content = "2022"
+        sales_log_xml.at_xpath("//xmlns:EXDAY").content = "1"
+        sales_log_xml.at_xpath("//xmlns:EXMONTH").content = "4"
+        sales_log_xml.at_xpath("//xmlns:EXYEAR").content = "2020"
+      end
+
+      it "intercepts the relevant validation error" do
+        expect(logger).to receive(:warn).with(/Removing exchange date as the exchange date is invalid/)
+        expect { sales_log_service.send(:create_log, sales_log_xml) }
+          .not_to raise_error
+      end
+
+      it "clears out the invalid answers" do
+        allow(logger).to receive(:warn)
+
+        sales_log_service.send(:create_log, sales_log_xml)
+        sales_log = SalesLog.find_by(old_id: sales_log_id)
+
+        expect(sales_log).not_to be_nil
+        expect(sales_log.saledate).to eq(Time.zone.local(2022, 10, 1))
+        expect(sales_log.exdate).to be_nil
+      end
+    end
+
+    context "and it has an invalid income" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:Q2Person1Income").content = "85000"
+        sales_log_xml.at_xpath("//xmlns:Q14ONSLACode").content = "E07000223"
+      end
+
+      it "intercepts the relevant validation error" do
+        expect(logger).to receive(:warn).with(/Removing income1 as the income1 is invalid/)
+        expect { sales_log_service.send(:create_log, sales_log_xml) }
+          .not_to raise_error
+      end
+
+      it "clears out the invalid answers" do
+        allow(logger).to receive(:warn)
+
+        sales_log_service.send(:create_log, sales_log_xml)
+        sales_log = SalesLog.find_by(old_id: sales_log_id)
+
+        expect(sales_log).not_to be_nil
+        expect(sales_log.income1).to be_nil
+      end
+    end
+
     context "when inferring default answers for completed sales logs" do
       context "when the armedforcesspouse is not answered" do
         let(:sales_log_id) { "discounted_ownership_sales_log" }

From 0a74ee75629df33c51698155638d70fdaa619a9c Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Mon, 20 Mar 2023 12:49:54 +0000
Subject: [PATCH 2/4] Return correct type if no LAs found for location (#1439)

* Default to location code if LocalAuthority is not found by location code

* Set local authority before validation
---
 app/models/lettings_log.rb                 |  2 +-
 app/models/location.rb                     |  4 ++--
 spec/features/schemes_helpers.rb           |  6 ++++--
 spec/features/schemes_spec.rb              |  6 ++++--
 spec/models/lettings_log_spec.rb           | 13 +++++++++++++
 spec/models/location_spec.rb               | 13 +++++++++++++
 spec/requests/locations_controller_spec.rb |  4 ++--
 7 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb
index 764b23f9a..4524ead12 100644
--- a/app/models/lettings_log.rb
+++ b/app/models/lettings_log.rb
@@ -84,7 +84,7 @@ class LettingsLog < Log
 
   def la
     if location
-      location.linked_local_authorities.active(form.start_date).first&.code
+      location.linked_local_authorities.active(form.start_date).first&.code || location.location_code
     else
       super
     end
diff --git a/app/models/location.rb b/app/models/location.rb
index 7002ce5a9..48689ef71 100644
--- a/app/models/location.rb
+++ b/app/models/location.rb
@@ -14,7 +14,7 @@ class Location < ApplicationRecord
 
   has_paper_trail
 
-  before_save :lookup_postcode!, if: :postcode_changed?
+  before_validation :lookup_postcode!, if: :postcode_changed?
 
   auto_strip_attributes :name
 
@@ -132,7 +132,7 @@ class Location < ApplicationRecord
 
   def linked_local_authorities
     la = LocalAuthority.find_by(code: location_code)
-    return [] unless la
+    return LocalAuthority.none unless la
 
     LocalAuthority.where(id: [la.id] + la.linked_local_authority_ids)
   end
diff --git a/spec/features/schemes_helpers.rb b/spec/features/schemes_helpers.rb
index 4ce1063c1..d5e5a8e22 100644
--- a/spec/features/schemes_helpers.rb
+++ b/spec/features/schemes_helpers.rb
@@ -64,7 +64,8 @@ module SchemesHelpers
     click_button "Add a location"
     fill_in with: "AA11AA"
     click_button "Save and continue"
-    fill_in with: "Adur"
+    select "Adur"
+    click_button "Save and continue"
     fill_in with: "Some name"
     click_button "Save and continue"
     fill_in with: 5
@@ -84,7 +85,8 @@ module SchemesHelpers
     click_button "Add a location"
     fill_in with: "AA12AA"
     click_button "Save and continue"
-    fill_in with: "Adur"
+    select "Adur"
+    click_button "Save and continue"
     fill_in with: "Other name"
     click_button "Save and continue"
     fill_in with: 2
diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb
index fefa3ea56..15487fc53 100644
--- a/spec/features/schemes_spec.rb
+++ b/spec/features/schemes_spec.rb
@@ -284,7 +284,8 @@ RSpec.describe "Schemes scheme Features" do
                 before do
                   fill_in with: "AA12AA"
                   click_button "Save and continue"
-                  fill_in with: "Adur"
+                  select "Adur"
+                  click_button "Save and continue"
                   fill_in with: location_name
                   click_button "Save and continue"
                   fill_in with: 1
@@ -904,7 +905,8 @@ RSpec.describe "Schemes scheme Features" do
                 before do
                   fill_in with: "AA12AA"
                   click_button "Save and continue"
-                  fill_in with: "Adur"
+                  select "Adur"
+                  click_button "Save and continue"
                   fill_in with: location_name
                   click_button "Save and continue"
                   fill_in with: 1
diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb
index e2431ff92..57b5de21a 100644
--- a/spec/models/lettings_log_spec.rb
+++ b/spec/models/lettings_log_spec.rb
@@ -2033,6 +2033,19 @@ RSpec.describe LettingsLog do
             end
           end
         end
+
+        context "and the location no local authorities associated with the location_code" do
+          before do
+            location.update!(location_code: "E01231231")
+            lettings_log.update!(location:)
+          end
+
+          it "returns the correct la" do
+            expect(location.location_code).to eq("E01231231")
+            expect(lettings_log["location_id"]).to eq(location.id)
+            expect(lettings_log.la).to eq("E01231231")
+          end
+        end
       end
 
       context "and not renewal" do
diff --git a/spec/models/location_spec.rb b/spec/models/location_spec.rb
index ceb87121f..eded7d1ee 100644
--- a/spec/models/location_spec.rb
+++ b/spec/models/location_spec.rb
@@ -32,6 +32,19 @@ RSpec.describe Location, type: :model do
       expect(location.linked_local_authorities.active(Time.zone.local(2022, 4, 1)).first.code).to eq("E07000030")
       expect(location.linked_local_authorities.active(Time.zone.local(2023, 4, 1)).first.code).to eq("E06000063")
     end
+
+    context "when location_code is no in LocalAuthorities table" do
+      before do
+        stub_request(:get, /api.postcodes.io\/postcodes\/CA101AA/)
+          .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Eden","codes":{"admin_district":"E01231231"}}}', headers: {})
+      end
+
+      it "defaults for location code for la" do
+        location.update!(postcode: "CA10 1AA")
+        expect(location.linked_local_authorities.count).to eq(0)
+        expect(location.location_code).to eq("E01231231")
+      end
+    end
   end
 
   describe "#postcode" do
diff --git a/spec/requests/locations_controller_spec.rb b/spec/requests/locations_controller_spec.rb
index 3b75339ff..154d18735 100644
--- a/spec/requests/locations_controller_spec.rb
+++ b/spec/requests/locations_controller_spec.rb
@@ -417,7 +417,7 @@ RSpec.describe LocationsController, type: :request do
 
         it "redirects correctly when postcodes.io does return a local authority" do
           follow_redirect!
-          expect(page).to have_content("What is the name of this location?")
+          expect(page).to have_content("What is the local authority")
         end
       end
 
@@ -478,7 +478,7 @@ RSpec.describe LocationsController, type: :request do
 
         it "redirects correctly when postcodes.io does return a local authority" do
           follow_redirect!
-          expect(page).to have_content("What is the name of this location?")
+          expect(page).to have_content("What is the local authority")
         end
       end
 

From 867a70a3938099d0895d7ca2f72e8dd73be69f46 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Mon, 20 Mar 2023 15:02:57 +0000
Subject: [PATCH 3/4] CLDC-2143 Set default and optional missing import fields
 (#1443)

* Make mortlen optional for 22/23

* Set extrabor to don't know it is not answered

* Make mortgage lender and othtype optional

* Add hidden don't know options to soctenant and fromprop

* Make frombeds optional

* Display soctenant questions if soctenant is don't know

* infer soctenant values if not given
---
 .../form/sales/pages/previous_bedrooms.rb     | 11 +++--
 .../sales/pages/previous_property_type.rb     | 11 +++--
 .../form/sales/pages/previous_tenure.rb       | 11 +++--
 .../form/sales/questions/buyer_previous.rb    |  8 ++++
 app/models/form/sales/questions/fromprop.rb   | 11 +++++
 app/models/sales_log.rb                       | 16 ++++++-
 .../imports/sales_logs_import_service.rb      | 16 ++-----
 .../sales/pages/previous_bedrooms_spec.rb     | 11 +++--
 .../pages/previous_property_type_spec.rb      | 11 +++--
 .../form/sales/pages/previous_tenure_spec.rb  | 11 +++--
 .../sales/questions/buyer_previous_spec.rb    |  8 ++++
 .../form/sales/questions/fromprop_spec.rb     | 11 +++++
 spec/models/sales_log_spec.rb                 |  6 +++
 .../imports/sales_logs_import_service_spec.rb | 48 +++++++++++++++++++
 14 files changed, 159 insertions(+), 31 deletions(-)

diff --git a/app/models/form/sales/pages/previous_bedrooms.rb b/app/models/form/sales/pages/previous_bedrooms.rb
index 87632e305..722822c2d 100644
--- a/app/models/form/sales/pages/previous_bedrooms.rb
+++ b/app/models/form/sales/pages/previous_bedrooms.rb
@@ -3,9 +3,14 @@ class Form::Sales::Pages::PreviousBedrooms < ::Form::Page
     super
     @id = "previous_bedrooms"
     @header = "About the buyers’ previous property"
-    @depends_on = [{
-      "soctenant" => 1,
-    }]
+    @depends_on = [
+      {
+        "soctenant" => 1,
+      },
+      {
+        "soctenant" => 0,
+      },
+    ]
   end
 
   def questions
diff --git a/app/models/form/sales/pages/previous_property_type.rb b/app/models/form/sales/pages/previous_property_type.rb
index 8555fdc79..8c46963d0 100644
--- a/app/models/form/sales/pages/previous_property_type.rb
+++ b/app/models/form/sales/pages/previous_property_type.rb
@@ -5,9 +5,14 @@ class Form::Sales::Pages::PreviousPropertyType < ::Form::Page
     @header = ""
     @description = ""
     @subsection = subsection
-    @depends_on = [{
-      "soctenant" => 1,
-    }]
+    @depends_on = [
+      {
+        "soctenant" => 1,
+      },
+      {
+        "soctenant" => 0,
+      },
+    ]
   end
 
   def questions
diff --git a/app/models/form/sales/pages/previous_tenure.rb b/app/models/form/sales/pages/previous_tenure.rb
index a830898a0..0f4a4b250 100644
--- a/app/models/form/sales/pages/previous_tenure.rb
+++ b/app/models/form/sales/pages/previous_tenure.rb
@@ -5,9 +5,14 @@ class Form::Sales::Pages::PreviousTenure < ::Form::Page
     @header = ""
     @description = ""
     @subsection = subsection
-    @depends_on = [{
-      "soctenant" => 1,
-    }]
+    @depends_on = [
+      {
+        "soctenant" => 1,
+      },
+      {
+        "soctenant" => 0,
+      },
+    ]
   end
 
   def questions
diff --git a/app/models/form/sales/questions/buyer_previous.rb b/app/models/form/sales/questions/buyer_previous.rb
index 4a851a487..aaea81cfc 100644
--- a/app/models/form/sales/questions/buyer_previous.rb
+++ b/app/models/form/sales/questions/buyer_previous.rb
@@ -12,5 +12,13 @@ class Form::Sales::Questions::BuyerPrevious < ::Form::Question
   ANSWER_OPTIONS = {
     "1" => { "value" => "Yes" },
     "2" => { "value" => "No" },
+    "0" => { "value" => "Don’t know" },
   }.freeze
+
+  def displayed_answer_options(_log, _user = nil)
+    {
+      "1" => { "value" => "Yes" },
+      "2" => { "value" => "No" },
+    }
+  end
 end
diff --git a/app/models/form/sales/questions/fromprop.rb b/app/models/form/sales/questions/fromprop.rb
index 88abc6b1b..9d35a4ae3 100644
--- a/app/models/form/sales/questions/fromprop.rb
+++ b/app/models/form/sales/questions/fromprop.rb
@@ -17,5 +17,16 @@ class Form::Sales::Questions::Fromprop < ::Form::Question
     "3" => { "value" => "House" },
     "4" => { "value" => "Bungalow" },
     "9" => { "value" => "Other" },
+    "0" => { "value" => "Don’t know" },
   }.freeze
+
+  def displayed_answer_options(_log, _user = nil)
+    {
+      "1" => { "value" => "Flat or maisonette" },
+      "2" => { "value" => "Bedsit" },
+      "3" => { "value" => "House" },
+      "4" => { "value" => "Bungalow" },
+      "9" => { "value" => "Other" },
+    }
+  end
 end
diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb
index c7fe68bca..7385d8b43 100644
--- a/app/models/sales_log.rb
+++ b/app/models/sales_log.rb
@@ -42,7 +42,7 @@ class SalesLog < Log
   }
   scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org) }
 
-  OPTIONAL_FIELDS = %w[saledate_check purchid monthly_charges_value_check old_persons_shared_ownership_value_check].freeze
+  OPTIONAL_FIELDS = %w[saledate_check purchid monthly_charges_value_check old_persons_shared_ownership_value_check mortgagelender othtype].freeze
   RETIREMENT_AGES = { "M" => 65, "F" => 60, "X" => 65 }.freeze
 
   def lettings?
@@ -78,6 +78,8 @@ class SalesLog < Log
   def dynamically_not_required
     not_required = []
     not_required << "proplen" if proplen_optional?
+    not_required << "mortlen" if mortlen_optional?
+    not_required << "frombeds" if frombeds_optional?
 
     not_required |= %w[address_line2 county postcode_full] if saledate && saledate.year >= 2023
 
@@ -90,6 +92,18 @@ class SalesLog < Log
     collection_start_year < 2023
   end
 
+  def mortlen_optional?
+    return false unless collection_start_year
+
+    collection_start_year < 2023
+  end
+
+  def frombeds_optional?
+    return false unless collection_start_year
+
+    collection_start_year < 2023
+  end
+
   def not_started?
     status == "not_started"
   end
diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb
index c5a49ec9d..35c94eb15 100644
--- a/app/services/imports/sales_logs_import_service.rb
+++ b/app/services/imports/sales_logs_import_service.rb
@@ -124,7 +124,7 @@ module Imports
       attributes["mortgagelenderother"] = mortgage_lender_other(xml_doc, attributes)
       attributes["postcode_full"] = parse_postcode(string_or_nil(xml_doc, "Q14Postcode"))
       attributes["pcodenk"] = 0 if attributes["postcode_full"].present? # known if given
-      attributes["soctenant"] = soctenant(attributes)
+      attributes["soctenant"] = 0 if attributes["ownershipsch"] == 1
       attributes["ethnic_group2"] = nil # 23/24 variable
       attributes["ethnicbuy2"] = nil # 23/24 variable
       attributes["prevshared"] = nil # 23/24 variable
@@ -375,17 +375,6 @@ module Imports
       end
     end
 
-    def soctenant(attributes)
-      return nil unless attributes["ownershipsch"] == 1
-
-      if attributes["frombeds"].blank? && attributes["fromprop"].blank? && attributes["socprevten"].blank?
-        2
-      else
-        1
-      end
-      # NO (2) if FROMBEDS, FROMPROP and socprevten are blank, and YES(1) if they are completed
-    end
-
     def still_serving(xml_doc)
       case unsafe_string_as_integer(xml_doc, "LeftArmedF")
       when 4
@@ -521,6 +510,9 @@ module Imports
       end
       attributes["pcodenk"] ||= 1
       attributes["prevten"] ||= 0
+      attributes["extrabor"] ||= 3 if attributes["mortgageused"] == 1
+      attributes["socprevten"] ||= 10 if attributes["ownershipsch"] == 1
+      attributes["fromprop"] ||= 0 if attributes["ownershipsch"] == 1
 
       # buyer 1 characteristics
       attributes["age1_known"] ||= 1
diff --git a/spec/models/form/sales/pages/previous_bedrooms_spec.rb b/spec/models/form/sales/pages/previous_bedrooms_spec.rb
index 5e1e18a52..8e2d03a20 100644
--- a/spec/models/form/sales/pages/previous_bedrooms_spec.rb
+++ b/spec/models/form/sales/pages/previous_bedrooms_spec.rb
@@ -28,8 +28,13 @@ RSpec.describe Form::Sales::Pages::PreviousBedrooms, type: :model do
   end
 
   it "has correct depends_on" do
-    expect(page.depends_on).to eq([{
-      "soctenant" => 1,
-    }])
+    expect(page.depends_on).to eq([
+      {
+        "soctenant" => 1,
+      },
+      {
+        "soctenant" => 0,
+      },
+    ])
   end
 end
diff --git a/spec/models/form/sales/pages/previous_property_type_spec.rb b/spec/models/form/sales/pages/previous_property_type_spec.rb
index 4cfae3b1c..21b3b3c0b 100644
--- a/spec/models/form/sales/pages/previous_property_type_spec.rb
+++ b/spec/models/form/sales/pages/previous_property_type_spec.rb
@@ -28,8 +28,13 @@ RSpec.describe Form::Sales::Pages::PreviousPropertyType, type: :model do
   end
 
   it "has correct depends_on" do
-    expect(page.depends_on).to eq([{
-      "soctenant" => 1,
-    }])
+    expect(page.depends_on).to eq([
+      {
+        "soctenant" => 1,
+      },
+      {
+        "soctenant" => 0,
+      },
+    ])
   end
 end
diff --git a/spec/models/form/sales/pages/previous_tenure_spec.rb b/spec/models/form/sales/pages/previous_tenure_spec.rb
index a4892d567..2ee7d5c02 100644
--- a/spec/models/form/sales/pages/previous_tenure_spec.rb
+++ b/spec/models/form/sales/pages/previous_tenure_spec.rb
@@ -28,8 +28,13 @@ RSpec.describe Form::Sales::Pages::PreviousTenure, type: :model do
   end
 
   it "has correct depends_on" do
-    expect(page.depends_on).to eq([{
-      "soctenant" => 1,
-    }])
+    expect(page.depends_on).to eq([
+      {
+        "soctenant" => 1,
+      },
+      {
+        "soctenant" => 0,
+      },
+    ])
   end
 end
diff --git a/spec/models/form/sales/questions/buyer_previous_spec.rb b/spec/models/form/sales/questions/buyer_previous_spec.rb
index a1348f17b..06aaca38f 100644
--- a/spec/models/form/sales/questions/buyer_previous_spec.rb
+++ b/spec/models/form/sales/questions/buyer_previous_spec.rb
@@ -46,10 +46,18 @@ RSpec.describe Form::Sales::Questions::BuyerPrevious, type: :model do
     expect(question.derived?).to be false
   end
 
+  it "has the correct displayed_answer_options" do
+    expect(question.displayed_answer_options(nil)).to eq({
+      "1" => { "value" => "Yes" },
+      "2" => { "value" => "No" },
+    })
+  end
+
   it "has the correct answer_options" do
     expect(question.answer_options).to eq({
       "1" => { "value" => "Yes" },
       "2" => { "value" => "No" },
+      "0" => { "value" => "Don’t know" },
     })
   end
 
diff --git a/spec/models/form/sales/questions/fromprop_spec.rb b/spec/models/form/sales/questions/fromprop_spec.rb
index ebfa218c5..c9aaa60b9 100644
--- a/spec/models/form/sales/questions/fromprop_spec.rb
+++ b/spec/models/form/sales/questions/fromprop_spec.rb
@@ -35,6 +35,16 @@ RSpec.describe Form::Sales::Questions::Fromprop, type: :model do
     expect(question.hint_text).to eq("")
   end
 
+  it "has the correct displayed_answer_options" do
+    expect(question.displayed_answer_options(nil)).to eq({
+      "1" => { "value" => "Flat or maisonette" },
+      "2" => { "value" => "Bedsit" },
+      "3" => { "value" => "House" },
+      "4" => { "value" => "Bungalow" },
+      "9" => { "value" => "Other" },
+    })
+  end
+
   it "has the correct answer_options" do
     expect(question.answer_options).to eq({
       "1" => { "value" => "Flat or maisonette" },
@@ -42,6 +52,7 @@ RSpec.describe Form::Sales::Questions::Fromprop, type: :model do
       "3" => { "value" => "House" },
       "4" => { "value" => "Bungalow" },
       "9" => { "value" => "Other" },
+      "0" => { "value" => "Don’t know" },
     })
   end
 end
diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb
index 7f6c74fc4..890fa069f 100644
--- a/spec/models/sales_log_spec.rb
+++ b/spec/models/sales_log_spec.rb
@@ -59,7 +59,11 @@ RSpec.describe SalesLog, type: :model do
           purchid
           monthly_charges_value_check
           old_persons_shared_ownership_value_check
+          mortgagelender
+          othtype
           proplen
+          mortlen
+          frombeds
         ])
       end
     end
@@ -73,6 +77,8 @@ RSpec.describe SalesLog, type: :model do
           purchid
           monthly_charges_value_check
           old_persons_shared_ownership_value_check
+          mortgagelender
+          othtype
           address_line2
           county
           postcode_full
diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb
index 3276e00b8..a9c085abc 100644
--- a/spec/services/imports/sales_logs_import_service_spec.rb
+++ b/spec/services/imports/sales_logs_import_service_spec.rb
@@ -1176,6 +1176,54 @@ RSpec.describe Imports::SalesLogsImportService do
           expect(sales_log&.mortgageused).to eq(1)
         end
       end
+
+      context "when the extrabor is not answered" do
+        let(:sales_log_id) { "discounted_ownership_sales_log" }
+
+        before do
+          sales_log_xml.at_xpath("//xmlns:Q35Borrowing").content = ""
+          allow(logger).to receive(:warn).and_return(nil)
+        end
+
+        it "sets extrabor to don't know" do
+          sales_log_service.send(:create_log, sales_log_xml)
+
+          sales_log = SalesLog.find_by(old_id: sales_log_id)
+          expect(sales_log&.extrabor).to be(3)
+        end
+      end
+
+      context "when the fromprop is not answered" do
+        let(:sales_log_id) { "shared_ownership_sales_log" }
+
+        before do
+          sales_log_xml.at_xpath("//xmlns:Q21PropertyType").content = ""
+          allow(logger).to receive(:warn).and_return(nil)
+        end
+
+        it "sets fromprop to don't know" do
+          sales_log_service.send(:create_log, sales_log_xml)
+
+          sales_log = SalesLog.find_by(old_id: sales_log_id)
+          expect(sales_log&.fromprop).to be(0)
+        end
+      end
+
+      context "when the socprevten is not answered" do
+        let(:sales_log_id) { "shared_ownership_sales_log" }
+
+        before do
+          sales_log_xml.at_xpath("//xmlns:PrevRentType").content = ""
+          allow(logger).to receive(:warn).and_return(nil)
+        end
+
+        it "sets socprevten to don't know" do
+          sales_log_service.send(:create_log, sales_log_xml)
+
+          sales_log = SalesLog.find_by(old_id: sales_log_id)
+          expect(sales_log&.socprevten).to be(10)
+        end
+      end
     end
   end
 end

From 3acd57c255551f6fc5bf485ded4f78d3bc6bfe2e Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Mon, 20 Mar 2023 15:25:23 +0000
Subject: [PATCH 4/4] Student child validation (#1441)

* Move household validation to soft validaiton

* Add student_not_child_value_check column

* Add soft validation pages

* Update import service

* typo

* Change class names and fix a typo

* More typos
---
 .../person_student_not_child_value_check.rb   |  21 ++++
 .../person_student_not_child_value_check.rb   |  24 ++++
 .../subsections/household_characteristics.rb  |  18 +++
 .../sales/household_validations.rb            |   6 -
 .../validations/sales/soft_validations.rb     |  11 ++
 .../imports/sales_logs_import_service.rb      |   4 +-
 config/locales/en.yml                         |   5 +-
 ...84057_add_student_not_child_value_check.rb |   5 +
 db/schema.rb                                  |   5 +-
 ...rson_student_not_child_value_check_spec.rb | 106 ++++++++++++++++++
 ...rson_student_not_child_value_check_spec.rb |  61 ++++++++++
 .../household_characteristics_spec.rb         |  36 ++++++
 .../sales/household_validations_spec.rb       |  11 +-
 .../sales/soft_validations_spec.rb            |  68 +++++++++++
 .../imports/sales_logs_import_service_spec.rb |  29 +++--
 15 files changed, 383 insertions(+), 27 deletions(-)
 create mode 100644 app/models/form/sales/pages/person_student_not_child_value_check.rb
 create mode 100644 app/models/form/sales/questions/person_student_not_child_value_check.rb
 create mode 100644 db/migrate/20230320084057_add_student_not_child_value_check.rb
 create mode 100644 spec/models/form/sales/pages/person_student_not_child_value_check_spec.rb
 create mode 100644 spec/models/form/sales/questions/person_student_not_child_value_check_spec.rb

diff --git a/app/models/form/sales/pages/person_student_not_child_value_check.rb b/app/models/form/sales/pages/person_student_not_child_value_check.rb
new file mode 100644
index 000000000..e3513e169
--- /dev/null
+++ b/app/models/form/sales/pages/person_student_not_child_value_check.rb
@@ -0,0 +1,21 @@
+class Form::Sales::Pages::PersonStudentNotChildValueCheck < Form::Sales::Pages::Person
+  def initialize(id, hsh, subsection, person_index:)
+    super
+    @depends_on = [
+      {
+        "person_#{person_index}_student_not_child?" => true,
+      },
+    ]
+    @person_index = person_index
+    @title_text = {
+      "translation" => "soft_validations.student_not_child.title_text",
+    }
+    @informative_text = {}
+  end
+
+  def questions
+    @questions ||= [
+      Form::Sales::Questions::PersonStudentNotChildValueCheck.new(nil, nil, self, person_index: @person_index),
+    ]
+  end
+end
diff --git a/app/models/form/sales/questions/person_student_not_child_value_check.rb b/app/models/form/sales/questions/person_student_not_child_value_check.rb
new file mode 100644
index 000000000..fba35d05b
--- /dev/null
+++ b/app/models/form/sales/questions/person_student_not_child_value_check.rb
@@ -0,0 +1,24 @@
+class Form::Sales::Questions::PersonStudentNotChildValueCheck < ::Form::Question
+  def initialize(id, hsh, page, person_index:)
+    super(id, hsh, page)
+    @id = "student_not_child_value_check"
+    @check_answer_label = "Student not a child confirmation"
+    @type = "interruption_screen"
+    @answer_options = {
+      "0" => { "value" => "Yes" },
+      "1" => { "value" => "No" },
+    }
+    @hidden_in_check_answers = {
+      "depends_on" => [
+        {
+          "student_not_child_value_check" => 0,
+        },
+        {
+          "student_not_child_value_check" => 1,
+        },
+      ],
+    }
+    @check_answers_card_number = person_index
+    @header = "Are you sure this person is not a child?"
+  end
+end
diff --git a/app/models/form/sales/subsections/household_characteristics.rb b/app/models/form/sales/subsections/household_characteristics.rb
index 311162e03..922d95edd 100644
--- a/app/models/form/sales/subsections/household_characteristics.rb
+++ b/app/models/form/sales/subsections/household_characteristics.rb
@@ -27,58 +27,76 @@ class Form::Sales::Subsections::HouseholdCharacteristics < ::Form::Subsection
       Form::Sales::Pages::Buyer1IncomeValueCheck.new("working_situation_buyer_1_income_value_check", nil, self),
       Form::Sales::Pages::Buyer1LiveInProperty.new(nil, nil, self),
       Form::Sales::Pages::Buyer2RelationshipToBuyer1.new(nil, nil, self),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("buyer_2_relationship_student_not_child_value_check", nil, self, person_index: 2),
       Form::Sales::Pages::Age2.new(nil, nil, self),
       Form::Sales::Pages::OldPersonsSharedOwnershipValueCheck.new("age_2_old_persons_shared_ownership_value_check", nil, self),
       Form::Sales::Pages::RetirementValueCheck.new("age_2_buyer_retirement_value_check", nil, self, person_index: 2),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("buyer_2_age_student_not_child_value_check", nil, self, person_index: 2),
       Form::Sales::Pages::GenderIdentity2.new(nil, nil, self),
       Form::Sales::Pages::RetirementValueCheck.new("gender_2_buyer_retirement_value_check", nil, self, person_index: 2),
       buyer_2_ethnicity_nationality_pages,
       Form::Sales::Pages::Buyer2WorkingSituation.new(nil, nil, self),
       Form::Sales::Pages::RetirementValueCheck.new("working_situation_2_retirement_value_check_joint_purchase", nil, self, person_index: 2),
       Form::Sales::Pages::Buyer2IncomeValueCheck.new("working_situation_buyer_2_income_value_check", nil, self),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("buyer_2_working_situation_student_not_child_value_check", nil, self, person_index: 2),
       Form::Sales::Pages::Buyer2LiveInProperty.new(nil, nil, self),
       Form::Sales::Pages::NumberOfOthersInProperty.new("number_of_others_in_property", nil, self, joint_purchase: false),
       Form::Sales::Pages::NumberOfOthersInProperty.new("number_of_others_in_property_joint_purchase", nil, self, joint_purchase: true),
       Form::Sales::Pages::PersonKnown.new("person_2_known", nil, self, person_index: 2),
       Form::Sales::Pages::PersonRelationshipToBuyer1.new("person_2_relationship_to_buyer_1", nil, self, person_index: 2),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("relationship_2_student_not_child_value_check", nil, self, person_index: 2),
       Form::Sales::Pages::PersonAge.new("person_2_age", nil, self, person_index: 2),
       Form::Sales::Pages::RetirementValueCheck.new("age_2_retirement_value_check", nil, self, person_index: 2),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("age_2_student_not_child_value_check", nil, self, person_index: 2),
       Form::Sales::Pages::PersonGenderIdentity.new("person_2_gender_identity", nil, self, person_index: 2),
       Form::Sales::Pages::RetirementValueCheck.new("gender_2_retirement_value_check", nil, self, person_index: 2),
       Form::Sales::Pages::PersonWorkingSituation.new("person_2_working_situation", nil, self, person_index: 2),
       Form::Sales::Pages::RetirementValueCheck.new("working_situation_2_retirement_value_check", nil, self, person_index: 2),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("working_situation_2_student_not_child_value_check", nil, self, person_index: 2),
       Form::Sales::Pages::PersonKnown.new("person_3_known", nil, self, person_index: 3),
       Form::Sales::Pages::PersonRelationshipToBuyer1.new("person_3_relationship_to_buyer_1", nil, self, person_index: 3),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("relationship_3_student_not_child_value_check", nil, self, person_index: 3),
       Form::Sales::Pages::PersonAge.new("person_3_age", nil, self, person_index: 3),
       Form::Sales::Pages::RetirementValueCheck.new("age_3_retirement_value_check", nil, self, person_index: 3),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("age_3_student_not_child_value_check", nil, self, person_index: 3),
       Form::Sales::Pages::PersonGenderIdentity.new("person_3_gender_identity", nil, self, person_index: 3),
       Form::Sales::Pages::RetirementValueCheck.new("gender_3_retirement_value_check", nil, self, person_index: 3),
       Form::Sales::Pages::PersonWorkingSituation.new("person_3_working_situation", nil, self, person_index: 3),
       Form::Sales::Pages::RetirementValueCheck.new("working_situation_3_retirement_value_check", nil, self, person_index: 3),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("working_situation_3_student_not_child_value_check", nil, self, person_index: 3),
       Form::Sales::Pages::PersonKnown.new("person_4_known", nil, self, person_index: 4),
       Form::Sales::Pages::PersonRelationshipToBuyer1.new("person_4_relationship_to_buyer_1", nil, self, person_index: 4),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("relationship_4_student_not_child_value_check", nil, self, person_index: 4),
       Form::Sales::Pages::PersonAge.new("person_4_age", nil, self, person_index: 4),
       Form::Sales::Pages::RetirementValueCheck.new("age_4_retirement_value_check", nil, self, person_index: 4),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("age_4_student_not_child_value_check", nil, self, person_index: 4),
       Form::Sales::Pages::PersonGenderIdentity.new("person_4_gender_identity", nil, self, person_index: 4),
       Form::Sales::Pages::RetirementValueCheck.new("gender_4_retirement_value_check", nil, self, person_index: 4),
       Form::Sales::Pages::PersonWorkingSituation.new("person_4_working_situation", nil, self, person_index: 4),
       Form::Sales::Pages::RetirementValueCheck.new("working_situation_4_retirement_value_check", nil, self, person_index: 4),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("working_situation_4_student_not_child_value_check", nil, self, person_index: 4),
       Form::Sales::Pages::PersonKnown.new("person_5_known", nil, self, person_index: 5),
       Form::Sales::Pages::PersonRelationshipToBuyer1.new("person_5_relationship_to_buyer_1", nil, self, person_index: 5),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("relationship_5_student_not_child_value_check", nil, self, person_index: 5),
       Form::Sales::Pages::PersonAge.new("person_5_age", nil, self, person_index: 5),
       Form::Sales::Pages::RetirementValueCheck.new("age_5_retirement_value_check", nil, self, person_index: 5),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("age_5_student_not_child_value_check", nil, self, person_index: 5),
       Form::Sales::Pages::PersonGenderIdentity.new("person_5_gender_identity", nil, self, person_index: 5),
       Form::Sales::Pages::RetirementValueCheck.new("gender_5_retirement_value_check", nil, self, person_index: 5),
       Form::Sales::Pages::PersonWorkingSituation.new("person_5_working_situation", nil, self, person_index: 5),
       Form::Sales::Pages::RetirementValueCheck.new("working_situation_5_retirement_value_check", nil, self, person_index: 5),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("working_situation_5_student_not_child_value_check", nil, self, person_index: 5),
       Form::Sales::Pages::PersonKnown.new("person_6_known", nil, self, person_index: 6),
       Form::Sales::Pages::PersonRelationshipToBuyer1.new("person_6_relationship_to_buyer_1", nil, self, person_index: 6),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("relationship_6_student_not_child_value_check", nil, self, person_index: 6),
       Form::Sales::Pages::PersonAge.new("person_6_age", nil, self, person_index: 6),
       Form::Sales::Pages::RetirementValueCheck.new("age_6_retirement_value_check", nil, self, person_index: 6),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("age_6_student_not_child_value_check", nil, self, person_index: 6),
       Form::Sales::Pages::PersonGenderIdentity.new("person_6_gender_identity", nil, self, person_index: 6),
       Form::Sales::Pages::RetirementValueCheck.new("gender_6_retirement_value_check", nil, self, person_index: 6),
       Form::Sales::Pages::PersonWorkingSituation.new("person_6_working_situation", nil, self, person_index: 6),
       Form::Sales::Pages::RetirementValueCheck.new("working_situation_6_retirement_value_check", nil, self, person_index: 6),
+      Form::Sales::Pages::PersonStudentNotChildValueCheck.new("working_situation_6_student_not_child_value_check", nil, self, person_index: 6),
     ].flatten.compact
   end
 
diff --git a/app/models/validations/sales/household_validations.rb b/app/models/validations/sales/household_validations.rb
index ec73b4abb..e53b7154e 100644
--- a/app/models/validations/sales/household_validations.rb
+++ b/app/models/validations/sales/household_validations.rb
@@ -45,12 +45,6 @@ private
     student = person_is_fulltime_student?(economic_status)
     child = person_is_child?(relationship)
 
-    if age_between_16_19 && student && !child
-      record.errors.add "age#{person_num}", I18n.t("validations.household.age.student_16_19.cannot_be_16_19.student_not_child")
-      record.errors.add "ecstat#{person_num}", I18n.t("validations.household.ecstat.student_16_19.cannot_be_student.16_19_not_child")
-      record.errors.add "relat#{person_num}", I18n.t("validations.household.relat.student_16_19.must_be_child")
-    end
-
     if age_between_16_19 && !student && child
       record.errors.add "age#{person_num}", I18n.t("validations.household.age.student_16_19.cannot_be_16_19.child_not_student")
       record.errors.add "ecstat#{person_num}", I18n.t("validations.household.ecstat.student_16_19.must_be_student")
diff --git a/app/models/validations/sales/soft_validations.rb b/app/models/validations/sales/soft_validations.rb
index a44e0a293..5819b0fe1 100644
--- a/app/models/validations/sales/soft_validations.rb
+++ b/app/models/validations/sales/soft_validations.rb
@@ -103,6 +103,17 @@ module Validations::Sales::SoftValidations
     mscharge > soft_max
   end
 
+  (2..6).each do |person_num|
+    define_method("person_#{person_num}_student_not_child?") do
+      relat = send("relat#{person_num}")
+      ecstat = send("ecstat#{person_num}")
+      age = send("age#{person_num}")
+      return unless age && ecstat && relat
+
+      age.between?(16, 19) && ecstat == 7 && relat != "C"
+    end
+  end
+
 private
 
   def sale_range
diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb
index 35c94eb15..d7a3c8534 100644
--- a/app/services/imports/sales_logs_import_service.rb
+++ b/app/services/imports/sales_logs_import_service.rb
@@ -152,6 +152,7 @@ module Imports
       attributes["old_persons_shared_ownership_value_check"] = 0
       attributes["income2_value_check"] = 0
       attributes["monthly_charges_value_check"] = 0
+      attributes["student_not_child_value_check"] = 0
 
       # Sets the log creator
       owner_id = meta_field_value(xml_doc, "owner-user-id").strip
@@ -253,7 +254,8 @@ module Imports
          staircase_bought_value_check
          monthly_charges_value_check
          hodate_check
-         saledate_check]
+         saledate_check
+         student_not_child_value_check]
     end
 
     def check_status_completed(sales_log, previous_status)
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 2d17cf207..8c9cd12c1 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -344,7 +344,6 @@ en:
         not_student_16_19: "Answer cannot be between 16 and 19 as person %{person_num} is a child of the lead tenant but is not a full-time student"
         student_16_19:
           cannot_be_16_19:
-            student_not_child: "Person cannot be aged 16-19 if they are a student but don't have relationship ‘child’"
             child_not_student: "Person cannot be aged 16-19 if they have relationship ‘child’ but are not a student"
           must_be_16_19: "Person must be aged 16-19 if they are a student and have relationship ‘child’"
         partner_under_16: "Cannot be under 16 if the relationship is partner"
@@ -358,7 +357,6 @@ en:
         student_16_19:
           cannot_be_student:
             child_not_16_19: "Person cannot be a student if they are not aged 16-19 but have relationship ‘child’"
-            16_19_not_child: "Person cannot be a student if they are aged 16-19 but don‘t have relationship ‘child’"
           must_be_student: "Person must be a student if they are aged 16-19 and have relationship ‘child’"
         retired_male: "Answer cannot be ‘retired’ as the male tenant is under 65"
         retired_female: "Answer cannot be ‘retired’ as the female tenant is under 60"
@@ -372,7 +370,6 @@ en:
           cannot_be_child:
             student_not_16_19: "Answer cannot be ‘child’ if the person is a student but not aged 16-19"
             16_19_not_student:  "Answer cannot be ‘child’ if the person is aged 16-19 but not a student"
-          must_be_child: "Answer must be ‘child’ if the person is aged 16-19 and a student"
       housingneeds_a:
         one_or_two_choices: "You can only select one option or ‘other disabled access needs’ plus ‘wheelchair-accessible housing’, ‘wheelchair access to essential rooms’ or ‘level access housing’"
       housingneeds_type:
@@ -526,6 +523,8 @@ en:
     staircase_bought_seems_high: "You said %{percentage}% was bought in this staircasing transaction, which seems high. Are you sure?"
     monthly_charges_over_soft_max:
       title_text: "The amount of monthly charges is high for this type of property and sale type"
+    student_not_child:
+      title_text: "You told us this person is a student aged beween 16 and 19"
 
   devise:
     two_factor_authentication:
diff --git a/db/migrate/20230320084057_add_student_not_child_value_check.rb b/db/migrate/20230320084057_add_student_not_child_value_check.rb
new file mode 100644
index 000000000..ecd037368
--- /dev/null
+++ b/db/migrate/20230320084057_add_student_not_child_value_check.rb
@@ -0,0 +1,5 @@
+class AddStudentNotChildValueCheck < ActiveRecord::Migration[7.0]
+  def change
+    add_column :sales_logs, :student_not_child_value_check, :integer
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 075cc4b5e..32d71dfeb 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_03_09_145740) do
+ActiveRecord::Schema[7.0].define(version: 2023_03_20_084057) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -560,7 +560,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_03_09_145740) do
     t.integer "buy2living"
     t.integer "prevtenbuy2"
     t.integer "pregblank"
-    t.integer "nationalbuy2"
     t.string "uprn"
     t.integer "uprn_known"
     t.integer "uprn_confirmed"
@@ -568,6 +567,8 @@ ActiveRecord::Schema[7.0].define(version: 2023_03_09_145740) do
     t.string "address_line2"
     t.string "town_or_city"
     t.string "county"
+    t.integer "nationalbuy2"
+    t.integer "student_not_child_value_check"
     t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id"
     t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id"
     t.index ["old_id"], name: "index_sales_logs_on_old_id", unique: true
diff --git a/spec/models/form/sales/pages/person_student_not_child_value_check_spec.rb b/spec/models/form/sales/pages/person_student_not_child_value_check_spec.rb
new file mode 100644
index 000000000..3499cda9f
--- /dev/null
+++ b/spec/models/form/sales/pages/person_student_not_child_value_check_spec.rb
@@ -0,0 +1,106 @@
+require "rails_helper"
+
+RSpec.describe Form::Sales::Pages::PersonStudentNotChildValueCheck, type: :model do
+  subject(:page) { described_class.new(page_id, page_definition, subsection, person_index:) }
+
+  let(:page_definition) { nil }
+  let(:subsection) { instance_double(Form::Subsection) }
+  let(:person_index) { 2 }
+
+  let(:page_id) { "person_2_student_not_child_value_check" }
+
+  it "has correct subsection" do
+    expect(page.subsection).to eq(subsection)
+  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
+
+  it "is interruption screen page" do
+    expect(page.interruption_screen?).to eq(true)
+  end
+
+  it "has correct title_text" do
+    expect(page.title_text).to eq({
+      "translation" => "soft_validations.student_not_child.title_text",
+    })
+  end
+
+  it "has correct informative_text" do
+    expect(page.informative_text).to eq({})
+  end
+
+  it "has correct questions" do
+    expect(page.questions.map(&:id)).to eq(%w[student_not_child_value_check])
+  end
+
+  context "with person 2" do
+    let(:person_index) { 2 }
+    let(:page_id) { "person_2_student_not_child_value_check" }
+
+    it "has the correct id" do
+      expect(page.id).to eq(page_id)
+    end
+
+    it "has correct depends_on" do
+      expect(page.depends_on).to eq([{ "person_2_student_not_child?" => true }])
+    end
+  end
+
+  context "with person 3" do
+    let(:person_index) { 3 }
+    let(:page_id) { "person_3_student_not_child_value_check" }
+
+    it "has the correct id" do
+      expect(page.id).to eq(page_id)
+    end
+
+    it "has correct depends_on" do
+      expect(page.depends_on).to eq([{ "person_3_student_not_child?" => true }])
+    end
+  end
+
+  context "with person 4" do
+    let(:person_index) { 4 }
+    let(:page_id) { "person_4_student_not_child_value_check" }
+
+    it "has the correct id" do
+      expect(page.id).to eq(page_id)
+    end
+
+    it "has correct depends_on" do
+      expect(page.depends_on).to eq([{ "person_4_student_not_child?" => true }])
+    end
+  end
+
+  context "with person 5" do
+    let(:person_index) { 5 }
+    let(:page_id) { "person_5_student_not_child_value_check" }
+
+    it "has the correct id" do
+      expect(page.id).to eq(page_id)
+    end
+
+    it "has correct depends_on" do
+      expect(page.depends_on).to eq([{ "person_5_student_not_child?" => true }])
+    end
+  end
+
+  context "with person 6" do
+    let(:person_index) { 6 }
+    let(:page_id) { "person_6_student_not_child_value_check" }
+
+    it "has the correct id" do
+      expect(page.id).to eq(page_id)
+    end
+
+    it "has correct depends_on" do
+      expect(page.depends_on).to eq([{ "person_6_student_not_child?" => true }])
+    end
+  end
+end
diff --git a/spec/models/form/sales/questions/person_student_not_child_value_check_spec.rb b/spec/models/form/sales/questions/person_student_not_child_value_check_spec.rb
new file mode 100644
index 000000000..f36e7ad57
--- /dev/null
+++ b/spec/models/form/sales/questions/person_student_not_child_value_check_spec.rb
@@ -0,0 +1,61 @@
+require "rails_helper"
+
+RSpec.describe Form::Sales::Questions::PersonStudentNotChildValueCheck, type: :model do
+  subject(:question) { described_class.new(question_id, question_definition, page, person_index: 1) }
+
+  let(:question_id) { nil }
+  let(:question_definition) { nil }
+  let(:page) { instance_double(Form::Page) }
+
+  it "has correct page" do
+    expect(question.page).to eq(page)
+  end
+
+  it "has the correct id" do
+    expect(question.id).to eq("student_not_child_value_check")
+  end
+
+  it "has the correct header" do
+    expect(question.header).to eq("Are you sure this person is not a child?")
+  end
+
+  it "has the correct check_answer_label" do
+    expect(question.check_answer_label).to eq("Student not a child confirmation")
+  end
+
+  it "has the correct type" do
+    expect(question.type).to eq("interruption_screen")
+  end
+
+  it "is not marked as derived" do
+    expect(question.derived?).to be false
+  end
+
+  it "has the correct hint" do
+    expect(question.hint_text).to be_nil
+  end
+
+  it "has a correct check_answers_card_number" do
+    expect(question.check_answers_card_number).to eq(1)
+  end
+
+  it "has the correct answer_options" do
+    expect(question.answer_options).to eq({
+      "0" => { "value" => "Yes" },
+      "1" => { "value" => "No" },
+    })
+  end
+
+  it "has the correct hidden_in_check_answers" do
+    expect(question.hidden_in_check_answers).to eq({
+      "depends_on" => [
+        {
+          "student_not_child_value_check" => 0,
+        },
+        {
+          "student_not_child_value_check" => 1,
+        },
+      ],
+    })
+  end
+end
diff --git a/spec/models/form/sales/subsections/household_characteristics_spec.rb b/spec/models/form/sales/subsections/household_characteristics_spec.rb
index b47ac970d..57a9ce019 100644
--- a/spec/models/form/sales/subsections/household_characteristics_spec.rb
+++ b/spec/models/form/sales/subsections/household_characteristics_spec.rb
@@ -40,57 +40,75 @@ RSpec.describe Form::Sales::Subsections::HouseholdCharacteristics, type: :model
           working_situation_buyer_1_income_value_check
           buyer_1_live_in_property
           buyer_2_relationship_to_buyer_1
+          buyer_2_relationship_student_not_child_value_check
           buyer_2_age
           age_2_old_persons_shared_ownership_value_check
           age_2_buyer_retirement_value_check
+          buyer_2_age_student_not_child_value_check
           buyer_2_gender_identity
           gender_2_buyer_retirement_value_check
           buyer_2_working_situation
           working_situation_2_retirement_value_check_joint_purchase
           working_situation_buyer_2_income_value_check
+          buyer_2_working_situation_student_not_child_value_check
           buyer_2_live_in_property
           number_of_others_in_property
           number_of_others_in_property_joint_purchase
           person_2_known
           person_2_relationship_to_buyer_1
+          relationship_2_student_not_child_value_check
           person_2_age
           age_2_retirement_value_check
+          age_2_student_not_child_value_check
           person_2_gender_identity
           gender_2_retirement_value_check
           person_2_working_situation
           working_situation_2_retirement_value_check
+          working_situation_2_student_not_child_value_check
           person_3_known
           person_3_relationship_to_buyer_1
+          relationship_3_student_not_child_value_check
           person_3_age
           age_3_retirement_value_check
+          age_3_student_not_child_value_check
           person_3_gender_identity
           gender_3_retirement_value_check
           person_3_working_situation
           working_situation_3_retirement_value_check
+          working_situation_3_student_not_child_value_check
           person_4_known
           person_4_relationship_to_buyer_1
+          relationship_4_student_not_child_value_check
           person_4_age
           age_4_retirement_value_check
+          age_4_student_not_child_value_check
           person_4_gender_identity
           gender_4_retirement_value_check
           person_4_working_situation
           working_situation_4_retirement_value_check
+          working_situation_4_student_not_child_value_check
           person_5_known
           person_5_relationship_to_buyer_1
+          relationship_5_student_not_child_value_check
           person_5_age
           age_5_retirement_value_check
+          age_5_student_not_child_value_check
           person_5_gender_identity
           gender_5_retirement_value_check
           person_5_working_situation
           working_situation_5_retirement_value_check
+          working_situation_5_student_not_child_value_check
           person_6_known
           person_6_relationship_to_buyer_1
+          relationship_6_student_not_child_value_check
           person_6_age
           age_6_retirement_value_check
+          age_6_student_not_child_value_check
           person_6_gender_identity
           gender_6_retirement_value_check
           person_6_working_situation
           working_situation_6_retirement_value_check
+          working_situation_6_student_not_child_value_check
         ],
       )
     end
@@ -124,9 +142,11 @@ RSpec.describe Form::Sales::Subsections::HouseholdCharacteristics, type: :model
           working_situation_buyer_1_income_value_check
           buyer_1_live_in_property
           buyer_2_relationship_to_buyer_1
+          buyer_2_relationship_student_not_child_value_check
           buyer_2_age
           age_2_old_persons_shared_ownership_value_check
           age_2_buyer_retirement_value_check
+          buyer_2_age_student_not_child_value_check
           buyer_2_gender_identity
           gender_2_buyer_retirement_value_check
           buyer_2_ethnic_group
@@ -139,49 +159,65 @@ RSpec.describe Form::Sales::Subsections::HouseholdCharacteristics, type: :model
           buyer_2_working_situation
           working_situation_2_retirement_value_check_joint_purchase
           working_situation_buyer_2_income_value_check
+          buyer_2_working_situation_student_not_child_value_check
           buyer_2_live_in_property
           number_of_others_in_property
           number_of_others_in_property_joint_purchase
           person_2_known
           person_2_relationship_to_buyer_1
+          relationship_2_student_not_child_value_check
           person_2_age
           age_2_retirement_value_check
+          age_2_student_not_child_value_check
           person_2_gender_identity
           gender_2_retirement_value_check
           person_2_working_situation
           working_situation_2_retirement_value_check
+          working_situation_2_student_not_child_value_check
           person_3_known
           person_3_relationship_to_buyer_1
+          relationship_3_student_not_child_value_check
           person_3_age
           age_3_retirement_value_check
+          age_3_student_not_child_value_check
           person_3_gender_identity
           gender_3_retirement_value_check
           person_3_working_situation
           working_situation_3_retirement_value_check
+          working_situation_3_student_not_child_value_check
           person_4_known
           person_4_relationship_to_buyer_1
+          relationship_4_student_not_child_value_check
           person_4_age
           age_4_retirement_value_check
+          age_4_student_not_child_value_check
           person_4_gender_identity
           gender_4_retirement_value_check
           person_4_working_situation
           working_situation_4_retirement_value_check
+          working_situation_4_student_not_child_value_check
           person_5_known
           person_5_relationship_to_buyer_1
+          relationship_5_student_not_child_value_check
           person_5_age
           age_5_retirement_value_check
+          age_5_student_not_child_value_check
           person_5_gender_identity
           gender_5_retirement_value_check
           person_5_working_situation
           working_situation_5_retirement_value_check
+          working_situation_5_student_not_child_value_check
           person_6_known
           person_6_relationship_to_buyer_1
+          relationship_6_student_not_child_value_check
           person_6_age
           age_6_retirement_value_check
+          age_6_student_not_child_value_check
           person_6_gender_identity
           gender_6_retirement_value_check
           person_6_working_situation
           working_situation_6_retirement_value_check
+          working_situation_6_student_not_child_value_check
         ],
       )
     end
diff --git a/spec/models/validations/sales/household_validations_spec.rb b/spec/models/validations/sales/household_validations_spec.rb
index 10367390e..5c7a921e4 100644
--- a/spec/models/validations/sales/household_validations_spec.rb
+++ b/spec/models/validations/sales/household_validations_spec.rb
@@ -84,17 +84,14 @@ RSpec.describe Validations::Sales::HouseholdValidations do
         .to include(match I18n.t("validations.household.age.child_over_20"))
     end
 
-    it "adds errors for a person aged 16-19 who is a student but not a child of the buyer" do
+    it "does not add and error for a person aged 16-19 who is a student but not a child of the buyer" do
       record.age2 = 18
       record.ecstat2 = "7"
       record.relat2 = "P"
       household_validator.validate_household_number_of_other_members(record)
-      expect(record.errors["relat2"])
-        .to include(match I18n.t("validations.household.relat.student_16_19.must_be_child"))
-      expect(record.errors["age2"])
-        .to include(match I18n.t("validations.household.age.student_16_19.cannot_be_16_19.student_not_child"))
-      expect(record.errors["ecstat2"])
-        .to include(match I18n.t("validations.household.ecstat.student_16_19.cannot_be_student.16_19_not_child"))
+      expect(record.errors["relat2"]).to be_empty
+      expect(record.errors["ecstat2"]).to be_empty
+      expect(record.errors["age2"]).to be_empty
     end
 
     it "adds errors for a person aged 16-19 who is a child of the buyer but not a student" do
diff --git a/spec/models/validations/sales/soft_validations_spec.rb b/spec/models/validations/sales/soft_validations_spec.rb
index 5d475b450..dc8bbfedd 100644
--- a/spec/models/validations/sales/soft_validations_spec.rb
+++ b/spec/models/validations/sales/soft_validations_spec.rb
@@ -730,4 +730,72 @@ RSpec.describe Validations::Sales::SoftValidations do
       end
     end
   end
+
+  describe "#person_2_student_not_child?" do
+    it "returns false if age is not given" do
+      record.age2 = nil
+      record.relat2 = "P"
+      record.ecstat2 = 7
+
+      expect(record).not_to be_person_2_student_not_child
+    end
+
+    it "returns false if retaltionship is not given" do
+      record.age2 = 17
+      record.relat2 = nil
+      record.ecstat2 = 7
+
+      expect(record).not_to be_person_2_student_not_child
+    end
+
+    it "returns false if economic status is not given" do
+      record.age2 = 17
+      record.relat2 = "P"
+      record.ecstat2 = nil
+
+      expect(record).not_to be_person_2_student_not_child
+    end
+
+    it "returns true if it's a student aged 16-19 and not a child" do
+      record.age2 = 17
+      record.relat2 = "P"
+      record.ecstat2 = 7
+
+      expect(record).to be_person_2_student_not_child
+    end
+  end
+
+  describe "#person_3_student_not_child?" do
+    it "returns false if age is not given" do
+      record.age3 = nil
+      record.relat3 = "P"
+      record.ecstat3 = 7
+
+      expect(record).not_to be_person_3_student_not_child
+    end
+
+    it "returns false if retaltionship is not given" do
+      record.age3 = 17
+      record.relat3 = nil
+      record.ecstat3 = 7
+
+      expect(record).not_to be_person_3_student_not_child
+    end
+
+    it "returns false if economic status is not given" do
+      record.age3 = 17
+      record.relat3 = "P"
+      record.ecstat3 = nil
+
+      expect(record).not_to be_person_3_student_not_child
+    end
+
+    it "returns true if it's a student aged 16-19 and not a child" do
+      record.age3 = 17
+      record.relat3 = "P"
+      record.ecstat3 = 7
+
+      expect(record).to be_person_3_student_not_child
+    end
+  end
 end
diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb
index a9c085abc..d0e1f807d 100644
--- a/spec/services/imports/sales_logs_import_service_spec.rb
+++ b/spec/services/imports/sales_logs_import_service_spec.rb
@@ -363,20 +363,34 @@ RSpec.describe Imports::SalesLogsImportService do
       end
     end
 
-    context "and it has an invalid record with invalid child, student and 16-19 age combination" do
+    context "and the student not child soft validation is triggered (student_not_child_value_check)" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:P2Rel").content = "P"
+        sales_log_xml.at_xpath("//xmlns:P2Eco").content = "7"
+        sales_log_xml.at_xpath("//xmlns:P2Age").content = "16"
+      end
+
+      it "completes the log" do
+        sales_log_service.send(:create_log, sales_log_xml)
+        sales_log = SalesLog.find_by(old_id: sales_log_id)
+        expect(sales_log.status).to eq("completed")
+      end
+    end
+
+    context "and it has an invalid record with invalid child age" do
       let(:sales_log_id) { "discounted_ownership_sales_log" }
 
       before do
         sales_log_xml.at_xpath("//meta:status").content = "submitted-invalid"
-        sales_log_xml.at_xpath("//xmlns:P2Age").content = 16
-        sales_log_xml.at_xpath("//xmlns:P2Eco").content = 7
-        sales_log_xml.at_xpath("//xmlns:P2Rel").content = "X"
+        sales_log_xml.at_xpath("//xmlns:P2Age").content = 17
+        sales_log_xml.at_xpath("//xmlns:P2Eco").content = 9
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Removing field age2 from log triggering validation: Person cannot be aged 16-19 if they are a student but don't have relationship ‘child’/)
-        expect(logger).to receive(:warn).with(/Removing field relat2 from log triggering validation: Answer must be ‘child’ if the person is aged 16-19 and a student/)
-        expect(logger).to receive(:warn).with(/Removing field ecstat2 from log triggering validation: Person cannot be a student if they are aged 16-19 but don‘t have relationship ‘child’/)
+        expect(logger).to receive(:warn).with(/Log discounted_ownership_sales_log: Removing field ecstat2 from log triggering validation: Answer cannot be ‘child under 16’ as you told us the person 2 is older than 16/)
+        expect(logger).to receive(:warn).with(/Log discounted_ownership_sales_log: Removing field age2 from log triggering validation: Answer cannot be over 16 as person’s 2 working situation is ‘child under 16‘/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -389,7 +403,6 @@ RSpec.describe Imports::SalesLogsImportService do
 
         expect(sales_log).not_to be_nil
         expect(sales_log.age2).to be_nil
-        expect(sales_log.relat2).to be_nil
         expect(sales_log.ecstat2).to be_nil
       end
     end