Facet URL helpers should use the facet configuration to determine the…

… key to use in the query parameters

app/helpers/blacklight/render_constraints_helper_behavior.rb

@@ -69,7 +69,7 @@ def render_filter_element(facet, values, localized_params)
 
     safe_join(values.map do |val|
       next if val.blank? # skip empty string
-      render_constraint_element( facet_field_label(facet), facet_display_value(facet, val),
+      render_constraint_element( facet_field_label(facet_config.key), facet_display_value(facet, val),
                   :remove => search_action_path(remove_facet_params(facet, val, localized_params)),
                   :classes => ["filter", "filter-" + facet.parameterize]
                 )

app/helpers/blacklight/url_helper_behavior.rb

@@ -215,17 +215,19 @@ def add_facet_params(field, item, source_params = params)
 
     facet_config = facet_configuration_for_field(field)
 
+    url_field = facet_config.key
+
     value = facet_value_for_facet_item(item)
 
     p = reset_search_params(source_params)
     p[:f] = (p[:f] || {}).dup # the command above is not deep in rails3, !@#$!@#$
-    p[:f][field] = (p[:f][field] || []).dup
+    p[:f][url_field] = (p[:f][url_field] || []).dup
 
-    if facet_config.single and not p[:f][field].empty?
-      p[:f][field] = []
+    if facet_config.single and not p[:f][url_field].empty?
+      p[:f][url_field] = []
     end
 
-    p[:f][field].push(value)
+    p[:f][url_field].push(value)
 
     if item and item.respond_to?(:fq) and item.fq
       item.fq.each do |f,v|
@@ -262,16 +264,20 @@ def remove_facet_params(field, item, source_params=params)
       field = item.field
     end
 
+    facet_config = facet_configuration_for_field(field)
+
+    url_field = facet_config.key
+
     value = facet_value_for_facet_item(item)
 
     p = reset_search_params(source_params)
     # need to dup the facet values too,
     # if the values aren't dup'd, then the values
     # from the session will get remove in the show view...
     p[:f] = (p[:f] || {}).dup
-    p[:f][field] = (p[:f][field] || []).dup
-    p[:f][field] = p[:f][field] - [value]
-    p[:f].delete(field) if p[:f][field].size == 0
+    p[:f][url_field] = (p[:f][url_field] || []).dup
+    p[:f][url_field] = p[:f][url_field] - [value]
+    p[:f].delete(url_field) if p[:f][url_field].size == 0
     p.delete(:f) if p[:f].empty?
     p
   end

spec/helpers/url_helper_spec.rb

@@ -343,6 +343,16 @@
       expect(result_params[:f]["facet_field"]).to eq ["facet_value"]
     end
 
+    it "should use the facet's key in the url" do
+      allow(helper).to receive(:facet_configuration_for_field).with('some_field').and_return(double(single: true, field: "a_solr_field", key: "some_key"))
+
+      result_params = helper.add_facet_params('some_field', 'my_value', {})
+
+      expect(result_params[:f]['some_key']).to have(1).item
+      expect(result_params[:f]['some_key'].first).to eq 'my_value'
+    end
+
+
     it "should add a facet param to existing facet constraints" do
       allow(helper).to receive(:params).and_return(@params_existing_facets)
 
@@ -374,7 +384,7 @@
     end    
 
     it "should replace facets for facets configured as single" do
-      allow(helper).to receive(:facet_configuration_for_field).with('single_value_facet_field').and_return(double(:single => true))
+      allow(helper).to receive(:facet_configuration_for_field).with('single_value_facet_field').and_return(double(single: true, key: "single_value_facet_field"))
       params = { :f => { 'single_value_facet_field' => 'other_value'}}
       allow(helper).to receive(:params).and_return params
 
@@ -441,6 +451,46 @@
       expect(added_facet_params_from_facet_action).to eq added_facet_params.except(Blacklight::Solr::FacetPaginator.request_keys[:page], Blacklight::Solr::FacetPaginator.request_keys[:sort])     
     end
   end
+
+  describe "#remove_facet_params" do
+    let(:query_params) { { f: facet_params }}
+    let(:facet_params) { { } }
+    it "should remove the facet / value tuple from the query parameters" do
+      facet_params['some_field'] = ['some_value', 'another_value']
+
+      params = helper.remove_facet_params('some_field', 'some_value', query_params)
+
+      expect(params[:f]['some_field']).not_to include 'some_value'
+      expect(params[:f]['some_field']).to include 'another_value'
+    end
+
+    it "should use the facet's key configuration" do
+      allow(helper).to receive(:facet_configuration_for_field).with('some_field').and_return(double(single: true, field: "a_solr_field", key: "some_key"))
+      facet_params['some_key'] = ['some_value', 'another_value']
+
+      params = helper.remove_facet_params('some_field', 'some_value', query_params)
+
+      expect(params[:f]['some_key']).not_to eq 'some_value'
+      expect(params[:f]['some_key']).to include 'another_value'
+    end
+
+    it "should remove the facet entirely when the last facet value is removed" do
+      facet_params['another_field'] = ['some_value']
+      facet_params['some_field'] = ['some_value']
+
+      params = helper.remove_facet_params('some_field', 'some_value', query_params)
+
+      expect(params[:f]).not_to include 'some_field'
+    end
+
+    it "should remove the 'f' parameter entirely when no facets remain" do
+      facet_params['some_field'] = ['some_value']
+
+      params = helper.remove_facet_params('some_field', 'some_value', query_params)
+
+      expect(params).not_to include :f
+    end
+  end
 
   describe "#bookmarks_export_url" do
     it "should be the bookmark url with an encrypted user token" do