diff --git a/web/src/peer_data.ts b/web/src/peer_data.ts index bf21e1117a..d1cb13486d 100644 --- a/web/src/peer_data.ts +++ b/web/src/peer_data.ts @@ -216,6 +216,21 @@ export function get_subscribers(stream_id: number): number[] { return [...subscribers.keys()]; } +export async function get_all_subscribers( + stream_id: number, + retry_on_failure = true, +): Promise { + // This function parallels `get_subscribers` but ensures we include all + // subscribers, possibly fetching that data from the server. + const subscribers = await get_full_subscriber_set(stream_id, retry_on_failure); + // This means the request failed, which can only happen if `retry_on_failure` + // is false. + if (subscribers === null) { + return null; + } + return [...subscribers.keys()]; +} + export function set_subscribers(stream_id: number, user_ids: number[], full_data = true): void { const subscribers = new LazySet(user_ids); stream_subscribers.set(stream_id, subscribers); diff --git a/web/src/stream_edit_subscribers.ts b/web/src/stream_edit_subscribers.ts index 476768d99d..7c66e0de29 100644 --- a/web/src/stream_edit_subscribers.ts +++ b/web/src/stream_edit_subscribers.ts @@ -15,6 +15,7 @@ import * as hash_parser from "./hash_parser.ts"; import {$t, $t_html} from "./i18n.ts"; import * as ListWidget from "./list_widget.ts"; import type {ListWidget as ListWidgetType} from "./list_widget.ts"; +import * as loading from "./loading.ts"; import * as peer_data from "./peer_data.ts"; import * as people from "./people.ts"; import type {User} from "./people.ts"; @@ -128,8 +129,30 @@ export function enable_subscriber_management({ $parent_container.find(".stream_subscription_request_result").empty(); }); - const user_ids = peer_data.get_subscribers(stream_id); const user_can_remove_subscribers = stream_data.can_unsubscribe_others(sub); + void render_subscriber_list_widget(sub, user_can_remove_subscribers, $parent_container); +} + +async function render_subscriber_list_widget( + sub: StreamSubscription, + user_can_remove_subscribers: boolean, + $parent_container: JQuery, +): Promise { + $(".subscriber_list_settings_container").toggleClass("no-display", true); + loading.make_indicator($(".subscriber-list-settings-loading"), { + text: $t({defaultMessage: "Loading…"}), + }); + + // Because we're using `retry_on_failure=true`, this will only return once it + // succeeds, so we can't get `null`. + const user_ids = await peer_data.get_all_subscribers(sub.stream_id, true); + assert(user_ids !== null); + + // Make sure we're still editing this stream after waiting for subscriber data. + if (!hash_parser.is_editing_stream(sub.stream_id)) { + blueslip.info("ignoring subscriber data for stream that is no longer being edited"); + return; + } // We track a single subscribers_list_widget for this module, since we // only ever have one list of subscribers visible at a time. @@ -139,6 +162,8 @@ export function enable_subscriber_management({ user_ids, user_can_remove_subscribers, }); + loading.destroy_indicator($(".subscriber-list-settings-loading")); + $(".subscriber_list_settings_container").toggleClass("no-display", false); } function make_list_widget({ @@ -424,16 +449,23 @@ export function update_subscribers_list(sub: StreamSubscription): void { // from an existing stream or a user group. const subscriber_ids = peer_data.get_subscribers(sub.stream_id); update_subscribers_list_widget(subscriber_ids); - $(".subscriber_list_settings_container").show(); } } function update_subscribers_list_widget(subscriber_ids: number[]): void { + // This can happen if we're still fetching user ids in + // `render_subscriber_list_widget`, but we'll render the widget with + // fetched ids after the fetch is complete, so we don't need to do + // anything here. + if (subscribers_list_widget === undefined) { + return; + } // This re-renders the subscribers_list_widget with a new // list of subscriber_ids. const users = people.get_users_from_ids(subscriber_ids); people.sort_but_pin_current_user_on_top(users); subscribers_list_widget.replace_list_data(users); + $(".subscriber_list_settings_container").show(); } export function rerender_subscribers_list(sub: sub_store.StreamSubscription): void { @@ -452,7 +484,6 @@ export function rerender_subscribers_list(sub: sub_store.StreamSubscription): vo return; } - const user_ids = peer_data.get_subscribers(sub.stream_id); const user_can_remove_subscribers = stream_data.can_unsubscribe_others(sub); const $parent_container = stream_settings_containers .get_edit_container(sub) @@ -463,12 +494,7 @@ export function rerender_subscribers_list(sub: sub_store.StreamSubscription): vo can_remove_subscribers: user_can_remove_subscribers, }), ); - subscribers_list_widget = make_list_widget({ - $parent_container, - name: "stream_subscribers", - user_ids, - user_can_remove_subscribers, - }); + void render_subscriber_list_widget(sub, user_can_remove_subscribers, $parent_container); } export function initialize(): void { diff --git a/web/styles/subscriptions.css b/web/styles/subscriptions.css index 65dc1f5ac2..0031ba5a08 100644 --- a/web/styles/subscriptions.css +++ b/web/styles/subscriptions.css @@ -136,6 +136,17 @@ h4.user_group_setting_subsection_title { line-height: 1.5; } +.subscriber_list_settings_container.no-display { + display: none; +} + +.subscriber-list-settings-loading { + display: flex; + align-items: center; + margin: auto; + margin-top: 100px; +} + .member-list-box, .subscriber-list-box { text-align: center; diff --git a/web/templates/stream_settings/stream_members.hbs b/web/templates/stream_settings/stream_members.hbs index 216fc15dc6..461c7286f0 100644 --- a/web/templates/stream_settings/stream_members.hbs +++ b/web/templates/stream_settings/stream_members.hbs @@ -1,5 +1,5 @@ {{#if render_subscribers}} -
+

{{t "Add subscribers" }}

@@ -20,4 +20,5 @@ {{> stream_members_table .}}
+
{{/if}} diff --git a/web/tests/peer_data.test.cjs b/web/tests/peer_data.test.cjs index a0e542a167..a3214de9bb 100644 --- a/web/tests/peer_data.test.cjs +++ b/web/tests/peer_data.test.cjs @@ -302,6 +302,19 @@ test("maybe_fetch_stream_subscribers", async () => { assert.equal(await peer_data.maybe_fetch_is_user_subscribed(india.stream_id, 2, false), true); assert.equal(await peer_data.maybe_fetch_is_user_subscribed(india.stream_id, 5, false), false); + peer_data.clear_for_testing(); + assert.deepEqual(await peer_data.get_subscribers(india.stream_id), []); + assert.deepEqual(await peer_data.get_all_subscribers(india.stream_id), [1, 2, 3, 4]); + + peer_data.clear_for_testing(); + channel.get = () => { + throw new Error("error"); + }; + blueslip.expect("error", "Failure fetching channel subscribers"); + // retry is false, so we get null because there was an error + assert.deepEqual(await peer_data.get_all_subscribers(india.stream_id, false), null); + blueslip.reset(); + channel.get = () => { throw new Error("error"); };