[#3512] Support facebook-style query params in cases where a facet pi…

…vot is configured (#3514)

each_with_object ignores the return value of the supplied block; you need to mutate
the passed object instead.

Prior to this change, the ramifications of bug are that:
* FilterField#permitted_params always includes empty hashes (the original, unmutated
  object we passed to each_with_object) for a facet pivot field.
* When Blacklight::Parameters::deep_merge_permitted_params comes across empty hashes,
  it discards a present hash in favor of the empty one.
* As the permitted params for all facet fields are merged together, the presence of
  even one facet pivot field leads us to discard all other permitted facet field keys.
* We have empty hashes, rather than hashes with permitted facet keys going into
  Blacklight::Parameters#deep_unmangle_params!
* Blacklight::Parameters#deep_unmangle_params! can find no permitted parameters that
  it needs to unmangle, so the facebook params never get unmangled, causing errors
  and unexpected behavior elsewhere in the stack.
* Facebook-style query params don't work if a facet pivot is configured.

Co-authored-by: Christina Chortaria <christinach@users.noreply.github.com>

lib/blacklight/search_state/filter_field.rb

@@ -145,8 +145,8 @@ def include?(item)
       def permitted_params
         if config.pivot
           {
-            filters_key => config.pivot.each_with_object({}) { |key, filter| filter.merge(key => [], "-#{key}" => []) },
-            inclusive_filters_key => config.pivot.each_with_object({}) { |key, filter| filter.merge(key => []) }
+            filters_key => config.pivot.each_with_object({}) { |key, filter| filter.merge!(key => [], "-#{key}" => []) },
+            inclusive_filters_key => config.pivot.each_with_object({}) { |key, filter| filter.merge!(key => []) }
           }
         else
           {

spec/controllers/catalog_controller_spec.rb

@@ -803,7 +803,7 @@ def export_as_mock
     end
 
     it "is empty for empty result sets", :integration do
-      get :page_links, params: { f: { "format" => 'empty-result-set' }, counter: 1 }
+      get :page_links, params: { f: { "format" => ['empty-result-set'] }, counter: 1 }
       expect(assigns(:page_link_data)).to be_empty
     end
   end

spec/features/facets_spec.rb

@@ -138,4 +138,11 @@
       end
     end
   end
+
+  describe 'facebook-style facet parameters' do
+    it 'can perform a search' do
+      visit '/?f[subject_ssim][0]=Iran.+Viza%CC%84rat-i+Kishvar'
+      expect(page).to have_text 'Naqdī barā-yi tamām-i fuṣūl'
+    end
+  end
 end

spec/lib/blacklight/parameters_spec.rb

@@ -52,7 +52,7 @@
     let(:search_state) { Blacklight::SearchState.new(query_params, blacklight_config) }
     let(:blacklight_config) { Blacklight::Configuration.new }
 
-    context 'with facebooks badly mangled query parameters' do
+    context 'with facebook\'s badly mangled query parameters' do
       let(:query_params) do
         ActionController::Parameters.new(
           f: { field: { '0': 'first', '1': 'second' } },
@@ -67,6 +67,17 @@
       it 'normalizes the facets to the expected format' do
         expect(params.permit_search_params.to_h.with_indifferent_access).to include f: { field: %w[first second] }, f_inclusive: { field: %w[first second] }
       end
+
+      context 'when several fields are configured' do
+        before do
+          blacklight_config.add_facet_field 'other_field'
+          blacklight_config.add_facet_field 'some_other_pivot_field', pivot: %w[abc def]
+        end
+
+        it 'normalizes the facets to the expected format' do
+          expect(params.permit_search_params.to_h.with_indifferent_access).to include f: { field: %w[first second] }, f_inclusive: { field: %w[first second] }
+        end
+      end
     end
 
     context 'with filter_search_state_fields set to false' do

spec/lib/blacklight/search_state/filter_field_spec.rb

@@ -198,4 +198,22 @@
       expect(search_state.filter('some_field').include?(OpenStruct.new(value: '1'))).to be true
     end
   end
+
+  describe '#permitted_params' do
+    context 'with a pivot facet' do
+      let(:blacklight_config) do
+        Blacklight::Configuration.new.configure do |config|
+          config.add_facet_field 'my_pivot', pivot: %w[format language_ssim]
+        end
+      end
+
+      it 'marks all the pivot fields as permitted' do
+        field = described_class.new blacklight_config.facet_fields['my_pivot'], search_state
+        expect(field.permitted_params).to eq({
+                                               f: { "-format" => [], "-language_ssim" => [], "format" => [], "language_ssim" => [] },
+                                               f_inclusive: { "format" => [], "language_ssim" => [] }
+                                             })
+      end
+    end
+  end
 end

spec/services/blacklight/search_service_spec.rb

@@ -59,7 +59,7 @@
         allow(blacklight_config).to receive(:default_solr_params).and_return(qt: 'custom_request_handler')
         allow(blacklight_solr).to receive(:send_and_receive) do |path, params|
           expect(path).to eq 'select'
-          expect(params[:params]['facet.field']).to eq ["format", "{!ex=pub_date_ssim_single}pub_date_ssim", "subject_ssim", "language_ssim", "lc_1letter_ssim", "subject_geo_ssim", "subject_era_ssim"]
+          expect(params[:params]['facet.field']).to contain_exactly "format", "{!ex=pub_date_ssim_single}pub_date_ssim", "subject_ssim", "language_ssim", "lc_1letter_ssim", "subject_geo_ssim", "subject_era_ssim"
           expect(params[:params]["facet.query"]).to eq ["pub_date_ssim:[#{5.years.ago.year} TO *]", "pub_date_ssim:[#{10.years.ago.year} TO *]", "pub_date_ssim:[#{25.years.ago.year} TO *]"]
           expect(params[:params]).to include('rows' => 10, 'qt' => "custom_request_handler", 'q' => "", "f.subject_ssim.facet.limit" => 21, 'sort' => "score desc, pub_date_si desc, title_si asc")
         end.and_return('response' => { 'docs' => [] })