From 19880797db2de4d60a23e17a46abe50ada3fb476 Mon Sep 17 00:00:00 2001 From: Evy Kassirer Date: Tue, 9 Sep 2025 15:59:51 -0700 Subject: [PATCH] channel_folders: Fix remaining test TODOs. Fixes #35494. --- tools/test-js-with-node | 4 -- web/src/channel_folders.ts | 2 - web/src/left_sidebar_navigation_area.ts | 8 +++- web/src/list_cursor.ts | 4 -- web/src/settings_data.ts | 2 - web/src/stream_list.ts | 6 ++- web/tests/channel_folders.test.cjs | 5 +++ .../left_sidebar_navigation_area.test.cjs | 19 +++----- web/tests/list_cursor.test.cjs | 6 +++ web/tests/settings_data.test.cjs | 19 ++++++++ web/tests/stream_list.test.cjs | 43 +++++++++++-------- 11 files changed, 72 insertions(+), 46 deletions(-) diff --git a/tools/test-js-with-node b/tools/test-js-with-node index 7755b60c50..4a71e4001c 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -309,8 +309,6 @@ EXEMPT_FILES = make_set( "web/src/zcommand.ts", "web/src/zform.js", "web/src/zulip_test.ts", - # TODO/channel-folders: Remove when tests are restored. - "web/tests/left_sidebar_navigation_area.test.cjs", # Test library code isn't always fully used. "web/tests/lib/example_user.cjs", "web/tests/lib/mdiff.cjs", @@ -320,8 +318,6 @@ EXEMPT_FILES = make_set( # There are some important functions which are not called right now but will # be reused when we add tests for dropdown widget so it doesn't make sense to remove them. "web/tests/recent_view.test.cjs", - # TODO/channel-folders: Remove when tests are restored. - "web/tests/stream_list.test.cjs", ] ) diff --git a/web/src/channel_folders.ts b/web/src/channel_folders.ts index 0e620d323c..1f536cad65 100644 --- a/web/src/channel_folders.ts +++ b/web/src/channel_folders.ts @@ -57,8 +57,6 @@ export function get_active_folder_ids(): Set { return active_channel_folder_ids; } -/* TODO/channel-folders: Remove when tests are restored */ -/* istanbul ignore next */ export function get_all_folder_ids(): Set { return new Set(channel_folder_by_id_dict.keys()); } diff --git a/web/src/left_sidebar_navigation_area.ts b/web/src/left_sidebar_navigation_area.ts index f71c3d92ee..6ff2dc2ab3 100644 --- a/web/src/left_sidebar_navigation_area.ts +++ b/web/src/left_sidebar_navigation_area.ts @@ -65,7 +65,7 @@ export function update_reminders_row(): void { ui_util.update_unread_count_in_dom($reminders_li, count); } -export function update_dom_with_unread_counts( +export let update_dom_with_unread_counts = function ( counts: unread.FullUnreadCountsData, skip_animations: boolean, ): void { @@ -85,6 +85,12 @@ export function update_dom_with_unread_counts( if (!skip_animations) { animate_mention_changes($mentioned_li, counts.mentioned_message_count); } +}; + +export function rewire_update_dom_with_unread_counts( + value: typeof update_dom_with_unread_counts, +): void { + update_dom_with_unread_counts = value; } export let select_top_left_corner_item = function (narrow_to_activate: string): void { diff --git a/web/src/list_cursor.ts b/web/src/list_cursor.ts index 27d56d0f14..ea37089a6a 100644 --- a/web/src/list_cursor.ts +++ b/web/src/list_cursor.ts @@ -93,12 +93,8 @@ export class ListCursor { const row = this.get_row(this.curr_key); if (row === undefined) { - /* TODO/channel-folders: Remove when tests are restored */ - /* istanbul ignore next */ return; } - /* TODO/channel-folders: Remove when tests are restored */ - /* istanbul ignore next */ row.highlight(); } diff --git a/web/src/settings_data.ts b/web/src/settings_data.ts index 831377bd13..2d6afa1877 100644 --- a/web/src/settings_data.ts +++ b/web/src/settings_data.ts @@ -278,8 +278,6 @@ export function user_can_delete_own_message(): boolean { ); } -/* TODO/channel-folders: Remove when tests are restored */ -/* istanbul ignore next */ export function should_mask_unread_count( sub_muted: boolean, unmuted_unread_count: number, diff --git a/web/src/stream_list.ts b/web/src/stream_list.ts index 728445f8d7..2c4ba3ef25 100644 --- a/web/src/stream_list.ts +++ b/web/src/stream_list.ts @@ -445,11 +445,15 @@ export function rewire_update_stream_section_mention_indicators( channel, we show it too. If there's no highlighted topic within the channel, then we should treat this like an empty topic list and remove the left bracket. */ -export function maybe_hide_topic_bracket(section_id: string): void { +export let maybe_hide_topic_bracket = function (section_id: string): void { const $container = $(`#stream-list-${section_id}-container`); const is_collapsed = collapsed_sections.has(section_id); const $highlighted_topic = $container.find(".topic-list-item.active-sub-filter"); $container.toggleClass("hide-topic-bracket", is_collapsed && $highlighted_topic.length === 0); +}; + +export function rewire_maybe_hide_topic_bracket(value: typeof maybe_hide_topic_bracket): void { + maybe_hide_topic_bracket = value; } function toggle_section_collapse($container: JQuery): void { diff --git a/web/tests/channel_folders.test.cjs b/web/tests/channel_folders.test.cjs index 96de601890..be9c0fa718 100644 --- a/web/tests/channel_folders.test.cjs +++ b/web/tests/channel_folders.test.cjs @@ -66,6 +66,11 @@ run_test("basics", () => { devops_folder, ]); + assert.deepEqual( + channel_folders.get_all_folder_ids(), + new Set([frontend_folder.id, backend_folder.id, devops_folder.id]), + ); + assert.ok(channel_folders.is_valid_folder_id(frontend_folder.id)); assert.ok(!channel_folders.is_valid_folder_id(999)); diff --git a/web/tests/left_sidebar_navigation_area.test.cjs b/web/tests/left_sidebar_navigation_area.test.cjs index 412d65634b..368a00a39c 100644 --- a/web/tests/left_sidebar_navigation_area.test.cjs +++ b/web/tests/left_sidebar_navigation_area.test.cjs @@ -86,11 +86,6 @@ run_test("narrowing", ({override_rewire}) => { }); run_test("update_count_in_dom", () => { - // TODO/channel-folders: Re-enable this test. - if (Filter !== undefined) { - return; - } - function make_elem($elem, count_selector) { const $count = $(count_selector); $elem.set_find_results(".unread_count", $count); @@ -114,6 +109,8 @@ run_test("update_count_in_dom", () => { make_elem($(".selected-home-view"), ""); + make_elem($(".top_left_condensed_unread_marker"), ""); + make_elem($(".top_left_starred_messages"), ""); make_elem($(".top_left_scheduled_messages"), ""); @@ -121,9 +118,6 @@ run_test("update_count_in_dom", () => { make_elem($(".top_left_reminders"), ""); make_elem($("#topics_header"), ""); - make_elem($("#stream-list-pinned-streams-container .markers-and-unreads"), ""); - make_elem($("#stream-list-normal-streams-container .markers-and-unreads"), ""); - make_elem($("#stream-list-dormant-streams-container .markers-and-unreads"), ""); left_sidebar_navigation_area.update_dom_with_unread_counts(counts, false); left_sidebar_navigation_area.update_starred_count(444, false); @@ -132,14 +126,11 @@ run_test("update_count_in_dom", () => { assert.equal($("").text(), "222"); assert.equal($("").text(), "333"); + assert.equal($("").text(), "333"); assert.equal($("").text(), "444"); - assert.equal($("").text(), "555"); - assert.equal($("").text(), "888"); + assert.equal($("").text(), ""); + assert.equal($("").text(), ""); assert.equal($("").text(), "666"); - // TODO/channel-folders: Do proper data setup so these are a number. - assert.equal($("").text(), ""); - assert.equal($("").text(), ""); - assert.equal($("").text(), ""); counts.mentioned_message_count = 0; diff --git a/web/tests/list_cursor.test.cjs b/web/tests/list_cursor.test.cjs index 61881f99ef..ea36d4e586 100644 --- a/web/tests/list_cursor.test.cjs +++ b/web/tests/list_cursor.test.cjs @@ -148,6 +148,12 @@ run_test("multiple item list", ({override}) => { assert.ok(!list_items[2].hasClass("highlight")); assert.ok(!list_items[3].hasClass("highlight")); + override(conf.list, "find_li", () => list_items[1]); + cursor.redraw(); + assert.ok(list_items[1].hasClass("highlight")); + assert.ok(!list_items[2].hasClass("highlight")); + assert.ok(!list_items[3].hasClass("highlight")); + cursor.clear(); assert.equal(cursor.get_key(), undefined); cursor.redraw(); diff --git a/web/tests/settings_data.test.cjs b/web/tests/settings_data.test.cjs index d31bc11f01..84af153e35 100644 --- a/web/tests/settings_data.test.cjs +++ b/web/tests/settings_data.test.cjs @@ -675,3 +675,22 @@ run_test("user_can_summarize_topics", ({override}) => { override(realm, "server_can_summarize_topics", false); assert.ok(!settings_data.user_can_summarize_topics()); }); + +run_test("should_mask_unread_count", ({override}) => { + override(user_settings, "web_stream_unreads_count_display_policy", 3); + let sub_muted = false; + let unmuted_unread_count = 0; + assert.equal(settings_data.should_mask_unread_count(sub_muted, unmuted_unread_count), true); + + override(user_settings, "web_stream_unreads_count_display_policy", 2); + assert.equal(settings_data.should_mask_unread_count(sub_muted, unmuted_unread_count), false); + + sub_muted = true; + assert.equal(settings_data.should_mask_unread_count(sub_muted, unmuted_unread_count), true); + + unmuted_unread_count = 2; + assert.equal(settings_data.should_mask_unread_count(sub_muted, unmuted_unread_count), false); + + override(user_settings, "web_stream_unreads_count_display_policy", 1); + assert.equal(settings_data.should_mask_unread_count(sub_muted, unmuted_unread_count), false); +}); diff --git a/web/tests/stream_list.test.cjs b/web/tests/stream_list.test.cjs index c7896c7816..4187fddb0f 100644 --- a/web/tests/stream_list.test.cjs +++ b/web/tests/stream_list.test.cjs @@ -36,12 +36,9 @@ mock_esm("../src/unread", { stream_has_any_unread_mentions: () => stream_has_any_unread_mentions, stream_has_any_unmuted_mentions: () => noop, }); -// TODO/channel-folders: Don't mock this. -mock_esm("../src/left_sidebar_navigation_area", { - update_dom_with_unread_counts: () => noop, -}); const {Filter} = zrequire("../src/filter"); +const left_sidebar_navigation_area = zrequire("left_sidebar_navigation_area"); const stream_data = zrequire("stream_data"); const stream_list = zrequire("stream_list"); stream_list.set_update_inbox_channel_view_callback(noop); @@ -51,8 +48,6 @@ const {initialize_user_settings} = zrequire("user_settings"); const settings_config = zrequire("settings_config"); mock_esm("../src/settings_data", { user_can_create_private_streams: () => true, - user_can_create_public_streams: () => true, - user_can_create_web_public_streams: () => true, user_has_permission_for_group_setting: () => true, should_mask_unread_count: () => false, }); @@ -174,6 +169,7 @@ test_ui("create_sidebar_row", ({override, override_rewire, mock_template}) => { }); override_rewire(stream_list, "update_dom_with_unread_counts", noop); override_rewire(stream_list, "update_stream_section_mention_indicators", noop); + override_rewire(left_sidebar_navigation_area, "update_dom_with_unread_counts", noop); const pinned_streams = []; $("#stream-list-pinned-streams").append = (stream) => { @@ -251,6 +247,7 @@ test_ui("create_sidebar_row", ({override, override_rewire, mock_template}) => { test_ui("pinned_streams_never_inactive", ({mock_template, override_rewire}) => { override_rewire(stream_list, "update_stream_section_mention_indicators", noop); override_rewire(stream_list, "update_dom_with_unread_counts", noop); + override_rewire(left_sidebar_navigation_area, "update_dom_with_unread_counts", noop); stream_data.add_sub(devel); stream_data.add_sub(social); @@ -447,6 +444,7 @@ test_ui("narrowing", ({override_rewire}) => { override_rewire(stream_list, "scroll_stream_into_view", noop); override_rewire(stream_list, "update_stream_section_mention_indicators", noop); override_rewire(stream_list, "update_dom_with_unread_counts", noop); + override_rewire(left_sidebar_navigation_area, "update_dom_with_unread_counts", noop); initialize_stream_data(); assert.ok(!$("").hasClass("active-filter")); @@ -485,11 +483,10 @@ test_ui("narrowing", ({override_rewire}) => { assert.ok(topics_closed); }); -test_ui("sort_streams", ({override_rewire}) => { - // TODO/channel-folders: Fix this test. - if (override_rewire !== undefined) { - return; - } +test_ui("sort_streams", ({override_rewire, mock_template}) => { + override_rewire(stream_list, "update_dom_with_unread_counts", noop); + override_rewire(stream_list, "update_stream_section_mention_indicators", noop); + override_rewire(left_sidebar_navigation_area, "update_dom_with_unread_counts", noop); // Get coverage on early-exit. stream_list.build_stream_list(); @@ -502,6 +499,8 @@ test_ui("sort_streams", ({override_rewire}) => { return ``; }); + mock_template("show_inactive_or_muted_channels.hbs", false, () => $("")); + const pinned_streams = []; $("#stream-list-pinned-streams").append = (stream) => { pinned_streams.push(stream); @@ -523,19 +522,22 @@ test_ui("sort_streams", ({override_rewire}) => { assert.deepEqual(normal_streams, [ $(""), $(""), + $(""), + $(""), ]); const streams = stream_list_sort.get_stream_ids(); assert.deepEqual(streams, [ - // three groups: pinned, normal, dormant + // two groups: pinned and normal (with dormant at the bottom of the list) + // pinned develSub.stream_id, RomeSub.stream_id, testSub.stream_id, - // + // normal announceSub.stream_id, DenmarkSub.stream_id, - // + // dormant in normal list carSub.stream_id, ]); @@ -549,6 +551,7 @@ test_ui("sort_streams", ({override_rewire}) => { test_ui("separators_only_pinned_and_dormant", ({override_rewire}) => { override_rewire(stream_list, "update_dom_with_unread_counts", noop); override_rewire(stream_list, "update_stream_section_mention_indicators", noop); + override_rewire(left_sidebar_navigation_area, "update_dom_with_unread_counts", noop); // Get coverage on early-exit. stream_list.build_stream_list(); @@ -604,6 +607,7 @@ test_ui("separators_only_pinned_and_dormant", ({override_rewire}) => { test_ui("rename_stream", ({mock_template, override, override_rewire}) => { override_rewire(stream_list, "update_dom_with_unread_counts", noop); override_rewire(stream_list, "update_stream_section_mention_indicators", noop); + override_rewire(left_sidebar_navigation_area, "update_dom_with_unread_counts", noop); override(user_settings, "web_stream_unreads_count_display_policy", 3); override(current_user, "user_id", me.user_id); initialize_stream_data(); @@ -647,10 +651,10 @@ test_ui("rename_stream", ({mock_template, override, override_rewire}) => { }); test_ui("refresh_pin", ({override_rewire, mock_template}) => { - // TODO/channel-folders: Un-disable this test - if (override_rewire !== undefined) { - return; - } + override_rewire(stream_list, "update_stream_section_mention_indicators", noop); + override_rewire(stream_list, "update_dom_with_unread_counts", noop); + override_rewire(stream_list, "maybe_hide_topic_bracket", noop); + override_rewire(left_sidebar_navigation_area, "update_dom_with_unread_counts", noop); initialize_stream_data(); const sub = { @@ -692,6 +696,9 @@ test_ui("refresh_pin", ({override_rewire, mock_template}) => { test_ui("create_initial_sidebar_rows", ({override, override_rewire, mock_template}) => { override(user_settings, "web_stream_unreads_count_display_policy", 2); // Test coverage for this setting. + override_rewire(stream_list, "update_stream_section_mention_indicators", noop); + override_rewire(stream_list, "update_dom_with_unread_counts", noop); + override_rewire(left_sidebar_navigation_area, "update_dom_with_unread_counts", noop); initialize_stream_data(); const html_dict = new Map();