CLDC-2240 Fix collection logs end date bug (#1551)

* Rename helper method not to conflict with Log method

* Add older_than_previous_collection_year helper method

* Fix spec

app/helpers/collection_time_helper.rb

@@ -1,15 +1,15 @@
 module CollectionTimeHelper
-  def collection_start_year(date)
+  def collection_start_year_for_date(date)
     window_end_date = Time.zone.local(date.year, 4, 1)
     date < window_end_date ? date.year - 1 : date.year
   end
 
   def current_collection_start_year
-    collection_start_year(Time.zone.now)
+    collection_start_year_for_date(Time.zone.now)
   end
 
   def collection_start_date(date)
-    Time.zone.local(collection_start_year(date), 4, 1)
+    Time.zone.local(collection_start_year_for_date(date), 4, 1)
   end
 
   def date_mid_collection_year_formatted(date)
@@ -22,7 +22,7 @@ module CollectionTimeHelper
   end
 
   def collection_end_date(date)
-    Time.zone.local(collection_start_year(date) + 1, 3, 31).end_of_day
+    Time.zone.local(collection_start_year_for_date(date) + 1, 3, 31).end_of_day
   end
 
   def current_collection_end_date

app/models/log.rb

@@ -1,4 +1,6 @@
 class Log < ApplicationRecord
+  include CollectionTimeHelper
+
   self.abstract_class = true
 
   belongs_to :owning_organisation, class_name: "Organisation", optional: true
@@ -87,6 +89,8 @@ class Log < ApplicationRecord
   end
 
   def collection_period_open?
+    return false if older_than_previous_collection_year?
+
     form.end_date > Time.zone.today
   end
 
@@ -133,6 +137,13 @@ class Log < ApplicationRecord
 
 private
 
+  # Handle logs that are older than previous collection start date
+  def older_than_previous_collection_year?
+    return false unless startdate
+
+    startdate < previous_collection_start_date
+  end
+
   def plural_gender_for_person(person_num)
     gender = public_send("sex#{person_num}".to_sym)
     return unless gender

app/models/validations/sales/sale_information_validations.rb

@@ -48,7 +48,7 @@ module Validations::Sales::SaleInformationValidations
   end
 
   def validate_discounted_ownership_value(record)
-    return unless record.saledate && collection_start_year(record.saledate) >= 2024
+    return unless record.saledate && collection_start_year_for_date(record.saledate) >= 2024
     return unless record.value && record.deposit && record.ownershipsch
     return unless record.mortgage || record.mortgageused == 2
     return unless record.discount || record.grant || record.type == 29

spec/helpers/collection_time_helper_spec.rb

@@ -50,7 +50,7 @@ RSpec.describe CollectionTimeHelper do
       let(:now) { Time.utc(2022, 8, 3) }
 
       it "returns the same year as the current start year" do
-        expect(collection_start_year(now)).to eq(2022)
+        expect(collection_start_year_for_date(now)).to eq(2022)
       end
 
       it "returns the correct current start date" do
@@ -66,7 +66,7 @@ RSpec.describe CollectionTimeHelper do
       let(:now) { Time.utc(2022, 2, 3) }
 
       it "returns the previous year as the current start year" do
-        expect(collection_start_year(now)).to eq(2021)
+        expect(collection_start_year_for_date(now)).to eq(2021)
       end
 
       it "returns the correct current start date" do

spec/models/lettings_log_spec.rb

@@ -1,6 +1,7 @@
 require "rails_helper"
 require "shared/shared_examples_for_derived_fields"
 
+# rubocop:disable RSpec/MessageChain
 # rubocop:disable RSpec/AnyInstance
 RSpec.describe LettingsLog do
   let(:different_managing_organisation) { create(:organisation) }
@@ -3188,5 +3189,55 @@ RSpec.describe LettingsLog do
       end
     end
   end
+
+  describe "#collection_period_open?" do
+    let(:log) { build(:lettings_log, startdate:) }
+
+    context "when startdate is nil" do
+      let(:startdate) { nil }
+
+      it "returns false" do
+        expect(log.collection_period_open?).to eq(true)
+      end
+    end
+
+    context "when older_than_previous_collection_year" do
+      let(:previous_collection_start_date) { Time.zone.local(2050, 4, 1) }
+      let(:startdate) { previous_collection_start_date - 1.day }
+
+      before do
+        allow(log).to receive(:previous_collection_start_date).and_return(previous_collection_start_date)
+      end
+
+      it "returns true" do
+        expect(log.collection_period_open?).to eq(false)
+      end
+    end
+
+    context "when form end date is in the future" do
+      let(:startdate) { nil }
+
+      before do
+        allow(log).to receive_message_chain(:form, :end_date).and_return(Time.zone.now + 1.day)
+      end
+
+      it "returns true" do
+        expect(log.collection_period_open?).to eq(true)
+      end
+    end
+
+    context "when form end date is in the past" do
+      let(:startdate) { Time.zone.local(2020, 4, 1) }
+
+      before do
+        allow(log).to receive_message_chain(:form, :end_date).and_return(Time.zone.now - 1.day)
+      end
+
+      it "returns false" do
+        expect(log.collection_period_open?).to eq(false)
+      end
+    end
+  end
 end
 # rubocop:enable RSpec/AnyInstance
+# rubocop:enable RSpec/MessageChain

spec/models/sales_log_spec.rb

@@ -1,6 +1,7 @@
 require "rails_helper"
 require "shared/shared_examples_for_derived_fields"
 
+# rubocop:disable RSpec/MessageChain
 # rubocop:disable RSpec/AnyInstance
 RSpec.describe SalesLog, type: :model do
   let(:owning_organisation) { create(:organisation) }
@@ -607,5 +608,55 @@ RSpec.describe SalesLog, type: :model do
       end
     end
   end
+
+  describe "#collection_period_open?" do
+    let(:log) { build(:sales_log, saledate:) }
+
+    context "when saledate is nil" do
+      let(:saledate) { nil }
+
+      it "returns false" do
+        expect(log.collection_period_open?).to eq(true)
+      end
+    end
+
+    context "when older_than_previous_collection_year" do
+      let(:previous_collection_start_date) { Time.zone.local(2050, 4, 1) }
+      let(:saledate) { previous_collection_start_date - 1.day }
+
+      before do
+        allow(log).to receive(:previous_collection_start_date).and_return(previous_collection_start_date)
+      end
+
+      it "returns true" do
+        expect(log.collection_period_open?).to eq(false)
+      end
+    end
+
+    context "when form end date is in the future" do
+      let(:saledate) { nil }
+
+      before do
+        allow(log).to receive_message_chain(:form, :end_date).and_return(Time.zone.now + 1.day)
+      end
+
+      it "returns true" do
+        expect(log.collection_period_open?).to eq(true)
+      end
+    end
+
+    context "when form end date is in the past" do
+      let(:saledate) { Time.zone.local(2020, 4, 1) }
+
+      before do
+        allow(log).to receive_message_chain(:form, :end_date).and_return(Time.zone.now - 1.day)
+      end
+
+      it "returns false" do
+        expect(log.collection_period_open?).to eq(false)
+      end
+    end
+  end
 end
 # rubocop:enable RSpec/AnyInstance
+# rubocop:enable RSpec/MessageChain

spec/requests/form_controller_spec.rb

@@ -201,25 +201,27 @@ RSpec.describe FormController, type: :request do
 
     describe "GET" do
       context "with form pages" do
-        context "when forms exist for multiple years" do
-          let(:lettings_log_year_1) { create(:lettings_log, owning_organisation: organisation, created_by: user) }
-          let(:lettings_log_year_2) { create(:lettings_log, :setup_completed, startdate: Time.zone.local(2022, 5, 1), owning_organisation: organisation, created_by: user) }
+        context "when forms exist" do
+          let(:lettings_log) { create(:lettings_log, :setup_completed, startdate: Time.zone.local(2022, 5, 1), owning_organisation: organisation, created_by: user) }
 
-          before do
-            Timecop.freeze(Time.zone.local(2021, 5, 1))
-            lettings_log_year_1.update!(startdate: Time.zone.local(2021, 5, 1))
-            Timecop.unfreeze
-            allow(lettings_log_year_1.form).to receive(:end_date).and_return(Time.zone.today + 1.day)
-          end
+          it "displays the question details" do
+            get "/lettings-logs/#{lettings_log.id}/tenant-code-test", headers: headers, params: {}
 
-          it "displays the correct question details for each lettings log based on form year" do
-            get "/lettings-logs/#{lettings_log_year_1.id}/tenant-code-test", headers: headers, params: {}
-            expect(response.body).to include("What is the tenant code?")
-            get "/lettings-logs/#{lettings_log_year_2.id}/tenant-code-test", headers: headers, params: {}
+            expect(response).to be_ok
             expect(response.body).to match("Different question header text for this year - 2023")
           end
         end
 
+        context "when question not routed to" do
+          let(:lettings_log) { create(:lettings_log, :setup_completed, startdate: Time.zone.local(2022, 5, 1), owning_organisation: organisation, created_by: user) }
+
+          it "redirects to log" do
+            get "/lettings-logs/#{lettings_log.id}/scheme", headers: headers, params: {}
+
+            expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}")
+          end
+        end
+
         context "when lettings logs are not owned or managed by your organisation" do
           it "does not show form pages for lettings logs you don't have access to" do
             get "/lettings-logs/#{unauthorized_lettings_log.id}/person-1-age", headers: headers, params: {}