Skip to content

Edition aware carousels #10619

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

Merged
merged 11 commits into from
Mar 25, 2025
Merged

Edition aware carousels #10619

merged 11 commits into from
Mar 25, 2025

Conversation

mekarpeles
Copy link
Member

Closes #7252 and #10580 (merging already reviewed code)

Technical

Testing

Screenshot

Stakeholders

@github-actions github-actions bot added the Priority: 2 Important, as time permits. [managed] label Mar 24, 2025
@mekarpeles
Copy link
Member Author

Load more terminating prematurely

By adding an exception into our code intentionally (to debug) we're able to see what URL is being created by BROWSE -- https://sentry.archive.org/organizations/ia-ux/issues/545144/?environment=ol-dev&environment=local&project=7&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D%20partials.json&referrer=issue-stream&stream_index=0. Here, we can see the advancedsearch url is https://archive.org/advancedsearch.php?q=openlibrary_work%3A%28%2A%29+AND+%28collection%3A%28inlibrary%29+OR+%28%21collection%3A%28printdisabled%29%29%29+AND+%28lending___available_to_browse%3Atrue+OR+lending___available_to_borrow%3Atrue%29+AND+languageSorter%3A%28%22English%22%29+AND+openlibrary_subject%3Aopenlibrary_staff_picks&fl%5B%5D=identifier&fl%5B%5D=openlibrary_edition&fl%5B%5D=openlibrary_work&rows=18&page=2&output=json&service=metadata__unlimited&sort%5B%5D=lending___last_browse+desc and that there are 18 results, however, ultimately our partial returns 10 editions, which is < limit 18 (and therefore it stops, as the Carousel.js code asks it to here: https://github.com/internetarchive/openlibrary/blob/master/openlibrary/plugins/openlibrary/js/carousel/Carousel.js#L168).

This happens because the IA response returns several items (editions) that have the same openlibrary_work ID and the Open Library API combines these into a single work/edition.

Carousel.js thus needs to be updated to continue paging results.

mekarpeles and others added 9 commits March 25, 2025 14:00
When a ListQuery carousel exhausts all its IDs, the query fallback to something that produces lots of results when instead none should be returned. This commit checks if no book keys remain in the list and falls back to a special query that will return no results.
This makes more sense for browse-ish carousels
- Adds providers to carousels query
- Edition-aware; update book -> work to edition where possible
- Fix ia carousels, use query instead of url
- Move compose_ia_url to backend
- Add subjects as loadmore field
- Standardizes application/json response
- Adds editions to fields
- Fixes carousel fetching
@mekarpeles mekarpeles force-pushed the edition-aware-carousels branch from da34782 to 814f68a Compare March 25, 2025 21:17
…austed-results

Fix list querying case when out of results
@mekarpeles mekarpeles merged commit 3153a88 into master Mar 25, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 2 Important, as time permits. [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make QueryCarousels edition-aware
4 participants