Skip to content
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

Set default author book count to zero #9004

Merged
merged 2 commits into from
Apr 9, 2024

Conversation

jimchamp
Copy link
Collaborator

@jimchamp jimchamp commented Apr 2, 2024

Closes #

Naive solution for these Sentry events.

books.num_found can sometimes be None. This change creates a new num_found variable that hold the immutable books.num_found value, or 0 if none exists.

Technical

Testing

This page fails to load in production, but works in testing with this branch deployed.

Screenshot

Stakeholders

@jimchamp jimchamp force-pushed the bug/default-count-on-null branch 2 times, most recently from 1a94b68 to 7ccec46 Compare April 2, 2024 06:54
@jimchamp jimchamp added On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing and removed Needs: Testing labels Apr 2, 2024
@jimchamp jimchamp marked this pull request as ready for review April 2, 2024 07:04
@jimchamp jimchamp changed the title [WIP] Set default author book count to zero Set default author book count to zero Apr 2, 2024
@cdrini cdrini self-assigned this Apr 8, 2024
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Lgtm one small code quality comment! Merge at your discretion.

Confirmed https://testing.openlibrary.org/authors/OL120205A/Marcel_Proust?has_fulltext=true&q=1%C0%A7%C0%A2%252527%252522%5C%27%5C%22&debug=true now loads on testing.

openlibrary/templates/type/author/view.html Outdated Show resolved Hide resolved
@cdrini cdrini added the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Apr 8, 2024
@jimchamp jimchamp added On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing Needs: Review This issue/PR needs to be reviewed in order to be closed or merged (see comments). [managed] and removed Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing labels Apr 9, 2024
@cdrini cdrini merged commit abe9f22 into internetarchive:master Apr 9, 2024
3 checks passed
Achorn pushed a commit to Achorn/openlibrary that referenced this pull request Apr 12, 2024
@jimchamp jimchamp deleted the bug/default-count-on-null branch April 24, 2024 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Review This issue/PR needs to be reviewed in order to be closed or merged (see comments). [managed] On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants