narrow: Mark messages as read when applying -is:dm filter.

It will mark messages as read when applying -is:dm filter alone or coupled with stream and topic filters. But it will not mark messages as read when there is also search term present. fixes #25113.
zulip · May 10, 2023 · 4b3b54e · 4b3b54e
1 parent 6e0ea3d
commit 4b3b54e
Show file tree

Hide file tree

Showing 2 changed files with 124 additions and 3 deletions.
diff --git a/web/src/filter.js b/web/src/filter.js
@@ -526,6 +526,37 @@ export class Filter {
         if (_.isEqual(term_types, ["is-dm"])) {
             return true;
         }
+        // -is:dm excludes direct messages from all messages
+        if (_.isEqual(term_types, ["not-is-dm"])) {
+            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"])) {
+            return true;
+        }
+
+        if (_.isEqual(term_types, ["topic", "not-is-dm"])) {
+            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"])) {
+            return true;
+        }
+
+        if (_.isEqual(term_types, ["stream", "not-is-dm"])) {
+            return true;
+        }
+
+        if (_.isEqual(term_types, ["topic", "not-is-dm"])) {
+            return true;
+        }
 
         if (_.isEqual(term_types, ["is-mentioned"])) {
             return true;
@@ -567,8 +598,9 @@ export class Filter {
     is_common_narrow() {
         // can_mark_messages_read tests the following filters:
         // stream, stream + topic,
-        // is:dm, dm,
-        // is:mentioned, is:resolved
+        // is: dm, dm,
+        // ( -is: dm, stream + -is:dm, topic + -is:dm )
+        // is: mentioned, is: resolved
         if (this.can_mark_messages_read()) {
             return true;
         }

diff --git a/web/tests/filter.test.js b/web/tests/filter.test.js
@@ -194,6 +194,52 @@ test("basics", () => {
     assert.ok(!filter.is_personal_filter());
     assert.ok(!filter.is_conversation_view());
 
+    operators = [
+        {operator: "stream", operand: "foo"},
+        {operator: "is", operand: "dm", negated: true},
+    ];
+    filter = new Filter(operators);
+    assert.ok(!filter.contains_only_private_messages());
+    assert.ok(filter.has_operator("stream"));
+    assert.ok(filter.can_mark_messages_read());
+    assert.ok(filter.supports_collapsing_recipients());
+    assert.ok(!filter.has_negated_operand("stream", "not-is-dm"));
+    assert.ok(filter.can_apply_locally());
+    assert.ok(!filter.is_personal_filter());
+
+    operators = [
+        {operator: "stream", operand: "foo"},
+        {operator: "topic", operand: "bar"},
+        {operator: "is", operand: "dm", negated: true},
+    ];
+    filter = new Filter(operators);
+    assert.ok(!filter.contains_only_private_messages());
+    assert.ok(filter.has_operator("stream"));
+    assert.ok(filter.has_operator("topic"));
+    assert.ok(filter.can_mark_messages_read());
+    assert.ok(filter.supports_collapsing_recipients());
+    assert.ok(!filter.has_operator("search"));
+    assert.ok(!filter.has_negated_operand("stream", "foo"));
+    assert.ok(!filter.has_negated_operand("topic", "bar"));
+    assert.ok(filter.can_apply_locally());
+    assert.ok(!filter.is_personal_filter());
+    assert.ok(filter.can_bucket_by("stream"));
+    assert.ok(filter.can_bucket_by("stream", "topic"));
+
+    operators = [
+        {operator: "topic", operand: "bar"},
+        {operator: "is", operand: "dm", negated: true},
+    ];
+    filter = new Filter(operators);
+    assert.ok(!filter.contains_only_private_messages());
+    assert.ok(filter.has_operator("topic"));
+    assert.ok(filter.can_mark_messages_read());
+    assert.ok(filter.supports_collapsing_recipients());
+    assert.ok(!filter.has_operator("search"));
+    assert.ok(!filter.has_negated_operand("topic", "bar"));
+    assert.ok(filter.can_apply_locally());
+    assert.ok(!filter.is_personal_filter());
+
     operators = [{operator: "is", operand: "dm"}];
     filter = new Filter(operators);
     assert.ok(filter.contains_only_private_messages());
@@ -210,6 +256,15 @@ test("basics", () => {
     assert.ok(filter.has_operand("is", "dm"));
     assert.ok(!filter.has_operand("is", "private"));
 
+    operators = [{operator: "is", operand: "dm", negated: true}];
+    filter = new Filter(operators);
+    assert.ok(!filter.contains_only_private_messages());
+    assert.ok(filter.can_mark_messages_read());
+    assert.ok(filter.supports_collapsing_recipients());
+    assert.ok(!filter.has_operator("search"));
+    assert.ok(!filter.is_personal_filter());
+    assert.ok(!filter.is_conversation_view());
+
     operators = [{operator: "is", operand: "mentioned"}];
     filter = new Filter(operators);
     assert.ok(!filter.contains_only_private_messages());
@@ -288,7 +343,6 @@ test("basics", () => {
     // filter.supports_collapsing_recipients loop.
     operators = [
         {operator: "is", operand: "resolved", negated: true},
-        {operator: "is", operand: "dm", negated: true},
         {operator: "stream", operand: "stream_name", negated: true},
         {operator: "streams", operand: "web-public", negated: true},
         {operator: "streams", operand: "public"},
@@ -444,6 +498,37 @@ test("can_mark_messages_read", () => {
     filter = new Filter(stream_negated_topic_operators);
     assert.ok(!filter.can_mark_messages_read());
 
+    const stream_negated_dm_operators = [
+        {operator: "stream", operand: "foo"},
+        {operator: "is", operand: "dm", negated: true},
+    ];
+    filter = new Filter(stream_negated_dm_operators);
+    assert.ok(filter.can_mark_messages_read());
+    assert_not_mark_read_with_has_operands(stream_negated_dm_operators);
+    assert_not_mark_read_with_is_operands(stream_negated_dm_operators);
+    assert_not_mark_read_when_searching(stream_negated_dm_operators);
+
+    const stream_topic_negated_dm_operators = [
+        {operator: "stream", operand: "foo"},
+        {operator: "topic", operand: "bar"},
+        {operator: "is", operand: "dm", negated: true},
+    ];
+    filter = new Filter(stream_topic_negated_dm_operators);
+    assert.ok(filter.can_mark_messages_read());
+    assert_not_mark_read_with_has_operands(stream_topic_negated_dm_operators);
+    assert_not_mark_read_with_is_operands(stream_topic_negated_dm_operators);
+    assert_not_mark_read_when_searching(stream_topic_negated_dm_operators);
+
+    const topic_negated_dm_operators = [
+        {operator: "topic", operand: "bar"},
+        {operator: "is", operand: "dm", negated: true},
+    ];
+    filter = new Filter(topic_negated_dm_operators);
+    assert.ok(filter.can_mark_messages_read());
+    assert_not_mark_read_with_is_operands(topic_negated_dm_operators);
+    assert_not_mark_read_when_searching(topic_negated_dm_operators);
+    assert_not_mark_read_when_searching(topic_negated_dm_operators);
+
     const dm = [{operator: "dm", operand: "joe@example.com,"}];
 
     const dm_negated = [{operator: "dm", operand: "joe@example.com,", negated: true}];
@@ -469,6 +554,10 @@ test("can_mark_messages_read", () => {
     assert_not_mark_read_with_has_operands(is_dm);
     assert_not_mark_read_when_searching(is_dm);
 
+    const not_is_dm = [{operator: "is", operand: "dm", negated: true}];
+    filter = new Filter(not_is_dm);
+    assert.ok(filter.can_mark_messages_read());
+
     const in_all = [{operator: "in", operand: "all"}];
     filter = new Filter(in_all);
     assert.ok(filter.can_mark_messages_read());