From 2af96752bfb7cbdbcf345f1f49dc74bd03e81bad Mon Sep 17 00:00:00 2001
From: Bruce Bolt <bruce.bolt@digital.cabinet-office.gov.uk>
Date: Wed, 17 Jul 2024 10:33:41 +0100
Subject: [PATCH 1/2] Do not attempt to show siblings if there aren't any

When a parent manual has been withdrawn and redirected, we don't get the
value for `child_section_groups` as the redirect content item doesn't
contain this value.

Therefore not showing any siblings items at all when this is the case,
as we can't be sure what the correct parent actually is for this manual
section.
---
 app/presenters/hmrc_manual_section_presenter.rb     |  4 ++++
 .../hmrc_manual_section_presenter_test.rb           | 13 +++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/app/presenters/hmrc_manual_section_presenter.rb b/app/presenters/hmrc_manual_section_presenter.rb
index 26d264025..40841490a 100644
--- a/app/presenters/hmrc_manual_section_presenter.rb
+++ b/app/presenters/hmrc_manual_section_presenter.rb
@@ -30,6 +30,8 @@ def breadcrumbs
   end
 
   def previous_and_next_links
+    return unless siblings
+
     siblings = {}
 
     if previous_sibling
@@ -68,6 +70,8 @@ def siblings
 
     child_section_groups = parent_for_section.dig("details", "child_section_groups")
 
+    return unless child_section_groups
+
     sibling_child_sections = child_section_groups.map do |group|
       included_section = group["child_sections"].find { |section| section["section_id"].include?(current_section_id) }
       group["child_sections"] if included_section.present?
diff --git a/test/presenters/hmrc_manual_section_presenter_test.rb b/test/presenters/hmrc_manual_section_presenter_test.rb
index 9227b7541..7de0ea49b 100644
--- a/test/presenters/hmrc_manual_section_presenter_test.rb
+++ b/test/presenters/hmrc_manual_section_presenter_test.rb
@@ -146,6 +146,19 @@ class PresentedHmrcManualSectionTest < HmrcManualSectionPresenterTestCase
       assert_equal expected_previous_link, presented_manual_section.previous_and_next_links
     end
 
+    test "presents no previous or next links if there is no previous or next section" do
+      manual_base_path = schema_item("vatgpb2000")["details"]["manual"]["base_path"]
+      manual = schema_item("vat-government-public-bodies", "hmrc_manual")
+
+      manual["details"]["child_section_groups"] = []
+
+      stub_content_store_has_item(manual_base_path, manual.to_json)
+
+      expected_links = {}
+
+      assert_equal expected_links, presented_manual_section.previous_and_next_links
+    end
+
     def presented_manual_section(overrides = {})
       presented_item("vatgpb2000", overrides)
     end

From c0e1751ad547b79443de3b9e925a130d5c5adcd7 Mon Sep 17 00:00:00 2001
From: Bruce Bolt <bruce.bolt@digital.cabinet-office.gov.uk>
Date: Wed, 17 Jul 2024 10:45:10 +0100
Subject: [PATCH 2/2] Rename variable

The `siblings` variable has the same name as a method, so renaming the
variable to avoid confusion.
---
 app/presenters/hmrc_manual_section_presenter.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/presenters/hmrc_manual_section_presenter.rb b/app/presenters/hmrc_manual_section_presenter.rb
index 40841490a..ba79a126b 100644
--- a/app/presenters/hmrc_manual_section_presenter.rb
+++ b/app/presenters/hmrc_manual_section_presenter.rb
@@ -32,23 +32,23 @@ def breadcrumbs
   def previous_and_next_links
     return unless siblings
 
-    siblings = {}
+    section_siblings = {}
 
     if previous_sibling
-      siblings[:previous_page] = {
+      section_siblings[:previous_page] = {
         title: I18n.t("manuals.previous_page"),
         url: previous_sibling["base_path"],
       }
     end
 
     if next_sibling
-      siblings[:next_page] = {
+      section_siblings[:next_page] = {
         title: I18n.t("manuals.next_page"),
         url: next_sibling["base_path"],
       }
     end
 
-    siblings
+    section_siblings
   end
 
 private