paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-10-30 19:43:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

message_edit: Add support for empty topic name.

Browse Source 
This commit is a part of the work to support empty string
as a topic name.

Previously, empty string was not a valid topic name.

Now, message edit operation supports empty topic name.

Adds backward compatibility for:
- `topic` field in the `delete_message` event type
- `orig_subject` and `subject` fields in the `update_message`
   event type
This commit is contained in:
Prakhar Pratyush
2024-11-13 12:38:49 +05:30
committed by Tim Abbott
parent 5d9beb3655
commit 2dea392d9e
5 changed files with 130 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
api_docs/changelog.md
View File

@@ -39,6 +39,8 @@ format used by the Zulip server that they are interacting with.
the following fields will have the value of `realm_empty_topic_display_name`
field replacing the empty string for channel messages:
* `subject` field in the `message` event type
* `topic` field in the `delete_message` event type
* `orig_subject` and `subject` fields in the `update_message` event type
* [`GET /messages`](/api/get-messages),
[`GET /messages/{message_id}`](/api/get-message): Added `allow_empty_topic_name`

2
zerver/actions/message_edit.py
View File

@@ -60,6 +60,7 @@ from zerver.lib.topic import (
RESOLVED_TOPIC_PREFIX,
TOPIC_LINKS,
TOPIC_NAME,
maybe_rename_general_chat_to_empty_topic,
messages_for_topic,
participants_for_topic,
save_message_for_edit_use_case,
@@ -1278,6 +1279,7 @@ def check_update_message(
# use OptionalTopic as well (or otherwise are guaranteed to strip input).
if topic_name is not None:
topic_name = topic_name.strip()
topic_name = maybe_rename_general_chat_to_empty_topic(topic_name)
if topic_name == message.topic_name():
topic_name = None

44
zerver/openapi/zulip.yaml
View File

@@ -2119,6 +2119,17 @@ paths:
Only present if `message_type` is `"stream"`.
The topic to which the message was sent.
For clients that don't support the `empty_topic_name` [client capability][client-capabilities],
if the actual topic name was empty string, this field's value will instead
be the value of `realm_empty_topic_display_name` found in the
[`POST /register`](/api/register-queue) response.
**Changes**: Before 10.0 (feature level 334), `empty_topic_name`
client capability didn't exist and empty string as the topic name for
channel messages wasn't allowed.
[client-capabilities]: /api/register-queue#parameter-client_capabilities
example:
{
"type": "delete_message",
@@ -2515,6 +2526,17 @@ paths:
The pre-edit topic for all of the messages with IDs in
`message_ids`.
For clients that don't support the `empty_topic_name` [client capability][client-capabilities],
if the actual pre-edit topic name is empty string, this field's value will instead
be the value of `realm_empty_topic_display_name` found in the
[`POST /register`](/api/register-queue) response.
**Changes**: Before 10.0 (feature level 334), `empty_topic_name`
client capability didn't exist and empty string as the topic name for
channel messages wasn't allowed.
[client-capabilities]: /api/register-queue#parameter-client_capabilities
subject:
type: string
description: |
@@ -2524,6 +2546,17 @@ paths:
The post-edit topic for all of the messages with IDs in
`message_ids`.
For clients that don't support the `empty_topic_name` [client capability][client-capabilities],
if the actual post-edit topic name is empty string, this field's value will instead
be the value of `realm_empty_topic_display_name` found in the
[`POST /register`](/api/register-queue) response.
**Changes**: Before 10.0 (feature level 334), `empty_topic_name`
client capability didn't exist and empty string as the topic name for
channel messages wasn't allowed.
[client-capabilities]: /api/register-queue#parameter-client_capabilities
topic_links:
type: array
items:
@@ -8409,8 +8442,15 @@ paths:
Should only be sent when changing the topic, and will throw an error
if the target message is not a channel message.
**Changes**: New in Zulip 2.0.0. Previous Zulip releases encoded
this as `subject`, which is currently a deprecated alias.
Note: When the value of `realm_empty_topic_display_name` found in
the [POST /register](/api/register-queue) response is used for this
parameter, it is interpreted as an empty string.
**Changes**: Before Zulip 10.0 (feature level 334), empty string
was not a valid topic name for channel messages.
New in Zulip 2.0.0. Previous Zulip releases encoded this as `subject`,
which is currently a deprecated alias.
type: string
example: Castle
propagate_mode:

70
zerver/tests/test_message_topics.py
View File

@@ -376,7 +376,7 @@ class EmptyTopicNameTest(ZulipTestCase):
apply_markdown=True,
client_type_name="website",
empty_topic_name=True,
event_types=["message"],
event_types=["message", "update_message", "delete_message"],
last_connection_time=time.time(),
queue_timeout=600,
realm_id=hamlet.realm.id,
@@ -385,14 +385,43 @@ class EmptyTopicNameTest(ZulipTestCase):
client = allocate_client_descriptor(queue_data)
self.assertTrue(client.event_queue.empty())
self.send_stream_message(iago, "Denmark", topic_name="")
message_id = self.send_stream_message(iago, "Denmark", topic_name="")
events = client.event_queue.contents()
self.assertEqual(events[0]["message"]["subject"], "")
self.send_stream_message(iago, "Denmark", topic_name=Message.EMPTY_TOPIC_FALLBACK_NAME)
message_id_2 = self.send_stream_message(
iago, "Denmark", topic_name=Message.EMPTY_TOPIC_FALLBACK_NAME
)
events = client.event_queue.contents()
self.assertEqual(events[1]["message"]["subject"], "")
self.login_user(iago)
with self.captureOnCommitCallbacks(execute=True):
params = {"topic": "new topic name", "send_notification_to_new_thread": "false"}
self.client_patch(f"/json/messages/{message_id}", params)
self.client_patch(f"/json/messages/{message_id_2}", params)
events = client.event_queue.contents()
self.assertEqual(events[2]["orig_subject"], "")
self.assertEqual(events[3]["orig_subject"], "")
# reset
message_id = self.send_stream_message(
iago, "Denmark", topic_name="", skip_capture_on_commit_callbacks=True
)
message_id_2 = self.send_stream_message(
iago,
"Verona",
topic_name=Message.EMPTY_TOPIC_FALLBACK_NAME,
skip_capture_on_commit_callbacks=True,
)
with self.captureOnCommitCallbacks(execute=True):
self.client_delete(f"/json/messages/{message_id}")
self.client_delete(f"/json/messages/{message_id_2}")
events = client.event_queue.contents()
self.assertEqual(events[4]["topic"], "")
self.assertEqual(events[5]["topic"], "")
def test_client_not_supports_empty_topic_name(self) -> None:
iago = self.example_user("iago")
hamlet = self.example_user("hamlet")
@@ -401,7 +430,7 @@ class EmptyTopicNameTest(ZulipTestCase):
apply_markdown=True,
client_type_name="zulip-mobile",
empty_topic_name=False,
event_types=["message"],
event_types=["message", "update_message", "delete_message"],
last_connection_time=time.time(),
queue_timeout=600,
realm_id=hamlet.realm.id,
@@ -410,14 +439,43 @@ class EmptyTopicNameTest(ZulipTestCase):
client = allocate_client_descriptor(queue_data)
self.assertTrue(client.event_queue.empty())
self.send_stream_message(iago, "Denmark", topic_name="")
message_id = self.send_stream_message(iago, "Denmark", topic_name="")
events = client.event_queue.contents()
self.assertEqual(events[0]["message"]["subject"], Message.EMPTY_TOPIC_FALLBACK_NAME)
self.send_stream_message(iago, "Denmark", topic_name=Message.EMPTY_TOPIC_FALLBACK_NAME)
message_id_2 = self.send_stream_message(
iago, "Denmark", topic_name=Message.EMPTY_TOPIC_FALLBACK_NAME
)
events = client.event_queue.contents()
self.assertEqual(events[1]["message"]["subject"], Message.EMPTY_TOPIC_FALLBACK_NAME)
self.login_user(iago)
with self.captureOnCommitCallbacks(execute=True):
params = {"topic": "new topic name", "send_notification_to_new_thread": "false"}
self.client_patch(f"/json/messages/{message_id}", params)
self.client_patch(f"/json/messages/{message_id_2}", params)
events = client.event_queue.contents()
self.assertEqual(events[2]["orig_subject"], Message.EMPTY_TOPIC_FALLBACK_NAME)
self.assertEqual(events[3]["orig_subject"], Message.EMPTY_TOPIC_FALLBACK_NAME)
# reset
message_id = self.send_stream_message(
iago, "Denmark", topic_name="", skip_capture_on_commit_callbacks=True
)
message_id_2 = self.send_stream_message(
iago,
"Verona",
topic_name=Message.EMPTY_TOPIC_FALLBACK_NAME,
skip_capture_on_commit_callbacks=True,
)
with self.captureOnCommitCallbacks(execute=True):
self.client_delete(f"/json/messages/{message_id}")
self.client_delete(f"/json/messages/{message_id_2}")
events = client.event_queue.contents()
self.assertEqual(events[4]["topic"], Message.EMPTY_TOPIC_FALLBACK_NAME)
self.assertEqual(events[5]["topic"], Message.EMPTY_TOPIC_FALLBACK_NAME)
def test_fetch_messages(self) -> None:
hamlet = self.example_user("hamlet")
self.login_user(hamlet)

28
zerver/tornado/event_queue.py
View File

@@ -28,8 +28,9 @@ from zerver.lib.narrow_helpers import narrow_dataclasses_from_tuples
from zerver.lib.narrow_predicate import build_narrow_predicate
from zerver.lib.notification_data import UserMessageNotificationsData
from zerver.lib.queue import queue_json_publish_rollback_unsafe, retry_event
from zerver.lib.topic import ORIG_TOPIC, TOPIC_NAME
from zerver.middleware import async_request_timer_restart
from zerver.models import CustomProfileField
from zerver.models import CustomProfileField, Message
from zerver.tornado.descriptors import clear_descriptor_by_handler_id, set_descriptor_by_handler_id
from zerver.tornado.exceptions import BadEventQueueIdError
from zerver.tornado.handlers import finish_handler, get_handler_by_id, handler_stats_string
@@ -1297,19 +1298,22 @@ def process_deletion_event(event: Mapping[str, Any], users: Iterable[int]) -> No
if not client.accepts_event(event):
continue
deletion_event = event
if deletion_event.get("topic") == "" and not client.empty_topic_name:
deletion_event = dict(event)
deletion_event["topic"] = Message.EMPTY_TOPIC_FALLBACK_NAME
# For clients which support message deletion in bulk, we
# send a list of msgs_ids together, otherwise we send a
# delete event for each message. All clients will be
# required to support bulk_message_deletion in the future;
# this logic is intended for backwards-compatibility only.
if client.bulk_message_deletion:
client.add_event(event)
client.add_event(deletion_event)
continue
for message_id in event["message_ids"]:
# We use the following rather than event.copy()
# because the read-only Mapping type doesn't support .copy().
compatibility_event = dict(event)
for message_id in deletion_event["message_ids"]:
compatibility_event = dict(deletion_event)
compatibility_event["message_id"] = message_id
del compatibility_event["message_ids"]
client.add_event(compatibility_event)
@@ -1452,10 +1456,18 @@ def process_message_update_event(
)
for client in get_client_descriptors_for_user(user_profile_id):
if client.accepts_event(user_event):
user_event_copy = user_event.copy()
if not client.empty_topic_name:
if user_event_copy.get(ORIG_TOPIC) == "":
user_event_copy[ORIG_TOPIC] = Message.EMPTY_TOPIC_FALLBACK_NAME
if user_event_copy.get(TOPIC_NAME) == "":
user_event_copy[TOPIC_NAME] = Message.EMPTY_TOPIC_FALLBACK_NAME
if client.accepts_event(user_event_copy):
# We need to do another shallow copy, or we risk
# sending the same event to multiple clients.
client.add_event(user_event)
client.add_event(user_event_copy)
def process_custom_profile_fields_event(event: Mapping[str, Any], users: Iterable[int]) -> None: