Fix broken template rendering on /search requests #8757
Merged
Conversation
This file contains 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
Work search template expects a SearchResponse object, but None was being sent if the request did not have any params. Updated request to always send a SearchResponse object. Also, removed an unused import.
for more information, see https://pre-commit.ci
ddominguez
commented
Jan 25, 2024
| @@ -19,7 +19,6 @@ | |||
| from openlibrary.core import cache | |||
| from openlibrary.core.lending import add_availability | |||
| from openlibrary.core.models import Edition | |||
| from openlibrary.plugins.inside.code import fulltext_search | |||
cdrini
approved these changes
Jan 27, 2024
There was a problem hiding this comment.
Lgtm! Good solution :) Tested on testing.openlibrary.org and works like a charm. Thank you @ddominguez !
Achorn
pushed a commit
to Achorn/openlibrary
that referenced
this pull request
Mar 14, 2024
* Fix broken template rendering on /search requests Work search template expects a SearchResponse object, but None was being sent if the request did not have any params. Updated request to always send a SearchResponse object. Also, removed an unused import.
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.
Work search template expects a SearchResponse object, but None was being sent if the request did not have any params. Updated request to always send a SearchResponse object.
Also, removed an unused import.
Closes #8730
Fix the broken rendering issues on the '/search' page when there are no query params.
Technical
There was existing logic to prevent the solr search query from being executed if the param dict was empty. I am assuming this was done to prevent a db hit. Executing the search query with the empty param was a solution that I tested and worked but I decided to send back an empty SearchResponse.
Testing
These are the steps I took to test.
/searchpage. Search form should be rendered./advancedsearchand submit and empty search. This will redirect to/searchand the Search form should render.ALLfilter and submit an empty search. This will redirect to/searchand the Search form should render.Screenshot
Stakeholders