Repository: zulip/zulip
Pull request: message-type: Add support for "direct" as value for `type` parameter.
For endpoints with a `type` parameter to indicate whether the message is a stream or direct  message, [`POST /typing`](https://zulip.com/api/set-typing-status) and [`POST /messages`](https://zulip.com/api/send-message), adds support for passing `"direct"` as a value for direct messages.

Maintains support for `"private"` as a deprecated alias for direct messages.

Fixes https://github.com/zulip/zulip/issues/24960.

---

**Notes**:
- For the **Usage examples** in the `POST /messages` documentation:
  - Comments now use 'direct messages' or 'DMs'
  - Updated the curl and javascript examples
  - Did not update the python example for "direct". I wasn't sure if we wanted to update that in tandem with an update to zulip-python-api; see question below.
- I added the validation check for the `type` parameter that was in `POST /typing` to also be in `POST /messages`.
- I did the refactor in the prep commit because any follow-up changes we do can be a bit clearer in that all instances of `message_type_name` now refer to these parameters passed to the view functions from clients. See `git-grep` below.
  - for `POST /messages`, we use "private" for the `message_type` string that's then used for the various function calls.
  - for `POST /typing`, we use "direct" for the default and updated `message_type` string.
- Updates these two API endpoint documentation pages to use "direct messages" or "DMs" and also added links to relevant help center documentation.

<details>
<summary>git-grep of `message_type_name` after changes</summary>

```console
$ git grep message_type_name
zerver/tests/test_message_send.py:        message_type_name = ["direct", "private"]
zerver/tests/test_message_send.py:        for message_type in message_type_name:
zerver/tests/test_message_send.py:        message_type_name = ["direct", "private"]
zerver/tests/test_message_send.py:        for message_type in message_type_name:
zerver/tests/test_typing.py:        message_type_name = ["direct", "private"]
zerver/tests/test_typing.py:        for message_type in message_type_name:
zerver/views/message_send.py:    message_type_name: str = REQ("type", str_validator=check_string_in(VALID_MESSAGE_TYPES)),
zerver/views/message_send.py:    message_type = message_type_name
zerver/views/typing.py:    message_type_name: str = REQ(
zerver/views/typing.py:    message_type = message_type_name

```
</details>

**Questions**:
- In the **Changes** notes, I stated that "private" would be removed in a future release, but I wasn't sure if that's the case?
- I added some TODO notes where `message_type` is set in both view functions, but these might not be something we plan on doing?
- Again, as noted above, we'll likely want to update the [zulip-python-api](https://github.com/zulip/python-zulip-api/blob/02586f1d348dbcab3bc8d30b805fffd48a524e18/zulip/zulip/send.py#L89-L101)?

---

**Screenshots and screen captures:**

<details>
<summary>API changelog update</summary>

![Screenshot from 2023-04-17 16-48-08](https://user-images.githubusercontent.com/63245456/232539073-ca9114d8-f9aa-471e-bd37-2e6ec6c4e8b7.png)
</details>

<details>
<summary>Send a message</summary>

[Current documentation](https://zulip.com/api/send-message)
<details>
<summary>Main endpoint description and usage examples</summary>

![Screenshot from 2023-04-17 16-50-21](https://user-images.githubusercontent.com/63245456/232538394-d2ed3339-3b7c-4f68-89dd-f5398c1f22d7.png)
![Screenshot from 2023-04-17 16-50-32](https://user-images.githubusercontent.com/63245456/232538398-492451a4-471a-4605-bc02-29875decb79a.png)
![Screenshot from 2023-04-17 16-50-40](https://user-images.githubusercontent.com/63245456/232538401-ba956f7a-9b0f-4f5c-b672-a306edb91c6b.png)
![Screenshot from 2023-04-17 16-50-47](https://user-images.githubusercontent.com/63245456/232538415-a9785037-5a4a-4249-8b1d-ef82f095fe85.png)
</details>
<details>
<summary>Parameters and response</summary>

![Screenshot from 2023-04-17 16-50-57](https://user-images.githubusercontent.com/63245456/232538419-25edea0c-d051-4b30-ac9e-ae255836535b.png)
![Screenshot from 2023-04-17 16-51-06](https://user-images.githubusercontent.com/63245456/232538421-708e15bf-1344-4966-a219-9adb0c162fc7.png)
</details>
</details>
<details>
<summary>Set typing status</summary>

[Current documentation](https://zulip.com/api/set-typing-status)
<details>
<summary>Main endpoint description</summary>

![Screenshot from 2023-04-17 16-49-12](https://user-images.githubusercontent.com/63245456/232540431-eb421fa7-c1de-4853-a0b8-f769eee22530.png)
</details>
<details>
<summary>Usage examples</summary>

![Screenshot from 2023-04-17 16-49-30](https://user-images.githubusercontent.com/63245456/232540157-7b0b9d8f-2e16-45dd-b0db-d07446fda0e7.png)
![Screenshot from 2023-04-17 16-49-36](https://user-images.githubusercontent.com/63245456/232540163-f4c557e4-1224-4d22-a2d8-71c265527a3c.png)
![Screenshot from 2023-04-17 16-49-44](https://user-images.githubusercontent.com/63245456/232540167-bd604c45-5ee6-491b-a909-12d114a8499a.png)
</details>
<details>
<summary>Parameters and response</summary>

![Screenshot from 2023-04-17 16-49-54](https://user-images.githubusercontent.com/63245456/232540171-d6a88467-b198-4309-932b-1bf93f9b60ed.png)
![Screenshot from 2023-04-17 16-50-02](https://user-images.githubusercontent.com/63245456/232540176-206d9437-f35a-4a75-b07a-7a9f67b2409d.png)
</details>
</details>
---

<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.-->

- [ ] [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.

- [ ] Explains differences from previous plans (e.g., issue description).
- [ ] Highlights technical choices and bugs encountered.
- [ ] Calls out remaining decisions and concerns.
- [ ] 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)).

- [ ] Each commit is a coherent idea.
- [ ] Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

- [ ] Visual appearance of the changes.
- [ ] Responsiveness and internationalization.
- [ ] Strings and tooltips.
- [ ] End-to-end functionality of buttons, interactions and flows.
- [ ] Corner cases, error conditions, and easily imagined bugs.
</details>


Problem statement:
Add API support for using "direct" as the message "type" for private messages
Since we renamed "private messages" to "direct messages" in the UI, we should start thinking about API changes in line with this change. This issue is for the simplest one, which I think should be a Zulip 7.0 release goal: Allowing `send_message` (and similar things like typing notifications requests) to use `direct` rather than `private` for the `type` parameter. 

`message_type_name: str = REQ("type")`

@laurynmm can I assign this to you? `git grep private zerver/views/` suggests it may just be those two endpoints that need attention here.

There's a significantly more difficult project of supporting a `direct` message type in events, which likely requires a client capability; the goal for this issue is just do the (mostly documentation, I think?) part of allowing things that send messages via the API to use the new name.

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

<!-- areaLabelAddition -->

@timabbott - Yeah, assigning to me makes sense. I already noted this down as a follow-up in the work I'm doing on #24806.
@laurynmm We noticed that you have not made any updates to this issue or linked PRs for 10 days. Please comment here if you are still actively working on it. Otherwise, we'd appreciate a quick `@zulipbot abandon` comment so that someone else can claim this issue and continue from where you left off.

If we don't hear back, you will be automatically unassigned in 4 days. Thanks!

<!-- inactiveWarning -->

Commit message: message: Use `recipient_type_name` for API message type references.

Refactors instances of `message_type_name` and `message_type`
that are referring to API message type value ("stream" or
"private") to use `recipient_type_name` instead.

Prep commit for adding "direct" as a value for endpoints with a
`type` parameter to indicate whether the message is a stream or
direct  message.

Patch to review:
diff --git a/zerver/actions/message_send.py b/zerver/actions/message_send.py
index 05e53e053e073..a5c316b3aab91 100644
--- a/zerver/actions/message_send.py
+++ b/zerver/actions/message_send.py
@@ -1154,7 +1154,7 @@ def check_send_private_message(
 def check_send_message(
     sender: UserProfile,
     client: Client,
-    message_type_name: str,
+    recipient_type_name: str,
     message_to: Union[Sequence[int], Sequence[str]],
     topic_name: Optional[str],
     message_content: str,
@@ -1168,7 +1168,7 @@ def check_send_message(
     *,
     skip_stream_access_check: bool = False,
 ) -> int:
-    addressee = Addressee.legacy_build(sender, message_type_name, message_to, topic_name)
+    addressee = Addressee.legacy_build(sender, recipient_type_name, message_to, topic_name)
     try:
         message = check_message(
             sender,
@@ -1192,7 +1192,7 @@ def check_send_message(
 def check_schedule_message(
     sender: UserProfile,
     client: Client,
-    message_type_name: str,
+    recipient_type_name: str,
     message_to: Union[Sequence[str], Sequence[int]],
     topic_name: Optional[str],
     message_content: str,
@@ -1202,7 +1202,7 @@ def check_schedule_message(
     realm: Optional[Realm] = None,
     forwarder_user_profile: Optional[UserProfile] = None,
 ) -> int:
-    addressee = Addressee.legacy_build(sender, message_type_name, message_to, topic_name)
+    addressee = Addressee.legacy_build(sender, recipient_type_name, message_to, topic_name)
 
     send_request = check_message(
         sender,
diff --git a/zerver/lib/addressee.py b/zerver/lib/addressee.py
index ada059d196a61..a5a172407c0ee 100644
--- a/zerver/lib/addressee.py
+++ b/zerver/lib/addressee.py
@@ -97,7 +97,7 @@ def topic(self) -> str:
     @staticmethod
     def legacy_build(
         sender: UserProfile,
-        message_type_name: str,
+        recipient_type_name: str,
         message_to: Union[Sequence[int], Sequence[str]],
         topic_name: Optional[str],
         realm: Optional[Realm] = None,
@@ -108,7 +108,7 @@ def legacy_build(
         if realm is None:
             realm = sender.realm
 
-        if message_type_name == "stream":
+        if recipient_type_name == "stream":
             if len(message_to) > 1:
                 raise JsonableError(_("Cannot send to multiple streams"))
 
@@ -131,7 +131,7 @@ def legacy_build(
                 return Addressee.for_stream_id(stream_name_or_id, topic_name)
 
             return Addressee.for_stream_name(stream_name_or_id, topic_name)
-        elif message_type_name == "private":
+        elif recipient_type_name == "private":
             if not message_to:
                 raise JsonableError(_("Message must have recipients"))
 
diff --git a/zerver/lib/email_mirror.py b/zerver/lib/email_mirror.py
index 9778088dddbf0..71c2626100014 100644
--- a/zerver/lib/email_mirror.py
+++ b/zerver/lib/email_mirror.py
@@ -198,7 +198,7 @@ def send_mm_reply_to_stream(
         check_send_message(
             sender=user_profile,
             client=get_client("Internal"),
-            message_type_name="stream",
+            recipient_type_name="stream",
             message_to=[stream.id],
             topic_name=topic,
             message_content=body,
diff --git a/zerver/lib/message.py b/zerver/lib/message.py
index e9e2a75c75362..f7bc7b521a8be 100644
--- a/zerver/lib/message.py
+++ b/zerver/lib/message.py
@@ -1273,17 +1273,17 @@ def apply_unread_message_event(
 ) -> None:
     message_id = message["id"]
     if message["type"] == "stream":
-        message_type = "stream"
+        recipient_type = "stream"
     elif message["type"] == "private":
         others = [recip for recip in message["display_recipient"] if recip["id"] != user_profile.id]
         if len(others) <= 1:
-            message_type = "private"
+            recipient_type = "private"
         else:
-            message_type = "huddle"
+            recipient_type = "huddle"
     else:
         raise AssertionError("Invalid message type {}".format(message["type"]))
 
-    if message_type == "stream":
+    if recipient_type == "stream":
         stream_id = message["stream_id"]
         topic = message[TOPIC_NAME]
         state["stream_dict"][message_id] = RawUnreadStreamDict(
@@ -1300,7 +1300,7 @@ def apply_unread_message_event(
         ):
             state["unmuted_stream_msgs"].add(message_id)
 
-    elif message_type == "private":
+    elif recipient_type == "private":
         if len(others) == 1:
             other_user_id = others[0]["id"]
         else:
diff --git a/zerver/lib/outgoing_webhook.py b/zerver/lib/outgoing_webhook.py
index d866ce64268d4..3a7fa17588210 100644
--- a/zerver/lib/outgoing_webhook.py
+++ b/zerver/lib/outgoing_webhook.py
@@ -191,7 +191,7 @@ def send_response_message(
     that might let someone send arbitrary messages to any stream through this.
     """
 
-    message_type = message_info["type"]
+    recipient_type_name = message_info["type"]
     display_recipient = message_info["display_recipient"]
     try:
         topic_name: Optional[str] = get_topic_from_message_info(message_info)
@@ -207,9 +207,9 @@ def send_response_message(
 
     widget_content = response_data.get("widget_content")
 
-    if message_type == "stream":
+    if recipient_type_name == "stream":
         message_to = [display_recipient]
-    elif message_type == "private":
+    elif recipient_type_name == "private":
         message_to = [recipient["email"] for recipient in display_recipient]
     else:
         raise JsonableError(_("Invalid message type"))
@@ -217,7 +217,7 @@ def send_response_message(
     check_send_message(
         sender=bot_user,
         client=client,
-        message_type_name=message_type,
+        recipient_type_name=recipient_type_name,
         message_to=message_to,
         topic_name=topic_name,
         message_content=content,
diff --git a/zerver/tests/test_event_system.py b/zerver/tests/test_event_system.py
index 0bb597a206c17..70b02ea9645d1 100644
--- a/zerver/tests/test_event_system.py
+++ b/zerver/tests/test_event_system.py
@@ -324,7 +324,7 @@ def test_get_events(self) -> None:
         check_send_message(
             sender=user_profile,
             client=get_client("whatever"),
-            message_type_name="private",
+            recipient_type_name="private",
             message_to=[recipient_email],
             topic_name=None,
             message_content="hello",
@@ -357,7 +357,7 @@ def test_get_events(self) -> None:
         check_send_message(
             sender=user_profile,
             client=get_client("whatever"),
-            message_type_name="private",
+            recipient_type_name="private",
             message_to=[recipient_email],
             topic_name=None,
             message_content="hello",
diff --git a/zerver/tests/test_message_send.py b/zerver/tests/test_message_send.py
index 6aee993dc8fd3..78810d2e0b017 100644
--- a/zerver/tests/test_message_send.py
+++ b/zerver/tests/test_message_send.py
@@ -2557,7 +2557,7 @@ def test_addressee_legacy_build_for_user_ids(self) -> None:
 
         result = Addressee.legacy_build(
             sender=self.example_user("hamlet"),
-            message_type_name="private",
+            recipient_type_name="private",
             message_to=user_ids,
             topic_name="random_topic",
             realm=realm,
@@ -2576,7 +2576,7 @@ def test_addressee_legacy_build_for_stream_id(self) -> None:
 
         result = Addressee.legacy_build(
             sender=sender,
-            message_type_name="stream",
+            recipient_type_name="stream",
             message_to=[stream.id],
             topic_name="random_topic",
             realm=realm,
diff --git a/zerver/tests/test_mirror_users.py b/zerver/tests/test_mirror_users.py
index 377256c898c2f..f9930935209b6 100644
--- a/zerver/tests/test_mirror_users.py
+++ b/zerver/tests/test_mirror_users.py
@@ -19,11 +19,13 @@ def test_invalid_client(self) -> None:
 
         recipients: List[str] = []
 
-        message_type = "private"
+        recipient_type_name = "private"
         client = get_client("banned_mirror")
 
         with self.assertRaises(InvalidMirrorInputError):
-            create_mirrored_message_users(client, user, recipients, sender.email, message_type)
+            create_mirrored_message_users(
+                client, user, recipients, sender.email, recipient_type_name
+            )
 
     def test_invalid_email(self) -> None:
         invalid_email = "alice AT example.com"
@@ -33,13 +35,15 @@ def test_invalid_email(self) -> None:
         user = self.mit_user("starnine")
         sender = user
 
-        message_type = "private"
+        recipient_type_name = "private"
 
         for client_name in ["zephyr_mirror", "irc_mirror", "jabber_mirror"]:
             client = get_client(client_name)
 
             with self.assertRaises(InvalidMirrorInputError):
-                create_mirrored_message_users(client, user, recipients, sender.email, message_type)
+                create_mirrored_message_users(
+                    client, user, recipients, sender.email, recipient_type_name
+                )
 
     @mock.patch(
         "DNS.dnslookup",
@@ -54,11 +58,11 @@ def test_zephyr_mirror_new_recipient(self, ignored: object) -> None:
 
         recipients = [user.email, new_user_email]
 
-        message_type = "private"
+        recipient_type_name = "private"
         client = get_client("zephyr_mirror")
 
         mirror_sender = create_mirrored_message_users(
-            client, user, recipients, sender.email, message_type
+            client, user, recipients, sender.email, recipient_type_name
         )
 
         self.assertEqual(mirror_sender, sender)
@@ -82,11 +86,11 @@ def test_zephyr_mirror_new_sender(self, ignored: object) -> None:
 
         recipients = ["stream_name"]
 
-        message_type = "stream"
+        recipient_type_name = "stream"
         client = get_client("zephyr_mirror")
 
         mirror_sender = create_mirrored_message_users(
-            client, user, recipients, sender_email, message_type
+            client, user, recipients, sender_email, recipient_type_name
         )
 
         assert mirror_sender is not None
@@ -105,11 +109,11 @@ def test_irc_mirror(self) -> None:
             self.nonreg_email("cordelia"),
         ]
 
-        message_type = "private"
+        recipient_type_name = "private"
         client = get_client("irc_mirror")
 
         mirror_sender = create_mirrored_message_users(
-            client, user, recipients, sender.email, message_type
+            client, user, recipients, sender.email, recipient_type_name
         )
 
         self.assertEqual(mirror_sender, sender)
@@ -134,11 +138,11 @@ def test_jabber_mirror(self) -> None:
             self.nonreg_email("cordelia"),
         ]
 
-        message_type = "private"
+        recipient_type_name = "private"
         client = get_client("jabber_mirror")
 
         mirror_sender = create_mirrored_message_users(
-            client, user, recipients, sender.email, message_type
+            client, user, recipients, sender.email, recipient_type_name
         )
 
         self.assertEqual(mirror_sender, sender)
diff --git a/zerver/tornado/event_queue.py b/zerver/tornado/event_queue.py
index d750765fbbc25..3604f07ce1dda 100644
--- a/zerver/tornado/event_queue.py
+++ b/zerver/tornado/event_queue.py
@@ -949,7 +949,7 @@ def process_message_event(
 
     sender_id: int = wide_dict["sender_id"]
     message_id: int = wide_dict["id"]
-    message_type: str = wide_dict["type"]
+    recipient_type_name: str = wide_dict["type"]
     sending_client: str = wide_dict["client"]
 
     @lru_cache(maxsize=None)
@@ -970,7 +970,7 @@ def get_client_payload(apply_markdown: bool, client_gravatar: bool) -> Dict[str,
 
         # If the recipient was offline and the message was a single or group PM to them
         # or they were @-notified potentially notify more immediately
-        private_message = message_type == "private"
+        private_message = recipient_type_name == "private"
         user_notifications_data = UserMessageNotificationsData.from_user_id_sets(
             user_id=user_profile_id,
             flags=flags,
diff --git a/zerver/views/message_send.py b/zerver/views/message_send.py
index bd402e02a9973..fabcee06018bb 100644
--- a/zerver/views/message_send.py
+++ b/zerver/views/message_send.py
@@ -51,11 +51,11 @@ def create_mirrored_message_users(
     user_profile: UserProfile,
     recipients: Iterable[str],
     sender: str,
-    message_type: str,
+    recipient_type_name: str,
 ) -> UserProfile:
     sender_email = sender.strip().lower()
     referenced_users = {sender_email}
-    if message_type == "private":
+    if recipient_type_name == "private":
         for email in recipients:
             referenced_users.add(email.lower())
 
@@ -142,7 +142,7 @@ def same_realm_jabber_user(user_profile: UserProfile, email: str) -> bool:
 def handle_deferred_message(
     sender: UserProfile,
     client: Client,
-    message_type_name: str,
+    recipient_type_name: str,
     message_to: Union[Sequence[str], Sequence[int]],
     topic_name: Optional[str],
     message_content: str,
@@ -176,7 +176,7 @@ def handle_deferred_message(
     check_schedule_message(
         sender,
         client,
-        message_type_name,
+        recipient_type_name,
         message_to,
         topic_name,
         message_content,
@@ -210,12 +210,14 @@ def send_message_backend(
     tz_guess: Optional[str] = REQ("tz_guess", default=None, documentation_pending=True),
     time: Optional[float] = REQ(default=None, converter=to_float, documentation_pending=True),
 ) -> HttpResponse:
+    recipient_type_name = message_type_name
+
     # If req_to is None, then we default to an
     # empty list of recipients.
     message_to: Union[Sequence[int], Sequence[str]] = []
 
     if req_to is not None:
-        if message_type_name == "stream":
+        if recipient_type_name == "stream":
             stream_indicator = extract_stream_indicator(req_to)
 
             # For legacy reasons check_send_message expects
@@ -259,7 +261,7 @@ def send_message_backend(
         # same-realm constraint.
         if req_sender is None:
             raise JsonableError(_("Missing sender"))
-        if message_type_name != "private" and not can_forge_sender:
+        if recipient_type_name != "private" and not can_forge_sender:
             raise JsonableError(_("User not authorized for this query"))
 
         # For now, mirroring only works with recipient emails, not for
@@ -274,7 +276,7 @@ def send_message_backend(
 
         try:
             mirror_sender = create_mirrored_message_users(
-                client, user_profile, message_to, req_sender, message_type_name
+                client, user_profile, message_to, req_sender, recipient_type_name
             )
         except InvalidMirrorInputError:
             raise JsonableError(_("Invalid mirrored message"))
@@ -294,7 +296,7 @@ def send_message_backend(
         deliver_at = handle_deferred_message(
             sender,
             client,
-            message_type_name,
+            recipient_type_name,
             message_to,
             topic_name,
             message_content,
@@ -310,7 +312,7 @@ def send_message_backend(
     ret = check_send_message(
         sender,
         client,
-        message_type_name,
+        recipient_type_name,
         message_to,
         topic_name,
         message_content,


Code review comments:
diff_hunk: @@ -210,12 +210,14 @@ def send_message_backend(
     tz_guess: Optional[str] = REQ("tz_guess", default=None, documentation_pending=True),
     time: Optional[float] = REQ(default=None, converter=to_float, documentation_pending=True),
 ) -> HttpResponse:
+    recipient_type_name = message_type_name
path: zerver/views/message_send.py
text: @user1:
This can be done a bit more cleanly be using the first positional argument to `REQ`, to not have set `message_type_name` at all.

@user1:
Ahh, but it gets replaced in the next commit anyway, yeah this is pretty reasonable as an intermediate state before we redo the internals plumbing to support `"direct"` rather than `"private"`.