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

feat: use last_read_at timestamp when available #1103

Merged
merged 3 commits into from May 13, 2024

Conversation

setchy
Copy link
Member

@setchy setchy commented May 10, 2024

Prefer last_read_at time when available. Helpful in conditions where you open a notification, then mark is as unread. Most noticeable when a decent amount of time has lapsed between reading and unreading.

Before
Screenshot 2024-05-10 at 4 43 39 PM

After
Screenshot 2024-05-10 at 4 43 18 PM

@setchy setchy changed the title fix: use last_read_at when available feat: use last_read_at timestamp when available May 10, 2024
@setchy setchy added the enhancement New feature or enhancement to existing functionality label May 10, 2024
@setchy setchy added this to the Release 5.6.0 milestone May 10, 2024
@setchy
Copy link
Member Author

setchy commented May 10, 2024

Hard to tell if this is a regression in the GitHub API around how they are setting updated_at timestamps when updating read/done notifications to unread... Feels unusual that I'm only just noticing this today... 🤷

Our GraphQL searches to return Discussions appear now finding no results for these types of notifications (as shown in the screenshots above) since it's using a time window of updatedAt timestamp - 2hrs. I've raised #1104 since I think we can remove the timestamp-range filtering from the query...

@setchy setchy marked this pull request as draft May 10, 2024 21:01
@setchy setchy marked this pull request as ready for review May 10, 2024 21:26
Copy link
Member

@afonsojramos afonsojramos left a comment

Choose a reason for hiding this comment

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

I see, so switching between unread/read would update the notification, that makes sense!

@afonsojramos afonsojramos merged commit b40e1ac into main May 13, 2024
7 checks passed
@afonsojramos afonsojramos deleted the fix/notification-updated-at branch May 13, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or enhancement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants