Don't show notifications for logged out accounts (part one for DMs) #4076
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Your Render PR Server URL is https://social-app-pr-4076.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cp3h9nn109ks73ao7230. |
|
The Pull Request introduced fingerprint changes against the base commit: 829f6a9 Fingerprint diff[{"type":"dir","filePath":"modules/expo-background-notification-handler/android","reasons":["expoAutolinkingAndroid"],"hash":"3734e9c5079d4c6419b07b7e17adfc58d5c6e1fe"},{"type":"dir","filePath":"modules/expo-background-notification-handler/ios","reasons":["expoAutolinkingIos"],"hash":"5752801a1c20a2b8b349268a2637acf7c4c578c2"}]Generated by PR labeler 🤖 |
|
haileyok
force-pushed
the
hailey/notifications-stuff
branch
from
bfaad29 to
ed8d9c0
Compare
rm a log hande async stuff better swift != javascript oops remove from store whenever we sign back in don't show notifications for dms on logged out accounts filter out while foregrounded smol add disabled dids to android migrate android to map update more js update types update extension to support no useless async add js functions revert oops
haileyok
force-pushed
the
hailey/notifications-stuff
branch
from
c0917c8 to
b5bb50c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
Notifications may be received for logged out accounts, even if we log out first. One option would be to add an endpoint to the backend such as
unregisterPushToken, but this wouldn't be a 100% reliable method. For example, something could happen during the logout where the request for unregistering fails but the client still removes the account from the logged in sessions.Instead, we can take advantage of the new notification handling logic to store logged out/disabled DIDs and prevent notifications for those accounts from showing up.
Right now, this will only work for DMs since we need to implement the
recipientDidor similar in notifications as well. (We might be able to parse the subject for some types, will have to look).For general performance reasons, I am also converting the all of the string array preference types to a map instead. These could get a little chunky for future use cases like thread mutes, so better to get ahead of that now.
Screen.Recording.2024-05-17.at.2.34.44.AM.mov