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

@@ -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
 

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

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" }

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

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