refactoring: moved initial logic of calc_can_mark_messages_read

core_filters This a pre commit to prepare for changes in zulip#25351 In web/src/filter, this initial logic is used by both can_mark_messages_read and is_common_narrow. Changes related to this pr zulip#25351 breaks is_common_narrow. As per TODO comment of is_common_narrow, we moved those filter logics to a core_filters method which is extended in both of them.
syed-rafat · May 24, 2023 · 29aa356 · 29aa356
1 parent baa77ef
commit 29aa356
Showing 1 changed file with 45 additions and 39 deletions.
diff --git a/web/src/filter.js b/web/src/filter.js
@@ -500,10 +500,9 @@ export class Filter {
         return true;
     }
 
-    calc_can_mark_messages_read() {
-        // 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).
+    // these filters work for both
+    // calc_can_mark_messages_read and is_common_narrow
+    core_filters() {
         const term_types = this.sorted_term_types();
 
         if (_.isEqual(term_types, ["stream", "topic"])) {
@@ -514,56 +513,70 @@ 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;
         }
 
         if (_.isEqual(term_types, ["is-dm"])) {
             return true;
         }
-        // -is:dm excludes direct messages from all messages
-        if (_.isEqual(term_types, ["not-is-dm"])) {
+
+        if (_.isEqual(term_types, ["is-mentioned"])) {
             return true;
         }
 
-        // Excluding direct messages from stream and topic does not
-        // accomplish anything, but we are still letting user mark
-        // mark messages as read to be consistent with our design.
-
-        if (_.isEqual(term_types, ["stream", "not-is-dm"])) {
+        if (_.isEqual(term_types, ["is-resolved"])) {
             return true;
         }
 
-        if (_.isEqual(term_types, ["topic", "not-is-dm"])) {
+        if (_.isEqual(term_types, [])) {
+            // All view
             return true;
         }
 
-        // Excluding direct messages from stream and topic does not
-        // accomplish anything, but we are still letting user mark
-        // mark messages as read to be consistent with our design.
-        if (_.isEqual(term_types, ["stream", "topic", "not-is-dm"])) {
+        if (term_types.length === 1 && ["in-home", "in-all"].includes(term_types[0])) {
             return true;
         }
 
-        if (_.isEqual(term_types, ["is-mentioned"])) {
+        return false
+    }
+
+    calc_can_mark_messages_read() {
+        // 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.initial_filters_for_mark_messages_read()) {
+            return true
+        }
+
+        const term_types = this.sorted_term_types();
+
+
+        // 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)
+
+
+        // -is:dm excludes direct messages from all messages
+        if (_.isEqual(term_types, ["not-is-dm"])) {
             return true;
         }
 
-        if (_.isEqual(term_types, ["is-resolved"])) {
+        // Excluding direct messages from stream and topic does not
+        // accomplish anything, but we are still letting user mark
+        // mark messages as read to be consistent with our design.
+
+        if (_.isEqual(term_types, ["stream", "not-is-dm"])) {
             return true;
         }
 
-        if (_.isEqual(term_types, [])) {
-            // All view
+        if (_.isEqual(term_types, ["topic", "not-is-dm"])) {
             return true;
         }
 
-        if (term_types.length === 1 && ["in-home", "in-all"].includes(term_types[0])) {
+        if (_.isEqual(term_types, ["stream", "topic", "not-is-dm"])) {
             return true;
         }
 
@@ -583,23 +596,16 @@ export class Filter {
     // 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: dm, stream + -is:dm, topic + -is:dm )
-        // is: mentioned, is: resolved
-        if (this.can_mark_messages_read()) {
+        const term_types = this.sorted_term_types();
+
+        if (this.core_filters()) {
             return true;
         }
+
         // that leaves us with checking:
-        // is: starred
-        // (which can_mark_messages_read_does not check as starred messages are always read)
-        const term_types = this.sorted_term_types();
+        // is: starred, streams-public
+        // (which core_filters does not check)
 
         if (_.isEqual(term_types, ["is-starred"])) {
             return true;