Conversation
Why these changes are being introduced: Configuring the results per page to a non-default number yielded unexpected numbers of results. This was because the per-page value is set at multiple points in the query processing chain, and not always consistently. Relevant ticket(s): - [USE-442](https://mitlibraries.atlassian.net/browse/USE-442) How this addresses that need: This updates how results per page is defined for TIMDEX queries. `SearchController#fetch_timdex_data`, now ensures that `per_page` is always set by checking env and falling back to the default value of '20'. This mirrors how the value is set for the corresponding Primo method. The query param name is also updated from `size` to `perPage`. That change had been made in TIMDEX API, but we forgot to make it here. Side effects of this change: - Some cassettes were regenerated. - We should, at some point, evaluate whether it's possible to simplify the query processing chain.
Pull Request Test Coverage Report for Build 22926501069Details
💛 - Coveralls |
❌ 3 blocking issues (3 total)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why these changes are being introduced:
Configuring the results per page to a non-default number yielded
unexpected numbers of results. This was because the per-page value is
set at multiple points in the query processing chain, and not always
consistently.
Relevant ticket(s):
How this addresses that need:
This updates how results per page is defined for TIMDEX queries.
SearchController#fetch_timdex_data, now ensures thatper_pageisalways set by checking env and falling back to the default value of
'20'. This mirrors how the value is set for the corresponding Primo
method.
The query param name is also updated from
sizetoperPage. Thatchange had been made in TIMDEX API, but we forgot to make it here.
Side effects of this change:
the query processing chain.
Developer
Accessibility
New ENV
Approval beyond code review
Additional context needed to review
RESULTS_PER_PAGEhas been set to 50 for this PR build. Each tab should list 50 results (or as many as possible).Code Reviewer
Code
added technical debt.
Documentation
(not just this pull request message).
Testing