From 108e3ef6cb30113543ae6279f808186a15f18b3f Mon Sep 17 00:00:00 2001
From: James Mead <james.mead@digital.cabinet-office.gov.uk>
Date: Wed, 20 Sep 2023 15:54:04 +0100
Subject: [PATCH] Reduce duplication in FindersController & facet_collection

The facet_collection partial only ever uses the filterable facets, so we
can avoid some duplication by selecting the filterable facets in
FindersController#facets. However, there are a few callsites within the
controller which use the full list of facets, so I've introduced a new
`#all_facets` method to handle that without making it available as a
view helper method.
---
 app/controllers/finders_controller.rb        | 14 ++++++++++----
 app/views/finders/_facet_collection.html.erb |  6 +++---
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/app/controllers/finders_controller.rb b/app/controllers/finders_controller.rb
index bef8ceaa03..f026221480 100644
--- a/app/controllers/finders_controller.rb
+++ b/app/controllers/finders_controller.rb
@@ -92,7 +92,7 @@ def render_component(partial, locals)
   def result_set_presenter
     @result_set_presenter ||= ResultSetPresenter.new(
       content_item,
-      facets,
+      all_facets,
       results,
       filter_params,
       sort_presenter,
@@ -106,12 +106,18 @@ def results
     @results ||= ResultSetParser.parse(search_results)
   end
 
+  def all_facets
+    return @facets if defined?(@facets)
+
+    FacetsBuilder.new(content_item:, search_results:, value_hash: filter_params).facets
+  end
+
   def facets
-    @facets ||= FacetsBuilder.new(content_item:, search_results:, value_hash: filter_params).facets
+    all_facets.select(&:filterable?)
   end
 
   def signup_links
-    @signup_links ||= SignupLinksPresenter.new(content_item, facets, keywords).signup_links
+    @signup_links ||= SignupLinksPresenter.new(content_item, all_facets, keywords).signup_links
   end
 
   def initialize_search_query(is_for_feed: false)
@@ -174,7 +180,7 @@ def keywords
 
   def facet_tags
     @facet_tags ||= FacetTagsPresenter.new(
-      facets.select(&:filterable?),
+      facets,
       sort_presenter,
       i_am_a_topic_page_finder:,
     )
diff --git a/app/views/finders/_facet_collection.html.erb b/app/views/finders/_facet_collection.html.erb
index cb198a5ee0..f6cd3f521a 100644
--- a/app/views/finders/_facet_collection.html.erb
+++ b/app/views/finders/_facet_collection.html.erb
@@ -18,12 +18,12 @@
   <% if content_item.all_content_finder? %>
     <%= render partial: 'filter_button'%>
   <% elsif !content_item.all_content_finder? %>
-    <% if facets.select(&:filterable?).any? %>
+    <% if facets.any? %>
       <%= render partial: 'filter_button'%>
     <% end %>
   <% end %>
 
-  <% if facets.select(&:filterable?).any? %>
+  <% if facets.any? %>
     <div data-module="gem-track-click" data-track-category="filterClicked"
         data-track-action="skip-Link" data-track-label="">
       <%= render "govuk_publishing_components/components/skip_link", {
@@ -46,7 +46,7 @@
           </div>
         </div>
         <div class="facets__content" data-module="ga4-event-tracker" data-ga4-filter-container>
-          <% facets.select(&:filterable?).each.with_index do |facet, index| %>
+          <% facets.each.with_index do |facet, index| %>
             <%= render partial: facet, object: facet, locals: { index: } %>
           <% end %>
           <div class="facets__tags-block js-mobile-facet-tag-block"  data-module="remove-filter">