Skip to content

Commit

Permalink
Add search summary panel and full filter support
Browse files Browse the repository at this point in the history
Why these changes are being introduced:

It would be helpful if users could see what filters they've
applied to a search directly below the search bar, which would
also provide another path to remove filters.

Relevant ticket(s):

https://mitlibraries.atlassian.net/browse/GDT-142

How this addresses that need:

This provides the requested panel. We had been calling this a
"filter removal" panel, but I ended up naming the partial
`search_summary` to avoid any confusion with the filter
sidebar. However, the currently displayed data has the
`filter-removal` class, because it's likely we will want to
show advanced search terms here, too (possibly just for screen
readers).

While working on this, it occurred to me that the existing TIMDEX
UI filter implementation is incomplete. It pulls in the aggregation
names and uses those as the internal filter names. This works fine
for the existing `contentType` and `source` filters, but it will
cause a name collision for filters that also exist as regular
search inputs (e.g. `contributors` vs. `contributorsFilter`). Since
we will be adding more aggregations and filters soon, it makes sense
to fix this problem now.

Side effects of these changes:

* The "You searched for" list has been removed, as this feature
replaces it. As mentioned above, we will likely want to provide
that information in some manner in the search summary, but that
will require some discussion with Darcy, who is out-of-office at
the moment.
* Filter keys are now cast as symbols throughout the application.
Previously, they would be strings in some places and symbols in
others. I found that confusing, but if this change is even more
confusing, I'm happy to revert it.
* A few tests have been skipped due to the removal of the "You
searched for list". I chose not to delete them in case we
reintroduce that feature in the near future.
* Most of the cassettes have been regenerated, due to changes to
the `TimdexSearch` model.
  • Loading branch information
jazairi committed Feb 16, 2024
1 parent fe1fa4b commit 979efa2
Show file tree
Hide file tree
Showing 42 changed files with 631 additions and 465 deletions.
33 changes: 33 additions & 0 deletions app/assets/stylesheets/partials/_panels.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
2 changes: 1 addition & 1 deletion app/assets/stylesheets/partials/_search.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
/* basic search bar */
.basic-search {
background-color: #989898;
margin-bottom: 1rem;
margin-bottom: 0rem;
padding: 1.6rem 2rem;

details {
Expand Down
3 changes: 1 addition & 2 deletions app/controllers/search_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ def extract_errors(response)
def extract_filters(response)
aggs = response&.data&.search&.to_h&.dig('aggregations')
return if aggs.blank?

aggs.select { |_, agg_values| agg_values.present? }
aggs.select { |_, agg_values| agg_values.present? }.transform_keys { |key| (key.dup << 'Filter').to_sym }
end

def extract_results(response)
Expand Down
15 changes: 12 additions & 3 deletions app/helpers/filter_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -21,8 +21,8 @@ def add_filter(query, filter, term)

def nice_labels
{
'contentType' => 'Content types',
'source' => 'Sources'
contentTypeFilter: 'Content type',
sourceFilter: 'Source'
}
end

Expand All @@ -44,4 +44,13 @@ 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
2 changes: 1 addition & 1 deletion app/helpers/form_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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)
<input type='checkbox' value='#{source.downcase}' name='sourceFilter[]' class='source'#{if params[:sourceFilter]&.include?(source.downcase)
' checked'
end}>
#{source}
Expand Down
2 changes: 1 addition & 1 deletion app/models/enhancer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
Expand Down
8 changes: 4 additions & 4 deletions app/models/query_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
Expand All @@ -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
4 changes: 2 additions & 2 deletions app/models/timdex_search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class TimdexSearch < TimdexBase
$sourceFilter: [String!]
$index: String
$from: String
$contentType: [String!]
$contentTypeFilter: [String!]
) {
search(
searchterm: $q
Expand All @@ -29,7 +29,7 @@ class TimdexSearch < TimdexBase
sourceFilter: $sourceFilter
index: $index
from: $from
contentTypeFilter: $contentType
contentTypeFilter: $contentTypeFilter
) {
hits
records {
Expand Down
2 changes: 1 addition & 1 deletion app/views/search/_filter.html.erb
Original file line number Diff line number Diff line change
@@ -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 %>"
Expand Down
15 changes: 15 additions & 0 deletions app/views/search/_search_summary.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<% return unless applied_filters.any? %>

<div class="filter-summary">
<h2 class="hd-filter-summary hd-5">You searched for: </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>
41 changes: 3 additions & 38 deletions app/views/search/results.html.erb
Original file line number Diff line number Diff line change
@@ -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') %>
Expand All @@ -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>
Expand Down
14 changes: 8 additions & 6 deletions test/controllers/search_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 979efa2

Please sign in to comment.