-
Notifications
You must be signed in to change notification settings - Fork 0
Implement support for faceting in UI #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is okay with the facets we currently are displaying because they are all "only one makes sense" kinda facets. We may need to revisit this in the future to ensure this doesn't remove facets that are applied during a drill down application (i.e. search > apply "foo" from group A > apply "bar" from group A... is the result just "bar" applied or are both being preserved in the query with this code? Even if this only works for our currently displayed types of facets, I'm okay with coming back to this later.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this work is predicated on only one value being applicable from a given vocabulary - so I'm not pushing values onto an array, or purging only one value, etc. This work does support applying facets from multiple vocabularies, which I think you've noted somewhere in these conversations, but more work will be needed if we get into dealing with arrays of values. |
||
| 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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to my note above. I think this works with contentType really well as it's a one value facet, but with facets in which multiple tiers of filtering can happen (such as subjects) this probably won't hold up. I'm okay with dealing with this later, but wanted to share the concern so we aren't surprised later if this needs to get more complex. |
||
| new_query | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fits my brain so much better. Thanks so much for talking that through with me so we both understood a bit better what was going on and why here :)