bluesky-social / atproto

Social networking technology created by Bluesky
Other
5.77k stars 406 forks source link

Send `mark-read-generic` notification on `updateSeen` #2567

Open haileyok opened 2 weeks ago

haileyok commented 2 weeks ago

Why

We've added support for incrementing the badge properly on iOS whenever push notifications are received for regular notifications (i.e. like, repost, etc.) However, there is no logic for decrementing the badge whenever notifications are read. We need to send an additional notification to those clients whenever they mark their notifications as read.

How

@devinivy added support for "client controlled" notifications in Courier for the chat service (though it looks like we needed to add that value here which I think I did correctly). We can therefore send one of those "client controlled" notifications to the recipient to tell the client to decrement the badge.

Since we currently mark all notifications as "read" based on the "last seen" value, we do not need to send a specific number of notifications to mark as read (though it looks like we maybe could do that by getting the unread notification count from the dataplane?) Therefore, I think the best move for now would be to just have the client reset the "generic" notification count to zero (separate from the DMs notification count) whenever a notification with reason mark-read-generic is received by the client.

Test Plan

We'll actually need to deploy this change to test it unfortunately, since the notifications need to come from Apple's server to test the Notification Service Extension on iOS. The good news is that since in this case we do not need to supply a title or message, these notifications will not appear for users anyway. This means that we can deploy this change whenever we are comfortable doing so and test the updated client internally.

haileyok commented 2 days ago

Hmm, considering here though that we might need some way to prevent some bizarre number of these notifications from going through. I'm pretty sure we are already a little irresponsible about calling this endpoint in the app (i think even just tapping the notifications icon twice will send two requests).

Think I have an idea here of how we can handle this nicely. Requires something in courier so I'll PR there as well.