From a1b221730bb86a00678e9abe5fe822e59cedbd27 Mon Sep 17 00:00:00 2001
From: Steve Howell <showell@zulipchat.com>
Date: Sat, 16 Dec 2017 10:53:27 -0500
Subject: [PATCH] refactor: Add unread.get_unread_message{_ids}().

This adds two similar functions to simplify
our batch processing of unread messages.

    unread.get_unread_messages
    unread.get_unread_message_ids

They are used to simplify two functions that loop
over messages.  Before this change, the functions
would short circuit the loop to ignore messages
that were already read; now they just use the
helpers before the loop.
---
 frontend_tests/node_tests/unread.js | 11 ++++++++--
 static/js/unread.js                 | 10 +++++++--
 static/js/unread_ops.js             | 32 +++++++++++------------------
 3 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/frontend_tests/node_tests/unread.js b/frontend_tests/node_tests/unread.js
index 606d60477f..1344772b00 100644
--- a/frontend_tests/node_tests/unread.js
+++ b/frontend_tests/node_tests/unread.js
@@ -76,9 +76,16 @@ var zero_counts = {
         subject: 'lunCH',
     };
 
-    assert(!unread.id_flagged_as_unread(15));
+    assert.deepEqual(unread.get_unread_message_ids([15, 16]), []);
+    assert.deepEqual(unread.get_unread_messages([message, other_message]), []);
+
     unread.process_loaded_messages([message, other_message]);
-    assert(unread.id_flagged_as_unread(15));
+
+    assert.deepEqual(unread.get_unread_message_ids([15, 16]), [15, 16]);
+    assert.deepEqual(
+        unread.get_unread_messages([message, other_message]),
+        [message, other_message]
+    );
 
     count = unread.num_unread_for_topic(stream_id, 'Lunch');
     assert.equal(count, 2);
diff --git a/static/js/unread.js b/static/js/unread.js
index 8fc767dae9..d0a67133dc 100644
--- a/static/js/unread.js
+++ b/static/js/unread.js
@@ -333,8 +333,14 @@ exports.message_unread = function (message) {
            message.flags.indexOf('read') === -1;
 };
 
-exports.id_flagged_as_unread = function (message_id) {
-    return unread_messages.has(message_id);
+exports.get_unread_message_ids = function (message_ids) {
+    return _.filter(message_ids, unread_messages.has);
+};
+
+exports.get_unread_messages = function (message) {
+    return _.filter(message, function (message) {
+        return unread_messages.has(message.id);
+    });
 };
 
 exports.update_unread_topics = function (msg, event) {
diff --git a/static/js/unread_ops.js b/static/js/unread_ops.js
index 0e977a9eae..b9acd0433b 100644
--- a/static/js/unread_ops.js
+++ b/static/js/unread_ops.js
@@ -38,14 +38,13 @@ exports.process_read_messages_event = function (message_ids) {
         loaded locally).
     */
     var options = {from: 'server'};
-    var processed = false;
+
+    message_ids = unread.get_unread_message_ids(message_ids);
+    if (message_ids.length === 0) {
+        return;
+    }
 
     _.each(message_ids, function (message_id) {
-        if (!unread.id_flagged_as_unread(message_id)) {
-            // Don't do anything if the message is already read.
-            return;
-        }
-
         if (current_msg_list === message_list.narrowed) {
             // I'm not sure this entirely makes sense for all server
             // notifications.
@@ -53,7 +52,6 @@ exports.process_read_messages_event = function (message_ids) {
         }
 
         unread.mark_as_read(message_id);
-        processed = true;
 
         var message = message_store.get(message_id);
 
@@ -62,22 +60,20 @@ exports.process_read_messages_event = function (message_ids) {
         }
     });
 
-    if (processed) {
-        unread_ui.update_unread_counts();
-    }
+    unread_ui.update_unread_counts();
 };
 
 
 // Takes a list of messages and marks them as read
 exports.mark_messages_as_read = function mark_messages_as_read(messages, options) {
     options = options || {};
-    var processed = false;
+
+    messages = unread.get_unread_messages(messages);
+    if (messages.length === 0) {
+        return;
+    }
 
     _.each(messages, function (message) {
-        if (!unread.id_flagged_as_unread(message.id)) {
-            // Don't do anything if the message is already read.
-            return;
-        }
         if (current_msg_list === message_list.narrowed) {
             unread.messages_read_in_narrow = true;
         }
@@ -85,13 +81,9 @@ exports.mark_messages_as_read = function mark_messages_as_read(messages, options
         message_flags.send_read(message);
         unread.mark_as_read(message.id);
         process_newly_read_message(message, options);
-
-        processed = true;
     });
 
-    if (processed) {
-        unread_ui.update_unread_counts();
-    }
+    unread_ui.update_unread_counts();
 };
 
 exports.mark_message_as_read = function mark_message_as_read(message, options) {