Notification Improvements (#2364)

Notification Improvements

js/background.js

@@ -240,7 +240,7 @@
     });
 
     window.addEventListener('focus', () => Whisper.Notifications.clear());
-    window.addEventListener('unload', () => Whisper.Notifications.clear());
+    window.addEventListener('unload', () => Whisper.Notifications.fastClear());
 
     Whisper.events.on('showConversation', function(conversation) {
       if (appView) {

js/notifications.js

@@ -8,6 +8,7 @@
 /* global Signal: false */
 /* global storage: false */
 /* global Whisper: false */
+/* global _: false */
 
 // eslint-disable-next-line func-names
 (function() {
@@ -29,12 +30,25 @@
       this.on('remove', this.onRemove);
 
       this.lastNotification = null;
+
+      // Testing indicated that trying to create/destroy notifications too quickly
+      //   resulted in notifications that stuck around forever, requiring the user
+      //   to manually close them. This introduces a minimum amount of time between calls,
+      //   and batches up the quick successive update() calls we get from an incoming
+      //   read sync, which might have a number of messages referenced inside of it.
+      this.fastUpdate = this.update;
+      this.update = _.debounce(this.update, 1000);
     },
     onClick(conversationId) {
       const conversation = ConversationController.get(conversationId);
       this.trigger('click', conversation);
     },
     update() {
+      if (this.lastNotification) {
+        this.lastNotification.close();
+        this.lastNotification = null;
+      }
+
       const { isEnabled } = this;
       const isAppFocused = isFocused();
       const isAudioNotificationEnabled =
@@ -62,7 +76,7 @@
 
       if (status.type !== 'ok') {
         if (status.shouldClearNotifications) {
-          this.clear();
+          this.reset([]);
         }
 
         return;
@@ -148,18 +162,19 @@
     },
     onRemove() {
       console.log('Remove notification');
-      if (this.length === 0) {
-        this.clear();
-      } else {
-        this.update();
-      }
+      this.update();
     },
     clear() {
       console.log('Remove all notifications');
-      if (this.lastNotification) {
-        this.lastNotification.close();
-      }
       this.reset([]);
+      this.update();
+    },
+    // We don't usually call this, but when the process is shutting down, we should at
+    //   least try to remove the notification immediately instead of waiting for the
+    //   normal debounce.
+    fastClear() {
+      this.reset([]);
+      this.fastUpdate();
     },
     enable() {
       const needUpdate = !this.isEnabled;

js/read_syncs.js

@@ -24,21 +24,32 @@
               message.get('source') === receipt.get('sender')
             );
           });
-          if (message) {
-            Whisper.Notifications.remove(message);
-            return message.markRead(receipt.get('read_at')).then(
-              function() {
-                this.notifyConversation(message);
-                this.remove(receipt);
-              }.bind(this)
-            );
-          } else {
-            console.log(
-              'No message for read sync',
-              receipt.get('sender'),
-              receipt.get('timestamp')
-            );
-          }
+          const notificationForMessage = message
+            ? Whisper.Notifications.findWhere({ messageId: message.id })
+            : null;
+          const removedNotification = Whisper.Notifications.remove(
+            notificationForMessage
+          );
+          const receiptSender = receipt.get('sender');
+          const receiptTimestamp = receipt.get('timestamp');
+          const wasMessageFound = Boolean(message);
+          const wasNotificationFound = Boolean(notificationForMessage);
+          const wasNotificationRemoved = Boolean(removedNotification);
+          console.log('Receive read sync:', {
+            receiptSender,
+            receiptTimestamp,
+            wasMessageFound,
+            wasNotificationFound,
+            wasNotificationRemoved,
+          });
+          return message
+            ? message.markRead(receipt.get('read_at')).then(
+                function() {
+                  this.notifyConversation(message);
+                  this.remove(receipt);
+                }.bind(this)
+              )
+            : Promise.resolve();
         }.bind(this)
       );
     },