Skip to content

Commit 32ea6bb

Browse files
authored
Switch facet suggest request to have query in URL query params, instead of path, with proper escaping (#3774)
It was failing prior on queries including `.` or `/`. It may have been possible to fix this with proper escaping and leave it in path component of URL -- but it's much more straightoforward to put it in query component after the `?`, and let existing escaping happening take care of it. This is a URL only used by JS fetch that shouldn't ordinarily be seen by the user, so aesthetics prob not a concern. The Blacklight CI sample data has a `topic_facet` value with a period in it (`Iran. Vizārat-i Kishvar`), and my own local data does too. I haven't seen any with `/`, but of course it's possible and should work.
1 parent 8c1d0e1 commit 32ea6bb

File tree

3 files changed

+14
-2
lines changed

3 files changed

+14
-2
lines changed

app/javascript/blacklight-frontend/facet_suggest.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@ const FacetSuggest = async (e) => {
1616

1717
// Drop facet.page so a filtered suggestion list will always start on page 1
1818
url.searchParams.delete('facet.page');
19+
// add our queryFragment for facet filtering
20+
url.searchParams.append('query_fragment', queryFragment);
21+
1922
const facetSearchParams = url.searchParams.toString();
2023
const basePathComponent = url.pathname.split('/')[1];
2124

22-
const urlToFetch = `/${basePathComponent}/facet_suggest/${facetField}/${queryFragment}?${facetSearchParams}`;
25+
const urlToFetch = `/${basePathComponent}/facet_suggest/${facetField}?${facetSearchParams}`;
2326

2427
const response = await fetch(urlToFetch);
2528
if (response.ok) {

lib/blacklight/routes/searchable.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def call(mapper, _options = {})
1818
mapper.get "opensearch"
1919
mapper.get 'suggest', as: 'suggest_index'
2020
mapper.get "facet/:id", action: 'facet', as: 'facet'
21-
mapper.get "facet_suggest/:id/(:query_fragment)", action: 'facet', as: 'facet_suggest', defaults: { only_values: true }
21+
mapper.get "facet_suggest/:id", action: 'facet', as: 'facet_suggest', defaults: { only_values: true }
2222
end
2323
end
2424
end

spec/features/facets_spec.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,15 @@
112112
expect(page).to have_css 'a.facet-select', count: 2
113113
end
114114

115+
it 'can show suggestions including a period', :js do
116+
visit '/catalog/facet/subject_ssim'
117+
fill_in 'facet_suggest_subject_ssim', with: 'iran.'
118+
119+
expect(page).to have_css '.facet-suggestions'
120+
expect(page).to have_link 'Iran. Vizārat-i Kishvar'
121+
expect(page).to have_css 'a.facet-select', count: 1
122+
end
123+
115124
it 'shows the user facet suggestions that are relevant to their q param', :js do
116125
visit '/catalog/facet/subject_ssim?q=tibet&search_field=all_fields'
117126
fill_in 'facet_suggest_subject_ssim', with: 'la'

0 commit comments

Comments
 (0)