CLDC-1969 completed sections count bug (#1509)

* feat: fix subsection count bug

* feat: add tests for sales and lettings behaviour separately

* refactor: linting

* feat: test fix wip

* feat: fix test

* feat: update test

* feat: update test

* reafactor: lint

* refactor: lint

app/helpers/tasklist_helper.rb

@@ -6,7 +6,7 @@ module TasklistHelper
   end
 
   def get_subsections_count(log, status = :all)
-    return log.form.subsections.count { |subsection| subsection.applicable_questions(log).count.positive? } if status == :all
+    return log.form.subsections.count { |subsection| subsection.displayed_in_tasklist?(log) } if status == :all
 
     log.form.subsections.count { |subsection| subsection.status(log) == status && subsection.applicable_questions(log).count.positive? }
   end

app/models/form/sales/subsections/discounted_ownership_scheme.rb

@@ -41,6 +41,6 @@ class Form::Sales::Subsections::DiscountedOwnershipScheme < ::Form::Subsection
   end
 
   def displayed_in_tasklist?(log)
-    log.ownershipsch == 2
+    log.ownershipsch.nil? || log.ownershipsch == 2
   end
 end

app/models/form/sales/subsections/outright_sale.rb

@@ -26,7 +26,7 @@ class Form::Sales::Subsections::OutrightSale < ::Form::Subsection
   end
 
   def displayed_in_tasklist?(log)
-    log.ownershipsch == 3
+    log.ownershipsch.nil? || log.ownershipsch == 3
   end
 
   def leasehold_charge_pages

app/models/form/sales/subsections/shared_ownership_scheme.rb

@@ -46,6 +46,6 @@ class Form::Sales::Subsections::SharedOwnershipScheme < ::Form::Subsection
   end
 
   def displayed_in_tasklist?(log)
-    log.ownershipsch == 1
+    log.ownershipsch.nil? || log.ownershipsch == 1
   end
 end

app/views/logs/edit.html.erb

@@ -11,7 +11,7 @@
     </h1>
 
     <% if @log.status == "in_progress" %>
-      <p class="govuk-body govuk-!-margin-bottom-7"><%= get_subsections_count(@log, :completed) %> of <%= get_subsections_count(@log, :all) %> sections completed.</p>
+      <p class="govuk-body govuk-!-margin-bottom-7"><%= get_subsections_count(@log, :completed) %> of <%= get_subsections_count(@log) %> subsections completed.</p>
       <p class="govuk-body govuk-!-margin-bottom-2">
         <% next_incomplete_section = get_next_incomplete_section(@log) %>
       </p>

spec/factories/lettings_log.rb

@@ -3,7 +3,7 @@ FactoryBot.define do
     created_by { FactoryBot.create(:user) }
     owning_organisation { created_by.organisation }
     managing_organisation { created_by.organisation }
-    trait :about_completed do
+    trait :setup_completed do
       renewal { 0 }
       needstype { 1 }
       rent_type { 1 }

spec/features/form/tasklist_page_spec.rb

@@ -33,7 +33,7 @@ RSpec.describe "Task List" do
   let(:setup_completed_log) do
     FactoryBot.create(
       :lettings_log,
-      :about_completed,
+      :setup_completed,
       owning_organisation: user.organisation,
       managing_organisation: user.organisation,
       created_by: user,
@@ -67,9 +67,17 @@ RSpec.describe "Task List" do
     expect(page).to have_content("This log has not been started.")
   end
 
-  it "shows number of completed sections if one section is completed" do
-    visit("/lettings-logs/#{setup_completed_log.id}")
-    expect(page).to have_content("1 of 8 sections completed.")
+  context "when testing completed subsection count" do
+    let(:real_2021_2022_form) { Form.new("config/forms/2021_2022.json") }
+
+    before do
+      allow(FormHandler.instance).to receive(:get_form).and_return(real_2021_2022_form)
+    end
+
+    it "shows number of completed sections if one section is completed" do
+      visit("/lettings-logs/#{setup_completed_log.id}")
+      expect(page).to have_content("1 of 7 subsections completed.")
+    end
   end
 
   it "show skip link for next incomplete section" do

spec/features/lettings_log_spec.rb

@@ -95,7 +95,7 @@ RSpec.describe "Lettings Log Features" do
     end
 
     context "when visiting a subsection check answers page" do
-      let(:lettings_log) { FactoryBot.create(:lettings_log, :about_completed) }
+      let(:lettings_log) { FactoryBot.create(:lettings_log, :setup_completed) }
 
       it "has the correct breadcrumbs with the correct links" do
         visit lettings_log_setup_check_answers_path(lettings_log)

spec/helpers/tasklist_helper_spec.rb

@@ -27,26 +27,30 @@ RSpec.describe TasklistHelper do
     end
 
     describe "get sections count" do
-      let(:fake_2021_2022_form) { Form.new("spec/fixtures/forms/2021_2022.json") }
+      let(:real_2021_2022_form) { Form.new("config/forms/2021_2022.json") }
 
       before do
-        allow(FormHandler.instance).to receive(:get_form).and_return(fake_2021_2022_form)
+        allow(FormHandler.instance).to receive(:get_form).and_return(real_2021_2022_form)
       end
 
-      it "returns the total of sections if no status is given" do
-        expect(get_subsections_count(empty_lettings_log)).to eq(8)
-      end
+      context "with an empty lettings log" do
+        it "returns the total displayed subsections count if no status is given" do
+          expect(get_subsections_count(empty_lettings_log)).to eq(7)
+        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)
+        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
       end
 
-      it "returns the number of not started sections" do
-        expect(get_subsections_count(empty_lettings_log, :not_started)).to eq(8)
-      end
+      context "with a partially complete lettings log" do
+        it "returns the total displayed subsections count if no status is given" do
+          expect(get_subsections_count(lettings_log)).to eq(7)
+        end
 
-      it "returns the number of sections in progress" do
-        expect(get_subsections_count(lettings_log, :in_progress)).to eq(2)
+        it "returns the completed sections count" do
+          expect(get_subsections_count(lettings_log, :completed)).to eq(1)
+        end
       end
 
       it "returns 0 for invalid state" do
@@ -77,6 +81,37 @@ RSpec.describe TasklistHelper do
     end
   end
 
+  describe "with sales" do
+    let(:empty_sales_log) { create(:sales_log, owning_organisation: nil) }
+    let(:completed_sales_log) { build(:sales_log, :completed) }
+
+    describe "get sections count" do
+      context "with an empty sales log" do
+        it "returns the total displayed subsections count if no status is given (includes all 3 sale information subsections)" do
+          expect(get_subsections_count(empty_sales_log)).to eq(9)
+        end
+
+        it "returns 0 sections for completed sections if no sections are completed" do
+          expect(get_subsections_count(empty_sales_log, :completed)).to eq(0)
+        end
+      end
+
+      context "with a completed sales log" do
+        it "returns the total displayed subsections count if no status is given (includes only the 1 relevant sale information subsection)" do
+          expect(get_subsections_count(completed_sales_log)).to eq(7)
+        end
+
+        it "returns the completed sections count" do
+          expect(get_subsections_count(completed_sales_log, :completed)).to eq(7)
+        end
+      end
+
+      it "returns 0 for invalid state" do
+        expect(get_subsections_count(completed_sales_log, :fake)).to eq(0)
+      end
+    end
+  end
+
   describe "#review_log_text" do
     context "with sales log" do
       context "when collection_period_open? == true" do

spec/models/validations/financial_validations_spec.rb

@@ -86,7 +86,7 @@ RSpec.describe Validations::FinancialValidations do
     end
 
     context "when outstanding rent or charges is yes" do
-      let(:record) { FactoryBot.create(:lettings_log, :about_completed, startdate: Time.zone.now) }
+      let(:record) { FactoryBot.create(:lettings_log, :setup_completed, startdate: Time.zone.now) }
 
       it "expects that a shortfall is provided" do
         record.hbrentshortfall = 1

spec/requests/form_controller_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe FormController, type: :request do
   let(:setup_complete_lettings_log) do
     create(
       :lettings_log,
-      :about_completed,
+      :setup_completed,
       status: 1,
       created_by: user,
     )
@@ -203,7 +203,7 @@ RSpec.describe FormController, type: :request 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, :about_completed, startdate: Time.zone.local(2022, 5, 1), 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) }
 
           before do
             Timecop.freeze(Time.zone.local(2021, 5, 1))