From 68a1c17ae5d7d457bd8ceeb8a09eaaaca6ae72b7 Mon Sep 17 00:00:00 2001 From: Jack <113976590+bibblobcode@users.noreply.github.com> Date: Thu, 23 Feb 2023 09:08:28 +0000 Subject: [PATCH] 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 | 1 + Gemfile.lock | 6 + app/helpers/collection_time_helper.rb | 16 ++ app/helpers/tasklist_helper.rb | 21 +-- app/models/form_handler.rb | 8 +- .../validations/sales/setup_validations.rb | 38 ++++- config/initializers/feature_toggle.rb | 7 +- config/locales/en.yml | 6 +- spec/helpers/tasklist_helper_spec.rb | 143 ++++++++---------- .../sales/setup_validations_spec.rb | 97 +++++++++--- spec/rails_helper.rb | 1 + 11 files changed, 222 insertions(+), 122 deletions(-) diff --git a/Gemfile b/Gemfile index 17e10865c..358e71070 100644 --- a/Gemfile +++ b/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 diff --git a/Gemfile.lock b/Gemfile.lock index fa61ec504..7da2ef437 100644 --- a/Gemfile.lock +++ b/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 diff --git a/app/helpers/collection_time_helper.rb b/app/helpers/collection_time_helper.rb index be0abae20..23014b8d1 100644 --- a/app/helpers/collection_time_helper.rb +++ b/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 diff --git a/app/helpers/tasklist_helper.rb b/app/helpers/tasklist_helper.rb index f4f1d51dd..30112894f 100644 --- a/app/helpers/tasklist_helper.rb +++ b/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 diff --git a/app/models/form_handler.rb b/app/models/form_handler.rb index 8f34288b3..4ba300552 100644 --- a/app/models/form_handler.rb +++ b/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 diff --git a/app/models/validations/sales/setup_validations.rb b/app/models/validations/sales/setup_validations.rb index dadf85650..e7090aabf 100644 --- a/app/models/validations/sales/setup_validations.rb +++ b/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 diff --git a/config/initializers/feature_toggle.rb b/config/initializers/feature_toggle.rb index 60a97c95c..dda281e10 100644 --- a/config/initializers/feature_toggle.rb +++ b/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 diff --git a/config/locales/en.yml b/config/locales/en.yml index bc54b74ce..8665c022a 100644 --- a/config/locales/en.yml +++ b/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" diff --git a/spec/helpers/tasklist_helper_spec.rb b/spec/helpers/tasklist_helper_spec.rb index 1ac6cf738..b5bfe89f5 100644 --- a/spec/helpers/tasklist_helper_spec.rb +++ b/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 diff --git a/spec/models/validations/sales/setup_validations_spec.rb b/spec/models/validations/sales/setup_validations_spec.rb index 74b7834ab..4571ae5cd 100644 --- a/spec/models/validations/sales/setup_validations_spec.rb +++ b/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 diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 81d9246ec..8e175ccf6 100644 --- a/spec/rails_helper.rb +++ b/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"