Makes facets usable

matt-bernhardt · matt-bernhardt · commit 8438bc947a03 · 2022-07-08T15:40:17.000-04:00
** Why are these changes being introduced: * We need to make the facets in the left sidebar into something usable. * Because of inconsistencies in the imported records, certain facets need to be removed - leaving only those that we feel comfortable providing. ** Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/rdi-197 ** How does this address that need: A number of things are happening here: * A $contentType query variable has been added to the TimdexSearch model, which maps to the contentTypeFacet variable in the API. This requires changes to the enhancer and query_builder models to support this new value. This variable has been added to the search feedback in the results.html.erb view template. * We are removing a number of facets that are no longer desired in the UI. Removing them from the aggregations of the TimdexSearch model requires regenerating a large number of cassettes - I've kept those changes to a second commit in this PR for hopefully easier review. * The facet_labels method on the search controller is moving to a new facet helper, named nice_labels. This is not so much a method as a hash which can be used to look up human-readable names for facet categories. Note the invocation in the _facet partial on line 4 to see what I mean here. * Because the hash is moving to a helper, the extract_facets method on the controller gets simpler. Thanks to Jeremy for working through the best way to achieve this. * The facet helper also includes two methods for manipulating the query passed to results_path in the view partial. add_facet and remove_facet should be named pretty clearly, and take the expected parameters. * There is a new set of unit tests for all of the facet helpers. * Facets can be removed by clicking on a link below that facet. Active facets are called out in black, which hopefully will help identify them to the user. This should still be evaluated by UX when they join the project, obviously. ** Document any side effects to this change: * This changes the search feedback in the results.html.erb to work from the @enhanced_query instance variable rather than the raw params object. This should better reflect what the search actually was, giving space for the user to be informed of any manipulations by the application, rather than simply working from whatever the user had requested.
diff --git a/app/assets/stylesheets/partials/_facets.scss b/app/assets/stylesheets/partials/_facets.scss
@@ -3,10 +3,15 @@
     margin-bottom: 0;
   }
 
+  .category {
+    margin-bottom: 1em;
+  }
+
   ul.category-terms {
     border-collapse: separate;
     border-spacing: 0px 4px;
     display: table;
+    margin-bottom: 0;
     table-layout: fixed;
     width: 100%;
 
@@ -18,7 +23,8 @@
         text-decoration: none;
 
         &:hover,
-        &:focus {
+        &:focus,
+        &.applied {
           background: #000;
           color: #fff;
         }
diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb
@@ -30,27 +30,13 @@ def extract_errors(response)
   end
 
   def extract_facets(response)
-    facets = response&.data&.search&.to_h&.dig('aggregations')
-
-    facets&.map { |k, v| { facet_labels[k] => v } }
+    response&.data&.search&.to_h&.dig('aggregations')
   end
 
   def extract_results(response)
     response&.data&.search&.to_h&.dig('records')
   end
 
-  def facet_labels
-    {
-      'contentFormat' => 'Formats',
-      'contentType' => 'Content types',
-      'contributors' => 'Contributors',
-      'languages' => 'Languages',
-      'literaryForm' => 'Literary forms',
-      'source' => 'Sources',
-      'subjects' => 'Subjects'
-    }
-  end
-
   def validate_q!
     return if params[:advanced]&.strip.present?
     return if params[:q]&.strip.present?
diff --git a/app/helpers/facet_helper.rb b/app/helpers/facet_helper.rb
@@ -0,0 +1,22 @@
+module FacetHelper
+  def add_facet(query, facet, term)
+    new_query = query.clone
+    new_query[:page] = 1
+    new_query[facet.to_sym] = term
+    new_query
+  end
+
+  def nice_labels
+    {
+      'contentType' => 'Content types',
+      'source' => 'Sources'
+    }
+  end
+
+  def remove_facet(query, facet)
+    new_query = query.clone
+    new_query[:page] = 1
+    new_query.delete facet.to_sym
+    new_query
+  end
+end
diff --git a/app/models/enhancer.rb b/app/models/enhancer.rb
@@ -1,7 +1,7 @@
 class Enhancer
   attr_accessor :enhanced_query
 
-  QUERY_PARAMS = %i[q citation contributors fundingInformation identifiers locations subjects title].freeze
+  QUERY_PARAMS = %i[q citation contentType contributors fundingInformation identifiers locations subjects title].freeze
   FILTER_PARAMS = [:source].freeze
   # accepts all params as each enhancer may require different data
   def initialize(params)
diff --git a/app/models/query_builder.rb b/app/models/query_builder.rb
@@ -2,7 +2,7 @@ class QueryBuilder
   attr_reader :query
 
   RESULTS_PER_PAGE = 20
-  QUERY_PARAMS = %w[q citation contributors fundingInformation identifiers locations subjects title].freeze
+  QUERY_PARAMS = %w[q citation contentType contributors fundingInformation identifiers locations subjects title].freeze
   FILTER_PARAMS = [:source].freeze
 
   def initialize(enhanced_query)
diff --git a/app/models/timdex_search.rb b/app/models/timdex_search.rb
@@ -14,6 +14,7 @@ class TimdexSearch < TimdexBase
       $title: String
       $sourceFacet: [String!]
       $from: String
+      $contentType: String
     ) {
       search(
         searchterm: $q
@@ -26,6 +27,7 @@ class TimdexSearch < TimdexBase
         title: $title
         sourceFacet: $sourceFacet
         from: $from
+        contentTypeFacet: $contentType
       ) {
         hits
         records {
@@ -51,34 +53,14 @@ class TimdexSearch < TimdexBase
           }
         }
         aggregations {
-          contentFormat {
-            key
-            docCount
-          }
           contentType {
             key
             docCount
           }
-          contributors {
-            key
-            docCount
-          }
-          languages {
-            key
-            docCount
-          }
-          literaryForm {
-            key
-            docCount
-          }
           source {
             key
             docCount
           }
-          subjects {
-            key
-            docCount
-          }
         }
       }
     }
diff --git a/app/views/search/_facet.html.erb b/app/views/search/_facet.html.erb
@@ -1,15 +1,19 @@
-<% return if facet.values[0].empty? %>
+<% return if values.empty? %>
 
 <div class="category">
-  <h3><%= facet.keys[0] %></h3>
+  <h3><%= nice_labels[category] || category %></h3>
   <ul class="category-terms list-unbulleted">
-  <% facet.values[0].each do |term| %>
+  <% values.each do |term| %>
     <li class="term">
-      <a href="#">
+      <a href="<%= results_path(add_facet(@enhanced_query, category, term['key'])) %>" class="<%= "applied" if @enhanced_query[category.to_sym] == term['key'] %>">
         <span class="name"><%= term['key'] %></span>
         <span class="count"><%= term['docCount'] %> <span class="sr">records</span></span>
       </a>
     </li>
   <% end %>
   </ul>
+  <% if @enhanced_query[category.to_sym].present? %>
+    <div><%= link_to "Show all #{nice_labels[category]&.downcase || category}", results_path(remove_facet(@enhanced_query, category)) %>
+    </div>
+  <% end %>
 </div>
diff --git a/app/views/search/results.html.erb b/app/views/search/results.html.erb
@@ -4,32 +4,35 @@
 
   <p>Showing results for:
       <ul>
-        <% if params[:q].present? %>
-          <li>Keyword anywhere: <%= params[:q] %></li>
+        <% if @enhanced_query[:q].present? %>
+          <li>Keyword anywhere: <%= @enhanced_query[:q] %></li>
         <% end %>
-        <% if params[:citation].present? %>
-          <li>Citation: <%= params[:citation] %></li>
+        <% if @enhanced_query[:citation].present? %>
+          <li>Citation: <%= @enhanced_query[:citation] %></li>
         <% end %>
-        <% if params[:contributors].present? %>
-          <li>Contributors: <%= params[:contributors] %></li>
+        <% if @enhanced_query[:contentType].present? %>
+          <li>Content type: <%= @enhanced_query[:contentType] %></li>
         <% end %>
-        <% if params[:fundingInformation].present? %>
-          <li>Funders: <%= params[:fundingInformation] %></li>
+        <% if @enhanced_query[:contributors].present? %>
+          <li>Contributors: <%= @enhanced_query[:contributors] %></li>
         <% end %>
-        <% if params[:identifiers].present? %>
-          <li>Identifiers: <%= params[:identifiers] %></li>
+        <% if @enhanced_query[:fundingInformation].present? %>
+          <li>Funders: <%= @enhanced_query[:fundingInformation] %></li>
         <% end %>
-        <% if params[:locations].present? %>
-          <li>Locations: <%= params[:locations] %></li>
+        <% if @enhanced_query[:identifiers].present? %>
+          <li>Identifiers: <%= @enhanced_query[:identifiers] %></li>
         <% end %>
-        <% if params[:subjects].present? %>
-          <li>Subjects: <%= params[:subjects] %></li>
+        <% if @enhanced_query[:locations].present? %>
+          <li>Locations: <%= @enhanced_query[:locations] %></li>
         <% end %>
-        <% if params[:title].present? %>
-          <li>Title: <%= params[:title] %></li>
+        <% if @enhanced_query[:subjects].present? %>
+          <li>Subjects: <%= @enhanced_query[:subjects] %></li>
         <% end %>
-        <% if params[:source].present? %>
-          <li>Source: <%= params[:source] %></li>
+        <% 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>
@@ -55,7 +58,9 @@
     <aside class="col1q facet-container">
       <div id="facets">
         <h2>Available filters</h2>
-        <%= render(partial: 'search/facet', collection: @facets) || render('facet_empty') %>
+        <% @facets&.each do |category, values| %>
+          <%= render(partial: 'search/facet', locals: {category: category, values: values}) %>
+        <% end %>
       </div>
     </aside>
   </div>
diff --git a/test/helpers/facet_helper_test.rb b/test/helpers/facet_helper_test.rb
@@ -0,0 +1,67 @@
+require 'test_helper'
+
+class FacetHelperTest < ActionView::TestCase
+  include FacetHelper
+
+  test 'add_facet will support adding a facet parameter to a search URL' do
+    original_query = {
+      page: 1,
+      q: 'data'
+    }
+    expected_query = {
+      page: 1,
+      q: 'data',
+      contentType: 'dataset'
+    }
+    assert_equal expected_query, add_facet(original_query, 'contentType', 'dataset')
+  end
+
+  test 'add_facet will reset a page count when called' do
+    original_query = {
+      page: 3,
+      q: 'data'
+    }
+    expected_query = {
+      page: 1,
+      q: 'data',
+      contentType: 'dataset'
+    }
+    assert_equal expected_query, add_facet(original_query, 'contentType', 'dataset')
+  end
+
+  test 'nice_labels allows translation of machine categories to human readable headings' do
+    needle = 'contentType'
+    assert_equal 'Content types', nice_labels[needle]
+  end
+
+  test 'nice_labels returns nil if a category is not mapped yet' do
+    needle = 'foo'
+    assert_nil nice_labels[needle]
+  end
+
+  test 'remove_facet will remove a specific facet parameter from a search URL' do
+    original_query = {
+      page: 1,
+      q: 'data',
+      contentType: 'dataset'
+    }
+    expected_query = {
+      page: 1,
+      q: 'data'
+    }
+    assert_equal expected_query, remove_facet(original_query, 'contentType')
+  end
+
+  test 'remove_facet will reset a page count when called' do
+    original_query = {
+      page: 3,
+      q: 'data',
+      contentType: 'dataset'
+    }
+    expected_query = {
+      page: 1,
+      q: 'data'
+    }
+    assert_equal expected_query, remove_facet(original_query, 'contentType')
+  end
+end
