Skip to content

Commit

Permalink
get_stream_topics: Add support for empty topic name.
Browse files Browse the repository at this point in the history
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 `allow_empty_topic_name` boolean parameter to
`GET /users/me/{stream_id}/topics` endpoint to decide
whether the topic names in the fetched `topics` array
can be empty strings.

If False, the topic names in the fetched response will
have the value of `realm_empty_topic_display_name` field
in `POST /register` response replacing "".

Fixes part of zulip#23291.
  • Loading branch information
prakhar1144 authored and ritwik-69 committed Jan 23, 2025
1 parent d35ef28 commit 3e97396
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 9 deletions.
4 changes: 4 additions & 0 deletions api_docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ deactivated groups.
Added `allow_empty_topic_name` boolean parameter to decide whether the
topic names in the fetched message history objects can be empty strings.

* [`GET /users/me/{stream_id}/topics`](/api/get-stream-topics):
Added `allow_empty_topic_name` boolean parameter to decide whether the
topic names in the fetched `topics` array can be empty strings.

* [`POST /register`](/api/register-queue): For clients that don't support
the `empty_topic_name` [client capability](/api/register-queue#parameter-client_capabilities),
the `topic` field in the `unread_msgs` object and `topic_name` field
Expand Down
2 changes: 1 addition & 1 deletion web/src/stream_topic_history_util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function get_server_history(stream_id: number, on_success: () => void): v

void channel.get({
url,
data: {},
data: {allow_empty_topic_name: true},
success(raw_data) {
const data = stream_topic_history_response_schema.parse(raw_data);
const server_history = data.topics;
Expand Down
2 changes: 1 addition & 1 deletion web/tests/stream_topic_history.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ test("server_history_end_to_end", () => {

channel.get = (opts) => {
assert.equal(opts.url, "/json/users/me/99/topics");
assert.deepEqual(opts.data, {});
assert.deepEqual(opts.data, {allow_empty_topic_name: true});
assert.ok(stream_topic_history.is_request_pending_for(stream_id));
get_success_callback = opts.success;
get_error_callback = opts.error;
Expand Down
28 changes: 22 additions & 6 deletions zerver/lib/topic.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,10 @@ def propagate() -> QuerySet[Message]:
return messages, propagate


def generate_topic_history_from_db_rows(rows: list[tuple[str, int]]) -> list[dict[str, Any]]:
def generate_topic_history_from_db_rows(
rows: list[tuple[str, int]],
allow_empty_topic_name: bool,
) -> list[dict[str, Any]]:
canonical_topic_names: dict[str, tuple[int, str]] = {}

# Sort rows by max_message_id so that if a topic
Expand All @@ -215,13 +218,19 @@ def generate_topic_history_from_db_rows(rows: list[tuple[str, int]]) -> list[dic

history = []
for max_message_id, topic_name in canonical_topic_names.values():
if topic_name == "" and not allow_empty_topic_name:
topic_name = Message.EMPTY_TOPIC_FALLBACK_NAME
history.append(
dict(name=topic_name, max_id=max_message_id),
)
return sorted(history, key=lambda x: -x["max_id"])


def get_topic_history_for_public_stream(realm_id: int, recipient_id: int) -> list[dict[str, Any]]:
def get_topic_history_for_public_stream(
realm_id: int,
recipient_id: int,
allow_empty_topic_name: bool,
) -> list[dict[str, Any]]:
cursor = connection.cursor()
# Uses index: zerver_message_realm_recipient_subject
# Note that this is *case-sensitive*, so that we can display the
Expand All @@ -244,14 +253,21 @@ def get_topic_history_for_public_stream(realm_id: int, recipient_id: int) -> lis
rows = cursor.fetchall()
cursor.close()

return generate_topic_history_from_db_rows(rows)
return generate_topic_history_from_db_rows(rows, allow_empty_topic_name)


def get_topic_history_for_stream(
user_profile: UserProfile, recipient_id: int, public_history: bool
user_profile: UserProfile,
recipient_id: int,
public_history: bool,
allow_empty_topic_name: bool,
) -> list[dict[str, Any]]:
if public_history:
return get_topic_history_for_public_stream(user_profile.realm_id, recipient_id)
return get_topic_history_for_public_stream(
user_profile.realm_id,
recipient_id,
allow_empty_topic_name,
)

cursor = connection.cursor()
# Uses index: zerver_message_realm_recipient_subject
Expand Down Expand Up @@ -279,7 +295,7 @@ def get_topic_history_for_stream(
rows = cursor.fetchall()
cursor.close()

return generate_topic_history_from_db_rows(rows)
return generate_topic_history_from_db_rows(rows, allow_empty_topic_name)


def get_topic_resolution_and_bare_name(stored_name: str) -> tuple[bool, str]:
Expand Down
16 changes: 16 additions & 0 deletions zerver/openapi/zulip.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10220,6 +10220,22 @@ paths:
user subscribed.
parameters:
- $ref: "#/components/parameters/ChannelIdInPath"
- name: allow_empty_topic_name
in: query
description: |
Whether the client supports processing the empty string as
a topic name in the returned data.

If `false`, the value of `realm_empty_topic_display_name`
found in the [`POST /register`](/api/register-queue) response is
returned replacing the empty string as the topic name.

**Changes**: New in Zulip 10.0 (feature level 334). Previously,
the empty string was not a valid topic.
schema:
type: boolean
default: false
example: true
responses:
"200":
description: Success.
Expand Down
25 changes: 25 additions & 0 deletions zerver/tests/test_message_topics.py
Original file line number Diff line number Diff line change
Expand Up @@ -843,3 +843,28 @@ def test_initial_state_data(self) -> None:
self.assertEqual(
state_data["user_topics"][1]["topic_name"], Message.EMPTY_TOPIC_FALLBACK_NAME
)

def test_get_channel_topics(self) -> None:
hamlet = self.example_user("hamlet")
self.login_user(hamlet)
channel_one = self.make_stream("channel_one")
channel_two = self.make_stream("channel_two")
self.subscribe(hamlet, channel_one.name)
self.subscribe(hamlet, channel_two.name)

self.send_stream_message(hamlet, channel_one.name, topic_name="")
self.send_stream_message(
hamlet, channel_two.name, topic_name=Message.EMPTY_TOPIC_FALLBACK_NAME
)

params = {"allow_empty_topic_name": "false"}
for channel_id in [channel_one.id, channel_two.id]:
result = self.client_get(f"/json/users/me/{channel_id}/topics", params)
data = self.assert_json_success(result)
self.assertEqual(data["topics"][0]["name"], Message.EMPTY_TOPIC_FALLBACK_NAME)

params = {"allow_empty_topic_name": "true"}
for channel_id in [channel_one.id, channel_two.id]:
result = self.client_get(f"/json/users/me/{channel_id}/topics", params)
data = self.assert_json_success(result)
self.assertEqual(data["topics"][0]["name"], "")
6 changes: 5 additions & 1 deletion zerver/views/streams.py
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,7 @@ def get_topics_backend(
maybe_user_profile: UserProfile | AnonymousUser,
*,
stream_id: PathOnly[NonNegativeInt],
allow_empty_topic_name: Json[bool] = False,
) -> HttpResponse:
if not maybe_user_profile.is_authenticated:
is_web_public_query = True
Expand All @@ -943,7 +944,9 @@ def get_topics_backend(
realm = get_valid_realm_from_request(request)
stream = access_web_public_stream(stream_id, realm)
result = get_topic_history_for_public_stream(
realm_id=realm.id, recipient_id=assert_is_not_none(stream.recipient_id)
realm_id=realm.id,
recipient_id=assert_is_not_none(stream.recipient_id),
allow_empty_topic_name=allow_empty_topic_name,
)

else:
Expand All @@ -956,6 +959,7 @@ def get_topics_backend(
user_profile=user_profile,
recipient_id=stream.recipient_id,
public_history=stream.is_history_public_to_subscribers(),
allow_empty_topic_name=allow_empty_topic_name,
)

return json_success(request, data=dict(topics=result))
Expand Down

0 comments on commit 3e97396

Please sign in to comment.