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: notification insanity #824

Conversation

nikkothari22
Copy link
Member

@nikkothari22 nikkothari22 commented Apr 7, 2024

Push notifications were being sent to the user who sent the message (only in channels - since topics)

Implemented the following:

On logging in - add current user ID to indexed DB.
On logging out - clear it.

In the service worker, fetch current user from IndexedDB and check if the message was sent by the current user. If not, create notification.

Tested this for a bit - seems to work fine. Sometimes the push notification doesn't fire (usually when the app is killed/started) - will investigate. This is hopefully not happening because of this change.

Other changes:

  1. Tag all notifications with channel ID.
  2. When the user opens the channel, clear the notifications.
  3. For Desk integration - publish a message_update event so that it works. The desk integration needs to be updated soon with pagination, polls etc.

@github-actions github-actions bot added 🐞 bug Something isn't working 📱mobile app UI labels Apr 7, 2024
@nikkothari22
Copy link
Member Author

So, now the "fix" is:

  1. Show the notification. If the push is sent from the current user, then add a "silent" property to the notification.
  2. After the notification is shown, close it.

@nikkothari22
Copy link
Member Author

This might still cause issues if getting the data from IndexedDB takes too much time. iOS wants devs to fire a notification immediately. Possible solution: fire notification immediately and then close it afterwards.

Note: only do this if the above solution does not work.

@nikkothari22
Copy link
Member Author

Looks like the only solution is to not use topic based notifications and send notifications to each user individually.

@nikkothari22 nikkothari22 mentioned this pull request Apr 9, 2024
@nikkothari22 nikkothari22 deleted the 813-notifications-are-shown-for-messages-sent-by-the-sender-in-channels branch June 11, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 📱mobile app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notifications are shown for messages sent by the sender in channels
1 participant