From 1bedb965e9b55a26529fd922e834ca42e5daeddf Mon Sep 17 00:00:00 2001
From: Steve Howell <showell@zulipchat.com>
Date: Wed, 8 Jan 2020 11:59:24 +0000
Subject: [PATCH] refactor: Clean up can_mark_messages_read.

We now explicitly enumerate various cases, which
should make it easier to change this code.
---
 frontend_tests/node_tests/filter.js |  6 ++++
 static/js/filter.js                 | 45 ++++++++++++++++++++++++-----
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/frontend_tests/node_tests/filter.js b/frontend_tests/node_tests/filter.js
index f89d26b118..127552a32d 100644
--- a/frontend_tests/node_tests/filter.js
+++ b/frontend_tests/node_tests/filter.js
@@ -275,11 +275,17 @@ run_test('can_mark_messages_read', () => {
         { operator: 'pm-with', operand: 'joe@example.com,' },
     ];
 
+    const pm_with_negated = [
+        { operator: 'pm-with', operand: 'joe@example.com,', negated: true},
+    ];
+
     const group_pm = [
         { operator: 'pm-with', operand: 'joe@example.com,STEVE@foo.com' },
     ];
     filter = new Filter(pm_with);
     assert(filter.can_mark_messages_read());
+    filter = new Filter(pm_with_negated);
+    assert(!filter.can_mark_messages_read());
     filter = new Filter(group_pm);
     assert(filter.can_mark_messages_read());
     assert_not_mark_read_with_is_operands(group_pm);
diff --git a/static/js/filter.js b/static/js/filter.js
index 4a2b821b45..7ffc5664f6 100644
--- a/static/js/filter.js
+++ b/static/js/filter.js
@@ -382,13 +382,44 @@ Filter.prototype = {
     },
 
     can_mark_messages_read: function () {
-        return _.every(this._operators, function (elem) {
-            return (_.contains(['stream', 'topic', 'pm-with'], elem.operator)
-                || elem.operator === 'is' && elem.operand === 'private'
-                || elem.operator === 'in' && elem.operand === 'all'
-                || elem.operator === 'in' && elem.operand === 'home')
-                && !elem.negated;
-        });
+        const term_types = this.sorted_term_types();
+
+        if (_.isEqual(term_types, ['stream', 'topic'])) {
+            return true;
+        }
+
+        if (_.isEqual(term_types, ['pm-with'])) {
+            return true;
+        }
+
+        // TODO: Some users really hate it when Zulip marks messages as read
+        // in interleaved views, so we will eventually have a setting
+        // that early-exits before the subsequent checks.
+
+        if (_.isEqual(term_types, ['stream'])) {
+            return true;
+        }
+
+        if (_.isEqual(term_types, ['is-private'])) {
+            return true;
+        }
+
+        if (_.isEqual(term_types, [])) {
+            // All view
+            return true;
+        }
+
+        if (this._operators.length === 1) {
+            const elem = this._operators[0];
+
+            if (elem.operator === 'in' && !elem.negated) {
+                if (elem.operand === 'home' || elem.operand === 'all') {
+                    return true;
+                }
+            }
+        }
+
+        return false;
     },
 
     allow_use_first_unread_when_narrowing: function () {