CLDC-1916 Handle sales forms and validations during comparison period (#1317)

* Add more time helpers

* CLDC-1916 Validate this and next year start date in saleslogs

* Add capybara-screenshot

* Use sales_in_crossover_period?

Gemfile

@@ -91,6 +91,7 @@ end
 group :test do
   gem "capybara", require: false
   gem "capybara-lockstep"
+  gem "capybara-screenshot"
   gem "faker"
   gem "rspec-rails", require: false
   gem "selenium-webdriver", require: false

Gemfile.lock

@@ -129,6 +129,9 @@ GEM
       capybara (>= 2.0)
       ruby2_keywords
       selenium-webdriver (>= 3)
+    capybara-screenshot (1.0.26)
+      capybara (>= 1.0, < 4)
+      launchy
     childprocess (4.1.0)
     coderay (1.1.3)
     concurrent-ruby (1.2.0)
@@ -201,6 +204,8 @@ GEM
     json-schema (3.0.0)
       addressable (>= 2.8)
     jwt (2.7.0)
+    launchy (2.5.2)
+      addressable (~> 2.8)
     listen (3.8.0)
       rb-fsevent (~> 0.10, >= 0.10.3)
       rb-inotify (~> 0.9, >= 0.9.10)
@@ -449,6 +454,7 @@ DEPENDENCIES
   byebug
   capybara
   capybara-lockstep
+  capybara-screenshot
   devise!
   devise_two_factor_authentication
   dotenv-rails

app/helpers/collection_time_helper.rb

@@ -23,4 +23,20 @@ module CollectionTimeHelper
   def current_collection_end_date
     Time.zone.local(current_collection_start_year + 1, 3, 31)
   end
+
+  def previous_collection_end_date
+    current_collection_end_date - 1.year
+  end
+
+  def next_collection_start_year
+    current_collection_start_year + 1
+  end
+
+  def previous_collection_start_year
+    current_collection_start_year - 1
+  end
+
+  def previous_collection_start_date
+    current_collection_start_date - 1.year
+  end
 end

app/helpers/tasklist_helper.rb

@@ -11,15 +11,6 @@ module TasklistHelper
     log.form.subsections.count { |subsection| subsection.status(log) == status && subsection.applicable_questions(log).count.positive? }
   end
 
-  def next_page_or_check_answers(subsection, log, current_user)
-    path = if subsection.is_started?(log)
-             "#{log.class.name.underscore}_#{subsection.id}_check_answers_path"
-           else
-             "#{log.class.name.underscore}_#{next_question_page(subsection, log, current_user)}_path"
-           end
-    send(path, log)
-  end
-
   def next_question_page(subsection, log, current_user)
     if subsection.pages.first.routed_to?(log, current_user)
       subsection.pages.first.id
@@ -46,4 +37,16 @@ module TasklistHelper
       "This log is from the #{log.form.start_date.year}/#{log.form.start_date.year + 1} collection window, which is now closed."
     end
   end
+
+private
+
+  def next_page_or_check_answers(subsection, log, current_user)
+    path = if subsection.is_started?(log)
+             "#{log.class.name.underscore}_#{subsection.id}_check_answers_path"
+           else
+             "#{log.class.name.underscore}_#{next_question_page(subsection, log, current_user)}_path"
+           end
+
+    send(path, log)
+  end
 end

app/models/form_handler.rb

@@ -35,8 +35,8 @@ class FormHandler
   def sales_forms
     {
       "current_sales" => Form.new(nil, current_collection_start_year, SALES_SECTIONS, "sales"),
-      "previous_sales" => Form.new(nil, current_collection_start_year - 1, SALES_SECTIONS, "sales"),
-      "next_sales" => Form.new(nil, current_collection_start_year + 1, SALES_SECTIONS, "sales"),
+      "previous_sales" => Form.new(nil, previous_collection_start_year, SALES_SECTIONS, "sales"),
+      "next_sales" => Form.new(nil, next_collection_start_year, SALES_SECTIONS, "sales"),
     }
   end
 
@@ -52,10 +52,10 @@ class FormHandler
     end
 
     if forms["previous_lettings"].blank? && current_collection_start_year >= 2022
-      forms["previous_lettings"] = Form.new(nil, current_collection_start_year - 1, LETTINGS_SECTIONS, "lettings")
+      forms["previous_lettings"] = Form.new(nil, previous_collection_start_year, LETTINGS_SECTIONS, "lettings")
     end
     forms["current_lettings"] = Form.new(nil, current_collection_start_year, LETTINGS_SECTIONS, "lettings") if forms["current_lettings"].blank?
-    forms["next_lettings"] = Form.new(nil, current_collection_start_year + 1, LETTINGS_SECTIONS, "lettings") if forms["next_lettings"].blank?
+    forms["next_lettings"] = Form.new(nil, next_collection_start_year, LETTINGS_SECTIONS, "lettings") if forms["next_lettings"].blank?
 
     forms
   end

app/models/validations/sales/setup_validations.rb

@@ -1,11 +1,45 @@
 module Validations::Sales::SetupValidations
   include Validations::SharedValidations
+  include CollectionTimeHelper
 
   def validate_saledate(record)
     return unless record.saledate && date_valid?("saledate", record)
 
-    unless record.saledate.between?(Time.zone.local(2022, 4, 1), Time.zone.local(2023, 3, 31)) || !FeatureToggle.saledate_collection_window_validation_enabled?
-      record.errors.add :saledate, I18n.t("validations.setup.saledate.financial_year")
+    unless record.saledate.between?(active_collection_start_date, current_collection_end_date) || !FeatureToggle.saledate_collection_window_validation_enabled?
+      record.errors.add :saledate, validation_error_message
+    end
+  end
+
+private
+
+  def active_collection_start_date
+    if FormHandler.instance.sales_in_crossover_period?
+      previous_collection_start_date
+    else
+      current_collection_start_date
+    end
+  end
+
+  def validation_error_message
+    current_end_year_long = current_collection_end_date.strftime("#{current_collection_end_date.day.ordinalize} %B %Y")
+
+    if FormHandler.instance.sales_in_crossover_period?
+      I18n.t(
+        "validations.setup.saledate.previous_and_current_financial_year",
+        previous_start_year_short: previous_collection_start_date.strftime("%y"),
+        previous_end_year_short: previous_collection_end_date.strftime("%y"),
+        previous_start_year_long: previous_collection_start_date.strftime("#{previous_collection_start_date.day.ordinalize} %B %Y"),
+        current_end_year_short: current_collection_end_date.strftime("%y"),
+        current_end_year_long:,
+      )
+    else
+      I18n.t(
+        "validations.setup.saledate.current_financial_year",
+        current_start_year_short: current_collection_start_date.strftime("%y"),
+        current_end_year_short: current_collection_end_date.strftime("%y"),
+        current_start_year_long: current_collection_start_date.strftime("#{current_collection_start_date.day.ordinalize} %B %Y"),
+        current_end_year_long:,
+      )
     end
   end
 end

config/initializers/feature_toggle.rb

@@ -1,13 +1,14 @@
 class FeatureToggle
-  def self.startdate_two_week_validation_enabled?
+  # Disable check on preview apps to allow for testing of future forms
+  def self.saledate_collection_window_validation_enabled?
     Rails.env.production? || Rails.env.test? || Rails.env.staging?
   end
 
-  def self.startdate_collection_window_validation_enabled?
+  def self.startdate_two_week_validation_enabled?
     Rails.env.production? || Rails.env.test? || Rails.env.staging?
   end
 
-  def self.saledate_collection_window_validation_enabled?
+  def self.startdate_collection_window_validation_enabled?
     Rails.env.production? || Rails.env.test? || Rails.env.staging?
   end
 

config/locales/en.yml

@@ -150,7 +150,11 @@ en:
       intermediate_rent_product_name:
         blank: "Enter name of other intermediate rent product"
       saledate:
-        financial_year: "Date must be from 22/23 financial year, which is between 1st April 2022 and 31st March 2023"
+        current_financial_year:
+          Enter a date within the %{current_start_year_short}/%{current_end_year_short} financial year, which is between %{current_start_year_long} and %{current_end_year_long}
+        previous_and_current_financial_year:
+          "Enter a date within the %{previous_start_year_short}/%{previous_end_year_short} or %{previous_end_year_short}/%{current_end_year_short} financial years, which is between %{previous_start_year_long} and %{current_end_year_long}"
+
       startdate:
         later_than_14_days_after: "The tenancy start date must not be later than 14 days from today’s date"
         before_scheme_end_date: "The tenancy start date must be before the end date for this supported housing scheme"

spec/helpers/tasklist_helper_spec.rb

@@ -1,100 +1,55 @@
 require "rails_helper"
 
 RSpec.describe TasklistHelper do
-  describe "with lettings" do
-    let(:empty_lettings_log) { FactoryBot.create(:lettings_log) }
-    let(:lettings_log) { FactoryBot.create(:lettings_log, :in_progress, needstype: 1) }
-    let(:fake_2021_2022_form) { Form.new("spec/fixtures/forms/2021_2022.json") }
+  let(:now) { Time.utc(2022, 6, 1) }
 
-    context "with 2021 2022 form" do
-      before do
-        allow(FormHandler.instance).to receive(:current_lettings_form).and_return(fake_2021_2022_form)
-      end
+  around do |example|
+    Timecop.freeze(now) do
+      Singleton.__init__(FormHandler)
+      example.run
+    end
+    Timecop.return
+    Singleton.__init__(FormHandler)
+  end
 
-      describe "get next incomplete section" do
-        it "returns the first subsection name if it is not completed" do
-          expect(get_next_incomplete_section(lettings_log).id).to eq("household_characteristics")
-        end
+  describe "with lettings" do
+    let(:empty_lettings_log) { create(:lettings_log) }
+    let(:lettings_log) { build(:lettings_log, :in_progress, needstype: 1) }
 
-        it "returns the first subsection name if it is partially completed" do
-          lettings_log["tenancycode"] = 123
-          expect(get_next_incomplete_section(lettings_log).id).to eq("household_characteristics")
-        end
+    describe "get next incomplete section" do
+      it "returns the first subsection name if it is not completed" do
+        expect(get_next_incomplete_section(lettings_log).id).to eq("household_characteristics")
       end
 
-      describe "get sections count" do
-        it "returns the total of sections if no status is given" do
-          expect(get_subsections_count(empty_lettings_log)).to eq(8)
-        end
-
-        it "returns 0 sections for completed sections if no sections are completed" do
-          expect(get_subsections_count(empty_lettings_log, :completed)).to eq(0)
-        end
-
-        it "returns the number of not started sections" do
-          expect(get_subsections_count(empty_lettings_log, :not_started)).to eq(8)
-        end
-
-        it "returns the number of sections in progress" do
-          expect(get_subsections_count(lettings_log, :in_progress)).to eq(3)
-        end
-
-        it "returns 0 for invalid state" do
-          expect(get_subsections_count(lettings_log, :fake)).to eq(0)
-        end
+      it "returns the first subsection name if it is partially completed" do
+        lettings_log["tenancycode"] = 123
+        expect(get_next_incomplete_section(lettings_log).id).to eq("household_characteristics")
       end
+    end
 
-      describe "get_next_page_or_check_answers" do
-        let(:subsection) { lettings_log.form.get_subsection("household_characteristics") }
-        let(:user) { FactoryBot.build(:user) }
-
-        it "returns the check answers page path if the section has been started already" do
-          expect(next_page_or_check_answers(subsection, lettings_log, user)).to match(/check-answers/)
-        end
-
-        it "returns the first question page path for the section if it has not been started yet" do
-          expect(next_page_or_check_answers(subsection, empty_lettings_log, user)).to match(/tenant-code-test/)
-        end
-
-        it "when first question being not routed to returns the next routed question link" do
-          empty_lettings_log.housingneeds_a = "No"
-          expect(next_page_or_check_answers(subsection, empty_lettings_log, user)).to match(/person-1-gender/)
-        end
+    describe "get sections count" do
+      it "returns the total of sections if no status is given" do
+        expect(get_subsections_count(empty_lettings_log)).to eq(1)
       end
 
-      describe "subsection link" do
-        let(:subsection) { lettings_log.form.get_subsection("household_characteristics") }
-        let(:user) { FactoryBot.build(:user) }
-
-        context "with a subsection that's enabled" do
-          it "returns the subsection link url" do
-            expect(subsection_link(subsection, lettings_log, user)).to match(/household-characteristics/)
-          end
-        end
+      it "returns 0 sections for completed sections if no sections are completed" do
+        expect(get_subsections_count(empty_lettings_log, :completed)).to eq(0)
+      end
 
-        context "with a subsection that cannot be started yet" do
-          before do
-            allow(subsection).to receive(:status).with(lettings_log).and_return(:cannot_start_yet)
-          end
+      it "returns the number of not started sections" do
+        expect(get_subsections_count(empty_lettings_log, :not_started)).to eq(1)
+      end
 
-          it "returns the label instead of a link" do
-            expect(subsection_link(subsection, lettings_log, user)).to match(subsection.label)
-          end
-        end
+      it "returns the number of sections in progress" do
+        expect(get_subsections_count(lettings_log, :in_progress)).to eq(2)
       end
-    end
-  end
 
-  describe "#review_log_text" do
-    around do |example|
-      Timecop.freeze(now) do
-        Singleton.__init__(FormHandler)
-        example.run
+      it "returns 0 for invalid state" do
+        expect(get_subsections_count(lettings_log, :fake)).to eq(0)
       end
-      Singleton.__init__(FormHandler)
     end
 
-    context "with lettings log" do
+    describe "review_log_text" do
       context "when collection_period_open? == true" do
         context "with 2023 deadline" do
           let(:now) { Time.utc(2022, 6, 1) }
@@ -129,6 +84,30 @@ RSpec.describe TasklistHelper do
       end
     end
 
+    describe "subsection link" do
+      let(:lettings_log) { create(:lettings_log, :completed) }
+      let(:subsection) { lettings_log.form.get_subsection("household_characteristics") }
+      let(:user) { build(:user) }
+
+      context "with a subsection that's enabled" do
+        it "returns the subsection link url" do
+          expect(subsection_link(subsection, lettings_log, user)).to match(/household-characteristics/)
+        end
+      end
+
+      context "with a subsection that cannot be started yet" do
+        before do
+          allow(subsection).to receive(:status).with(lettings_log).and_return(:cannot_start_yet)
+        end
+
+        it "returns the label instead of a link" do
+          expect(subsection_link(subsection, lettings_log, user)).to match(subsection.label)
+        end
+      end
+    end
+  end
+
+  describe "#review_log_text" do
     context "with sales log" do
       context "when collection_period_open? == true" do
         let(:now) { Time.utc(2022, 6, 1) }
@@ -142,11 +121,13 @@ RSpec.describe TasklistHelper do
       end
 
       context "when collection_period_open? == false" do
-        let(:now) { Time.utc(2023, 7, 8) }
-        let(:sales_log) { create(:sales_log, :completed, saledate: Time.utc(2023, 2, 8)) }
+        let(:now) { Time.utc(2022, 6, 1) }
+        let!(:sales_log) { create(:sales_log, :completed) }
 
         it "returns relevant text" do
-          expect(review_log_text(sales_log)).to eq("This log is from the 2022/2023 collection window, which is now closed.")
+          Timecop.freeze(now + 1.year) do
+            expect(review_log_text(sales_log)).to eq("This log is from the 2021/2022 collection window, which is now closed.")
+          end
         end
       end
     end

spec/models/validations/sales/setup_validations_spec.rb

@@ -6,43 +6,96 @@ RSpec.describe Validations::Sales::SetupValidations do
   let(:validator_class) { Class.new { include Validations::Sales::SetupValidations } }
 
   describe "#validate_saledate" do
-    context "when saledate is blank" do
-      let(:record) { FactoryBot.build(:sales_log, saledate: nil) }
+    context "with sales_in_crossover_period == false" do
+      context "when saledate is blank" do
+        let(:record) { build(:sales_log, saledate: nil) }
 
-      it "does not add an error" do
-        setup_validator.validate_saledate(record)
+        it "does not add an error" do
+          setup_validator.validate_saledate(record)
 
-        expect(record.errors).to be_empty
+          expect(record.errors).to be_empty
+        end
       end
-    end
 
-    context "when saledate is in the 22/23 financial year" do
-      let(:record) { FactoryBot.build(:sales_log, saledate: Time.zone.local(2023, 1, 1)) }
+      context "when saledate is in the 22/23 financial year" do
+        let(:record) { build(:sales_log, saledate: Time.zone.local(2023, 1, 1)) }
 
-      it "does not add an error" do
-        setup_validator.validate_saledate(record)
+        it "does not add an error" do
+          setup_validator.validate_saledate(record)
 
-        expect(record.errors).to be_empty
+          expect(record.errors).to be_empty
+        end
+      end
+
+      context "when saledate is before the 22/23 financial year" do
+        let(:record) { build(:sales_log, saledate: Time.zone.local(2020, 1, 1)) }
+
+        it "adds error" do
+          setup_validator.validate_saledate(record)
+
+          expect(record.errors[:saledate]).to include("Enter a date within the 22/23 financial year, which is between 1st April 2022 and 31st March 2023")
+        end
       end
-    end
 
-    context "when saledate is before the 22/23 financial year" do
-      let(:record) { FactoryBot.build(:sales_log, saledate: Time.zone.local(2022, 1, 1)) }
+      context "when saledate is after the 22/23 financial year" do
+        let(:record) { build(:sales_log, saledate: Time.zone.local(2025, 4, 1)) }
 
-      it "adds error" do
-        setup_validator.validate_saledate(record)
+        it "adds error" do
+          setup_validator.validate_saledate(record)
 
-        expect(record.errors[:saledate]).to include(I18n.t("validations.setup.saledate.financial_year"))
+          expect(record.errors[:saledate]).to include("Enter a date within the 22/23 financial year, which is between 1st April 2022 and 31st March 2023")
+        end
       end
     end
 
-    context "when saledate is after the 22/23 financial year" do
-      let(:record) { FactoryBot.build(:sales_log, saledate: Time.zone.local(2023, 4, 1)) }
+    context "with sales_in_crossover_period == true" do
+      around do |example|
+        Timecop.freeze(Time.zone.local(2024, 5, 1)) do
+          Singleton.__init__(FormHandler)
+          example.run
+        end
+        Timecop.return
+        Singleton.__init__(FormHandler)
+      end
+
+      context "when saledate is blank" do
+        let(:record) { build(:sales_log, saledate: nil) }
+
+        it "does not add an error" do
+          setup_validator.validate_saledate(record)
+
+          expect(record.errors).to be_empty
+        end
+      end
+
+      context "when saledate is in the 22/23 financial year" do
+        let(:record) { build(:sales_log, saledate: Time.zone.local(2024, 1, 1)) }
+
+        it "does not add an error" do
+          setup_validator.validate_saledate(record)
+
+          expect(record.errors).to be_empty
+        end
+      end
+
+      context "when saledate is before the 22/23 financial year" do
+        let(:record) { build(:sales_log, saledate: Time.zone.local(2020, 5, 1)) }
+
+        it "adds error" do
+          setup_validator.validate_saledate(record)
+
+          expect(record.errors[:saledate]).to include("Enter a date within the 23/24 or 24/25 financial years, which is between 1st April 2023 and 31st March 2025")
+        end
+      end
+
+      context "when saledate is after the 22/23 financial year" do
+        let(:record) { build(:sales_log, saledate: Time.zone.local(2025, 4, 1)) }
 
-      it "adds error" do
-        setup_validator.validate_saledate(record)
+        it "adds error" do
+          setup_validator.validate_saledate(record)
 
-        expect(record.errors[:saledate]).to include(I18n.t("validations.setup.saledate.financial_year"))
+          expect(record.errors[:saledate]).to include("Enter a date within the 23/24 or 24/25 financial years, which is between 1st April 2023 and 31st March 2025")
+        end
       end
     end
   end

spec/rails_helper.rb

@@ -6,6 +6,7 @@ require File.expand_path("../config/environment", __dir__)
 abort("The Rails environment is running in production mode!") if Rails.env.production?
 require "rspec/rails"
 require "capybara/rspec"
+require "capybara-screenshot/rspec"
 require "selenium-webdriver"
 require "view_component/test_helpers"