Skip to content

Commit f2c8e40

Browse files
committed
notif [nfc]: Keep more comments in remove-notif from zulip-mobile implementation
These comments in the corresponding code in zulip-mobile: https://github.com/zulip/zulip-mobile/blob/2217c858e207f9f092651dd853051843c3f04422/android/app/src/main/java/com/zulipmobile/notifications/NotificationUiManager.kt#L116-L168 are explaining the same logic we've copied into this code. So the comments are equally relevant for explaining the code here too.
1 parent b9addf2 commit f2c8e40

File tree

1 file changed

+19
-2
lines changed

1 file changed

+19
-2
lines changed

lib/notifications/display.dart

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,15 +209,32 @@ class NotificationDisplayManager {
209209
}
210210

211211
static void _onRemoveFcmMessage(RemoveFcmMessage data) async {
212+
// We have an FCM message telling us that some Zulip messages were read
213+
// and should no longer appear as notifications. We'll remove their
214+
// conversations' notifications, if appropriate, and then the whole
215+
// notification group if it's now empty.
212216
assert(debugLog('notif remove zulipMessageIds: ${data.zulipMessageIds}'));
213217

218+
// There may be a lot of messages mentioned here, across a lot of
219+
// conversations. But they'll all be for one account, so they'll
220+
// fall under one notification group.
214221
final groupKey = _groupKey(data);
215-
final activeNotifications = await _androidHost.getActiveNotifications(
216-
desiredExtras: [kExtraZulipMessageId]);
217222

223+
// Find any conversations we can cancel the notification for.
224+
// The API doesn't lend itself to removing individual messages as
225+
// they're read, so we wait until we're ready to remove the whole
226+
// conversation's notification. For background discussion, see:
227+
// https://github.com/zulip/zulip-mobile/pull/4842#pullrequestreview-725817909
218228
var haveRemaining = false;
229+
final activeNotifications = await _androidHost.getActiveNotifications(
230+
desiredExtras: [kExtraZulipMessageId]);
219231
for (final statusBarNotification in activeNotifications) {
220232
if (statusBarNotification == null) continue; // TODO(pigeon) eliminate this case
233+
234+
// The StatusBarNotification object describes an active notification in the UI.
235+
// Its `.tag`, `.id`, and `.notification` are the same values as we passed to
236+
// [AndroidNotificationHostApi.notify] (and so to `NotificationManager#notify`
237+
// in the underlying Android APIs). So these are good to match on and inspect.
221238
final notification = statusBarNotification.notification;
222239

223240
// Sadly we don't get toString on Pigeon data classes: flutter#59027

0 commit comments

Comments
 (0)