stream_pill: Fetch all subscribers before getting user ids for a stream.

Work towards #34244.
This commit is contained in:
Evy Kassirer
2025-04-09 21:20:17 -07:00
committed by Tim Abbott
parent c406db2480
commit ec65dad063
9 changed files with 105 additions and 23 deletions

View File

@@ -1,8 +1,10 @@
import $ from "jquery";
import assert from "minimalistic-assert"; import assert from "minimalistic-assert";
import * as add_subscribers_pill from "./add_subscribers_pill.ts"; import * as add_subscribers_pill from "./add_subscribers_pill.ts";
import * as input_pill from "./input_pill.ts"; import * as input_pill from "./input_pill.ts";
import * as keydown_util from "./keydown_util.ts"; import * as keydown_util from "./keydown_util.ts";
import * as loading from "./loading.ts";
import * as people from "./people.ts"; import * as people from "./people.ts";
import type {User} from "./people.ts"; import type {User} from "./people.ts";
import * as stream_pill from "./stream_pill.ts"; import * as stream_pill from "./stream_pill.ts";
@@ -13,9 +15,9 @@ import * as user_groups from "./user_groups.ts";
import type {UserGroup} from "./user_groups.ts"; import type {UserGroup} from "./user_groups.ts";
import * as user_pill from "./user_pill.ts"; import * as user_pill from "./user_pill.ts";
function get_pill_user_ids(pill_widget: CombinedPillContainer): number[] { async function get_pill_user_ids(pill_widget: CombinedPillContainer): Promise<number[]> {
const user_ids = user_pill.get_user_ids(pill_widget); const user_ids = user_pill.get_user_ids(pill_widget);
const stream_user_ids = stream_pill.get_user_ids(pill_widget); const stream_user_ids = await stream_pill.get_user_ids(pill_widget);
return [...user_ids, ...stream_user_ids]; return [...user_ids, ...stream_user_ids];
} }
@@ -108,14 +110,22 @@ export function create({
if (onPillCreateAction) { if (onPillCreateAction) {
pill_widget.onPillCreate(() => { pill_widget.onPillCreate(() => {
onPillCreateAction(get_pill_user_ids(pill_widget), get_pill_group_ids(pill_widget)); void (async () => {
loading.make_indicator($(".add-group-member-loading-spinner"), {
height: 56, // 4em at 14px / 1em
});
const user_ids = await get_pill_user_ids(pill_widget);
onPillCreateAction(user_ids, get_pill_group_ids(pill_widget));
loading.destroy_indicator($(".add-group-member-loading-spinner"));
})();
}); });
} }
if (onPillRemoveAction) { if (onPillRemoveAction) {
pill_widget.onPillRemove(() => { void (async () => {
onPillRemoveAction(get_pill_user_ids(pill_widget), get_pill_group_ids(pill_widget)); const user_ids = await get_pill_user_ids(pill_widget);
}); onPillRemoveAction(user_ids, get_pill_group_ids(pill_widget));
})();
} }
function get_users(): User[] { function get_users(): User[] {
@@ -175,9 +185,23 @@ export function set_up_handlers({
*/ */
function callback(): void { function callback(): void {
const pill_widget = get_pill_widget(); const pill_widget = get_pill_widget();
const pill_user_ids = get_pill_user_ids(pill_widget); void (async () => {
loading.make_indicator($(".add-group-member-loading-spinner"), {
height: 56, // 4em at 14px / 1em
});
const pill_user_ids = await get_pill_user_ids(pill_widget);
// If we're no longer in the same view after fetching
// subscriber data, don't update the UI. We don't need
// to destroy the loading spinner because the tab re-renders
// every time it opens, and also there might be a new tab
// with a current loading spinner.
if (get_pill_widget() !== pill_widget) {
return;
}
loading.destroy_indicator($(".add-group-member-loading-spinner"));
const pill_group_ids = get_pill_group_ids(pill_widget); const pill_group_ids = get_pill_group_ids(pill_widget);
action({pill_user_ids, pill_group_ids}); action({pill_user_ids, pill_group_ids});
})();
} }
$parent_container.on("keyup", pill_selector, (e) => { $parent_container.on("keyup", pill_selector, (e) => {

View File

@@ -1,8 +1,10 @@
import $ from "jquery";
import assert from "minimalistic-assert"; import assert from "minimalistic-assert";
import * as blueslip from "./blueslip.ts"; import * as blueslip from "./blueslip.ts";
import * as input_pill from "./input_pill.ts"; import * as input_pill from "./input_pill.ts";
import * as keydown_util from "./keydown_util.ts"; import * as keydown_util from "./keydown_util.ts";
import * as loading from "./loading.ts";
import type {User} from "./people.ts"; import type {User} from "./people.ts";
import * as pill_typeahead from "./pill_typeahead.ts"; import * as pill_typeahead from "./pill_typeahead.ts";
import * as stream_pill from "./stream_pill.ts"; import * as stream_pill from "./stream_pill.ts";
@@ -151,13 +153,23 @@ export function create({
if (onPillCreateAction) { if (onPillCreateAction) {
pill_widget.onPillCreate(() => { pill_widget.onPillCreate(() => {
onPillCreateAction(get_pill_user_ids(pill_widget)); void (async () => {
loading.make_indicator($(".add-subscriber-loading-spinner"), {
height: 56, // 4em at 14px / 1em
});
const user_ids = await get_pill_user_ids(pill_widget);
onPillCreateAction(user_ids);
loading.destroy_indicator($(".add-subscriber-loading-spinner"));
})();
}); });
} }
if (onPillRemoveAction) { if (onPillRemoveAction) {
pill_widget.onPillRemove(() => { pill_widget.onPillRemove(() => {
onPillRemoveAction(get_pill_user_ids(pill_widget)); void (async () => {
const user_ids = await get_pill_user_ids(pill_widget);
onPillRemoveAction(user_ids);
})();
}); });
} }
@@ -202,9 +214,9 @@ export function append_user_group_from_name(
user_group_pill.append_user_group(user_group, pill_widget); user_group_pill.append_user_group(user_group, pill_widget);
} }
function get_pill_user_ids(pill_widget: CombinedPillContainer): number[] { async function get_pill_user_ids(pill_widget: CombinedPillContainer): Promise<number[]> {
const user_ids = user_pill.get_user_ids(pill_widget); const user_ids = user_pill.get_user_ids(pill_widget);
const stream_user_ids = stream_pill.get_user_ids(pill_widget); const stream_user_ids = await stream_pill.get_user_ids(pill_widget);
const group_user_ids = user_group_pill.get_user_ids(pill_widget); const group_user_ids = user_group_pill.get_user_ids(pill_widget);
return [...user_ids, ...stream_user_ids, ...group_user_ids]; return [...user_ids, ...stream_user_ids, ...group_user_ids];
} }
@@ -251,8 +263,22 @@ export function set_up_handlers({
*/ */
function callback(): void { function callback(): void {
const pill_widget = get_pill_widget(); const pill_widget = get_pill_widget();
const pill_user_ids = get_pill_user_ids(pill_widget); void (async () => {
loading.make_indicator($(".add-subscriber-loading-spinner"), {
height: 56, // 4em at 14px / 1em
});
const pill_user_ids = await get_pill_user_ids(pill_widget);
// If we're no longer in the same view after fetching
// subscriber data, don't update the UI. We don't need
// to destroy the loading spinner because the tab re-renders
// every time it opens, and also there might be a new tab
// with a current loading spinner.
if (get_pill_widget() !== pill_widget) {
return;
}
loading.destroy_indicator($(".add-subscriber-loading-spinner"));
action({pill_user_ids}); action({pill_user_ids});
})();
} }
$parent_container.on("keyup", pill_selector, (e) => { $parent_container.on("keyup", pill_selector, (e) => {

View File

@@ -60,12 +60,25 @@ export function get_stream_name_from_item(item: StreamPill): string {
return stream.name; return stream.name;
} }
export function get_user_ids(pill_widget: StreamPillWidget | CombinedPillContainer): number[] { export async function get_user_ids(
let user_ids = pill_widget pill_widget: StreamPillWidget | CombinedPillContainer,
.items() ): Promise<number[]> {
.flatMap((item) => const stream_ids = get_stream_ids(pill_widget);
item.type === "stream" ? peer_data.get_subscribers(item.stream_id) : [], const results = await Promise.all(
stream_ids.map(async (stream_id) => peer_data.get_all_subscribers(stream_id, true)),
); );
const current_stream_ids_in_widget = get_stream_ids(pill_widget);
let user_ids: number[] = [];
for (const [index, stream_id] of stream_ids.entries()) {
const subscribers = results[index]!;
// Double check if the stream pill has been removed from the pill
// widget while we were doing fetches.
if (current_stream_ids_in_widget.includes(stream_id)) {
user_ids = [...user_ids, ...subscribers];
}
}
user_ids = [...new Set(user_ids)]; user_ids = [...new Set(user_ids)];
user_ids.sort((a, b) => a - b); user_ids.sort((a, b) => a - b);
return user_ids; return user_ids;

View File

@@ -147,6 +147,19 @@ h4.user_group_setting_subsection_title {
margin-top: 100px; margin-top: 100px;
} }
#people_to_add_in_group,
#people_to_add,
.member_list_settings_container,
.subscriber_list_settings_container {
display: flex;
flex-direction: column;
}
.add-group-member-loading-spinner,
.add-subscriber-loading-spinner {
align-self: center;
}
.member-list-box, .member-list-box,
.subscriber-list-box { .subscriber-list-box {
text-align: center; text-align: center;

View File

@@ -11,6 +11,8 @@
autocomplete="off" placeholder="{{t 'Filter' }}" /> autocomplete="off" placeholder="{{t 'Filter' }}" />
</div> </div>
<div class="add-subscriber-loading-spinner"></div>
<div class="subscriber-list-box"> <div class="subscriber-list-box">
<div class="subscriber_list_container" data-simplebar data-simplebar-tab-index="-1"> <div class="subscriber_list_container" data-simplebar data-simplebar-tab-index="-1">
<table class="subscriber-list table table-striped"> <table class="subscriber-list table table-striped">

View File

@@ -16,6 +16,7 @@
<input type="text" class="search filter_text_input" placeholder="{{t 'Filter' }}" /> <input type="text" class="search filter_text_input" placeholder="{{t 'Filter' }}" />
</span> </span>
</div> </div>
<div class="add-subscriber-loading-spinner"></div>
<div class="subscriber-list-box"> <div class="subscriber-list-box">
{{> stream_members_table .}} {{> stream_members_table .}}
</div> </div>

View File

@@ -9,6 +9,8 @@
autocomplete="off" placeholder="{{t 'Filter' }}" /> autocomplete="off" placeholder="{{t 'Filter' }}" />
</div> </div>
<div class="add-group-member-loading-spinner"></div>
<div class="member-list-box"> <div class="member-list-box">
<div class="member_list_container" data-simplebar data-simplebar-tab-index="-1"> <div class="member_list_container" data-simplebar data-simplebar-tab-index="-1">
<table class="member-list table table-striped"> <table class="member-list table table-striped">

View File

@@ -18,6 +18,7 @@
<input type="text" class="search filter_text_input" placeholder="{{t 'Filter' }}" /> <input type="text" class="search filter_text_input" placeholder="{{t 'Filter' }}" />
</span> </span>
</div> </div>
<div class="add-group-member-loading-spinner"></div>
<div class="member-list-box"> <div class="member-list-box">
{{> user_group_members_table .}} {{> user_group_members_table .}}
</div> </div>

View File

@@ -132,11 +132,11 @@ run_test("get_stream_id", () => {
assert.equal(stream_pill.get_stream_name_from_item(denmark_pill), denmark.name); assert.equal(stream_pill.get_stream_name_from_item(denmark_pill), denmark.name);
}); });
run_test("get_user_ids", () => { run_test("get_user_ids", async () => {
const items = [denmark_pill, sweden_pill]; const items = [denmark_pill, sweden_pill];
const widget = {items: () => items}; const widget = {items: () => items};
const user_ids = stream_pill.get_user_ids(widget); const user_ids = await stream_pill.get_user_ids(widget);
assert.deepEqual(user_ids, [1, 2, 3, 4, 5, 77]); assert.deepEqual(user_ids, [1, 2, 3, 4, 5, 77]);
}); });