Fix reading log carousel to check edition-level availability#11417
Conversation
When a specific edition is logged in reading logs, fetch availability at the edition level instead of work level. This ensures correct borrow buttons are shown instead of 'Locate' buttons when the logged edition is available. Fixes internetarchive#11329
for more information, see https://pre-commit.ci
| want_to_read.docs = add_availability_with_edition_preference( | ||
| want_to_read.docs | ||
| )[:5] |
There was a problem hiding this comment.
You can do this more simply with something like
| want_to_read.docs = add_availability_with_edition_preference( | |
| want_to_read.docs | |
| )[:5] | |
| want_to_read.docs = add_availability( | |
| [safeget(lambda: d['editions']['docs'][0], d) for d in want_to_read.docs if d.get('title')] | |
| )[:5] |
safeget is a helper method we have for situations just like these!
|
Nice, great work @Aayushdev18 ! We can likely simplify this by calling The |
| docs_without_editions = [] | ||
|
|
||
| for doc in filtered_docs: | ||
| # editions can be a list [edition] or dict {'docs': [edition]} |
There was a problem hiding this comment.
Oh good catch! I expect this should only be the dict, but let me know if you find otherwise!
Simplify the add_availability_with_edition_preference function by using the safeget helper to extract editions directly, following the pattern from work_search.html. This makes the code more concise and handles both dict and list edition structures gracefully.
for more information, see https://pre-commit.ci
|
@cdrini Regarding the editions structure: I checked the codebase and found that in link_editions_to_works (line 273 in bookshelves.py), it currently sets work_doc.editions = [edition] (as a list). However, I noticed the comment at line 255 mentions doc["editions"]["docs"], suggesting a dict format. So there might be an inconsistency in how editions are structured in different parts of the codebase. The good news is that using safeget(lambda: d['editions']['docs'][0], d) handles both cases gracefully: Thanks again for the guidance. |
|
Hi @cdrini, just a gentle follow-up on this PR. All checks have passed , would you be able to take a look when you have time? Thanks! |
|
Hi @Aayushdev18 ! I tested this, and it seemed to alas not fix the issue! The buttons still displayed "locate". This might need some more debugging, I reckon. |
The previous implementation was calling add_availability on editions,
but the template reads availability from work documents. This fix:
- Extracts editions using safeget
- Gets edition-level availability using get_availability('openlibrary_edition')
- Attaches availability to work documents (not editions)
- Falls back to work-level availability when no edition is available
This should fix the 'Locate' button issue by properly setting
availability on work docs that the template expects.
for more information, see https://pre-commit.ci
The link_editions_to_works function sets editions as a list [edition],
but the code was only checking for dict format {'docs': [edition]}.
This fix handles both cases to properly extract editions and get
edition-level availability.
for more information, see https://pre-commit.ci
|
Hi @cdrini! Thanks for testing. I found the issue: link_editions_to_works sets editions as a list [edition], but the code was only checking the dict format {'docs': [edition]}. |
|
Hi @Aayushdev18 thanks for the update! I spent some time on this one, and man what a headache to debug! I think you're exactly correct, the editions on the works here are stored as a raw list, not as a |
When a specific edition is logged in reading logs, fetch availability at the edition level instead of work level. This ensures correct borrow buttons are shown instead of 'Locate' buttons when the logged edition is available.
Fixes #11329
Closes #11329
Fix
Technical
When a reading log entry has a logged edition (from
link_editions_to_works), the code now:editionsfield (handles both list and dict formats)'openlibrary_edition'mode instead of work-levelTesting
/people/your-username/booksto see the carousel@cdrini