Merge pull request #3608 from projectblacklight/simplify-facet-search

Simplify FacetSuggestInput by only passing the presenter argument

Author: cgalarza

app/components/blacklight/search/facet_suggest_input.html.erb

@@ -1,12 +1,12 @@
-<label for="facet_suggest_<%= facet.key %>">
-  <%= I18n.t('blacklight.search.facets.suggest.label', field_label: presenter&.label) %>
+<label for="facet_suggest_<%= key %>">
+  <%= I18n.t('blacklight.search.facets.suggest.label', field_label: label) %>
 </label>
-<%= text_field_tag "facet_suggest_#{facet.key}",
+<%= text_field_tag "facet_suggest_#{key}",
   nil,
   class: "facet-suggest form-control mb-3",
   data: {
-    facet_field: facet.key,
-    facet_search_context: presenter.view_context.search_facet_path(id: facet.key)
+    facet_field: key,
+    facet_search_context: helpers.search_facet_path(id: key)
   },
   placeholder: I18n.t('blacklight.search.form.search.placeholder')
 %>

app/components/blacklight/search/facet_suggest_input.rb

@@ -3,17 +3,19 @@
 module Blacklight
   module Search
     class FacetSuggestInput < Blacklight::Component
-      def initialize(facet:, presenter:)
-        @facet = facet
+      def initialize(presenter:)
         @presenter = presenter
       end
 
       private
 
-      attr_accessor :facet, :presenter
+      attr_accessor :presenter
+
+      delegate :suggest, :key, :label, to: :presenter
 
       def render?
-        facet&.suggest != false
+        # Draw if suggest is true or not present
+        suggest != false
       end
     end
   end

app/controllers/concerns/blacklight/catalog.rb

@@ -15,7 +15,6 @@ module Blacklight::Catalog
   included do
     if respond_to? :helper_method
       helper_method :sms_mappings, :has_search_parameters?
-      helper_method :search_facet_path
     end
 
     record_search_parameters
@@ -138,15 +137,6 @@ def has_search_parameters?
     params[:search_field].present? || search_state.has_constraints?
   end
 
-  def search_facet_path(options = {})
-    opts = search_state
-           .to_h
-           .merge(action: "facet", only_path: true)
-           .merge(options)
-           .except(:page)
-    url_for opts
-  end
-
   private
 
   #

app/helpers/blacklight/facets_helper_behavior.rb

@@ -7,6 +7,15 @@ def facet_field_presenter(facet_config, display_facet)
     facet_config.presenter.new(facet_config, display_facet, self)
   end
 
+  def search_facet_path(options = {})
+    opts = search_state
+           .to_h
+           .merge(action: "facet", only_path: true)
+           .merge(options)
+           .except(:page)
+    url_for opts
+  end
+
   private
 
   def facet_value_for_facet_item item

app/presenters/blacklight/facet_field_presenter.rb

@@ -4,7 +4,7 @@ module Blacklight
   class FacetFieldPresenter
     attr_reader :facet_field, :display_facet, :view_context, :search_state
 
-    delegate :key, to: :facet_field
+    delegate :key, :suggest, to: :facet_field
     delegate :field_name, to: :display_facet
 
     def initialize(facet_field, display_facet, view_context, search_state = view_context.search_state)

app/views/catalog/facet.html.erb

@@ -3,7 +3,7 @@
   <% view_config = blacklight_config.view_config(document_index_view_type) %>
 
   <div class="facet-filters card card-body bg-light p-3 mb-3 border-0">
-    <%= render Blacklight::Search::FacetSuggestInput.new(facet: @facet, presenter: @presenter) %>
+    <%= render Blacklight::Search::FacetSuggestInput.new(presenter: @presenter) %>
     <%= render partial: 'facet_index_navigation' if @facet.index_range && @display_facet.index? %>
   </div>
 

spec/components/blacklight/search/facet_suggest_input_spec.rb

@@ -4,55 +4,65 @@
 
 RSpec.describe Blacklight::Search::FacetSuggestInput, type: :component do
   let(:facet) { Blacklight::Configuration::FacetField.new key: 'language_facet', suggest: true }
-  let(:presenter) { instance_double(Blacklight::FacetFieldPresenter) }
-  let(:view_context) { double(ActionView::Base) }
+  let(:presenter) { Blacklight::FacetFieldPresenter.new(facet, nil, nil, nil) }
+  let(:component) { described_class.new(presenter: presenter) }
 
   before do
-    allow(presenter).to receive_messages(label: 'Language', view_context: view_context)
-    allow(view_context).to receive(:search_facet_path).and_return('/catalog/facet/language_facet')
+    allow(presenter).to receive_messages(label: 'Language')
   end
 
   it 'has an input with the facet-suggest class, which the javascript needs to find it' do
-    rendered = render_inline(described_class.new(facet: facet, presenter: presenter))
-    expect(rendered.css("input.facet-suggest").count).to eq 1
+    with_request_url "/catalog/facet/language_facet" do
+      rendered = render_inline component
+      expect(rendered.css("input.facet-suggest").count).to eq 1
+    end
   end
 
   it 'has an input with the data-facet-field attribute, which the javascript needs to determine the correct query' do
-    rendered = render_inline(described_class.new(facet: facet, presenter: presenter))
-    expect(rendered.css('input[data-facet-field="language_facet"]').count).to eq 1
+    with_request_url "/catalog/facet/language_facet" do
+      rendered = render_inline component
+      expect(rendered.css('input[data-facet-field="language_facet"]').count).to eq 1
+    end
   end
 
   it 'has an input with the data-facet-search-context attribute, which the javascript needs to determine the current search context' do
-    allow(view_context).to receive(:search_facet_path).and_return('/catalog/facet/language_facet?f%5Bformat%5D%5B%5D=Book&facet.prefix=R&facet.sort=index&q=tibet&search_field=all_fields')
-    rendered = render_inline(described_class.new(facet: facet, presenter: presenter))
-    expect(rendered.css('input[data-facet-search-context="/catalog/facet/language_facet?f%5Bformat%5D%5B%5D=Book&facet.prefix=R&facet.sort=index&q=tibet&search_field=all_fields"]').count).to eq 1
+    with_request_url "/catalog/facet/language_facet?f%5Bformat%5D%5B%5D=Book&facet.prefix=R&facet.sort=index&q=tibet&search_field=all_fields" do
+      rendered = render_inline component
+      expect(rendered.css('input[data-facet-search-context="/catalog/facet/language_facet.html?f%5Bformat%5D%5B%5D=Book&facet.prefix=R&facet.sort=index&q=tibet&search_field=all_fields"]').count).to eq 1
+    end
   end
 
   it 'has a visible label that is associated with the input' do
-    rendered = render_inline(described_class.new(facet: facet, presenter: presenter))
-    label = rendered.css('label').first
-    expect(label.text.strip).to eq 'Filter Language'
+    with_request_url "/catalog/facet/language_facet" do
+      rendered = render_inline component
+      label = rendered.css('label').first
+      expect(label.text.strip).to eq 'Filter Language'
 
-    id_in_label_for = label.attribute('for').text
-    expect(id_in_label_for).to eq('facet_suggest_language_facet')
+      id_in_label_for = label.attribute('for').text
+      expect(id_in_label_for).to eq('facet_suggest_language_facet')
 
-    expect(rendered.css('input').first.attribute('id').text).to eq id_in_label_for
+      expect(rendered.css('input').first.attribute('id').text).to eq id_in_label_for
+    end
   end
 
   context 'when the facet is explicitly configured to suggest: false' do
     let(:facet) { Blacklight::Configuration::FacetField.new key: 'language_facet', suggest: false }
 
     it 'does not display' do
-      expect(render_inline(described_class.new(facet: facet, presenter: presenter)).to_s).to eq ''
+      with_request_url "/catalog/facet/language_facet" do
+        expect(render_inline(component).to_s).to eq ''
+      end
     end
   end
 
   context 'when the facet is not explicitly configured with a suggest key' do
     let(:facet) { Blacklight::Configuration::FacetField.new key: 'language_facet' }
 
     it 'displays' do
-      rendered = render_inline(described_class.new(facet: facet, presenter: presenter))
-      expect(rendered.css("input.facet-suggest").count).to eq 1
+      with_request_url "/catalog/facet/language_facet" do
+        rendered = render_inline component
+        expect(rendered.css("input.facet-suggest").count).to eq 1
+      end
     end
   end
 end

spec/controllers/catalog_controller_spec.rb

@@ -785,13 +785,6 @@ def export_as_mock
     end
   end
 
-  describe "search_facet_path" do
-    it "is the same as the catalog path" do
-      get :index, params: { page: 1 }
-      expect(controller.send(:search_facet_path, id: "some_facet", page: 5)).to eq facet_catalog_path(id: "some_facet")
-    end
-  end
-
   describe "page_links" do
     it "has prev/next docs and result set data for non-empty result sets", :integration do
       get :page_links, params: { f: { "format" => 'Book' }, counter: 2 }

spec/helpers/blacklight/facets_helper_behavior_spec.rb

@@ -26,4 +26,14 @@
       expect(presenter).to be_a SomePresenter
     end
   end
+
+  describe "#search_facet_path" do
+    before do
+      params[:controller] = 'catalog'
+    end
+
+    it "is the same as the catalog path" do
+      expect(helper.search_facet_path(id: "some_facet", page: 5)).to eq facet_catalog_path(id: "some_facet")
+    end
+  end
 end