Merge pull request #1565 from alphagov/341-replace-bodged-parent-brea…

…dcrumbs-with-specialist-topic-breadcrumbs

341 Replace bodged parent breadcrumbs with specialist topic breadcrumbs

CHANGELOG.md

@@ -7,6 +7,10 @@
   useful summary for people upgrading their application, not a replication
   of the commit log.
 
+## unreleased
+
+* Replace bodged parent breadcrumbs with specialist topic breadcrumbs ([PR #1565](https://github.com/alphagov/govuk_publishing_components/pull/1565))
+
 ## 21.26.0
 
 * Update summary list component to allow delete action at the group level and custom heading levels ([PR #1574](https://github.com/alphagov/govuk_publishing_components/pull/1574))

lib/govuk_publishing_components.rb

@@ -12,9 +12,10 @@
 require "govuk_publishing_components/presenters/related_navigation_helper"
 require "govuk_publishing_components/presenters/step_by_step_nav_helper"
 require "govuk_publishing_components/presenters/page_with_step_by_step_navigation"
-require "govuk_publishing_components/presenters/content_breadcrumbs_based_on_parent"
+require "govuk_publishing_components/presenters/content_breadcrumbs_based_on_ancestors"
 require "govuk_publishing_components/presenters/content_breadcrumbs_based_on_priority"
 require "govuk_publishing_components/presenters/content_breadcrumbs_based_on_taxons"
+require "govuk_publishing_components/presenters/content_breadcrumbs_based_on_topic"
 require "govuk_publishing_components/presenters/checkboxes_helper"
 require "govuk_publishing_components/presenters/select"
 require "govuk_publishing_components/presenters/meta_tags"

lib/govuk_publishing_components/presenters/breadcrumb_selector.rb

@@ -25,54 +25,20 @@ def priority_breadcrumbs
 
     private
 
-      def content_item_navigation
-        @content_item_navigation ||= ContextualNavigation.new(content_item, request)
-      end
-
-      def parent_item_navigation
-        @parent_item_navigation ||= ContextualNavigation.new(parent_item, request)
-      end
-
-      def parent_item
-        @parent_item ||= Services.content_store.content_item(content_item_navigation.parent_api_path)
-      rescue GdsApi::ContentStore::ItemNotFound
-        # Do nothing
-      end
-
-      def parent_item_options
-        @parent_item_options ||= options(parent_item_navigation)
-      end
-
-      def content_item_options
-        @content_item_options ||= options(content_item_navigation)
-      end
-
-      def parent_breadcrumbs
-        breadcrumbs = [parent_item_options[:breadcrumbs]].flatten # to ensure breadcrumbs always an array
-        breadcrumbs.last[:is_page_parent] = false
-        breadcrumbs << {
-          title: parent_item["title"],
-          url: parent_item["base_path"],
-          is_page_parent: true,
-        }
-      end
-
       def best_match_option
         return content_item_options unless content_item_navigation.html_publication_with_parent?
 
-        step_by_step_header = parent_item_options[:step_by_step]
-
         {
-          step_by_step: step_by_step_header,
-          breadcrumbs: step_by_step_header ? parent_breadcrumbs.first : parent_breadcrumbs,
+          step_by_step: parent_is_step_by_step?,
+          breadcrumbs: parent_is_step_by_step? ? parent_breadcrumbs.first : parent_breadcrumbs,
         }
       end
 
       def options(navigation)
         if navigation.content_tagged_to_a_finder?
           {
             step_by_step: false,
-            breadcrumbs: navigation.breadcrumbs,
+            breadcrumbs: navigation.finder_breadcrumbs,
           }
         elsif navigation.content_tagged_to_current_step_by_step?
           {
@@ -89,10 +55,10 @@ def options(navigation)
             step_by_step: false,
             breadcrumbs: navigation.breadcrumbs,
           }
-        elsif navigation.content_has_curated_related_items?
+        elsif navigation.content_has_a_topic?
           {
             step_by_step: false,
-            breadcrumbs: navigation.breadcrumbs,
+            breadcrumbs: navigation.topic_breadcrumbs,
           }
         elsif navigation.use_taxon_breadcrumbs?
           {
@@ -108,6 +74,42 @@ def options(navigation)
           {}
         end
       end
+
+      def content_item_navigation
+        @content_item_navigation ||= ContextualNavigation.new(content_item, request)
+      end
+
+      def parent_item_navigation
+        @parent_item_navigation ||= ContextualNavigation.new(parent_item, request)
+      end
+
+      def parent_item
+        @parent_item ||= Services.content_store.content_item(content_item_navigation.parent_api_path)
+      rescue GdsApi::ContentStore::ItemNotFound
+        # Do nothing
+      end
+
+      def parent_item_options
+        @parent_item_options ||= options(parent_item_navigation)
+      end
+
+      def content_item_options
+        @content_item_options ||= options(content_item_navigation)
+      end
+
+      def parent_breadcrumbs
+        breadcrumbs = [parent_item_options[:breadcrumbs]].flatten # to ensure breadcrumbs always an array
+        breadcrumbs.last[:is_page_parent] = false
+        breadcrumbs << {
+          title: parent_item["title"],
+          url: parent_item["base_path"],
+          is_page_parent: true,
+        }
+      end
+
+      def parent_is_step_by_step?
+        parent_item_options[:step_by_step]
+      end
     end
   end
 end

lib/govuk_publishing_components/presenters/content_breadcrumbs_based_on_ancestors.rb

@@ -0,0 +1,41 @@
+module GovukPublishingComponents
+  module Presenters
+    # @private
+    class ContentBreadcrumbsBasedOnAncestors
+      def self.call(content_item)
+        new(content_item).breadcrumbs
+      end
+
+      def initialize(content_item)
+        @content_item = ContentItem.new(content_item)
+      end
+
+      def breadcrumbs
+        ordered_ancestors = all_ancestors.map do |ancestor|
+          { title: ancestor.title, url: ancestor.base_path }
+        end
+
+        ordered_ancestors << { title: "Home", url: "/" }
+        ordered_ancestors.reverse!
+        ordered_ancestors
+      end
+
+    private
+
+      attr_reader :content_item
+
+      def all_ancestors
+        ancestors = []
+
+        parent = content_item.parent
+        while parent
+          ancestors << parent
+
+          parent = parent.parent
+        end
+
+        ancestors
+      end
+    end
+  end
+end

lib/govuk_publishing_components/presenters/content_breadcrumbs_based_on_taxons.rb

@@ -2,6 +2,10 @@ module GovukPublishingComponents
   module Presenters
     # @private
     class ContentBreadcrumbsBasedOnTaxons
+      def self.call(content_item)
+        new(content_item).breadcrumbs
+      end
+
       def initialize(content_item)
         @content_item = ContentItem.new(content_item)
       end

lib/govuk_publishing_components/presenters/content_breadcrumbs_based_on_topic.rb

@@ -0,0 +1,38 @@
+module GovukPublishingComponents
+  module Presenters
+    class ContentBreadcrumbsBasedOnTopic
+      def self.call(content_item)
+        new(content_item).breadcrumbs
+      end
+
+      attr_reader :content_item
+
+      def initialize(content_item)
+        @content_item = content_item
+      end
+
+      def breadcrumbs
+        breadcrumbs = [{ title: "Home", url: "/" }]
+        return breadcrumbs unless topics.present?
+
+        breadcrumbs << topic_breadcrumb
+        breadcrumbs
+      end
+
+      def topic_breadcrumb
+        {
+          title: first_topic["title"],
+          url: first_topic["base_path"],
+        }
+      end
+
+      def first_topic
+        topics.first
+      end
+
+      def topics
+        @topics ||= content_item.dig("links", "topics")
+      end
+    end
+  end
+end

lib/govuk_publishing_components/presenters/contextual_navigation.rb

@@ -26,15 +26,22 @@ def parent_api_path
       end
 
       def taxon_breadcrumbs
-        @taxon_breadcrumbs ||= ContentBreadcrumbsBasedOnTaxons.new(content_item).breadcrumbs
+        @taxon_breadcrumbs ||= ContentBreadcrumbsBasedOnTaxons.call(content_item)
       end
 
       def priority_breadcrumbs
         @priority_breadcrumbs ||= ContentBreadcrumbsBasedOnPriority.call(content_item)
       end
 
+      def topic_breadcrumbs
+        @topic_breadcrumbs ||= ContentBreadcrumbsBasedOnTopic.call(content_item)
+      end
+
       def breadcrumbs
-        return breadcrumbs_based_on_parent unless content_tagged_to_a_finder?
+        breadcrumbs_based_on_ancestors
+      end
+
+      def finder_breadcrumbs
         return [] unless parent_finder
 
         [
@@ -77,6 +84,10 @@ def content_is_a_html_publication?
         content_item["document_type"] == "html_publication"
       end
 
+      def content_has_a_topic?
+        content_item.dig("links", "topics").present?
+      end
+
       def tagged_to_brexit?
         taxons = content_item.dig("links", "taxons").to_a
         brexit_taxon = "d6c2de5d-ef90-45d1-82d4-5f2438369eea"
@@ -119,8 +130,8 @@ def content_tagged_to_other_step_by_steps?
         step_nav_helper.show_also_part_of_step_nav?
       end
 
-      def breadcrumbs_based_on_parent
-        ContentBreadcrumbsBasedOnParent.new(content_item).breadcrumbs[:breadcrumbs]
+      def breadcrumbs_based_on_ancestors
+        ContentBreadcrumbsBasedOnAncestors.call(content_item)
       end
 
       def step_nav_helper

lib/govuk_publishing_components/presenters/page_with_step_by_step_navigation.rb

@@ -3,6 +3,8 @@ module Presenters
     # @private
     # Only used by the step by step component
     class PageWithStepByStepNavigation
+      MAX_SECTION_LENGTH = RelatedNavigationHelper::MAX_SECTION_LENGTH
+
       def initialize(content_store_response, current_path, query_parameters = {})
         @content_item = content_store_response.to_h.deep_symbolize_keys
         @current_path = current_path
@@ -48,7 +50,7 @@ def show_related_links?
       end
 
       def show_also_part_of_step_nav?
-        active_step_by_step? && also_part_of_step_nav.any? && step_navs_combined_list.count < 5
+        active_step_by_step? && also_part_of_step_nav.any? && step_navs_combined_list.count < MAX_SECTION_LENGTH
       end
 
       def related_links

lib/govuk_publishing_components/presenters/related_navigation_helper.rb

@@ -31,7 +31,7 @@ def related_navigation
           }
         when :footer
           {
-            "topics" => related_topics,
+            "topics" => related_topics_or_taxons,
             "topical_events" => related_topical_events,
             "world_locations" => related_world_locations,
             "statistical_data_sets" => related_statistical_data_sets,
@@ -43,7 +43,7 @@ def related_navigation
             "related_items" => related_items,
             "related_guides" => related_guides,
             "collections" => related_document_collections,
-            "topics" => related_topics,
+            "topics" => related_topics_or_taxons,
             "topical_events" => related_topical_events,
             "world_locations" => related_world_locations,
             "statistical_data_sets" => related_statistical_data_sets,
@@ -162,26 +162,27 @@ def related_statistical_data_sets
       end
 
       def related_taxons
-        content_item_links_for("taxons", only: "taxon")
+        @related_taxons ||= content_item_links_for("taxons", only: "taxon")
       end
 
-      def related_topics
-        if related_legacy_topics.any?
-          related_legacy_topics
-        elsif related_taxons.any?
-          related_taxons
-        else
-          []
-        end
+      def related_topics_or_taxons
+        return related_topics if related_topics.any?
+        return related_taxons if related_taxons.any?
+
+        []
       end
 
-      def related_legacy_topics
-        mainstream_browse_page_links = content_item_links_for("mainstream_browse_pages", only: "mainstream_browse_page")
-        topic_links = content_item_links_for("topics", only: "topic")
+      def related_topics
+        @related_topics ||= begin
+          mainstream_browse_page_links = content_item_links_for("mainstream_browse_pages", only: "mainstream_browse_page")
+          topic_links = content_item_links_for("topics", only: "topic")
+
+          return topic_links if topic_links.present? && mainstream_browse_page_links.empty?
 
-        mainstream_browse_page_links + topic_links.find_all do |topic_link|
-          mainstream_browse_page_links.none? do |mainstream_browse_page_link|
-            mainstream_browse_page_link[:text] == topic_link[:text]
+          mainstream_browse_page_links + topic_links.find_all do |topic_link|
+            mainstream_browse_page_links.none? do |mainstream_browse_page_link|
+              mainstream_browse_page_link[:text] == topic_link[:text]
+            end
           end
         end
       end