From c606970c84931583370640c7bca5906482ff1f42 Mon Sep 17 00:00:00 2001
From: Jessica Jones <jessica.jones@digital.cabinet-office.gov.uk>
Date: Mon, 14 Oct 2024 12:23:20 +0100
Subject: [PATCH] Minor refactor of contents list logic

The show_contents_list? method is hard to understand, and bugs
have surfaced here.

The special logic around showing contents lists on MOJ manual sections
only should only be called by the manual section presenter class.
---
 app/presenters/content_item/contents_list.rb  |  8 -----
 app/presenters/manual_section_presenter.rb    | 10 ++++++
 .../content_item/contents_list_test.rb        | 31 +------------------
 .../manual_section_presenter_test.rb          | 14 +++++++++
 4 files changed, 25 insertions(+), 38 deletions(-)

diff --git a/app/presenters/content_item/contents_list.rb b/app/presenters/content_item/contents_list.rb
index 85cbfb70d..c420fa877 100644
--- a/app/presenters/content_item/contents_list.rb
+++ b/app/presenters/content_item/contents_list.rb
@@ -19,7 +19,6 @@ def contents_items
     end
 
     def show_contents_list?
-      return false if exclude_contents_list_for_manual_section?
       return false if contents_items.count < 2
       return true if contents_items.count > 2
       return false if no_first_item?
@@ -111,12 +110,5 @@ def first_item
     def no_first_item?
       first_item.nil?
     end
-
-    def exclude_contents_list_for_manual_section?
-      # MOJ require contents lists for manual section pages.
-
-      moj_content = content_item.dig("links", "organisations", 0, "content_id") == "dcc907d6-433c-42df-9ffb-d9c68be5dc4d"
-      document_type == "manual_section" && !moj_content
-    end
   end
 end
diff --git a/app/presenters/manual_section_presenter.rb b/app/presenters/manual_section_presenter.rb
index f22ba5fd1..97e6a1499 100644
--- a/app/presenters/manual_section_presenter.rb
+++ b/app/presenters/manual_section_presenter.rb
@@ -4,6 +4,8 @@ class ManualSectionPresenter < ContentItemPresenter
   include ContentItem::ContentsList
   include ContentItem::ManualSection
 
+  MOJ_ORGANISATION_CONTENT_ID = "dcc907d6-433c-42df-9ffb-d9c68be5dc4d".freeze
+
   def base_path
     manual["base_path"]
   end
@@ -52,6 +54,14 @@ def main
     end
   end
 
+  def show_contents_list?
+    organisation_content_id == MOJ_ORGANISATION_CONTENT_ID
+  end
+
+  def organisation_content_id
+    content_item.dig("links", "organisations", 0, "content_id")
+  end
+
 private
 
   def manual
diff --git a/test/presenters/content_item/contents_list_test.rb b/test/presenters/content_item/contents_list_test.rb
index f84048c39..a9e9fb3a7 100644
--- a/test/presenters/content_item/contents_list_test.rb
+++ b/test/presenters/content_item/contents_list_test.rb
@@ -2,20 +2,9 @@
 
 class ContentItemContentsListTest < ActiveSupport::TestCase
   def setup
-    content_item = {
-      "title" => "thing",
-      "document_type" => "manual_section",
-      "links" => {
-        "organisations" => [
-          {
-            "content_id" => "dcc907d6-433c-42df-9ffb-d9c68be5dc4d",
-          },
-        ],
-      },
-    }
+    content_item = { "title" => "thing" }
     @contents_list = Object.new
     @contents_list.stubs(:content_item).returns(content_item)
-    @contents_list.stubs(:document_type).returns(content_item["document_type"])
     @contents_list.stubs(:view_context)
                   .returns(ApplicationController.new.view_context)
     @contents_list.extend(ContentItem::ContentsList)
@@ -108,24 +97,6 @@ def body
     assert @contents_list.show_contents_list?
   end
 
-  test "#show_contents_list? returns false if the content item is a manual section but excluded from displaying content lists" do
-    content_item = {
-      "title" => "thing",
-      "document_type" => "manual_section",
-      "links" => {
-        "organisations" => [
-          {
-            "content_id" => "91cd6143-69d5-4f27-99ff-a52fb0d51c74",
-          },
-        ],
-      },
-    }
-
-    @contents_list.stubs(:content_item).returns(content_item)
-    @contents_list.stubs(:document_type).returns(content_item["document_type"])
-    assert_not @contents_list.show_contents_list?
-  end
-
   test "#show_contents_list? returns true if number of contents items is 2 and the first item's character count is above 415 including a list" do
     class << @contents_list
       def body
diff --git a/test/presenters/manual_section_presenter_test.rb b/test/presenters/manual_section_presenter_test.rb
index 48be20d6c..ab9802eb4 100644
--- a/test/presenters/manual_section_presenter_test.rb
+++ b/test/presenters/manual_section_presenter_test.rb
@@ -76,6 +76,20 @@ class PresentedManualSectionTest < ManualSectionPresenterTestCase
       assert_match first_section_content_sample, presented_manual_section.main.first[:content]
     end
 
+    test "hides the contents list by default" do
+      manual_section = schema_item("what-is-content-design")
+      assert_equal false, present_example(manual_section).show_contents_list?
+    end
+
+    test "shows the contents list if organisation is MOJ" do
+      content_item = schema_item("what-is-content-design", "manual_section")
+      moj_content_id = ManualSectionPresenter::MOJ_ORGANISATION_CONTENT_ID
+      content_item["links"]["organisations"] = [{ "content_id" => moj_content_id }]
+      presented = present_example(content_item)
+
+      assert_equal true, presented.show_contents_list?
+    end
+
     def presented_manual_section(overrides = {})
       presented_item("what-is-content-design", overrides)
     end