Improve how adv search handles incoming params (#3278)

* Prevent duplicate hidden fields in adv search form

* Apply incoming inclusive facets to adv search form

* Correct misleading test description
- use rspec `is_expected` syntax to simplify tests

app/components/blacklight/advanced_search_form_component.html.erb

@@ -8,7 +8,7 @@
 <% end %>
 
 <%= form_tag @url, method: @method, class: @classes.join(' '), role: 'search', 'aria-label' => t('blacklight.search.form.submit') do %>
-  <%= render Blacklight::HiddenSearchStateComponent.new(params: @params) %>
+  <%= render Blacklight::HiddenSearchStateComponent.new(params: hidden_search_state_params) %>
 
   <div class="input-criteria">
     <div class="query-criteria mb-4">

app/components/blacklight/advanced_search_form_component.rb

@@ -34,6 +34,12 @@ def sort_fields_select
       select_tag(:sort, options_for_select(options, params[:sort]), class: "form-select custom-select sort-select w-auto", aria: { labelledby: 'advanced-search-sort-label' })
     end
 
+    # Filtered params to pass to hidden search fields
+    # @return [ActiveSupport::HashWithIndifferentAccess]
+    def hidden_search_state_params
+      @params.except(:clause, :f_inclusive, :op, :sort)
+    end
+
     private
 
     def initialize_search_field_controls

app/components/blacklight/facet_field_checkboxes_component.rb

@@ -17,7 +17,7 @@ def presenters
       return to_enum(:presenters) unless block_given?
 
       @facet_field.paginator.items.each do |item|
-        yield @facet_field.facet_field.item_presenter.new(item, @facet_field.facet_field, helpers, @facet_field.key, @facet_field.search_state)
+        yield Blacklight::FacetCheckboxItemPresenter.new(item, @facet_field.facet_field, helpers, @facet_field.key, @facet_field.search_state)
       end
     end
   end

app/presenters/blacklight/facet_checkbox_item_presenter.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module Blacklight
+  class FacetCheckboxItemPresenter < Blacklight::FacetItemPresenter
+    # Check if the query parameters have any inclusive facets with the given value
+    # @return [Boolean]
+    def selected?
+      search_state.filter(facet_config).values(except: [:filters, :missing]).flatten.include?(value)
+    end
+  end
+end

spec/features/advanced_search_spec.rb

@@ -105,11 +105,23 @@
 
   describe "prepopulated advanced search form" do
     before do
-      visit '/catalog/advanced?op=must&clause[0][field]=title&clause[0]query=medicine'
+      visit '/catalog/advanced?op=must&clause[1][field]=title&clause[1]query=medicine&f_inclusive[language_ssim][]=Tibetan&sort=author'
     end
 
-    it "does not create hidden inputs for search fields" do
+    it 'prepopulates the expected fields' do
       expect(page).to have_field 'Title', with: 'medicine'
+      expect(page).to have_field 'Tibetan', checked: true
+      expect(page).to have_select 'op', selected: 'all'
+      expect(page).to have_select 'sort', selected: 'author'
+    end
+
+    it "does not create hidden inputs for fields included in adv search form" do
+      within('form.advanced') do
+        expect(page).to have_no_field('clause[1][query]', type: :hidden, with: 'medicine')
+        expect(page).to have_no_field('f_inclusive[language_ssim][]', type: :hidden, with: 'Tibetan')
+        expect(page).to have_no_field('op', type: :hidden, with: 'must')
+        expect(page).to have_no_field('sort', type: :hidden, with: 'author')
+      end
     end
 
     it "does not have multiple parameters for a search field" do
@@ -121,8 +133,10 @@
 
     it "clears the prepopulated fields when the Start Over button is pressed" do
       expect(page).to have_field 'Title', with: 'medicine'
+      expect(page).to have_field 'Tibetan', checked: true
       click_on 'Start over'
       expect(page).to have_no_field 'Title', with: 'medicine'
+      expect(page).to have_no_field 'Tibetan', checked: true
     end
   end
 end

spec/presenters/blacklight/facet_checkbox_item_presenter_spec.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Blacklight::FacetCheckboxItemPresenter, type: :presenter do
+  subject(:presenter) do
+    described_class.new(facet_item, facet_config, view_context, facet_field, search_state)
+  end
+
+  let(:facet_item) { Blacklight::Solr::Response::Facets::FacetItem.new(value: 'Book', hits: 30) }
+  let(:facet_config) { Blacklight::Configuration::FacetField.new(key: 'format') }
+  let(:view_context) { controller.view_context }
+  let(:filter_field) { Blacklight::SearchState::FilterField.new(facet_config, search_state) }
+  let(:facet_field) { Blacklight::Solr::Response::Facets::FacetField.new('format', [facet_item]) }
+  let(:params) { ActionController::Parameters.new(f_inclusive: { format: ["Book"] }) }
+  let(:blacklight_config) { Blacklight::Configuration.new }
+  let(:search_state) { Blacklight::SearchState.new(params, blacklight_config) }
+
+  before do
+    blacklight_config.add_facet_field 'format'
+  end
+
+  describe '#selected?' do
+    subject { presenter.selected? }
+
+    context 'with a matching inclusive filter' do
+      it { is_expected.to be true }
+    end
+
+    context 'with an inclusive filter that does not match' do
+      let(:params) { ActionController::Parameters.new(f_inclusive: { format: ["Manuscript"] }) }
+
+      it { is_expected.to be false }
+    end
+
+    context 'with a matching exclusive filter' do
+      let(:params) { ActionController::Parameters.new(f: { format: ["Book"] }) }
+
+      it { is_expected.to be false }
+    end
+  end
+end