Allow bookmarks to be sorted by most recent - #5573
Conversation
c622d03 to
3ef400b
Compare
3e0deb6 to
0f281f0
Compare
0586d18 to
770d6c4
Compare
sandbergja
left a comment
sandbergja
left a comment
There was a problem hiding this comment.
Thanks, @rladdusaw 🐈⬛ . A few questions.
| def sort_by_most_recently_bookmarked(documents, user) | ||
| update_blank_sort_param | ||
| if params[:sort] == 'score desc' && bookmarks? && user | ||
| user.bookmarks.order(updated_at: :desc) | ||
| .collect { |bookmark| bookmark.document_id.to_s } | ||
| .filter_map { |id| documents.index_by(&:id)[id] } | ||
| else | ||
| documents | ||
| end | ||
| end |
There was a problem hiding this comment.
Could this logic be moved to the Bookmarks Controller? The helpers have global scope, so it would be more cohesive to keep the logic there.
| this.update_sort_text(); | ||
| } | ||
|
|
||
| update_sort_text() { |
There was a problem hiding this comment.
Are you sure we couldn't handle this on the ruby side? It seems odd for us to render the wrong options in Ruby only to correct them in JS.
There was a problem hiding this comment.
The text of the dropdown says sort by recently bookmarked on the catalog search page even though it's not an option in the dropdown for that page and the relevance sort is the one being used on the page. I spent a lot of time trying to get it to display properly, but nothing we tried worked. It's not an ideal solution, but the Blacklight config seems resistant to configuring a dynamic first sort option and this was the only way we could get it working.
There was a problem hiding this comment.
One question: did you consider modifying the choices in a custom Blacklight::Response::SortComponent or similar?
| it 'excludes scsb items when pul location filter is applied' do | ||
| get '/catalog.json?per_page=100&f_inclusive%5Badvanced_location_s%5D%5B%5D=pul' | ||
| expect(response.body).to include('"99118884033506421"') | ||
| expect(response.body).to include('"9918150543506421"') |
4 participants
Resolves #4948