Push notifications: prevent duplication between remote push notifications and local notifications #1704
Comments
|
On Mon, Nov 13, 2023 at 10:27:56AM -0800, Daniel D’Aquino wrote:
For notifications we have two mechanisms:
- **Local notifications:** Which locally shows notifications when the app is running on the foreground or the background
- Local notifications have the advantage of not relying on a central server which is nice in cases when the user does not want to or cannot use our notification relay (e.g. If they only use private relays)
- **Remote push notifications:** Shows notifications even when the app is not running.
- Push notifications have the advantage of always working even when the app is not running.
Ideally we should keep both to enjoy the benefits of each. However, we need to ensure that the user is not getting duplicated notifications.
**Acceptance criteria:** Implement some mechanism to detect if a notification was already sent either locally or remotely, and ignore duplicate notifications.
should we add this to the note metadata structure in nostrdb? downside
is we won't have ndb in remote notifications v1 :(
|
That would be a nice way of doing it, when we have ndb support on the extension. If we need to implement this ticket before adding ndb support, perhaps using UserDefaults might work as well. Given the transient nature of notifications, I imagine we don't even need to keep this type of info longer than 1 week or so. We can probably make the whole system suppress notifications of events older than 1 week, so that we can model it as a temporary cache more than a permanent DB, and thus eliminate certain complexities like DB migrations and so on. |
This commit adds a new cache that serves as a coordinator between local and remote/push notifications to avoid showing two notifications for the same event to the user. Testing ------- Device: iPhone 15 Pro simulator iOS: 17.0.1 Damus: This commit Coverage: 1. New unit tests are passing 2. Triggered a local notification with the app open, and then sent a remote push notification for the same event. Local notification shown and remote push notification suppressed. PASS 3. Triggered a push notification for an event that was not yet in the cache. Push notification is displayed as expected. PASS 4. Triggered a new push notification while the app was turned off. Remote push notification is displayed first, and local notification for the same event is suppressed. PASS Changelog-Added: Prevent duplicate notifications for the same events Closes: #1704
|
Sent a patch via email! 🤙 All testing, code, and notes can be visualized here: |
danieldaquino
modified the milestones:
Push Notifications,
1.7.0 — Damus Purple et al. (TestFlight release)
|
Need to add switch local <-> push notifications |
alltheseas
removed this from the "Madeira" AppStore 1.7 purple follow on + search design milestone
|
A relevant note from an old patch email conversation to further elaborate the switch between local <-> push notifications mentioned above:
|
|
Sent patches!
@jb55 please let me know if you have any questions or concerns, thank you! |
Changing the notification mode setting requires successfully sending or revoking the device token to the server. As this is an action that might fail, it is important to have a clear UX feedback in case this fails. Testing -------- PASS Device: iPhone 15 simulator iOS: 17.4 Damus: This commit strfry-push-notify: d6c2ff289c80e0a90874a7499ed6408394659fc9 Coverage: 1. Checked that push notification mode setting is invisible when experimental push notifications mode is disabled 2. Checked that push notification mode setting is visible when experimental push notifications mode is enabled 3. Checked that switching between push and local notifications sends requests to the server 4. Checked that switching to push notification mode will cause local notifications to be suppressed and push notifications will be sent to the APNS server 5. Checked that switching back to local notification mode will cause local notifications to be displayed, and push notifications will NOT be sent to APNS 6. Checked that if the API server is off, switching from local to push notification modes is not possible and shows an error to the user. 7. Checked that sending APNS payload to Apple's test APNS page will actually deliver the push notification successfully. Closes: #1704
For notifications we have two mechanisms:
Ideally we should keep both to enjoy the benefits of each. However, we need to ensure that the user is not getting duplicated notifications.
Acceptance criteria: Implement some mechanism to detect if a notification was already sent either locally or remotely, and ignore duplicate notifications.
The text was updated successfully, but these errors were encountered: