Skip to content

Commit 3f138a6

Browse files
roanster007timabbott
authored andcommitted
topics: Change topic links of left sidebar to use new permalinks.
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 zulip#21505.
1 parent cb6f0fd commit 3f138a6

13 files changed

+87
-10
lines changed

web/shared/src/internal_url.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,36 @@ export function encode_stream_id(
5050
return encodeHashComponent(slug);
5151
}
5252

53+
export function maybe_encode_with_operator(
54+
channel_topic_url: string,
55+
message_id: number | undefined,
56+
): string {
57+
if (message_id === undefined) {
58+
return channel_topic_url;
59+
}
60+
61+
return channel_topic_url + `/with/${message_id}`;
62+
}
63+
5364
export function by_stream_url(
5465
stream_id: number,
5566
maybe_get_stream_name: MaybeGetStreamName,
5667
): string {
5768
return `#narrow/channel/${encode_stream_id(stream_id, maybe_get_stream_name)}`;
5869
}
5970

71+
// The message_id parameter is used to obtain topic permalinks,
72+
// by using it in a `with` operator.
6073
export function by_stream_topic_url(
6174
stream_id: number,
6275
topic: string,
6376
maybe_get_stream_name: MaybeGetStreamName,
77+
message_id?: number,
6478
): string {
65-
return `#narrow/channel/${encode_stream_id(
79+
const channel_topic_url = `#narrow/channel/${encode_stream_id(
6680
stream_id,
6781
maybe_get_stream_name,
6882
)}/topic/${encodeHashComponent(topic)}`;
83+
84+
return maybe_encode_with_operator(channel_topic_url, message_id);
6985
}

web/src/filter.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,6 +1046,17 @@ export class Filter {
10461046
return this._terms.length === 1 && this.has_operand("in", "home");
10471047
}
10481048

1049+
is_in_channel_topic_narrow(): boolean {
1050+
if (
1051+
this.terms().length === 2 &&
1052+
this.has_operator("channel") &&
1053+
this.has_operator("topic")
1054+
) {
1055+
return true;
1056+
}
1057+
return false;
1058+
}
1059+
10491060
is_keyword_search(): boolean {
10501061
return this.has_operator("search");
10511062
}

web/src/hash_util.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as people from "./people.ts";
77
import * as settings_data from "./settings_data.ts";
88
import type {NarrowTerm} from "./state_data.ts";
99
import * as stream_data from "./stream_data.ts";
10+
import {get_latest_message_id_in_topic} from "./stream_topic_history.ts";
1011
import * as sub_store from "./sub_store.ts";
1112
import type {StreamSubscription} from "./sub_store.ts";
1213
import * as user_groups from "./user_groups.ts";
@@ -84,6 +85,21 @@ export function by_stream_topic_url(stream_id: number, topic: string): string {
8485
return internal_url.by_stream_topic_url(stream_id, topic, sub_store.maybe_get_stream_name);
8586
}
8687

88+
// We use the topic permalinks if we have access to the last message id of the
89+
// topic in the cache, by encoding it at the end of the traditional channel-topic
90+
// url using a `with` operator.
91+
// Else, we fallback to using the traditional channel-topic urls.
92+
export function by_channel_topic_permalink(stream_id: number, topic: string): string {
93+
const latest_msg_in_topic = get_latest_message_id_in_topic(stream_id, topic);
94+
95+
return internal_url.by_stream_topic_url(
96+
stream_id,
97+
topic,
98+
sub_store.maybe_get_stream_name,
99+
latest_msg_in_topic,
100+
);
101+
}
102+
87103
// Encodes a term list into the
88104
// corresponding hash: the # component
89105
// of the narrow URL

web/src/inbox_ui.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ function format_topic(
449449
is_empty_string_topic: topic === "",
450450
unread_count: topic_unread_count,
451451
conversation_key: get_topic_key(stream_id, topic),
452-
topic_url: hash_util.by_stream_topic_url(stream_id, topic),
452+
topic_url: hash_util.by_channel_topic_permalink(stream_id, topic),
453453
is_hidden: filter_should_hide_stream_row({stream_id, topic}),
454454
is_collapsed: collapsed_containers.has(STREAM_HEADER_PREFIX + stream_id),
455455
mention_in_unread: unread.topic_has_any_unread_mentions(stream_id, topic),

web/src/recent_view_ui.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ function format_conversation(conversation_data: ConversationData): ConversationC
659659
const topic = last_msg.topic;
660660
const topic_display_name = util.get_final_topic_display_name(topic);
661661
const is_empty_string_topic = topic === "";
662-
const topic_url = hash_util.by_stream_topic_url(stream_id, topic);
662+
const topic_url = hash_util.by_channel_topic_permalink(stream_id, topic);
663663

664664
// We hide the row according to filters or if it's muted.
665665
// We only supply the data to the topic rows and let jquery

web/src/stream_list.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -910,7 +910,7 @@ export function set_event_handlers({
910910
const topic_list_info = topic_list_data.get_list_info(stream_id, false, "");
911911
const topic_item = topic_list_info.items[0];
912912
if (topic_item !== undefined) {
913-
const destination_url = hash_util.by_stream_topic_url(
913+
const destination_url = hash_util.by_channel_topic_permalink(
914914
stream_id,
915915
topic_item.topic_name,
916916
);

web/src/stream_topic_history.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,14 @@ export function get_max_message_id(stream_id: number): number {
372372
return history.get_max_message_id();
373373
}
374374

375+
export function get_latest_message_id_in_topic(
376+
stream_id: number,
377+
topic_name: string,
378+
): number | undefined {
379+
const history = stream_dict.get(stream_id);
380+
return history?.topics.get(topic_name)?.message_id;
381+
}
382+
375383
export function reset(): void {
376384
// This is only used by tests.
377385
stream_dict.clear();

web/src/topic_popover.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,13 @@ import render_delete_topic_modal from "../templates/confirm_dialog/confirm_delet
77
import render_left_sidebar_topic_actions_popover from "../templates/popovers/left_sidebar/left_sidebar_topic_actions_popover.hbs";
88

99
import * as confirm_dialog from "./confirm_dialog.ts";
10+
import * as hash_util from "./hash_util.ts";
1011
import {$t_html} from "./i18n.ts";
1112
import * as message_edit from "./message_edit.ts";
1213
import * as message_summary from "./message_summary.ts";
1314
import * as popover_menus from "./popover_menus.ts";
1415
import * as popover_menus_data from "./popover_menus_data.ts";
1516
import * as starred_messages_ui from "./starred_messages_ui.ts";
16-
import {realm} from "./state_data.ts";
1717
import * as stream_popover from "./stream_popover.ts";
1818
import * as ui_util from "./ui_util.ts";
1919
import * as unread_ops from "./unread_ops.ts";
@@ -27,20 +27,18 @@ function get_conversation(instance: tippy.Instance): {
2727
} {
2828
let stream_id;
2929
let topic_name;
30-
let url;
3130

3231
if (!instance.reference.classList.contains("topic-sidebar-menu-icon")) {
3332
const $elt = $(instance.reference);
3433
stream_id = Number.parseInt($elt.attr("data-stream-id")!, 10);
3534
topic_name = $elt.attr("data-topic-name")!;
36-
url = new URL($elt.attr("data-topic-url")!, realm.realm_url).href;
3735
} else {
3836
const $elt = $(instance.reference).closest(".topic-sidebar-menu-icon").expectOne();
3937
const $stream_li = $elt.closest(".narrow-filter").expectOne();
4038
topic_name = $elt.closest("li").expectOne().attr("data-topic-name")!;
41-
url = util.the($elt.closest("li").find<HTMLAnchorElement>("a.sidebar-topic-name")).href;
4239
stream_id = stream_popover.elem_to_stream_id($stream_li);
4340
}
41+
const url = hash_util.by_channel_topic_permalink(stream_id, topic_name);
4442

4543
return {stream_id, topic_name, url};
4644
}

web/src/ui_init.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,10 +667,18 @@ export function initialize_everything(state_data) {
667667
topic_list.initialize({
668668
on_topic_click(stream_id, topic) {
669669
const sub = sub_store.get(stream_id);
670+
const latest_msg_id = stream_topic_history.get_latest_message_id_in_topic(
671+
stream_id,
672+
topic,
673+
);
674+
675+
assert(latest_msg_id !== undefined);
676+
670677
message_view.show(
671678
[
672679
{operator: "channel", operand: sub.stream_id.toString()},
673680
{operator: "topic", operand: topic},
681+
{operator: "with", operand: latest_msg_id},
674682
],
675683
{trigger: "sidebar"},
676684
);

web/tests/filter.test.cjs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,7 @@ test("basics", () => {
338338
assert.ok(!filter.is_personal_filter());
339339
assert.ok(filter.is_conversation_view());
340340
assert.ok(!filter.is_conversation_view_with_near());
341+
assert.ok(!filter.is_in_channel_topic_narrow());
341342

342343
terms = [
343344
{operator: "dm", operand: "[email protected]"},
@@ -364,6 +365,7 @@ test("basics", () => {
364365
assert.ok(!filter.is_personal_filter());
365366
assert.ok(filter.is_conversation_view());
366367
assert.ok(!filter.is_conversation_view_with_near());
368+
assert.ok(!filter.is_in_channel_topic_narrow());
367369

368370
terms = [
369371
{operator: "dm", operand: "[email protected],[email protected]"},
@@ -450,6 +452,7 @@ test("basics", () => {
450452
assert.ok(!filter.is_personal_filter());
451453
assert.ok(filter.is_conversation_view());
452454
assert.ok(!filter.is_conversation_view_with_near());
455+
assert.ok(filter.is_in_channel_topic_narrow());
453456

454457
terms = [
455458
{operator: "channel", operand: foo_stream_id.toString()},

web/tests/internal_url.test.cjs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ run_test("test by_stream_url", () => {
4747

4848
run_test("test by_stream_topic_url", () => {
4949
const maybe_get_stream_name = () => "a test stream";
50-
const result = internal_url.by_stream_topic_url(123, "test topic", maybe_get_stream_name);
50+
// Test stream_topic_url is a traditional topic link when the
51+
// message_id to be encoded is undefined.
52+
let result = internal_url.by_stream_topic_url(123, "test topic", maybe_get_stream_name);
5153
assert.equal(result, "#narrow/channel/123-a-test-stream/topic/test.20topic");
54+
55+
// Test stream_topic_url is a topic permaling when the
56+
// message_id to be encoded is not undefined.
57+
result = internal_url.by_stream_topic_url(123, "test topic", maybe_get_stream_name, 12);
58+
assert.equal(result, "#narrow/channel/123-a-test-stream/topic/test.20topic/with/12");
5259
});

web/tests/recent_view.test.cjs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ initialize_user_settings({user_settings});
1414

1515
window.scrollTo = noop;
1616
const test_url = () => "https://www.example.com";
17+
const test_permalink = () => "https://www.example.com/with/12";
1718

1819
// We assign this in our test() wrapper.
1920
let messages;
@@ -99,6 +100,7 @@ mock_esm("../src/compose_closed_ui", {
99100
mock_esm("../src/hash_util", {
100101
by_stream_url: test_url,
101102
by_stream_topic_url: test_url,
103+
by_channel_topic_permalink: test_permalink,
102104
by_conversation_and_time_url: test_url,
103105
});
104106
mock_esm("../src/message_list_data", {
@@ -425,7 +427,7 @@ function generate_topic_data(topic_info_array) {
425427
topic_display_name: util.get_final_topic_display_name(topic),
426428
is_empty_string_topic: topic === "",
427429
conversation_key: get_topic_key(stream_id, topic),
428-
topic_url: "https://www.example.com",
430+
topic_url: "https://www.example.com/with/12",
429431
unread_count,
430432
mention_in_unread: false,
431433
visibility_policy,

web/tests/stream_topic_history.test.cjs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,14 @@ test("server_history_end_to_end", () => {
358358
const history = stream_topic_history.get_recent_topic_names(stream_id);
359359
assert.deepEqual(history, ["topic3", "topic2", "topic1"]);
360360

361+
for (const topic of topics) {
362+
const last_msg_id_in_topic = stream_topic_history.get_latest_message_id_in_topic(
363+
stream_id,
364+
topic.name,
365+
);
366+
assert.deepEqual(last_msg_id_in_topic, topic.max_id);
367+
}
368+
361369
// Try getting server history for a second time.
362370

363371
/* istanbul ignore next */

0 commit comments

Comments
 (0)