From 3a4cb692fb18a12cb2ebdf2f07eaee66d9d277f1 Mon Sep 17 00:00:00 2001
From: palashb01 <palash.baderia@outlook.com>
Date: Mon, 4 Sep 2023 19:34:10 +0530
Subject: [PATCH] filter: Create helper function that checks basic filter
 terms.

Currently, `is_common_narrow` relies on `calc_can_mark_messages_read`
to check a number of filters that are "common narrows" (which means
they have a special message header title and different behavior when
exiting a search view).

Creates a helper function,
`single_term_type_returns_all_messages_of_conversation`, that is
used in both the above functions.

The check for an empty array of term types is unnecessary for
`is_common_narrow` because the "All messages" view has an undefined
filter and the "in-home" term type used in the "zhome" message list
is covered. So that empty array check is not moved to the helper
function, and instead stays only in `calc_can_mark_messages_read`.

The helper function checks for a single term type (except in the case
of topic, which is checked in combination with stream) that will return
all the messages of a conversation.

To ensure consistency in the function, split the combined if
condition of in-home and in-all, further improved the comments, and
deleted the unnecessary ones.
---
 web/src/filter.js | 55 ++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 29 deletions(-)

diff --git a/web/src/filter.js b/web/src/filter.js
index fcee5901ed..47eabad331 100644
--- a/web/src/filter.js
+++ b/web/src/filter.js
@@ -499,8 +499,31 @@ export class Filter {
         // Arguably this should match supports_collapsing_recipients.
         // We may want to standardize on that in the future.  (At
         // present, this function does not allow combining valid filters).
+        if (this.single_term_type_returns_all_messages_of_conversation()) {
+            return true;
+        }
+        const term_types = this.sorted_term_types();
+        if (_.isEqual(term_types, [])) {
+            // "All messages" view
+            return true;
+        }
+        return false;
+    }
+
+    can_mark_messages_read() {
+        if (this._can_mark_messages_read === undefined) {
+            this._can_mark_messages_read = this.calc_can_mark_messages_read();
+        }
+        return this._can_mark_messages_read;
+    }
+
+    single_term_type_returns_all_messages_of_conversation() {
         const term_types = this.sorted_term_types();
 
+        // "topic" alone cannot guarantee all messages of a conversation because
+        // it is limited by the user's message history. Therefore, we check "stream"
+        // and "topic" together to ensure that the current filter will return all the
+        // messages of a conversation.
         if (_.isEqual(term_types, ["stream", "topic"])) {
             return true;
         }
@@ -509,11 +532,6 @@ export class Filter {
             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.
-        // (in which case, is_common_narrow would also need to be modified)
-
         if (_.isEqual(term_types, ["stream"])) {
             return true;
         }
@@ -526,48 +544,27 @@ export class Filter {
             return true;
         }
 
-        if (_.isEqual(term_types, [])) {
-            // All view
+        if (_.isEqual(term_types, ["in-home"])) {
             return true;
         }
 
-        if (term_types.length === 1 && ["in-home", "in-all"].includes(term_types[0])) {
+        if (_.isEqual(term_types, ["in-all"])) {
             return true;
         }
 
         return false;
     }
 
-    can_mark_messages_read() {
-        if (this._can_mark_messages_read === undefined) {
-            this._can_mark_messages_read = this.calc_can_mark_messages_read();
-        }
-        return this._can_mark_messages_read;
-    }
-
     // This is used to control the behaviour for "exiting search",
     // given the ability to flip between displaying the search bar and the narrow description in UI
     // here we define a narrow as a "common narrow" on the basis of
     // https://paper.dropbox.com/doc/Navbar-behavior-table--AvnMKN4ogj3k2YF5jTbOiVv_AQ-cNOGtu7kSdtnKBizKXJge
     // common narrows show a narrow description and allow the user to
     // close search bar UI and show the narrow description UI.
-    //
-    // TODO: We likely will want to rewrite this to not piggy-back on
-    // can_mark_messages_read, since that might gain some more complex behavior
-    // with near: narrows.
     is_common_narrow() {
-        // can_mark_messages_read tests the following filters:
-        // stream, stream + topic,
-        // is:dm, dm,
-        // is:resolved
-        if (this.can_mark_messages_read()) {
+        if (this.single_term_type_returns_all_messages_of_conversation()) {
             return true;
         }
-        // that leaves us with checking:
-        // is: starred
-        // (which can_mark_messages_read does not check as starred messages are always read)
-        // is: mentioned
-        // (for which can_mark_messages_read returns false, since they don't have conversation context)
         const term_types = this.sorted_term_types();
         if (_.isEqual(term_types, ["is-mentioned"])) {
             return true;