Fix issue #2723: Improve list search functionality #10460
Conversation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Awesome great work @isabellabonilla ! This was a trickier issue, nice work getting all the difference pieces implemented and playing together -- and as a first time contributor! 😊
A few pieces of feedback to fix some issues with wrong field names, and allowing for backwards compatibility of the public search API 👍
| 'key', # unique identifier for the list | ||
| 'name', # name/title of the list | ||
| 'description', # short description of the list | ||
| 'created', # timestamp when the list was created |
There was a problem hiding this comment.
This field unfortunately doesn't exist in our solr, so we can't use it.
| 'created', # timestamp when the list was created |
| all_fields = { | ||
| 'key', # unique identifier for the list | ||
| 'name', # name/title of the list | ||
| 'description', # short description of the list |
There was a problem hiding this comment.
Ditto with this one, but this does exist in our db, so you can add it to the non_solr_fields so that it can be fetched for reading.
| 'created desc': 'created desc', # sort by newest lists first (default) | ||
| 'created asc': 'created asc', # sort by oldest lists first |
There was a problem hiding this comment.
Ditto here, these fields don't exist in our solr.
| 'created desc': 'created desc', # sort by newest lists first (default) | |
| 'created asc': 'created asc', # sort by oldest lists first |
| 'name', # name/title of the list | ||
| 'description', # short description of the list | ||
| 'created', # timestamp when the list was created | ||
| } |
There was a problem hiding this comment.
We do have a few other keys that will be available for search:
| } | |
| "subject", | |
| "subject_key", | |
| "person", | |
| "person_key", | |
| "place", | |
| "place_key", | |
| "time", | |
| "time_key", | |
| } |
| 'description', | ||
| 'created', |
There was a problem hiding this comment.
| 'description', | |
| 'created', |
| fields=fields, | ||
| sort="created_desc", # default sorting with most recently created lists first |
There was a problem hiding this comment.
| fields=fields, | |
| sort="created_desc", # default sorting with most recently created lists first |
|
|
||
| web.header('Content-Type', 'application/json') | ||
| return delegate.RawText(json.dumps(response)) | ||
| return delegate.RawText(json.dumps(raw_resp)) |
There was a problem hiding this comment.
Ah this is going to be a breaking change to our public lists search API... we'll likely have to have both for a little while. To do this, let's add this code:
| return delegate.RawText(json.dumps(raw_resp)) | |
| if i.api == 'next': | |
| return delegate.RawText(json.dumps(raw_resp)) | |
| else: | |
| # Default to the old API shape for a while, then we'll flip | |
| return delegate.RawText(json.dumps({ | |
| 'start': offset, | |
| 'docs': [ | |
| lst.preview() | |
| for lst in web.ctx.site.get_many(doc['key'] for doc in raw_resp['docs']) | |
| ] | |
| }) |
This will let us use the better solr-based search, while maintaining the same shape of response.
| "limit": int(limit), | ||
| "offset": int(offset), | ||
| } | ||
| def get_results(elf, q, offset=0, limit=100, fields='*', sort=''): |
There was a problem hiding this comment.
We want to avoid defaulting to * now since that results in large responses ; I reckon you got this from the authors page which we have yet to fix 😁 Defaulting it to None should do the trick 👍 The down stream code should then just the default fields from the Scheme.
| def get_results(elf, q, offset=0, limit=100, fields='*', sort=''): | |
| def get_results(elf, q, offset=0, limit=100, fields=None, sort=''): |
|
|
||
| docs = self.get_results(i.q, offset=offset, limit=limit) | ||
| response = self.get_results(i.q, offset=offset, limit=limit) |
There was a problem hiding this comment.
Let's also add the sort=i.sort and fields=i.fields.split(',') if i.fields else None so we can test those are working correctly.
cdrini
added
the
Needs: Submitter Input
Closes #2723
Fix: Improves list search functionality to allow partial matches in list titles.
Technical
ListSearchSchemeto leverage Solr for list searches./search/listscontroller to use the new Solr-backed search.openlibrary/plugins/worksearch/code.pyopenlibrary/plugins/worksearch/schemes/lists.py(new file)openlibrary/templates/search/lists.htmlTesting
/search/lists.Stakeholders
@cdrini
@seabelis
@el4ctr0n