Skip to content

Simplify FacetSuggestInput by only passing the presenter argument #3608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions app/components/blacklight/search/facet_suggest_input.html.erb
Original file line number Diff line number Diff line change
@@ -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')
%>
10 changes: 6 additions & 4 deletions app/components/blacklight/search/facet_suggest_input.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions app/controllers/concerns/blacklight/catalog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

#
Expand Down
9 changes: 9 additions & 0 deletions app/helpers/blacklight/facets_helper_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/blacklight/facet_field_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion app/views/catalog/facet.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<% component.with_title { facet_field_label(@facet.key) } %>

<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>

Expand Down
50 changes: 30 additions & 20 deletions spec/components/blacklight/search/facet_suggest_input_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 0 additions & 7 deletions spec/controllers/catalog_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
10 changes: 10 additions & 0 deletions spec/helpers/blacklight/facets_helper_behavior_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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