zulip / zulip-mobile

Zulip mobile apps for Android and iOS.
https://zulip.com/apps/
Apache License 2.0
1.27k stars 643 forks source link

Support revoking notifications when message is deleted #5837

Open alya opened 5 months ago

alya commented 5 months ago

As specified in https://github.com/zulip/zulip/issues/26584, it would be good to revoke push notifications when a message is deleted, and to modify the notification when the message is edited. As @gnprice wrote in https://github.com/zulip/zulip/issues/26584#issuecomment-1738150571:

From the mobile client perspective: revoking notifications will work great on Android, only requiring the server to send an appropriate FCM blob in the same way it does when the message is read.

That functionality isn't currently available in our iOS app; it's tracked as zulip/zulip-mobile#3119 . But I believe that's basically orthogonal to this issue — the implementation of this issue should send those revocations the same way as we send other revocations, which is for Android only, and then in the future when we have an iOS client handling revocations and start sending them there, we'll do so uniformly for all causes of revocations, including the ones added as part of this issue.

Editing the content of the message in a notification is another matter. Properly supporting that will mean adding a new type of push-notification blob to the protocol between the server and the client. It would be good to do, but should be tracked as its own issue, because unlike the revocation cases it will be more work and will happen later. (Probably after 8.0 and after the Flutter migration of the mobile app.)

Thus, only support for message deletion should be considered a 9.0 goal / something we may implement in the legacy app.

Server issue:

gnprice commented 5 months ago

Just to make this explicit: there shouldn't be any code changes needed in mobile in order to support this. The server just needs to start sending an appropriate FCM blob of the same kind that it sends when a message (that had caused a notification) is read.

But we'll be happy to consult and answer questions when that's being implemented, and to do an extra round of testing.