paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-03 05:23:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

topics: Change topic links of left sidebar to use new permalinks.

Browse Source 
This commit updates the topic links obtained from clicking
the topics in the left sidebar, recent view and inbox, and
those obtained from "Copy link to topic" to use the new
topic permalinks.

Fixes part of #21505.
This commit is contained in:
roanster007
2024-06-01 19:12:54 +05:30
committed by Tim Abbott
parent e67786154a
commit 41f30e1052
12 changed files with 79 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
web/shared/src/internal_url.ts
View File

@@ -57,13 +57,22 @@ export function by_stream_url(
return `#narrow/channel/${encode_stream_id(stream_id, maybe_get_stream_name)}`;
}
// The message_id parameter is used to obtain topic permalinks,
// by using it in a `with` operator.
export function by_stream_topic_url(
stream_id: number,
topic: string,
maybe_get_stream_name: MaybeGetStreamName,
message_id?: number,
): string {
if (message_id === undefined) {
return `#narrow/channel/${encode_stream_id(
stream_id,
maybe_get_stream_name,
)}/topic/${encodeHashComponent(topic)}`;
}
return `#narrow/channel/${encode_stream_id(
stream_id,
maybe_get_stream_name,
)}/topic/${encodeHashComponent(topic)}/with/${message_id}`;
}

24
web/src/hash_util.ts
View File

@@ -7,6 +7,7 @@ import * as people from "./people.ts";
import * as settings_data from "./settings_data.ts";
import type {NarrowTerm} from "./state_data.ts";
import * as stream_data from "./stream_data.ts";
import * as stream_topic_history from "./stream_topic_history.ts";
import * as sub_store from "./sub_store.ts";
import type {StreamSubscription} from "./sub_store.ts";
import * as user_groups from "./user_groups.ts";
@@ -84,6 +85,29 @@ export function by_stream_topic_url(stream_id: number, topic: string): string {
return internal_url.by_stream_topic_url(stream_id, topic, sub_store.maybe_get_stream_name);
}
// We use the topic permalinks if we have access to the last message
// id of the topic in the cache, by encoding it at the end of the
// traditional channel-topic url using a `with` operator. If client
// cache doesn't have a message, we use the traditional link format.
export function by_channel_topic_permalink(stream_id: number, topic: string): string {
// From an API perspective, any message ID in the topic is a valid
// choice. In the client code, we choose the latest message ID in
// the topic, since display in recent conversations, the left
// sidebar, and most other elements are placed in a way reflecting
// the recency of the latest message in the topic.
const target_message_id = stream_topic_history.get_latest_known_message_id_in_topic(
stream_id,
topic,
);
return internal_url.by_stream_topic_url(
stream_id,
topic,
sub_store.maybe_get_stream_name,
target_message_id,
);
}
// Encodes a term list into the
// corresponding hash: the # component
// of the narrow URL

2
web/src/inbox_ui.ts
View File

@@ -449,7 +449,7 @@ function format_topic(
is_empty_string_topic: topic === "",
unread_count: topic_unread_count,
conversation_key: get_topic_key(stream_id, topic),
topic_url: hash_util.by_stream_topic_url(stream_id, topic),
topic_url: hash_util.by_channel_topic_permalink(stream_id, topic),
is_hidden: filter_should_hide_stream_row({stream_id, topic}),
is_collapsed: collapsed_containers.has(STREAM_HEADER_PREFIX + stream_id),
mention_in_unread: unread.topic_has_any_unread_mentions(stream_id, topic),

2
web/src/recent_view_ui.ts
View File

@@ -659,7 +659,7 @@ function format_conversation(conversation_data: ConversationData): ConversationC
const topic = last_msg.topic;
const topic_display_name = util.get_final_topic_display_name(topic);
const is_empty_string_topic = topic === "";
const topic_url = hash_util.by_stream_topic_url(stream_id, topic);
const topic_url = hash_util.by_channel_topic_permalink(stream_id, topic);
// We hide the row according to filters or if it's muted.
// We only supply the data to the topic rows and let jquery

2
web/src/stream_list.ts
View File

@@ -910,7 +910,7 @@ export function set_event_handlers({
const topic_list_info = topic_list_data.get_list_info(stream_id, false, "");
const topic_item = topic_list_info.items[0];
if (topic_item !== undefined) {
const destination_url = hash_util.by_stream_topic_url(
const destination_url = hash_util.by_channel_topic_permalink(
stream_id,
topic_item.topic_name,
);

8
web/src/stream_topic_history.ts
View File

@@ -372,6 +372,14 @@ export function get_max_message_id(stream_id: number): number {
return history.get_max_message_id();
}
export function get_latest_known_message_id_in_topic(
stream_id: number,
topic_name: string,
): number | undefined {
const history = stream_dict.get(stream_id);
return history?.topics.get(topic_name)?.message_id;
}
export function reset(): void {
// This is only used by tests.
stream_dict.clear();

2
web/src/topic_list_data.ts
View File

@@ -135,7 +135,7 @@ function choose_topics(
is_followed: is_topic_followed,
is_unmuted_or_followed: is_topic_unmuted_or_followed,
is_active_topic,
url: hash_util.by_stream_topic_url(stream_id, topic_name),
url: hash_util.by_channel_topic_permalink(stream_id, topic_name),
contains_unread_mention,
};

8
web/src/ui_init.js
View File

@@ -667,10 +667,18 @@ export function initialize_everything(state_data) {
topic_list.initialize({
on_topic_click(stream_id, topic) {
const sub = sub_store.get(stream_id);
const latest_msg_id = stream_topic_history.get_latest_known_message_id_in_topic(
stream_id,
topic,
);
assert(latest_msg_id !== undefined);
message_view.show(
[
{operator: "channel", operand: sub.stream_id.toString()},
{operator: "topic", operand: topic},
{operator: "with", operand: latest_msg_id},
],
{trigger: "sidebar"},
);

9
web/tests/internal_url.test.cjs
View File

@@ -47,6 +47,13 @@ run_test("test by_stream_url", () => {
run_test("test by_stream_topic_url", () => {
const maybe_get_stream_name = () => "a test stream";
const result = internal_url.by_stream_topic_url(123, "test topic", maybe_get_stream_name);
// Test stream_topic_url is a traditional topic link when the
// message_id to be encoded is undefined.
let result = internal_url.by_stream_topic_url(123, "test topic", maybe_get_stream_name);
assert.equal(result, "#narrow/channel/123-a-test-stream/topic/test.20topic");
// Test stream_topic_url is a topic permaling when the
// message_id to be encoded is not undefined.
result = internal_url.by_stream_topic_url(123, "test topic", maybe_get_stream_name, 12);
assert.equal(result, "#narrow/channel/123-a-test-stream/topic/test.20topic/with/12");
});

4
web/tests/recent_view.test.cjs
View File

@@ -14,6 +14,7 @@ initialize_user_settings({user_settings});
window.scrollTo = noop;
const test_url = () => "https://www.example.com";
const test_permalink = () => "https://www.example.com/with/12";
// We assign this in our test() wrapper.
let messages;
@@ -99,6 +100,7 @@ mock_esm("../src/compose_closed_ui", {
mock_esm("../src/hash_util", {
by_stream_url: test_url,
by_stream_topic_url: test_url,
by_channel_topic_permalink: test_permalink,
by_conversation_and_time_url: test_url,
});
mock_esm("../src/message_list_data", {
@@ -425,7 +427,7 @@ function generate_topic_data(topic_info_array) {
topic_display_name: util.get_final_topic_display_name(topic),
is_empty_string_topic: topic === "",
conversation_key: get_topic_key(stream_id, topic),
topic_url: "https://www.example.com",
topic_url: "https://www.example.com/with/12",
unread_count,
mention_in_unread: false,
visibility_policy,

8
web/tests/stream_topic_history.test.cjs
View File

@@ -358,6 +358,14 @@ test("server_history_end_to_end", () => {
const history = stream_topic_history.get_recent_topic_names(stream_id);
assert.deepEqual(history, ["topic3", "topic2", "topic1"]);
for (const topic of topics) {
const last_msg_id_in_topic = stream_topic_history.get_latest_known_message_id_in_topic(
stream_id,
topic.name,
);
assert.deepEqual(last_msg_id_in_topic, topic.max_id);
}
// Try getting server history for a second time.
/* istanbul ignore next */

9
web/tests/topic_list_data.test.cjs
View File

@@ -100,6 +100,9 @@ test("get_list_info w/real stream_topic_history", ({override}) => {
assert.equal(list_info.more_topics_unreads, 0);
assert.equal(list_info.more_topics_have_unread_mention_messages, false);
assert.equal(list_info.num_possible_topics, 11);
// The topic link is not a permalink since the topic has no
// messages sent yet.
assert.deepEqual(list_info.items[0], {
topic_name: "topic 11",
topic_resolved_prefix: "",
@@ -137,7 +140,7 @@ test("get_list_info w/real stream_topic_history", ({override}) => {
topic_resolved_prefix: "✔ ",
is_empty_string_topic: false,
unread: 0,
url: "#narrow/channel/556-general/topic/.E2.9C.94.20topic.209",
url: `#narrow/channel/556-general/topic/.E2.9C.94.20topic.209/with/${1000 + 9}`,
});
assert.deepEqual(list_info.items[1], {
@@ -153,7 +156,7 @@ test("get_list_info w/real stream_topic_history", ({override}) => {
topic_resolved_prefix: "",
is_empty_string_topic: false,
unread: 0,
url: "#narrow/channel/556-general/topic/topic.208",
url: `#narrow/channel/556-general/topic/topic.208/with/${1000 + 8}`,
});
// Empty string as topic name.
@@ -178,7 +181,7 @@ test("get_list_info w/real stream_topic_history", ({override}) => {
topic_resolved_prefix: "",
is_empty_string_topic: true,
unread: 0,
url: "#narrow/channel/556-general/topic/",
url: "#narrow/channel/556-general/topic//with/2025",
});
// If we zoom in, our results are based on topic filter.