android notif: Fix bugs in un-notifying read messages

I added this TODO comment a few commits ago, based on studying the
code, and just now I confirmed that the bug happens empirically:
reading several conversations at once can leave behind a ghost group.
There's also a related bug when the user reads a conversation whose
notification has already been dismissed: if there's just one active
notification remaining, we'll end up dismissing the whole group and
that notification with it.

The root cause of both of these is that we're relying on an assumption
that the loop looking for notifications to cancel found exactly one
such notification.  In fact it may have found fewer or more than that.
This can cause this code to leave the summary notification in place
when it should cancel it, and vice versa.

To fix, while we're looping through the notifications in order to
possibly cancel some, we just note whether there are also any (in
the same group) that we aren't cancelling.

Fixes: zulip#5119
Fixes: zulip#5120

android/app/src/main/java/com/zulipmobile/notifications/FCMPushNotifications.kt

@@ -132,13 +132,13 @@ private fun removeNotification(context: Context, fcmMessage: RemoveFcmMessage) {
     // fall under one notification group.
     val groupKey = extractGroupKey(fcmMessage.identity)
 
-    val statusBarNotifications = getNotificationManager(context).activeNotifications
     // Find any conversations we can cancel the notification for.
     // The API doesn't lend itself to removing individual messages as
     // they're read, so we wait until we're ready to remove the whole
     // conversation's notification.
     // See: https://github.com/zulip/zulip-mobile/pull/4842#pullrequestreview-725817909
-    for (statusBarNotification in statusBarNotifications) {
+    var haveRemaining = false
+    for (statusBarNotification in getNotificationManager(context).activeNotifications) {
         // The StatusBarNotification object describes an active notification in the UI.
         // Its relationship to the Notification and to our metadata is a bit confusing:
         //  * The `.tag`, `.id`, and `.notification` are the same values as we passed to
@@ -152,25 +152,25 @@ private fun removeNotification(context: Context, fcmMessage: RemoveFcmMessage) {
         // Don't act on notifications that are for other Zulip accounts/identities.
         if (notification.group != groupKey) continue;
 
+        // Don't act on the summary notification for the group.
+        if (statusBarNotification.tag == groupKey) continue;
+
         val lastMessageId = notification.extras.getInt("lastZulipMessageId")
         if (fcmMessage.messageIds.contains(lastMessageId)) {
             // The latest Zulip message in this conversation was read.
             // That's our cue to cancel the notification for the conversation.
             NotificationManagerCompat.from(context).cancel(statusBarNotification.tag, statusBarNotification.id)
+        } else {
+            // This notification is for another conversation that's still unread.
+            // We won't cancel the summary notification.
+            haveRemaining = true
         }
     }
-    var counter = 0
-    for (statusBarNotification in statusBarNotifications) {
-        if (statusBarNotification.notification.group == groupKey) {
-            counter++
-        }
-    }
-    if (counter == 2) {
-        // counter will be 2 only when summary notification and last notification are
-        // present; in this case, we remove summary notification.
+
+    if (!haveRemaining) {
+        // The notification group is now empty; it had no notifications we didn't
+        // just cancel, except the summary notification.  Cancel that one too.
         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.)
     }
 }