Skip to content

Commit

Permalink
android notif [nfc]: Add TODO for apparent bug in clearing notifications
Browse files Browse the repository at this point in the history
If the user reads the last messages in several conversations at once,
and those are their only active notifications -- for example, if they
have a couple of recent PMs in different threads, and visit the
"Private messages" view in the web app -- then it looks from the code
here like we'll delete those individual conversations' notifications,
but not the summary for the group, leaving behind a ghost header.

I noticed this just now from reading the code.  I haven't confirmed
empirically that this indeed happens.  If there's a subtle reason
it doesn't, though, we should resolve this TODO by making that clear.

[Added later: Indeed, confirmed this empirically; filed as zulip#5119.]
  • Loading branch information
gnprice authored and chetas411 committed Nov 28, 2021
1 parent 9400bf3 commit 85db0ff
Showing 1 changed file with 2 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ private fun removeNotification(context: Context, fcmMessage: RemoveFcmMessage) {
// counter will be 2 only when summary notification and last notification are
// present; in this case, we remove summary notification.
NotificationManagerCompat.from(context).cancel(groupKey, NOTIFICATION_ID)
// TODO: What if several conversations get read at once? Does this leave a ghost group?
// (Yep, it does: #5119.)
}
}

Expand Down

0 comments on commit 85db0ff

Please sign in to comment.