buddy_data: Add participants to the top of sorted user list.

This was causing a bug where the participants weren't necessarily
all getting rendered, specifically when there were many subscribers
to a channel, because we render users in batches as buddy list
scrolls, and those users would only show up after some scrolling.

This fix makes sure we always load the participants first.
This commit is contained in:
evykassirer 2024-10-01 22:09:29 -07:00 committed by Tim Abbott
parent 8e5672dd98
commit 9c8cc8c333
4 changed files with 58 additions and 10 deletions

View File

@ -90,7 +90,17 @@ export function compare_function(
b: number,
current_sub: StreamSubscription | undefined,
pm_ids: Set<number>,
conversation_participants: Set<number>,
): number {
const a_is_participant = conversation_participants.has(a);
const b_is_participant = conversation_participants.has(b);
if (a_is_participant && !b_is_participant) {
return -1;
}
if (!a_is_participant && b_is_participant) {
return 1;
}
const a_would_receive_message = user_matches_narrow(a, pm_ids, current_sub?.stream_id);
const b_would_receive_message = user_matches_narrow(b, pm_ids, current_sub?.stream_id);
if (a_would_receive_message && !b_would_receive_message) {
@ -117,11 +127,13 @@ export function compare_function(
return util.strcmp(full_name_a, full_name_b);
}
export function sort_users(user_ids: number[]): number[] {
export function sort_users(user_ids: number[], conversation_participants: Set<number>): number[] {
// TODO sort by unread count first, once we support that
const current_sub = narrow_state.stream_sub();
const pm_ids_set = narrow_state.pm_ids_set();
user_ids.sort((a, b) => compare_function(a, b, current_sub, pm_ids_set));
user_ids.sort((a, b) =>
compare_function(a, b, current_sub, pm_ids_set, conversation_participants),
);
return user_ids;
}
@ -430,7 +442,7 @@ export function get_filtered_and_sorted_user_ids(user_filter_text: string): numb
const conversation_participants = get_conversation_participants();
user_ids = get_filtered_user_id_list(user_filter_text, conversation_participants);
user_ids = maybe_shrink_list(user_ids, user_filter_text, conversation_participants);
return sort_users(user_ids);
return sort_users(user_ids, conversation_participants);
}
export function matches_filter(user_filter_text: string, user_id: number): boolean {

View File

@ -776,7 +776,13 @@ export class BuddyList extends BuddyListConf {
const i = user_id_list.findIndex(
(list_user_id) =>
this.compare_function(user_id, list_user_id, current_sub, pm_ids_set) < 0,
this.compare_function(
user_id,
list_user_id,
current_sub,
pm_ids_set,
this.render_data.participant_ids_set,
) < 0,
);
return i === -1 ? user_id_list.length : i;
}

View File

@ -184,7 +184,7 @@ test("sort_users", () => {
presence.presence_info.delete(alice.user_id);
buddy_data.sort_users(user_ids);
buddy_data.sort_users(user_ids, new Set());
assert.deepEqual(user_ids, [fred.user_id, jill.user_id, alice.user_id]);
});

View File

@ -455,18 +455,18 @@ test("compare_function", () => {
peer_data.set_subscribers(stream_id, []);
assert.equal(
second_user_shown_higher,
buddy_data.compare_function(fred.user_id, alice.user_id, sub, new Set()),
buddy_data.compare_function(fred.user_id, alice.user_id, sub, new Set(), new Set()),
);
// Fred is higher because they're in the narrow and Alice isn't.
peer_data.set_subscribers(stream_id, [fred.user_id]);
assert.equal(
first_user_shown_higher,
buddy_data.compare_function(fred.user_id, alice.user_id, sub, new Set()),
buddy_data.compare_function(fred.user_id, alice.user_id, sub, new Set(), new Set()),
);
assert.equal(
second_user_shown_higher,
buddy_data.compare_function(alice.user_id, fred.user_id, sub, new Set()),
buddy_data.compare_function(alice.user_id, fred.user_id, sub, new Set(), new Set()),
);
// Fred is higher because they're in the DM conversation and Alice isn't.
@ -477,19 +477,49 @@ test("compare_function", () => {
alice.user_id,
undefined,
new Set([fred.user_id]),
new Set(),
),
);
// Fred is higher because they're in the conversation and Alice isn't.
assert.equal(
first_user_shown_higher,
buddy_data.compare_function(
fred.user_id,
alice.user_id,
undefined,
new Set(),
new Set([fred.user_id]),
),
);
assert.equal(
second_user_shown_higher,
buddy_data.compare_function(
alice.user_id,
fred.user_id,
undefined,
new Set(),
new Set([fred.user_id]),
),
);
// Alice is higher because of alphabetical sorting.
assert.equal(
second_user_shown_higher,
buddy_data.compare_function(fred.user_id, alice.user_id, undefined, new Set()),
buddy_data.compare_function(fred.user_id, alice.user_id, undefined, new Set(), new Set()),
);
// The user is part of a DM conversation, though that's not explicitly in the DM list.
assert.equal(
first_user_shown_higher,
buddy_data.compare_function(me.user_id, alice.user_id, undefined, new Set([fred.user_id])),
buddy_data.compare_function(
me.user_id,
alice.user_id,
undefined,
new Set([fred.user_id]),
new Set(),
),
);
});