Skip to content

Skip missing books in IA carousels#12804

Open
MahdiiFakhfakh wants to merge 3 commits into
internetarchive:masterfrom
MahdiiFakhfakh:recently-returned-carousel
Open

Skip missing books in IA carousels#12804
MahdiiFakhfakh wants to merge 3 commits into
internetarchive:masterfrom
MahdiiFakhfakh:recently-returned-carousel

Conversation

@MahdiiFakhfakh
Copy link
Copy Markdown
Contributor

Closes #12740

fix

Skips missing entries returned from Archive carousel data so one bad/missing item does not break the IA carousel rendering.

Technical

get_ia_carousel_books() already skipped "error" entries from lending.get_available(), but did not skip missing/falsey entries like None.

This PR updates the filtering so only real book objects are passed to format_book_data().

Testing

docker compose run --rm home pytest openlibrary/plugins/openlibrary/tests/test_home.py -q
docker compose run --rm home ruff check openlibrary/plugins/openlibrary/home.py openlibrary/plugins/openlibrary/tests/test_home.py

Screenshot

N/A. This is a backend carousel data handling fix.

Stakeholders

@RayBB @mekarpeles @seabelis

@mekarpeles mekarpeles requested a review from Copilot May 24, 2026 14:35
@github-actions github-actions Bot added the Priority: 2 Important, as time permits. [managed] label May 24, 2026
@github-project-automation github-project-automation Bot moved this to Waiting Review/Merge from Staff in Ray's Project May 24, 2026
@mekarpeles
Copy link
Copy Markdown
Member

Thanks for the pull request!

🤖 Copilot has been assigned for an initial review.

A reviewer must first be assigned. There are currently 17 open PRs of equal or higher priority ahead of yours.

PR triage checklist (maintainers / Pam)
  • PR description — not empty; explains what the change does and how to verify it
  • References an issue — PR body contains a #NNN reference
    • Linked issue is triaged — has a Priority: * label (not just Needs: Triage)
    • Linked issue is assigned — has at least one assignee
  • Commit history clean — no WIP/fixup/conflict noise; commit messages are meaningful
  • CI passing — no failing check-runs
  • Test cases present — if the change touches substantive logic, test coverage exists or is explained
  • Proof of testing — PR body includes a description of what was tested, a screenshot, or a video

Note

This comment was automatically generated by Pam, Open Library's Project AI Manager, on behalf of @mekarpeles. Pam is designed to provide status visibility, perform basic project management functions and relevant codebase research, and provide actionable feedback so contributors aren't left waiting.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes IA carousel rendering on the home page by filtering out missing/falsey entries returned from lending.get_available() so a single bad item doesn’t break formatting.

Changes:

  • Filter IA carousel results to skip None/falsey entries (in addition to 'error') before calling format_book_data().
  • Add a regression test ensuring missing books are skipped and valid books are still formatted.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
openlibrary/plugins/openlibrary/home.py Filters IA carousel book list to exclude falsey entries before formatting.
openlibrary/plugins/openlibrary/tests/test_home.py Adds a unit test covering the “skip missing books” behavior.

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

Status: Waiting Review/Merge from Staff

Development

Successfully merging this pull request may close these issues.

Recently Returned carousel missing from home page

4 participants