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

Fix: "Mark as Read" buttons don't work (#398) #399

Merged
merged 1 commit into from
May 16, 2023

Conversation

mbestavros
Copy link
Collaborator

@mbestavros mbestavros commented May 14, 2023

This PR implements two separate fixes for the issues outlined in #398. Building Read You with these changes fixes the "Mark as Read" buttons on my environment.

Here are the details:

Part 1: Send the before parameter correctly

As detailed in the original docs, the Fever API requires a before parameter (formatted as a Unix timestamp) in requests to update a feed or category as read (before a certain time). My prior understanding was that this is meant to be represented in seconds; the Wikipedia entry for Unix time and some of my own investigations [1][2][3] support this. Most applications (including popular Fever endpoints) seem to expect seconds.

Read You, however, appears to send the Fever before parameter as milliseconds instead of seconds. This causes the Fever endpoints to think that the request is to mark all stories before some time in the extremely distant future as read. This is obviously not what the user wants.

Unfortunately, the original Fever API docs don't explicitly state whether that timestamp is supposed to be in seconds or milliseconds, but given all the context clues, I think it makes most sense to send the before parameter as seconds. Therefore, this PR simply converts milliseconds to seconds before performing the POST request against the Fever API.

Part 2: Send the before parameter correctly (...again)

When clicking the Mark All as Read button, Read You defaults the Fever before parameter to a value of Date(Long.MAX_VALUE).time. While this makes some sense, it appears to break at least some RSS readers, who just drop the request and don't mark anything as read. Converting this time to seconds doesn't help. Instead, this PR sets that value to the current time, which makes more logical sense and - crucially - appears to actually work.

Tagging @Ashinch for review. Resolves #398 and resolves #383 (please confirm if you're able @scottbetza)

Love the app, and can't wait to see where it goes next! 😄

References

[1]: Read You's own Fever documentation contains Unix timestamp values that make most sense as seconds, rather than milliseconds.
[2]: Miniflux's Fever handler expects time in seconds.
[3]: FreshRSS' Fever API documentation contains example timestamps that also make the most sense in seconds rather than milliseconds.

Signed-off-by: Mark Bestavros <markbest@bu.edu>
@scottbetza
Copy link

Any chance you can build a pre-release somehow of this or show me where to find apk. Sorry not a dev and been a while, but glad to check it out and test.

@mbestavros
Copy link
Collaborator Author

mbestavros commented May 14, 2023

@scottbetza It looks like test APKs are built as part of CI, which is awesome! You can find the artifact here: https://github.com/Ashinch/ReadYou/actions/runs/4973703129?pr=399

Grab either the fdroid-... or github-... files (shouldn't matter too much which one, I did the fdroid version), unzip it, and install the APK file inside. That should have not only my changes, but also a number of other nice enhancements since the last stable release in January.

Let me know if your RSS service also marks stories as read when you use the batch Mark as Read feature.

@Ashinch
Copy link
Owner

Ashinch commented May 15, 2023

@mbestavros

Thank you for contributing many code, I will take some time to review them ❤️🙏

@Ashinch Ashinch merged commit 470dff5 into Ashinch:main May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants