CLDC-2460 Update edit end date functionality (#1721)

* Redirect lettings log to review page if the collection year is closed for editing

* Redirect sales log to review page if the collection year is closed for editing

* Update location default deactivation date

* Update location default deactivation date

* Do not display resources for a closed collection year

* lint

* Extract a method

* tests

* Update breadcrumbs for closed period

app/controllers/lettings_logs_controller.rb

@@ -63,6 +63,8 @@ class LettingsLogsController < LogsController
 
     if @log.unresolved
       redirect_to(send(@log.form.unresolved_log_path, @log))
+    elsif @log.collection_closed_for_editing?
+      redirect_to review_lettings_log_path(@log)
     else
       render("logs/edit", locals: { current_user: })
     end

app/controllers/locations_controller.rb

@@ -280,7 +280,7 @@ private
     if params[:location_deactivation_period].blank?
       return
     elsif params[:location_deactivation_period]["#{key}_type".to_sym] == "default"
-      return FormHandler.instance.start_date_of_earliest_open_collection_period
+      return FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period
     elsif params[:location_deactivation_period][key.to_sym].present?
       return params[:location_deactivation_period][key.to_sym]
     end

app/controllers/sales_logs_controller.rb

@@ -37,8 +37,12 @@ class SalesLogsController < LogsController
 
   def edit
     @log = current_user.sales_logs.visible.find(params[:id])
+    if @log.collection_closed_for_editing?
+      redirect_to review_sales_log_path(@log, sales_log: true)
+    else
       render "logs/edit", locals: { current_user: }
     end
+  end
 
   def destroy
     @log = SalesLog.visible.find(params[:id])

app/controllers/schemes_controller.rb

@@ -318,7 +318,7 @@ private
     if params[:scheme_deactivation_period].blank?
       return
     elsif params[:scheme_deactivation_period]["#{key}_type".to_sym] == "default"
-      return FormHandler.instance.start_date_of_earliest_open_collection_period
+      return FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period
     elsif params[:scheme_deactivation_period][key.to_sym].present?
       return params[:scheme_deactivation_period][key.to_sym]
     end

app/helpers/review_helper.rb

@@ -0,0 +1,28 @@
+module ReviewHelper
+  include CollectionTimeHelper
+
+  def review_log_info_text(log)
+    if log.collection_period_open?
+      "You can review and make changes to this log until #{log.form.submission_deadline.to_formatted_s(:govuk_date)}.".html_safe
+    else
+      start_year = log.startdate ? collection_start_year_for_date(log.startdate) : log.form.start_date.year
+      "This log is from the #{start_year}/#{start_year + 1} collection window, which is now closed."
+    end
+  end
+
+  def review_breadcrumbs(log)
+    class_name = log.class.model_name.human.downcase
+    if log.collection_closed_for_editing?
+      content_for :breadcrumbs, govuk_breadcrumbs(breadcrumbs: {
+        "Logs" => url_for(log.class),
+        "Log #{log.id}" => "",
+      })
+    else
+      content_for :breadcrumbs, govuk_breadcrumbs(breadcrumbs: {
+        "Logs" => url_for(log.class),
+        "Log #{log.id}" => url_for(log),
+        "Review #{class_name}" => "",
+      })
+    end
+  end
+end

app/models/form_handler.rb

@@ -115,10 +115,18 @@ class FormHandler
     in_crossover_period? ? previous_collection_start_date : current_collection_start_date
   end
 
+  def start_date_of_earliest_open_for_editing_collection_period
+    in_edit_crossover_period? ? previous_collection_start_date : current_collection_start_date
+  end
+
   def in_crossover_period?(now: Time.zone.now)
     lettings_in_crossover_period?(now:) || sales_in_crossover_period?(now:)
   end
 
+  def in_edit_crossover_period?(now: Time.zone.now)
+    lettings_in_edit_crossover_period?(now:) || sales_in_edit_crossover_period?(now:)
+  end
+
   def lettings_in_crossover_period?(now: Time.zone.now)
     forms = lettings_forms.values
     forms.count { |form| now.between?(form.start_date, form.new_logs_end_date) } > 1
@@ -157,6 +165,14 @@ class FormHandler
     end
   end
 
+  def earliest_open_for_editing_collection_start_date(now: Time.zone.now)
+    if in_edit_crossover_period?(now:)
+      collection_start_date(now) - 1.year
+    else
+      collection_start_date(now)
+    end
+  end
+
 private
 
   def get_all_forms

app/models/location_deactivation_period.rb

@@ -34,8 +34,8 @@ class LocationDeactivationPeriodValidator < ActiveModel::Validator
       end
     elsif location.location_deactivation_periods.any? { |period| period.reactivation_date.present? && record.deactivation_date.between?(period.deactivation_date, period.reactivation_date - 1.day) }
       record.errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation.during_deactivated_period"))
-    elsif record.deactivation_date.before? FormHandler.instance.start_date_of_earliest_open_collection_period
-      record.errors.add(:deactivation_date, message: I18n.t("validations.location.toggle_date.out_of_range", date: FormHandler.instance.start_date_of_earliest_open_collection_period.to_formatted_s(:govuk_date)))
+    elsif record.deactivation_date.before? FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period
+      record.errors.add(:deactivation_date, message: I18n.t("validations.location.toggle_date.out_of_range", date: FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period.to_formatted_s(:govuk_date)))
     elsif record.deactivation_date.before? location.available_from
       record.errors.add(:deactivation_date, message: I18n.t("validations.location.toggle_date.before_creation", date: location.available_from.to_formatted_s(:govuk_date)))
     end

app/models/log.rb

@@ -188,6 +188,10 @@ class Log < ApplicationRecord
     bulk_upload_id.present?
   end
 
+  def collection_closed_for_editing?
+    form.edit_end_date < Time.zone.now || older_than_previous_collection_year?
+  end
+
 private
 
   # Handle logs that are older than previous collection start date

app/models/scheme_deactivation_period.rb

@@ -34,8 +34,8 @@ class SchemeDeactivationPeriodValidator < ActiveModel::Validator
       end
     elsif scheme.scheme_deactivation_periods.any? { |period| period.reactivation_date.present? && record.deactivation_date.between?(period.deactivation_date, period.reactivation_date - 1.day) }
       record.errors.add(:deactivation_date, message: I18n.t("validations.scheme.deactivation.during_deactivated_period"))
-    elsif record.deactivation_date.before? FormHandler.instance.start_date_of_earliest_open_collection_period
-      record.errors.add(:deactivation_date, message: I18n.t("validations.scheme.toggle_date.out_of_range", date: FormHandler.instance.start_date_of_earliest_open_collection_period.to_formatted_s(:govuk_date)))
+    elsif record.deactivation_date.before? FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period
+      record.errors.add(:deactivation_date, message: I18n.t("validations.scheme.toggle_date.out_of_range", date: FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period.to_formatted_s(:govuk_date)))
     elsif record.deactivation_date.before? scheme.available_from
       record.errors.add(:deactivation_date, message: I18n.t("validations.scheme.toggle_date.before_creation", date: scheme.available_from.to_formatted_s(:govuk_date)))
     end

app/views/form/review.html.erb

@@ -1,10 +1,6 @@
 <% class_name = @log.class.model_name.human.downcase %>
 <% content_for :title, "Review #{class_name}" %>
-<% content_for :breadcrumbs, govuk_breadcrumbs(breadcrumbs: {
-  "Logs" => url_for(@log.class),
-  "Log #{@log.id}" => url_for(@log),
-  "Review #{class_name}" => "",
-}) %>
+<% review_breadcrumbs(@log) %>
 
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds-from-desktop">
@@ -12,7 +8,7 @@
       <%= content_for(:title) %>
     </h1>
     <p class="govuk-body">
-      You can review and make changes to this log until <%= @log.form.submission_deadline.to_formatted_s(:govuk_date) %>.
+      <%= review_log_info_text(@log) %>
     </p>
     <% @log.form.sections.map do |section| %>
       <h2 class="govuk-heading-m"><%= section.label %></h2>

app/views/layouts/_collection_resources.html.erb

@@ -2,6 +2,7 @@
   <h2 class="govuk-heading-m">Collection resources</h2>
   <p class="govuk-body-s">For lettings starting during 1 April 2023 to 31 March 2024 and sales completing during the same period, use the 2023/24 forms.</p>
 
+  <% if FormHandler.instance.lettings_form_for_start_year(2023) && FormHandler.instance.lettings_form_for_start_year(2023).edit_end_date > Time.zone.today %>
     <h3 class="govuk-heading-s">Lettings 2023/24</h3>
     <%= render DocumentListComponent.new(items: [
       {
@@ -49,7 +50,9 @@
         metadata: file_type_size_and_pages("bulk-upload-sales-specification-2023-24.xlsx"),
       },
     ]) %>
+  <% end %>
 
+  <% if FormHandler.instance.lettings_form_for_start_year(2022) && FormHandler.instance.lettings_form_for_start_year(2022).edit_end_date > Time.zone.today %>
     <h3 class="govuk-heading-s">Lettings 2022/23</h3>
     <%= render DocumentListComponent.new(items: [
       {
@@ -87,4 +90,5 @@
         metadata: file_type_size_and_pages("bulk-upload-sales-template-2022-23.xlsx"),
       },
     ]) %>
+  <% end %>
 </div>

app/views/locations/toggle_active.html.erb

@@ -10,7 +10,7 @@
 <%= form_with model: @location_deactivation_period, url: toggle_location_form_path(action, @location), method: "patch", local: true do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
-    <% start_date = FormHandler.instance.earliest_open_collection_start_date(now: @location.available_from) %>
+    <% start_date = FormHandler.instance.earliest_open_for_editing_collection_start_date %>
     <%= f.govuk_error_summary %>
         <%= f.govuk_radio_buttons_fieldset date_type_question(action),
                                             legend: { text: I18n.t("questions.location.toggle_active.apply_from") },

app/views/schemes/toggle_active.html.erb

@@ -10,7 +10,7 @@
 <%= form_with model: @scheme_deactivation_period, url: toggle_scheme_form_path(action, @scheme), method: "patch", local: true do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
-      <% start_date = FormHandler.instance.start_date_of_earliest_open_collection_period %>
+      <% start_date = FormHandler.instance.earliest_open_for_editing_collection_start_date %>
       <%= f.govuk_error_summary %>
       <%= f.govuk_radio_buttons_fieldset date_type_question(action),
                                          legend: { text: I18n.t("questions.scheme.toggle_active.apply_from") },

spec/requests/lettings_logs_controller_spec.rb

@@ -908,15 +908,21 @@ RSpec.describe LettingsLogsController, type: :request do
         end
 
         context "with a signed in user" do
-          context "with lettings logs that are owned or managed by your organisation" do
           before do
-              sign_in user
-              get "/lettings-logs/#{lettings_log.id}", headers:, params: {}
             Timecop.freeze(2021, 4, 1)
             Singleton.__init__(FormHandler)
             completed_lettings_log.update!(startdate: Time.zone.local(2021, 4, 1), voiddate: Time.zone.local(2021, 4, 1), mrcdate: Time.zone.local(2021, 4, 1))
             completed_lettings_log.reload
-              Timecop.unfreeze
+          end
+
+          context "with lettings logs that are owned or managed by your organisation" do
+            before do
+              sign_in user
+              get "/lettings-logs/#{lettings_log.id}", headers:, params: {}
+            end
+
+            after do
+              Timecop.return
               Singleton.__init__(FormHandler)
             end
 
@@ -933,19 +939,51 @@ RSpec.describe LettingsLogsController, type: :request do
             end
 
             it "displays a link to update the log for currently editable logs" do
-              completed_lettings_log.update!(startdate: Time.zone.local(2022, 4, 1), tenancylength: nil)
+              completed_lettings_log.update!(startdate: Time.zone.local(2021, 4, 1), tenancylength: nil)
               completed_lettings_log.reload
 
               get "/lettings-logs/#{completed_lettings_log.id}", headers:, params: {}
-              expect(completed_lettings_log.form.new_logs_end_date).to eq(Time.zone.local(2023, 12, 31))
+              expect(completed_lettings_log.form.new_logs_end_date).to eq(Time.zone.local(2022, 12, 31))
               expect(completed_lettings_log.status).to eq("completed")
               expect(page).to have_link("review and make changes to this log", href: "/lettings-logs/#{completed_lettings_log.id}/review")
             end
+          end
 
-            xit "displays a closed collection window message for previous collection year logs" do
+          context "with lettings logs from a closed collection period before the previous collection" do
+            before do
+              sign_in user
+              Timecop.return
+              Singleton.__init__(FormHandler)
               get "/lettings-logs/#{completed_lettings_log.id}", headers:, params: {}
-              expect(completed_lettings_log.form.new_logs_end_date).to eq(Time.zone.local(2022, 7, 1))
+            end
+
+            it "redirects to review page" do
+              expect(response).to redirect_to("/lettings-logs/#{completed_lettings_log.id}/review")
+            end
+          end
+
+          context "with lettings logs from a closed previous collection period" do
+            before do
+              sign_in user
+              Timecop.freeze(2023, 2, 1)
+              Singleton.__init__(FormHandler)
+              get "/lettings-logs/#{completed_lettings_log.id}", headers:, params: {}
+            end
+
+            after do
+              Timecop.return
+              Singleton.__init__(FormHandler)
+            end
+
+            it "redirects to review page" do
+              expect(response).to redirect_to("/lettings-logs/#{completed_lettings_log.id}/review")
+            end
+
+            it "displays a closed collection window message for previous collection year logs" do
+              get "/lettings-logs/#{completed_lettings_log.id}", headers:, params: {}
+              expect(completed_lettings_log.form.new_logs_end_date).to eq(Time.zone.local(2022, 12, 31))
               expect(completed_lettings_log.status).to eq("completed")
+              follow_redirect!
               expect(page).to have_content("This log is from the 2021/2022 collection window, which is now closed.")
             end
           end
@@ -992,10 +1030,17 @@ RSpec.describe LettingsLogsController, type: :request do
             end
 
             before do
+              Timecop.freeze(2021, 4, 1)
+              Singleton.__init__(FormHandler)
               sign_in user
               get "/lettings-logs/#{section_completed_lettings_log.id}", headers:, params: {}
             end
 
+            after do
+              Timecop.unfreeze
+              Singleton.__init__(FormHandler)
+            end
+
             it "displays a section status for a lettings log" do
               assert_select ".govuk-tag", text: /Not started/, count: 6
               assert_select ".govuk-tag", text: /Completed/, count: 1
@@ -1019,12 +1064,19 @@ RSpec.describe LettingsLogsController, type: :request do
             let!(:location) { FactoryBot.create(:location, scheme:) }
 
             before do
+              Timecop.freeze(2021, 4, 1)
+              Singleton.__init__(FormHandler)
               FactoryBot.create_list(:lettings_log, 3, unresolved: true, created_by: user)
               lettings_log.update!(needstype: 2, scheme:, location:, unresolved: true)
               sign_in user
               get "/lettings-logs/#{lettings_log.id}", headers:, params: {}
             end
 
+            after do
+              Timecop.return
+              Singleton.__init__(FormHandler)
+            end
+
             it "marks it as resolved when both scheme and location exist" do
               lettings_log.reload
               expect(lettings_log.unresolved).to eq(false)

spec/requests/locations_controller_spec.rb

@@ -1832,7 +1832,7 @@ RSpec.describe LocationsController, type: :request do
       let(:startdate) { Time.utc(2022, 9, 11) }
 
       before do
-        Timecop.freeze(Time.utc(2023, 1, 10))
+        Timecop.freeze(Time.utc(2023, 9, 10))
         sign_in user
         create(:location_deactivation_period, deactivation_date:, location:)
         location.save!

spec/requests/organisations_controller_spec.rb

@@ -175,7 +175,10 @@ RSpec.describe OrganisationsController, type: :request do
 
     describe "#show" do
       context "with an organisation that the user belongs to" do
+        let(:set_time) {}
+
         before do
+          set_time
           sign_in user
           get "/organisations/#{organisation.id}", headers:, params: {}
         end
@@ -183,6 +186,42 @@ RSpec.describe OrganisationsController, type: :request do
         it "redirects to details" do
           expect(response).to have_http_status(:redirect)
         end
+
+        context "and 2022 collection window is open" do
+          let(:set_time) { allow(Time).to receive(:now).and_return(Time.zone.local(2023, 1, 1)) }
+
+          it "displays correct resources for 2022/23 and 2023/24 collection years" do
+            follow_redirect!
+            expect(page).to have_content("Lettings 2023/24")
+            expect(page).to have_content("Sales 2023/24")
+            expect(page).to have_content("Lettings 2022/23")
+            expect(page).to have_content("Sales 2022/23")
+          end
+        end
+
+        context "and 2022 collection window is closed for editing" do
+          let(:set_time) { allow(Time).to receive(:now).and_return(Time.zone.local(2024, 1, 1)) }
+
+          it "displays correct resources for 2022/23 and 2023/24 collection years" do
+            follow_redirect!
+            expect(page).to have_content("Lettings 2023/24")
+            expect(page).to have_content("Sales 2023/24")
+            expect(page).not_to have_content("Lettings 2022/23")
+            expect(page).not_to have_content("Sales 2022/23")
+          end
+        end
+
+        context "and 2023 collection window is closed for editing" do
+          let(:set_time) { allow(Time).to receive(:now).and_return(Time.zone.local(2025, 1, 1)) }
+
+          it "displays correct resources for 2022/23 and 2023/24 collection years" do
+            follow_redirect!
+            expect(page).not_to have_content("Lettings 2023/24")
+            expect(page).not_to have_content("Sales 2023/24")
+            expect(page).not_to have_content("Lettings 2022/23")
+            expect(page).not_to have_content("Sales 2022/23")
+          end
+        end
       end
 
       context "with an organisation that are not in scope for the user, i.e. that they do not belong to" do

spec/requests/sales_logs_controller_spec.rb

@@ -556,6 +556,84 @@ RSpec.describe SalesLogsController, type: :request do
       end
     end
 
+    context "when viewing a sales log" do
+      let(:headers) { { "Accept" => "text/html" } }
+      let(:completed_sales_log) { FactoryBot.create(:sales_log, :completed, owning_organisation: user.organisation, created_by: user) }
+
+      before do
+        sign_in user
+        Timecop.freeze(2021, 4, 1)
+        Singleton.__init__(FormHandler)
+        completed_sales_log.update!(saledate: Time.zone.local(2021, 4, 1))
+        completed_sales_log.reload
+      end
+
+      context "with sales logs that are owned by your organisation" do
+        before do
+          get "/sales-logs/#{completed_sales_log.id}", headers:, params: {}
+        end
+
+        after do
+          Timecop.return
+          Singleton.__init__(FormHandler)
+        end
+
+        it "shows the tasklist for sales logs you have access to" do
+          expect(response.body).to match("Log")
+          expect(response.body).to match(completed_sales_log.id.to_s)
+        end
+
+        it "displays a link to update the log for currently editable logs" do
+          completed_sales_log.update!(saledate: Time.zone.local(2021, 4, 1))
+          completed_sales_log.reload
+
+          get "/sales-logs/#{completed_sales_log.id}", headers:, params: {}
+          expect(completed_sales_log.form.new_logs_end_date).to eq(Time.zone.local(2022, 12, 31))
+          expect(completed_sales_log.status).to eq("completed")
+          expect(page).to have_link("review and make changes to this log", href: "/sales-logs/#{completed_sales_log.id}/review?sales_log=true")
+        end
+      end
+
+      context "with sales logs from a closed collection period before the previous collection" do
+        before do
+          sign_in user
+          Timecop.return
+          Singleton.__init__(FormHandler)
+          get "/sales-logs/#{completed_sales_log.id}", headers:, params: {}
+        end
+
+        it "redirects to review page" do
+          expect(response).to redirect_to("/sales-logs/#{completed_sales_log.id}/review?sales_log=true")
+        end
+      end
+
+      context "with sales logs from a closed previous collection period" do
+        before do
+          sign_in user
+          Timecop.freeze(2023, 2, 1)
+          Singleton.__init__(FormHandler)
+          get "/sales-logs/#{completed_sales_log.id}", headers:, params: {}
+        end
+
+        after do
+          Timecop.return
+          Singleton.__init__(FormHandler)
+        end
+
+        it "redirects to review page" do
+          expect(response).to redirect_to("/sales-logs/#{completed_sales_log.id}/review?sales_log=true")
+        end
+
+        it "displays a closed collection window message for previous collection year logs" do
+          get "/sales-logs/#{completed_sales_log.id}", headers:, params: {}
+          expect(completed_sales_log.form.new_logs_end_date).to eq(Time.zone.local(2022, 12, 31))
+          expect(completed_sales_log.status).to eq("completed")
+          follow_redirect!
+          expect(page).to have_content("This log is from the 2021/2022 collection window, which is now closed.")
+        end
+      end
+    end
+
     context "when requesting CSV download" do
       let(:headers) { { "Accept" => "text/html" } }
       let(:search_term) { "foot" }