Repository: zulip/zulip
Pull request: search: Add is-muted search operator. 
1st commit fixes the issues existing with `-in:home` search operator.
Previously, mentions from muted channels were incorrectly excluded when narrowed down to `-in:home`. Additionally, messages from all muted topics were missing in the results.
This commit solves the above listed issues.

2nd commit adds `-is:muted` as an alias of `in:home`.

Referenced PR: #31077.
Fixes: #16943.

<!-- If the PR makes UI changes, always include one or more still screenshots to demonstrate your changes. If it seems helpful, add a screen capture of the new functionality as well.

Tooling tips: https://zulip.readthedocs.io/en/latest/tutorials/screenshot-and-gif-software.html
-->

**Screenshots and screen captures:**

Search suggestions
![image](https://github.com/user-attachments/assets/cb134de0-2f97-419a-ada1-c5c1ed2d4282)

Help menu
![image](https://github.com/user-attachments/assets/1e34af1c-693a-488a-80e6-aa2b28d1414a)

![image](https://github.com/user-attachments/assets/907a1f56-af84-40fb-8792-c56cec9f879b)

<details>
<summary>Self-review checklist</summary>

<!-- Prior to submitting a PR, follow our step-by-step guide to review your own code:
https://zulip.readthedocs.io/en/latest/contributing/code-reviewing.html#how-to-review-code -->

<!-- Once you create the PR, check off all the steps below that you have completed.
If any of these steps are not relevant or you have not completed, leave them unchecked.-->

- [x] [Self-reviewed](https://zulip.readthedocs.io/en/latest/contributing/code-reviewing.html#how-to-review-code) the changes for clarity and maintainability
      (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

- [x] Explains differences from previous plans (e.g., issue description).
- [x] Highlights technical choices and bugs encountered.
- [ ] Calls out remaining decisions and concerns.
- [x] Automated tests verify logic where appropriate.

Individual commits are ready for review (see [commit discipline](https://zulip.readthedocs.io/en/latest/contributing/commit-discipline.html)).

- [x] Each commit is a coherent idea.
- [x] Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

- [x] Visual appearance of the changes.
- [x] Responsiveness and internationalization.
- [x] Strings and tooltips.
- [x] End-to-end functionality of buttons, interactions and flows.
- [x] Corner cases, error conditions, and easily imagined bugs.
</details>


Problem statement:
Add -is:muted as alias for in:home.
This is a relatively simple fix for somebody who's worked with our message filtering logic.  It's probably a couple hours to get stuff working and a day or two to polish.

We want to make -is:muted the new alias for in:home.

We probably want to deprecate this somewhat slowly.  I think we can add code for is:muted and move all the help/docs to the new name, but we probably might want to still support in:home for a version or so.

A lot of the work here will be about being thoughtful about documentation and testing.  Most of the actual code will be in `NarrowBuilder`, and it's basically copy/paste three lines from `by_in` to `by_is`.  And then there are similar changes on the client side.

Hints from issue discussion:
Hello @zulip/server-search members, this issue was labeled with the "area: search" label, so you may want to check it out!

<!-- areaLabelAddition -->
@zulipbot claim
Hello @tushar912, it looks like you've currently claimed 1 issue in this repository. We encourage new contributors to focus their efforts on at most 1 issue at a time, so please complete your work on your other claimed issues before trying to claim this issue again.

We look forward to your valuable contributions!
@zulipbot claim
Hello @Bhanu-mbvg, you claimed this issue to work on it, but this issue and any referenced pull requests haven't been updated for 10 days. Are you still working on this issue?

If so, please update this issue by leaving a comment on this issue to let me know that you're still working on it. Otherwise, I'll automatically remove you from this issue in 4 days.

If you've decided to work on something else, simply comment `@zulipbot abandon` so that someone else can claim it and continue from where you left off.

Thank you for your valuable contributions to Zulip!

<!-- inactiveWarning -->
**ERROR:** This active issue has no assignee.
@Bhanu-mbvg are you working on it.  If not can I take it up.
@tushar912 I assigned it to you for now.  If @Bhanu-mbvg responds, you two can coordinate as needed.
@zulipbot claim
Welcome to Zulip, @ahmedabuamra! We just sent you an invite to collaborate on this repository at https://github.com/zulip/zulip/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip!

Here's some tips to get you off to a good start:
* Join me on the [Zulip developers' server](https://chat.zulip.org), to get help, chat about this issue, and meet the other developers.
* [Unwatch this repository](https://help.github.com/articles/unwatching-repositories/), so that you don't get 100 emails a day.

As you work on this issue, you'll also want to refer to the [Zulip code contribution guide](https://zulip.readthedocs.io/en/latest/contributing/index.html), as well as the rest of the developer documentation on that site.

See you on the other side (that is, the pull request side)!

Commit message: search: Fix `-in:home` muting logic.

Previously, mentions from muted channels were incorrectly excluded when
narrowing down to `-in:home`. Additionally, messages from all muted
topics were missing in the results.
This commit solves the above listed issues.

The relevant legacy logic being removed dates to when we didn't
support unmuting or following topics.

Patch to review:
diff --git a/web/src/filter.ts b/web/src/filter.ts
index 4361a680ffbe3..7180106c5d5f7 100644
--- a/web/src/filter.ts
+++ b/web/src/filter.ts
@@ -120,16 +120,15 @@ function zephyr_topic_name_match(message: Message & {type: "stream"}, operand: s
 
 function message_in_home(message: Message): boolean {
     // The home view contains messages not sent to muted channels,
-    // with additional logic for unmuted topics, mentions, and
+    // with additional logic for unmuted topics and
     // single-channel windows.
     if (message.type === "private") {
         return true;
     }
     const stream_name = stream_data.get_stream_name_from_id(message.stream_id);
     if (
-        message.mentioned ||
-        (page_params.narrow_stream !== undefined &&
-            stream_name.toLowerCase() === page_params.narrow_stream.toLowerCase())
+        page_params.narrow_stream !== undefined &&
+        stream_name.toLowerCase() === page_params.narrow_stream.toLowerCase()
     ) {
         return true;
     }
@@ -1763,7 +1762,9 @@ export class Filter {
             // not narrowed to dms
             !(this.has_operator("dm") || this.has_operand("is", "dm")) &&
             // not narrowed to starred messages
-            !this.has_operand("is", "starred")
+            !this.has_operand("is", "starred") &&
+            // not narrowed to negated home messages
+            !this.has_negated_operand("in", "home")
         );
     }
 
diff --git a/web/tests/filter.test.cjs b/web/tests/filter.test.cjs
index bb8890dfd4cef..39b0996d28005 100644
--- a/web/tests/filter.test.cjs
+++ b/web/tests/filter.test.cjs
@@ -2699,6 +2699,26 @@ run_test("is_in_home", () => {
     assert.ok(!filter3.is_in_home());
 });
 
+run_test("excludes_muted_topics", () => {
+    let filter = new Filter([{operator: "is", operand: "starred"}]);
+    assert.ok(!filter.excludes_muted_topics());
+
+    filter = new Filter([{operator: "in", operand: "home", negated: true}]);
+    assert.ok(!filter.excludes_muted_topics());
+
+    filter = new Filter([{operator: "search", operand: "pizza"}]);
+    assert.ok(!filter.excludes_muted_topics());
+
+    filter = new Filter([
+        {operator: "channel", operand: foo_stream_id.toString()},
+        {operator: "topic", operand: "bar"},
+    ]);
+    assert.ok(!filter.excludes_muted_topics());
+
+    filter = new Filter([{operator: "is", operand: "dm"}]);
+    assert.ok(!filter.excludes_muted_topics());
+});
+
 run_test("equals", () => {
     let terms = [{operator: "channel", operand: foo_stream_id.toString()}];
     let filter = new Filter(terms);


Code review comments:
diff_hunk: @@ -432,6 +432,14 @@ def by_is(self, query: Select, operand: str, maybe_negate: ConditionTransform) -
         elif operand == "followed":
             cond = get_followed_topic_condition_sa(self.user_profile.id)
             return query.where(maybe_negate(cond))
+        elif operand == "muted":
+            conditions = exclude_muting_conditions(
+                self.user_profile, [NarrowParameter(operator="is", operand="muted")]
+            )
+            if conditions:
+                return query.where(maybe_negate(not_(and_(*conditions))))
+            return query  # nocoverage
path: zerver/lib/narrow.py
text: @user1:
The double negate feels a bit crude for `-in:home`, but should work. 

@user1:
Adding a test revealed that this logic is wrong if the user hasn't muted anything.

``` diff
diff --git a/zerver/lib/narrow.py b/zerver/lib/narrow.py
index 32f7b95fb8..6d40db1e3b 100644
--- a/zerver/lib/narrow.py
+++ b/zerver/lib/narrow.py
@@ -433,12 +433,17 @@ class NarrowBuilder:
             cond = get_followed_topic_condition_sa(self.user_profile.id)
             return query.where(maybe_negate(cond))
         elif operand == "muted":
+            # TODO: If we also have a channel operator, this could be
+            # a lot more efficient if limited to only those muting
+            # rules that appear in such channels.
             conditions = exclude_muting_conditions(
                 self.user_profile, [NarrowParameter(operator="is", operand="muted")]
             )
             if conditions:
                 return query.where(maybe_negate(not_(and_(*conditions))))
-            return query  # nocoverage
+
+            # This is the case where no channels or topics were muted.
+            return query.where(maybe_negate(false()))
 
         raise BadNarrowOperatorError("unknown 'is' operand " + operand)
 
```

diff_hunk: @@ -287,6 +287,11 @@ export function pick_empty_narrow_banner(current_filter: Filter): NarrowBannerDa
                     return {
                         title: $t({defaultMessage: "You aren't following any topics."}),
                     };
+
+                case "muted":
+                    return {
+                        title: $t({defaultMessage: "You have no muted topics."}),
+                    };
path: web/src/narrow_banner.ts
text: @user1:
This isn't quite correct -- it's that you don't have any messages in muted locations.

@user1:
``` diff
diff --git a/web/src/narrow_banner.ts b/web/src/narrow_banner.ts
index adfc895ff9..213f57be81 100644
--- a/web/src/narrow_banner.ts
+++ b/web/src/narrow_banner.ts
@@ -290,7 +290,7 @@ export function pick_empty_narrow_banner(current_filter: Filter): NarrowBannerDa
 
                 case "muted":
                     return {
-                        title: $t({defaultMessage: "You have no muted topics."}),
+                        title: $t({defaultMessage: "You have no messages in muted topics and channels."}),
                     };
             }
             // fallthrough to default case if no match is found
diff --git a/web/tests/message_view.test.cjs b/web/tests/message_view.test.cjs
index fb2f07e516..c604ebb13c 100644
--- a/web/tests/message_view.test.cjs
+++ b/web/tests/message_view.test.cjs
@@ -374,7 +374,7 @@ run_test("show_empty_narrow_message", ({mock_template, override}) => {
     narrow_banner.show_empty_narrow_message(current_filter);
     assert.equal(
         $(".empty_feed_notice_main").html(),
-        empty_narrow_html("translated: You have no muted topics."),
+        empty_narrow_html("translated: You have no messages in muted topics and channels."),
     );
     // organization has disabled sending direct messages
     override(realm, "realm_direct_message_permission_group", nobody.id);
```

diff_hunk: @@ -293,6 +293,16 @@ def test_add_term_using_is_operator_for_negated_followed_topics(self) -> None:
             "NOT (EXISTS (SELECT 1 \nFROM zerver_usertopic \nWHERE zerver_usertopic.user_profile_id = %(param_1)s AND zerver_usertopic.visibility_policy = %(param_2)s AND upper(zerver_usertopic.topic_name) = upper(zerver_message.subject) AND zerver_usertopic.recipient_id = zerver_message.recipient_id))",
         )
 
+    def test_add_term_using_is_operator_for_muted_topics(self) -> None:
+        mute_channel(self.realm, self.user_profile, "Verona")
+        term = NarrowParameter(operator="is", operand="muted", negated=False)
+        self._do_add_term_test(term, "WHERE recipient_id IN (__[POSTCOMPILE_recipient_id_1])")
+
+    def test_add_term_using_is_operator_for_negated_muted_topics(self) -> None:
+        mute_channel(self.realm, self.user_profile, "Verona")
+        term = NarrowParameter(operator="is", operand="muted", negated=True)
+        self._do_add_term_test(term, "WHERE (recipient_id NOT IN (__[POSTCOMPILE_recipient_id_1]))")
path: zerver/tests/test_message_fetch.py
text: @user1:
These tests don't verify the operator actually works, just the SQL generated. 

@user1:
Added a proper test. @author one shouldn't write any server code without making sure you have a test that verifies the logic correctly -- not just these shallow SQL query type tests. 

Proof in point is the fact that adding this test revealed the operator did not work at all.
``` diff
diff --git a/zerver/tests/test_message_fetch.py b/zerver/tests/test_message_fetch.py
index d43776db6a..6b6106afec 100644
--- a/zerver/tests/test_message_fetch.py
+++ b/zerver/tests/test_message_fetch.py
@@ -5262,6 +5262,54 @@ class MessageIsTest(ZulipTestCase):
         messages = self.assert_json_success(result)["messages"]
         self.assert_length(messages, 1)
 
+    def test_message_is_muted(self) -> None:
+        self.login("iago")
+        is_muted_narrow = orjson.dumps([dict(operator="is", operand="muted")]).decode()
+        is_unmuted_narrow = orjson.dumps(
+            [dict(operator="is", operand="muted", negated=True)]
+        ).decode()
+
+        # Have another user generate a message in a topic that isn't muted by the user.
+        msg_id = self.send_stream_message(self.example_user("hamlet"), "Denmark", topic_name="hey")
+        result = self.client_get(
+            "/json/messages",
+            dict(narrow=is_muted_narrow, anchor=msg_id, num_before=0, num_after=0),
+        )
+        messages = self.assert_json_success(result)["messages"]
+        self.assert_length(messages, 0)
+        result = self.client_get(
+            "/json/messages",
+            dict(narrow=is_unmuted_narrow, anchor=msg_id, num_before=0, num_after=0),
+        )
+        messages = self.assert_json_success(result)["messages"]
+        self.assert_length(messages, 1)
+
+        stream_id = self.get_stream_id("Denmark", self.example_user("hamlet").realm)
+
+        # Mute the topic.
+        payload = {
+            "stream_id": stream_id,
+            "topic": "hey",
+            "visibility_policy": int(UserTopic.VisibilityPolicy.MUTED),
+        }
+        self.client_post("/json/user_topics", payload)
+        result = self.client_get(
+            "/json/messages",
+            dict(narrow=is_muted_narrow, anchor=msg_id, num_before=0, num_after=0),
+        )
+        messages = self.assert_json_success(result)["messages"]
+        self.assert_length(messages, 1)
+
+        result = self.client_get(
+            "/json/messages",
+            dict(narrow=is_unmuted_narrow, anchor=msg_id, num_before=0, num_after=0),
+        )
+        messages = self.assert_json_success(result)["messages"]
+        self.assert_length(messages, 0)
+
+        # We could do more tests, but test_exclude_muting_conditions
+        # covers that code path pretty well.
+
 
 class MessageVisibilityTest(ZulipTestCase):
```