From a4ebcc3f460025c954903c6c88d4779e2de1e678 Mon Sep 17 00:00:00 2001
From: Arthur Campbell <51094020+arfacamble@users.noreply.github.com>
Date: Thu, 15 Jun 2023 15:02:19 +0100
Subject: [PATCH] CLDC-2449 production recursion error on finding url for next
 incomplete section (#1698)

* protect against stack level errors in the case where a log has the status in progress but due to changes in the form should now have the status completed
further update the lettings log factory as the :completed trait was producing logs that were not copmlete

* add tests for calculate_status that implicitly also check if the factory traits for copmleted are working
also some minor renaming and refactoring

* correct linting errors

* remove status update but protect against error resurfacing in the future
---
 app/models/form.rb             |  2 +-
 app/models/log.rb              |  8 ++++----
 spec/factories/lettings_log.rb |  2 ++
 spec/models/form_spec.rb       | 16 ++++++++++++++--
 spec/models/log_spec.rb        | 22 ++++++++++++++++++++++
 5 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/app/models/form.rb b/app/models/form.rb
index 16c896e4c..7a55b2e0d 100644
--- a/app/models/form.rb
+++ b/app/models/form.rb
@@ -124,7 +124,7 @@ class Form
   def next_incomplete_section_redirect_path(subsection, log)
     subsection_ids = subsections.map(&:id)
 
-    if log.status == "completed"
+    if log.status == "completed" || log.calculate_status == "completed" # if a log's status in in progress but then fields are made optional, all its subsections are complete, resulting in a stack error
       return first_question_in_last_subsection(subsection_ids)
     end
 
diff --git a/app/models/log.rb b/app/models/log.rb
index 452aa67c6..c02fdb02d 100644
--- a/app/models/log.rb
+++ b/app/models/log.rb
@@ -139,9 +139,9 @@ class Log < ApplicationRecord
   def calculate_status
     return "deleted" if discarded_at.present?
 
-    if all_fields_completed? && errors.empty?
+    if all_subsections_completed? && errors.empty?
       "completed"
-    elsif all_fields_nil?
+    elsif all_subsections_unstarted?
       "not_started"
     else
       "in_progress"
@@ -210,11 +210,11 @@ private
     self.status = calculate_status
   end
 
-  def all_fields_completed?
+  def all_subsections_completed?
     form.subsections.all? { |subsection| subsection.complete?(self) || subsection.not_displayed_in_tasklist?(self) }
   end
 
-  def all_fields_nil?
+  def all_subsections_unstarted?
     not_started_statuses = %i[not_started cannot_start_yet]
     form.subsections.all? { |subsection| not_started_statuses.include? subsection.status(self) }
   end
diff --git a/spec/factories/lettings_log.rb b/spec/factories/lettings_log.rb
index ec119fbdc..8cb459117 100644
--- a/spec/factories/lettings_log.rb
+++ b/spec/factories/lettings_log.rb
@@ -146,6 +146,8 @@ FactoryBot.define do
       joint { 3 }
       address_line1 { "fake address" }
       town_or_city { "London" }
+      ppcodenk { 0 }
+      tshortfall_known { 1 }
     end
     trait :export do
       tenancycode { "987654" }
diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb
index b63d05f62..439cd24b1 100644
--- a/spec/models/form_spec.rb
+++ b/spec/models/form_spec.rb
@@ -190,8 +190,6 @@ RSpec.describe Form, type: :model do
         FormHandler.instance.use_real_forms!
 
         example.run
-
-        FormHandler.instance.use_fake_forms!
       end
 
       it "finds the path to the section after" do
@@ -202,6 +200,20 @@ RSpec.describe Form, type: :model do
         expect(form.next_incomplete_section_redirect_path(subsection, lettings_log)).to eq("joint")
       end
     end
+
+    context "when a log has status in progress but all subsections are complete" do
+      let(:lettings_log) { build(:lettings_log, :completed, status: "in_progress") }
+      let(:subsection) { form.get_subsection("setup") }
+
+      before do
+        Timecop.return
+        FormHandler.instance.use_real_forms!
+      end
+
+      it "does not raise a Stack Error" do
+        expect { form.next_incomplete_section_redirect_path(subsection, lettings_log) }.not_to raise_error
+      end
+    end
   end
 
   describe "#reset_not_routed_questions_and_invalid_answers" do
diff --git a/spec/models/log_spec.rb b/spec/models/log_spec.rb
index 0cba24086..fb686e744 100644
--- a/spec/models/log_spec.rb
+++ b/spec/models/log_spec.rb
@@ -5,4 +5,26 @@ RSpec.describe Log, type: :model do
     expect(SalesLog).to be < described_class
     expect(LettingsLog).to be < described_class
   end
+
+  describe "#calculate_status" do
+    it "returns the correct status for a completed sales log" do
+      complete_sales_log = create(:sales_log, :completed, status: nil)
+      expect(complete_sales_log.calculate_status).to eq "completed"
+    end
+
+    it "returns the correct status for an in progress sales log" do
+      in_progress_sales_log = create(:sales_log, :in_progress, status: nil)
+      expect(in_progress_sales_log.calculate_status).to eq "in_progress"
+    end
+
+    it "returns the correct status for a completed lettings log" do
+      complete_lettings_log = create(:lettings_log, :completed, status: nil)
+      expect(complete_lettings_log.calculate_status).to eq "completed"
+    end
+
+    it "returns the correct status for an in progress lettings log" do
+      in_progress_lettings_log = create(:lettings_log, :in_progress, status: nil)
+      expect(in_progress_lettings_log.calculate_status).to eq "in_progress"
+    end
+  end
 end