paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-10-31 03:53:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

message_send: Add support to send message with an 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.

Adds a `empty_topic_name` client capability to allow client
to specify whether it supports empty string as a topic name.

Adds backward compatibility for:
- `subject` field in the `message` event type
This commit is contained in:
Prakhar Pratyush
2024-11-12 19:16:51 +05:30
committed by Tim Abbott
parent 9f1dc08ff2
commit 3ba198e79a
18 changed files with 165 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
api_docs/changelog.md
View File

@@ -26,6 +26,20 @@ format used by the Zulip server that they are interacting with.
`realm_empty_topic_display_name` field for clients to use
while adding support for empty string as topic name.
* [`POST /register`](/api/register-queue): Added `empty_topic_name`
[client capability](/api/register-queue#parameter-client_capabilities)
to allow client to specify whether it supports empty string as a topic name
in `register` response or events involving topic names.
Clients that don't support this client capability receive
`realm_empty_topic_display_name` field value as the topic name replacing
the empty string.
* [`GET /events`](/api/get-events): For clients that don't support
the `empty_topic_name` [client capability](/api/register-queue#parameter-client_capabilities),
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
**Feature level 333**
* [Message formatting](/api/message-formatting): System groups can now

1
tools/linter_lib/custom_check.py
View File

@@ -37,6 +37,7 @@ FILES_WITH_LEGACY_SUBJECT = {
# This has lots of query data embedded, so it's hard
# to fix everything until we migrate the DB to "topic".
"zerver/tests/test_message_fetch.py",
"zerver/tests/test_message_topics.py",
}
shebang_rules: list["Rule"] = [

2
version.py
View File

@@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3"
# new level means in api_docs/changelog.md, as well as "**Changes**"
# entries in the endpoint's documentation in `zulip.yaml`.
API_FEATURE_LEVEL = 333 # Last bumped for can_send_message_group setting.
API_FEATURE_LEVEL = 334 # Last bumped for adding empty_topic_name client capability.
# Bump the minor PROVISION_VERSION to indicate that folks should provision
# only when going from an old version of the code to a newer version. Bump

2
zerver/actions/message_send.py
View File

@@ -1767,7 +1767,7 @@ def check_message(
# else can sneak past the access check.
assert sender.bot_type == sender.OUTGOING_WEBHOOK_BOT
if realm.mandatory_topics and topic_name == "(no topic)":
if realm.mandatory_topics and topic_name in ("(no topic)", ""):
raise JsonableError(_("Topics are required in this organization"))
elif addressee.is_private():

4
zerver/lib/addressee.py
View File

@@ -5,6 +5,7 @@ from django.utils.translation import gettext as _
from zerver.lib.exceptions import JsonableError
from zerver.lib.string_validation import check_stream_topic
from zerver.lib.topic import maybe_rename_general_chat_to_empty_topic
from zerver.models import Realm, Stream, UserProfile
from zerver.models.users import (
get_user_by_id_in_realm_including_cross_realm,
@@ -146,6 +147,7 @@ class Addressee:
@staticmethod
def for_stream(stream: Stream, topic_name: str) -> "Addressee":
topic_name = topic_name.strip()
topic_name = maybe_rename_general_chat_to_empty_topic(topic_name)
check_stream_topic(topic_name)
return Addressee(
msg_type="stream",
@@ -156,6 +158,7 @@ class Addressee:
@staticmethod
def for_stream_name(stream_name: str, topic_name: str) -> "Addressee":
topic_name = topic_name.strip()
topic_name = maybe_rename_general_chat_to_empty_topic(topic_name)
check_stream_topic(topic_name)
return Addressee(
msg_type="stream",
@@ -166,6 +169,7 @@ class Addressee:
@staticmethod
def for_stream_id(stream_id: int, topic_name: str) -> "Addressee":
topic_name = topic_name.strip()
topic_name = maybe_rename_general_chat_to_empty_topic(topic_name)
check_stream_topic(topic_name)
return Addressee(
msg_type="stream",

6
zerver/lib/events.py
View File

@@ -80,6 +80,7 @@ from zerver.models import (
Client,
CustomProfileField,
Draft,
Message,
NamedUserGroup,
Realm,
RealmUserDefault,
@@ -479,7 +480,7 @@ def fetch_initial_state_data(
settings.MAX_DEACTIVATED_REALM_DELETION_DAYS
)
state["realm_empty_topic_display_name"] = "testing general chat"
state["realm_empty_topic_display_name"] = Message.EMPTY_TOPIC_FALLBACK_NAME
if want("realm_user_settings_defaults"):
realm_user_default = RealmUserDefault.objects.get(realm=realm)
state["realm_user_settings_defaults"] = {}
@@ -1751,6 +1752,7 @@ class ClientCapabilities(TypedDict):
user_list_incomplete: NotRequired[bool]
include_deactivated_groups: NotRequired[bool]
archived_channels: NotRequired[bool]
empty_topic_name: NotRequired[bool]
DEFAULT_CLIENT_CAPABILITIES = ClientCapabilities(notification_settings_null=False)
@@ -1792,6 +1794,7 @@ def do_events_register(
user_list_incomplete = client_capabilities.get("user_list_incomplete", False)
include_deactivated_groups = client_capabilities.get("include_deactivated_groups", False)
archived_channels = client_capabilities.get("archived_channels", False)
empty_topic_name = client_capabilities.get("empty_topic_name", False)
if fetch_event_types is not None:
event_types_set: set[str] | None = set(fetch_event_types)
@@ -1865,6 +1868,7 @@ def do_events_register(
user_list_incomplete=user_list_incomplete,
include_deactivated_groups=include_deactivated_groups,
archived_channels=archived_channels,
empty_topic_name=empty_topic_name,
)
if queue_id is None:

1
zerver/lib/home.py
View File

@@ -157,6 +157,7 @@ def build_page_params_for_home_page_load(
user_list_incomplete=True,
include_deactivated_groups=True,
archived_channels=True,
empty_topic_name=True,
)
if user_profile is not None:

3
zerver/lib/string_validation.py
View File

@@ -56,9 +56,6 @@ def check_stream_name(stream_name: str) -> None:
def check_stream_topic(topic_name: str) -> None:
if topic_name.strip() == "":
raise JsonableError(_("Topic can't be empty!"))
invalid_character_pos = check_string_is_printable(topic_name)
if invalid_character_pos is not None:
raise JsonableError(

6
zerver/lib/topic.py
View File

@@ -321,3 +321,9 @@ def participants_for_topic(realm_id: int, recipient_id: int, topic_name: str) ->
).values_list("id", flat=True)
)
return participants
def maybe_rename_general_chat_to_empty_topic(topic_name: str) -> str:
if topic_name == Message.EMPTY_TOPIC_FALLBACK_NAME:
topic_name = ""
return topic_name

4
zerver/models/messages.py
View File

@@ -157,6 +157,10 @@ class Message(AbstractMessage):
DEFAULT_SELECT_RELATED = ["sender", "realm", "recipient", "sending_client"]
# Name to be used for the empty topic with clients that have not
# yet migrated to have the `empty_topic_name` client capability.
EMPTY_TOPIC_FALLBACK_NAME = "test general chat"
class Meta:
indexes = [
GinIndex("search_tsvector", fastupdate=False, name="zerver_message_search_tsvector"),

29
zerver/openapi/zulip.yaml
View File

@@ -7067,7 +7067,14 @@ paths:
[`POST /register`](/api/register-queue) endpoint to determine
the maximum topic length.
**Changes**: New in Zulip 2.0.0. Previous Zulip releases encoded
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
@@ -14138,6 +14145,16 @@ paths:
access archived channels, without breaking backwards-compatibility for
existing clients.
- `empty_topic_name`: Boolean for whether the client supports processing
the empty string as a topic name. Clients not declaring this capability
will be sent the value of `realm_empty_topic_display_name` found in the
[POST /register](/api/register-queue) response instead of the empty string
wherever topic names appear in the register response or events involving
topic names.
<br/>
**Changes**: New in Zulip 10.0 (feature level 334). Previously,
the empty string was not a valid topic name.
[help-linkifiers]: /help/add-a-custom-linkifier
[rfc6570]: https://www.rfc-editor.org/rfc/rfc6570.html
[events-linkifiers]: /api/get-events#realm_linkifiers
@@ -23445,6 +23462,16 @@ components:
The field name is a legacy holdover from when topics were
called "subjects" and will eventually change.
For clients that don't support the `empty_topic_name` [client capability][client-capabilities],
the empty string value is replaced with the value of `realm_empty_topic_display_name`
found in the [POST /register](/api/register-queue) response, for channel messages.
**Changes**: Before Zulip 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
submessages:
type: array
description: |

1
zerver/tests/test_event_system.py
View File

@@ -979,6 +979,7 @@ class ClientDescriptorsTest(ZulipTestCase):
self.client_gravatar = client_gravatar
self.client_type_name = "whatever"
self.events: list[dict[str, Any]] = []
self.empty_topic_name = True
def accepts_messages(self) -> bool:
return True

5
zerver/tests/test_message_move_topic.py
View File

@@ -93,7 +93,7 @@ class MessageMoveTopicTest(ZulipTestCase):
self.assert_json_error(result, "Invalid propagate_mode without topic edit")
self.check_topic(id1, topic_name="topic1")
def test_edit_message_no_topic(self) -> None:
def test_edit_message_empty_topic_with_extra_space(self) -> None:
self.login("hamlet")
msg_id = self.send_stream_message(
self.example_user("hamlet"), "Denmark", topic_name="editing", content="before edit"
@@ -104,7 +104,8 @@ class MessageMoveTopicTest(ZulipTestCase):
"topic": " ",
},
)
self.assert_json_error(result, "Topic can't be empty!")
self.assert_json_success(result)
self.check_topic(msg_id, "")
def test_edit_message_invalid_topic(self) -> None:
self.login("hamlet")

36
zerver/tests/test_message_send.py
View File

@@ -789,22 +789,6 @@ class MessagePOSTTest(ZulipTestCase):
)
self.assert_json_error(result, "Message must not be empty")
def test_empty_string_topic(self) -> None:
"""
Sending a message that has empty string topic should fail
"""
self.login("hamlet")
result = self.client_post(
"/json/messages",
{
"type": "channel",
"to": "Verona",
"content": "Test message",
"topic": "",
},
)
self.assert_json_error(result, "Topic can't be empty!")
def test_missing_topic(self) -> None:
"""
Sending a message without topic should fail
@@ -3342,3 +3326,23 @@ class CheckMessageTest(ZulipTestCase):
realm.refresh_from_db()
ret = check_message(sender, client, addressee, message_content, realm)
self.assertEqual(ret.message.sender.id, sender.id)
def test_empty_topic_message(self) -> None:
realm = get_realm("zulip")
sender = self.example_user("iago")
client = make_client(name="test suite")
stream = get_stream("Denmark", realm)
topic_name = ""
message_content = "whatever"
addressee = Addressee.for_stream(stream, topic_name)
do_set_realm_property(realm, "mandatory_topics", True, acting_user=None)
realm.refresh_from_db()
with self.assertRaisesRegex(JsonableError, "Topics are required in this organization"):
check_message(sender, client, addressee, message_content, realm)
do_set_realm_property(realm, "mandatory_topics", False, acting_user=None)
realm.refresh_from_db()
ret = check_message(sender, client, addressee, message_content, realm)
self.assertEqual(ret.message.topic_name(), topic_name)

54
zerver/tests/test_message_topics.py
View File

@@ -1,3 +1,4 @@
import time
from unittest import mock
from django.utils.timezone import now as timezone_now
@@ -9,6 +10,7 @@ from zerver.models import Message, UserMessage, UserTopic
from zerver.models.clients import get_client
from zerver.models.realms import get_realm
from zerver.models.streams import get_stream
from zerver.tornado.event_queue import allocate_client_descriptor
class TopicHistoryTest(ZulipTestCase):
@@ -362,3 +364,55 @@ class TopicDeleteTest(ZulipTestCase):
)
result_dict = self.assert_json_success(result)
self.assertFalse(result_dict["complete"])
class EmptyTopicNameTest(ZulipTestCase):
def test_client_supports_empty_topic_name(self) -> None:
iago = self.example_user("iago")
hamlet = self.example_user("hamlet")
queue_data = dict(
all_public_streams=True,
apply_markdown=True,
client_type_name="website",
empty_topic_name=True,
event_types=["message"],
last_connection_time=time.time(),
queue_timeout=600,
realm_id=hamlet.realm.id,
user_profile_id=hamlet.id,
)
client = allocate_client_descriptor(queue_data)
self.assertTrue(client.event_queue.empty())
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)
events = client.event_queue.contents()
self.assertEqual(events[1]["message"]["subject"], "")
def test_client_not_supports_empty_topic_name(self) -> None:
iago = self.example_user("iago")
hamlet = self.example_user("hamlet")
queue_data = dict(
all_public_streams=True,
apply_markdown=True,
client_type_name="zulip-mobile",
empty_topic_name=False,
event_types=["message"],
last_connection_time=time.time(),
queue_timeout=600,
realm_id=hamlet.realm.id,
user_profile_id=hamlet.id,
)
client = allocate_client_descriptor(queue_data)
self.assertTrue(client.event_queue.empty())
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)
events = client.event_queue.contents()
self.assertEqual(events[1]["message"]["subject"], Message.EMPTY_TOPIC_FALLBACK_NAME)

2
zerver/tornado/django_api.py
View File

@@ -92,6 +92,7 @@ def request_event_queue(
user_list_incomplete: bool = False,
include_deactivated_groups: bool = False,
archived_channels: bool = False,
empty_topic_name: bool = False,
) -> str | None:
if not settings.USING_TORNADO:
return None
@@ -117,6 +118,7 @@ def request_event_queue(
"user_list_incomplete": orjson.dumps(user_list_incomplete),
"include_deactivated_groups": orjson.dumps(include_deactivated_groups),
"archived_channels": orjson.dumps(archived_channels),
"empty_topic_name": orjson.dumps(empty_topic_name),
}
if event_types is not None:

16
zerver/tornado/event_queue.py
View File

@@ -29,7 +29,7 @@ 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.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
@@ -82,6 +82,7 @@ class ClientDescriptor:
user_list_incomplete: bool,
include_deactivated_groups: bool,
archived_channels: bool,
empty_topic_name: bool,
) -> None:
# TODO: We eventually want to upstream this code to the caller, but
# serialization concerns make it a bit difficult.
@@ -115,6 +116,7 @@ class ClientDescriptor:
self.user_list_incomplete = user_list_incomplete
self.include_deactivated_groups = include_deactivated_groups
self.archived_channels = archived_channels
self.empty_topic_name = empty_topic_name
# Default for lifespan_secs is DEFAULT_EVENT_QUEUE_TIMEOUT_SECS;
# but users can set it as high as MAX_QUEUE_TIMEOUT_SECS.
@@ -147,6 +149,7 @@ class ClientDescriptor:
user_list_incomplete=self.user_list_incomplete,
include_deactivated_groups=self.include_deactivated_groups,
archived_channels=self.archived_channels,
empty_topic_name=self.empty_topic_name,
)
@override
@@ -186,6 +189,7 @@ class ClientDescriptor:
user_list_incomplete=d.get("user_list_incomplete", False),
include_deactivated_groups=d.get("include_deactivated_groups", False),
archived_channels=d.get("archived_channels", False),
empty_topic_name=d.get("empty_topic_name", False),
)
ret.last_connection_time = d["last_connection_time"]
return ret
@@ -1222,6 +1226,16 @@ def process_message_event(
is_incoming_1_to_1=wide_dict["recipient_id"] == client.user_recipient_id,
)
# Compatibility code to change subject="" to subject=Message.EMPTY_TOPIC_FALLBACK_NAME
# for older clients with no UI support for empty topic name.
if (
recipient_type_name == "stream"
and not client.empty_topic_name
and wide_dict["subject"] == ""
):
message_dict = message_dict.copy()
message_dict["subject"] = Message.EMPTY_TOPIC_FALLBACK_NAME
# Make sure Zephyr mirroring bots know whether stream is invite-only
if "mirror" in client.client_type_name and event_template.get("invite_only"):
message_dict = message_dict.copy()

5
zerver/tornado/views.py
View File

@@ -214,6 +214,10 @@ def get_events_backend(
Json[bool],
ApiParamConfig(documentation_status=DocumentationStatus.INTENTIONALLY_UNDOCUMENTED),
] = False,
empty_topic_name: Annotated[
Json[bool],
ApiParamConfig(documentation_status=DocumentationStatus.INTENTIONALLY_UNDOCUMENTED),
] = False,
) -> HttpResponse:
if narrow is None:
narrow = []
@@ -254,6 +258,7 @@ def get_events_backend(
user_list_incomplete=user_list_incomplete,
include_deactivated_groups=include_deactivated_groups,
archived_channels=archived_channels,
empty_topic_name=empty_topic_name,
)
result = in_tornado_thread(fetch_events)(