Skip to content
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

Hack to fix collections year limiter #931

Merged
merged 5 commits into from
Nov 28, 2018

Conversation

murny
Copy link
Contributor

@murny murny commented Nov 27, 2018

Fixes #899

There is a bigger issue here at play. We shouldn't always be redirecting to /search from pages that have search results (collections/users/items) when they change the year range limit. They should remain under the same page they previously were.

Example:

If on a collections page (http://localhost:3000/communities/24a186e9-1356-4569-be37-bc3adce8b55c/collections/d677ccdb-606e-4841-bfba-46919e9beb7e) and you change the year limit and hit limit button, it should remain on this page, and only add some query params to the url. Instead we are redirecting this to the search page so it can handle it (http://localhost:3000/search).

How to fix this? Well this requires a bit more work. We have a "ItemSearch" Concern that has been neglected. Ideally, everything should be using this concern. However currently, "SearchController" is doing it's own thing. We should refactor the "SearchController" and move all it's custom "search" stuff into the "ItemSearch" concern so all search pages get it (items/collections/users) and then instead of redirecting to the search page like this, we can just handle this in its originating controller and this would fix this issue.

For a quick fix, I am just adding a facet for the member paths (community/collection pair), so we can limit the search page to only that collection.

What it looks like now, after you limit year range on a collection page, you will enter the "search" page with the year range and faceted by the collection you were previously on:
screen shot 2018-11-27 at 12 15 32 pm

Copy link
Member

@pgwillia pgwillia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duct-tape
Thanks for your analysis of this.

@murny murny merged commit 69c345e into ualbertalib:master Nov 28, 2018
@murny murny deleted the fix-year-limiter-on-collection branch November 28, 2018 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Jupiter] Year limiter does not reflect existing collection parameter
2 participants