Merge pull request #117 from MITLibraries/gdt-142-filter-removal-panel

Add search summary panel and full filter support
MITLibraries · Feb 20, 2024 · 4783334 · 4783334
2 parents fe1fa4b + 02db6ad
commit 4783334
Show file tree

Hide file tree

Showing 42 changed files with 641 additions and 466 deletions.
diff --git a/app/assets/stylesheets/partials/_panels.scss b/app/assets/stylesheets/partials/_panels.scss
@@ -99,3 +99,36 @@
     }
   }
 }
+
+.filter-summary {
+  color: $black;
+  background-color: $gray-l3;
+  margin-top: 0;
+  border: 0;
+  padding: 2rem;
+  padding-top: 1.5rem;
+  display: flex;
+  .hd-filter-summary {
+    margin-right: 2rem;
+    padding-top: 0.3rem;
+  }
+  a {
+    font-size: $fs-small;
+    text-decoration: none;
+    padding: .5rem;
+    &::before {
+      font-family: FontAwesome;
+      content: '\f00d';
+      padding-right: .25rem;
+    }
+    &:hover,
+    &:focus {
+      color: $white;
+      background-color: $black;
+    }
+    margin-right: 2rem;
+  }
+  @media (max-width: $bp-screen-md) {
+    display: block;
+  }
+}
diff --git a/app/assets/stylesheets/partials/_search.scss b/app/assets/stylesheets/partials/_search.scss
@@ -5,7 +5,7 @@
 /* basic search bar */
 .basic-search {
   background-color: #989898;
-  margin-bottom: 1rem;
+  margin-bottom: 0rem;
   padding: 1.6rem 2rem;
 
   details {

diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb
@@ -33,7 +33,11 @@ def extract_filters(response)
     aggs = response&.data&.search&.to_h&.dig('aggregations')
     return if aggs.blank?
 
-    aggs.select { |_, agg_values| agg_values.present? }
+    # We use aggregations to determine which terms can be filtered. However, agg names do not include 'filter', whereas
+    # our filter fields do (e.g., 'source' vs 'sourceFilter'). Because of this mismatch, we need to modify the
+    # aggregation key names before collecting them as filters, so that when a filter is applied, it searches the
+    # correct field name.
+    aggs.select { |_, agg_values| agg_values.present? }.transform_keys { |key| (key.dup << 'Filter').to_sym }
   end
 
   def extract_results(response)

diff --git a/app/helpers/filter_helper.rb b/app/helpers/filter_helper.rb
@@ -9,7 +9,7 @@ def add_filter(query, filter, term)
     # seems like the best solution as each record only has a single source
     # in the data so there will never be a case to apply multiple in an AND
     # which is all we support in filter application.
-    if new_query[filter].present? && filter != 'source'
+    if new_query[filter].present? && filter != :sourceFilter
       new_query[filter] << term
       new_query[filter].uniq!
     else
@@ -21,8 +21,8 @@ def add_filter(query, filter, term)
 
   def nice_labels
     {
-      'contentType' => 'Content types',
-      'source' => 'Sources'
+      contentTypeFilter: 'Content type',
+      sourceFilter: 'Source'
     }
   end
 
@@ -44,4 +44,14 @@ def filter_applied?(terms, term)
 
     terms.include?(term)
   end
+
+  def applied_filters
+    filters = []
+    @enhanced_query.map do |param, values|
+      next unless param.to_s.include? 'Filter'
+
+      values.each { |value| filters << { param => value } }
+    end
+    filters
+  end
 end
diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb
@@ -2,9 +2,8 @@ module FormHelper
   def source_checkbox(source, params)
     "<div class='field-subitem'>
       <label class='field-checkbox'>
-        <input type='checkbox' value='#{source.downcase}' name='source[]' class='source'#{if params[:source]&.include?(source.downcase)
-                                                                                            ' checked'
-                                                                                          end}>
+        <input type='checkbox' value='#{source.downcase}' name='sourceFilter[]'
+               class='source'#{' checked' if params[:sourceFilter]&.include?(source.downcase) }>
         #{source}
       </label>
     </div>".html_safe

diff --git a/app/models/enhancer.rb b/app/models/enhancer.rb
@@ -2,7 +2,7 @@ class Enhancer
   attr_accessor :enhanced_query
 
   QUERY_PARAMS = %i[q citation contentType contributors fundingInformation identifiers locations subjects title].freeze
-  FILTER_PARAMS = [:source].freeze
+  FILTER_PARAMS = %i[sourceFilter contentTypeFilter].freeze
   # accepts all params as each enhancer may require different data
   def initialize(params)
     @enhanced_query = {}

diff --git a/app/models/query_builder.rb b/app/models/query_builder.rb
@@ -3,7 +3,7 @@ class QueryBuilder
 
   RESULTS_PER_PAGE = 20
   QUERY_PARAMS = %w[q citation contributors fundingInformation identifiers locations subjects title].freeze
-  FILTER_PARAMS = %i[source contentType].freeze
+  FILTER_PARAMS = %i[sourceFilter contentTypeFilter].freeze
 
   def initialize(enhanced_query)
     @query = {}
@@ -29,8 +29,8 @@ def extract_query(enhanced_query)
   end
 
   def extract_filters(enhanced_query)
-    # NOTE: ui and backend naming are not aligned so we can't loop here. we should fix in UI
-    @query['sourceFilter'] = enhanced_query[:source]
-    @query['contentType'] = enhanced_query[:contentType]
+    FILTER_PARAMS.each do |qp|
+      @query[qp] = enhanced_query[qp]
+    end
   end
 end
diff --git a/app/models/timdex_search.rb b/app/models/timdex_search.rb
@@ -15,7 +15,7 @@ class TimdexSearch < TimdexBase
       $sourceFilter: [String!]
       $index: String
       $from: String
-      $contentType: [String!]
+      $contentTypeFilter: [String!]
     ) {
       search(
         searchterm: $q
@@ -29,7 +29,7 @@ class TimdexSearch < TimdexBase
         sourceFilter: $sourceFilter
         index: $index
         from: $from
-        contentTypeFilter: $contentType
+        contentTypeFilter: $contentTypeFilter
       ) {
         hits
         records {

diff --git a/app/views/search/_filter.html.erb b/app/views/search/_filter.html.erb
@@ -1,4 +1,4 @@
-<% return if values.empty? %>
+<% return if values.blank? %>
 
 <div class="category">
   <button class="filter-label <%= 'expanded' if @enhanced_query[category.to_sym].present? || first == true %>"

diff --git a/app/views/search/_search_summary.html.erb b/app/views/search/_search_summary.html.erb
@@ -0,0 +1,15 @@
+<% return unless applied_filters.any? %>
+
+<div class="filter-summary">
+  <h2 class="hd-filter-summary hd-5">Applied filters: </h2>
+  <ul class="list-filter-summary list-inline">
+  <% applied_filters.each do |filter| %>
+    <li>
+      <a href="<%= results_path(remove_filter(@enhanced_query, filter.keys[0], filter.values[0])) %>">
+        <%= "#{nice_labels[filter.keys[0]] || filter.keys[0]}: #{filter.values[0]}" %>
+        <span class="sr">Remove applied filter?</span>
+      </a>
+    </li>
+  <% end %>
+  </ul>
+</div>
diff --git a/app/views/search/results.html.erb b/app/views/search/results.html.erb
@@ -1,43 +1,8 @@
 <%= content_for(:title, "Search Results | MIT Libraries") %>
 
 <div class="space-wrap">
-
-  <p>Showing results for:
-      <ul>
-        <% if @enhanced_query[:q].present? %>
-          <li>Keyword anywhere: <%= @enhanced_query[:q] %></li>
-        <% end %>
-        <% if @enhanced_query[:citation].present? %>
-          <li>Citation: <%= @enhanced_query[:citation] %></li>
-        <% end %>
-        <% if @enhanced_query[:contentType].present? %>
-          <li>Content type: <%= @enhanced_query[:contentType] %></li>
-        <% end %>
-        <% if @enhanced_query[:contributors].present? %>
-          <li>Contributors: <%= @enhanced_query[:contributors] %></li>
-        <% end %>
-        <% if @enhanced_query[:fundingInformation].present? %>
-          <li>Funders: <%= @enhanced_query[:fundingInformation] %></li>
-        <% end %>
-        <% if @enhanced_query[:identifiers].present? %>
-          <li>Identifiers: <%= @enhanced_query[:identifiers] %></li>
-        <% end %>
-        <% if @enhanced_query[:locations].present? %>
-          <li>Locations: <%= @enhanced_query[:locations] %></li>
-        <% end %>
-        <% if @enhanced_query[:subjects].present? %>
-          <li>Subjects: <%= @enhanced_query[:subjects] %></li>
-        <% end %>
-        <% if @enhanced_query[:title].present? %>
-          <li>Title: <%= @enhanced_query[:title] %></li>
-        <% end %>
-        <% if @enhanced_query[:source].present? %>
-          <li>Source: <%= @enhanced_query[:source] %></li>
-        <% end %>
-      </ul>
-  </p>
-
   <%= render partial: "form" %>
+  <%= render partial: "search_summary" %>
 
   <div id="hint" aria-live="polite">
     <%= render(partial: 'search/issn') %>
@@ -56,9 +21,9 @@
           <h3 class="hd-4"><em><%= results_summary(@pagination[:hits]) %></em></h3>
           <% @filters&.each_with_index do |(category, values), index| %>
             <% if index == 0 %>
-              <%= render(partial: 'search/filter', locals: {category: category, values: values, first: true }) %>
+              <%= render(partial: 'search/filter', locals: { category: category, values: values, first: true }) %>
             <% else %>
-              <%= render(partial: 'search/filter', locals: {category: category, values: values, first: false }) %>
+              <%= render(partial: 'search/filter', locals: { category: category, values: values, first: false }) %>
             <% end %>
           <% end %>
         </div>

diff --git a/test/controllers/search_controller_test.rb b/test/controllers/search_controller_test.rb
@@ -91,7 +91,7 @@ class SearchControllerTest < ActionDispatch::IntegrationTest
       assert_response :success
       assert_nil flash[:error]
 
-      assert_select 'li', 'Keyword anywhere: hallo'
+      assert_select 'input[value=?]', 'hallo'
     end
   end
 
@@ -272,6 +272,8 @@ class SearchControllerTest < ActionDispatch::IntegrationTest
 
   # Advanced search behavior
   test 'advanced search by keyword' do
+    skip("We no longer display a list of search terms in the UI; leaving this in in case we decide to reintroduce" \
+         "that feature soon.")
     VCR.use_cassette('advanced keyword asdf',
                      allow_playback_repeats: true,
                      match_requests_on: %i[method uri body]) do
@@ -291,12 +293,12 @@ class SearchControllerTest < ActionDispatch::IntegrationTest
       get '/results?citation=asdf&advanced=true'
       assert_response :success
       assert_nil flash[:error]
-
-      assert_select 'li', 'Citation: asdf'
     end
   end
 
   test 'advanced search can accept values from all fields' do
+    skip("We no longer display a list of search terms in the UI; leaving this in in case we decide to reintroduce" \
+         "that feature soon.")
     VCR.use_cassette('advanced all',
                      allow_playback_repeats: true,
                      match_requests_on: %i[method uri body]) do
@@ -359,7 +361,7 @@ class SearchControllerTest < ActionDispatch::IntegrationTest
   end
 
   def source_filter_count(controller)
-    controller.view_context.assigns['filters']['source'].count
+    controller.view_context.assigns['filters'][:sourceFilter].count
   end
 
   test 'advanced search can limit to a single source' do
@@ -369,7 +371,7 @@ def source_filter_count(controller)
       query = {
         q: 'data',
         advanced: 'true',
-        source: ['Woods Hole Open Access Server']
+        sourceFilter: ['Woods Hole Open Access Server']
       }.to_query
       get "/results?#{query}"
       assert_response :success
@@ -406,7 +408,7 @@ def source_filter_count(controller)
       query = {
         q: 'data',
         advanced: 'true',
-        source: ['Abdul Latif Jameel Poverty Action Lab Dataverse', 'Woods Hole Open Access Server']
+        sourceFilter: ['Abdul Latif Jameel Poverty Action Lab Dataverse', 'Woods Hole Open Access Server']
       }.to_query
       get "/results?#{query}"
       assert_response :success