From bb6cbee22c3e28f95e6659cd8d858597b2be46c6 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Thu, 26 Jul 2018 11:30:01 -0700 Subject: [PATCH] Add a remove link for selected facets in the JSON response Fixes #1951 --- .../blacklight/facets_helper_behavior.rb | 4 +- app/presenters/blacklight/json_presenter.rb | 16 ++---- app/views/catalog/index.json.jbuilder | 23 +++++---- lib/blacklight/search_state.rb | 2 + spec/controllers/catalog_controller_spec.rb | 4 +- .../blacklight/json_presenter_spec.rb | 26 +++++----- .../views/catalog/index.json.jbuilder_spec.rb | 51 ++++++++++++------- 7 files changed, 68 insertions(+), 58 deletions(-) diff --git a/app/helpers/blacklight/facets_helper_behavior.rb b/app/helpers/blacklight/facets_helper_behavior.rb index f68815364d..24f5ef3906 100644 --- a/app/helpers/blacklight/facets_helper_behavior.rb +++ b/app/helpers/blacklight/facets_helper_behavior.rb @@ -185,8 +185,8 @@ def facet_field_in_params? field # Check if the query parameters have the given facet field with the # given value. # - # @param [Object] field - # @param [Object] item facet value + # @param [String] field + # @param [String] item facet value # @return [Boolean] def facet_in_params?(field, item) value = facet_value_for_facet_item(item) diff --git a/app/presenters/blacklight/json_presenter.rb b/app/presenters/blacklight/json_presenter.rb index 565b9a89a5..f8b5417fc0 100644 --- a/app/presenters/blacklight/json_presenter.rb +++ b/app/presenters/blacklight/json_presenter.rb @@ -5,7 +5,6 @@ class JsonPresenter # @param [Solr::Response] response raw solr response. # @param [Configuration] blacklight_config the configuration - # @param [Array] facets list of facets def initialize(response, blacklight_config) @response = response @blacklight_config = blacklight_config @@ -17,18 +16,9 @@ def documents @response.documents end - def search_facets_as_json - facets_from_request.map do |display_facet| - next if display_facet.items.empty? - f = display_facet.as_json - f.stringify_keys! - f.delete "options" - f["label"] = facet_configuration_for_field(f["name"]).label - f["items"] = f["items"].as_json.each do |i| - i['label'] ||= i['value'] - end - f - end.compact + # @return [Array] + def search_facets + facets_from_request.select { |display_facet| display_facet.items.present? } end # extract the pagination info from the response object diff --git a/app/views/catalog/index.json.jbuilder b/app/views/catalog/index.json.jbuilder index 75d1cc98a5..4c5bd1ccf5 100644 --- a/app/views/catalog/index.json.jbuilder +++ b/app/views/catalog/index.json.jbuilder @@ -37,27 +37,32 @@ json.data do end json.included do - json.array! @presenter.search_facets_as_json do |facet| + json.array! @presenter.search_facets do |facet| json.type 'facet' - json.id facet['name'] + json.id facet.name json.attributes do - json.label facet['label'] + facet_config = facet_configuration_for_field(facet.name) + json.label facet_field_label(facet_config.key) json.items do - json.array! facet['items'] do |item| + json.array! facet.items do |item| json.id json.attributes do - json.label item['label'] - json.value item['value'] - json.hits item['hits'] + json.label item.label + json.value item.value + json.hits item.hits end json.links do - json.self path_for_facet(facet['name'], item['value'], only_path: false) + if facet_in_params?(facet.name, item.value) + json.remove search_action_path(search_state.remove_facet_params(facet.name, item.value)) + else + json.self path_for_facet(facet.name, item.value, only_path: false) + end end end end end json.links do - json.self search_facet_path(id: facet['name'], only_path: false) + json.self search_facet_path(id: facet.name, only_path: false) end end diff --git a/lib/blacklight/search_state.rb b/lib/blacklight/search_state.rb index 93a79cdb53..1ace7bd32a 100644 --- a/lib/blacklight/search_state.rb +++ b/lib/blacklight/search_state.rb @@ -97,6 +97,8 @@ def add_facet_params_and_redirect(field, item) # removes the field value from params[:f] # removes the field if there are no more values in params[:f][field] # removes additional params (page, id, etc..) + # @param [String] field + # @param [String] item def remove_facet_params(field, item) if item.respond_to? :field field = item.field diff --git a/spec/controllers/catalog_controller_spec.rb b/spec/controllers/catalog_controller_spec.rb index 1d90684bcc..01396cbda6 100644 --- a/spec/controllers/catalog_controller_spec.rb +++ b/spec/controllers/catalog_controller_spec.rb @@ -130,8 +130,8 @@ expect(response).to be_successful end let(:json) { JSON.parse(response.body) } - let(:pages) { json['meta']['pages'] } - let(:docs) { json['data'] } + let(:pages) { json['meta']['pages'] } + let(:docs) { json['data'] } let(:facets) { json['included'].select { |x| x['type'] == 'facet' } } let(:search_fields) { json['included'].select { |x| x['type'] == 'search_field' } } diff --git a/spec/presenters/blacklight/json_presenter_spec.rb b/spec/presenters/blacklight/json_presenter_spec.rb index 2b02a60725..2a63835397 100644 --- a/spec/presenters/blacklight/json_presenter_spec.rb +++ b/spec/presenters/blacklight/json_presenter_spec.rb @@ -11,14 +11,14 @@ aggregations: aggregations) end let(:docs) do - [ - SolrDocument.new(id: '123', title_tsim: 'Book1', author_tsim: 'Julie'), - SolrDocument.new(id: '456', title_tsim: 'Book2', author_tsim: 'Rosie') - ] + [ + SolrDocument.new(id: '123', title_tsim: 'Book1', author_tsim: 'Julie'), + SolrDocument.new(id: '456', title_tsim: 'Book2', author_tsim: 'Rosie') + ] end let(:aggregations) do - { 'format_si' => Blacklight::Solr::Response::Facets::FacetField.new("format_si", [{ label: "Book", value: 'Book', hits: 20 }])} + { 'format_si' => Blacklight::Solr::Response::Facets::FacetField.new("format_si", [{ label: "Book", value: 'Book', hits: 20 }]) } end let(:config) do @@ -30,17 +30,15 @@ let(:presenter) { described_class.new(response, config) } + describe '#search_facets' do + let(:search_facets) { presenter.search_facets } - describe '#search_facets_as_json' do - subject { presenter.search_facets_as_json } - - context 'for defined facets that are present in the response' do - it 'has a label' do - expect(subject.first["label"]).to eq 'Format' + context 'with defined facets that are present in the response' do + it 'returns them' do + expect(search_facets.map(&:name)).to eq ['format_si'] end end - context 'when there are defined facets that are not in the response' do before do config.add_facet_field 'example_query_facet_field', label: 'Publish Date', query: {} @@ -53,8 +51,8 @@ } end - it 'shows only facets that are defined' do - expect(subject.map { |f| f['name'] }).to eq ['format_si'] + it 'filters out the facets that are not defined' do + expect(search_facets.map(&:name)).to eq ['format_si'] end end end diff --git a/spec/views/catalog/index.json.jbuilder_spec.rb b/spec/views/catalog/index.json.jbuilder_spec.rb index 9acf3f40f3..2edd5f8652 100644 --- a/spec/views/catalog/index.json.jbuilder_spec.rb +++ b/spec/views/catalog/index.json.jbuilder_spec.rb @@ -20,15 +20,23 @@ JSON.parse(rendered).with_indifferent_access end + let(:book_facet_item) do + Blacklight::Solr::Response::Facets::FacetItem.new('value' => 'Book', 'hits' => 30, 'label' => 'Book') + end + + let(:format_facet) do + Blacklight::Solr::Response::Facets::FacetField.new('format', + [book_facet_item], + 'label' => 'Format') + end + before do allow(view).to receive(:blacklight_config).and_return(config) allow(view).to receive(:search_action_path).and_return('http://test.host/some/search/url') allow(view).to receive(:search_facet_path).and_return('http://test.host/some/facet/url') allow(presenter).to receive(:pagination_info).and_return({ current_page: 1, next_page: 2, prev_page: nil }) - allow(presenter).to receive(:search_facets_as_json).and_return( - [{ 'name' => "format", 'label' => "Format", - 'items' => [{ 'value' => 'Book', 'hits' => 30, 'label' => 'Book' }] }]) + allow(presenter).to receive(:search_facets).and_return([format_facet]) assign :presenter, presenter assign :response, response end @@ -82,22 +90,29 @@ ]) end - it "has facet information and links" do - expect(hash).to include(:included) - - facets = hash[:included].select { |x| x['type'] == 'facet' } - expect(facets).to be_present + describe 'facets' do + let(:facets) { hash[:included].select { |x| x['type'] == 'facet' } } + let(:format) { facets.find { |x| x['id'] == 'format' } } + let(:format_items) { format['attributes']['items'] } + let(:format_item_attributes) { format_items.map { |x| x['attributes'] } } - expect(facets.map { |x| x['id'] }).to include 'format' - - format = facets.find { |x| x['id'] == 'format' } - - expect(format['links']).to include self: 'http://test.host/some/facet/url' - expect(format['attributes']).to include :items - expect(format['attributes']['label']).to eq 'Format' - - format_items = format['attributes']['items'].map { |x| x['attributes'] } + context 'when no facets have been selected' do + it 'has facet information and links' do + expect(facets).to be_present + expect(facets.map { |x| x['id'] }).to include 'format' + expect(format['links']).to include self: 'http://test.host/some/facet/url' + expect(format['attributes']['label']).to eq 'Format' + expect(format_item_attributes).to match_array [{ value: 'Book', hits: 30, label: 'Book' }] + end + end - expect(format_items).to match_array [{value: 'Book', hits: 30, label: 'Book'}] + context 'when facets have been selected' do + before do + params[:f] = { format: ['Book'] } + end + it 'has a link to remove the selected value' do + expect(format_items.first['links']).to eq('remove' => 'http://test.host/some/search/url') + end + end end end