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

Relax yearly already-read page query #8596

Merged
merged 1 commit into from
Dec 7, 2023
Merged

Relax yearly already-read page query #8596

merged 1 commit into from
Dec 7, 2023

Conversation

mekarpeles
Copy link
Member

@mekarpeles mekarpeles commented Dec 7, 2023

Fixes yearly already-read reading log by showing books with checkins that have since been moved to other shelves, e.g. a book that was marked as read but then moved to e.g. "Already Read" or "Currently Reading"

Technical

Testing

Screenshot

Stakeholders

Fixes yearly already-read reading log by showing books with checkins that have since been moved to other shelves, e.g. a book that was marked as read but then moved to e.g. "Already Read" or "Currently Reading"
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.

Ah this is causing a bug; see https://testing.openlibrary.org/people/ScarTissue/books/already-read/year/2022

I haven't read the deep learning book. And it doesn't appear to have a check in? I'm not sure why it appears there...

@cdrini cdrini added Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] On testing.openlibrary.org labels Dec 7, 2023
@@ -383,8 +383,7 @@ def get_sorted_reading_log_books(
FROM bookshelves_books b
INNER JOIN bookshelves_events e
ON b.work_id = e.work_id AND b.username = e.username
WHERE b.bookshelf_id = $bookshelf_id
AND b.username = $username
WHERE b.username = $username
AND e.event_date LIKE $checkin_year || '%'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we maybe filter by a certain check-in type?

class BookshelfEvent(IntEnum):
START = 1
UPDATE = 2
FINISH = 3

Copy link
Collaborator

Choose a reason for hiding this comment

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

They are all FINISH. The other ones are there to support multiple check-ins per work, and are not used.

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.

Ah not a bug! @jimchamp found the issue! Looks like I had a funny case where that book contained a START event , which I don't think we use anymore. Must've been a testing thing!

@cdrini cdrini merged commit 4c609a2 into master Dec 7, 2023
3 checks passed
@cdrini cdrini deleted the mekarpeles-patch-2 branch December 7, 2023 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] Theme: My Books Theme: Yearly Reading Goals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants