paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-03 21:43:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

typeahead_helper: Rename functions used for sorting.

Browse Source 
d11537a8cd modified how we sort options in typeahead for stream
subscribers and group members. The function names added in that
commit were confusing since the names included "settings" when
the function is used to sort the typehead for adding and updating
stream subscribers and group members. This commit fixes the names
of these functions.

This commit also renames sort_setting_options since that function
is now used for adding and updating stream subscribers and group
members apart from being used for sorting typeahead for group
settings.
This commit is contained in:
Sahil Batra
2025-04-17 16:47:25 +05:30
committed by Tim Abbott
parent 9ebc877bbe
commit b63b484f8d
4 changed files with 78 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
web/src/pill_typeahead.ts
View File

@@ -382,7 +382,7 @@ export function set_up_combined(
}
}
return typeahead_helper.sort_stream_setting_options({
return typeahead_helper.sort_stream_or_group_members_options({
users,
query,
groups,

49
web/src/typeahead_helper.ts
View File

@@ -703,29 +703,27 @@ export function compare_group_setting_options(
return 1;
}
export const sort_setting_options = ({
export const sort_users_and_groups_options = ({
users,
query,
groups,
compare_setting_options,
compare_options,
target_group,
}: {
users: UserPillData[];
query: string;
groups: UserGroupPillData[];
compare_setting_options: (
compare_options: (
option_a: UserPillData | UserGroupPillData,
option_b: UserPillData | UserGroupPillData,
target_group: UserGroup | undefined,
) => number;
target_group: UserGroup | undefined;
}): (UserPillData | UserGroupPillData)[] => {
function sort_setting_items(
function sort_items(
objs: (UserPillData | UserGroupPillData)[],
): (UserPillData | UserGroupPillData)[] {
objs.sort((option_a, option_b) =>
compare_setting_options(option_a, option_b, target_group),
);
objs.sort((option_a, option_b) => compare_options(option_a, option_b, target_group));
return objs;
}
@@ -745,13 +743,13 @@ export const sort_setting_options = ({
return [user_groups.get_display_group_name(g.name)];
});
const exact_matches = sort_setting_items([
const exact_matches = sort_items([
...groups_results.exact_matches,
...users_name_results.exact_matches,
...email_results.exact_matches,
]);
const prefix_matches = sort_setting_items([
const prefix_matches = sort_items([
...groups_results.begins_with_case_sensitive_matches,
...groups_results.begins_with_case_insensitive_matches,
...users_name_results.begins_with_case_sensitive_matches,
@@ -760,16 +758,13 @@ export const sort_setting_options = ({
...email_results.begins_with_case_insensitive_matches,
]);
const word_boundary_matches = sort_setting_items([
const word_boundary_matches = sort_items([
...groups_results.word_boundary_matches,
...users_name_results.word_boundary_matches,
...email_results.word_boundary_matches,
]);
const no_matches = sort_setting_items([
...groups_results.no_matches,
...email_results.no_matches,
]);
const no_matches = sort_items([...groups_results.no_matches, ...email_results.no_matches]);
const getters: {
getter: (UserPillData | UserGroupPillData)[];
@@ -788,18 +783,18 @@ export const sort_setting_options = ({
},
];
const setting_options: (UserPillData | UserGroupPillData)[] = [];
const options: (UserPillData | UserGroupPillData)[] = [];
for (const getter of getters) {
if (setting_options.length >= MAX_ITEMS) {
if (options.length >= MAX_ITEMS) {
break;
}
for (const item of getter.getter) {
setting_options.push(item);
options.push(item);
}
}
return setting_options.slice(0, MAX_ITEMS);
return options.slice(0, MAX_ITEMS);
};
export let sort_group_setting_options = ({
@@ -813,11 +808,11 @@ export let sort_group_setting_options = ({
groups: UserGroupPillData[];
target_group: UserGroup | undefined;
}): (UserPillData | UserGroupPillData)[] =>
sort_setting_options({
sort_users_and_groups_options({
users,
query,
groups,
compare_setting_options: compare_group_setting_options,
compare_options: compare_group_setting_options,
target_group,
});
@@ -825,7 +820,7 @@ export function rewire_sort_group_setting_options(value: typeof sort_group_setti
sort_group_setting_options = value;
}
export function compare_stream_setting_options(
export function compare_stream_or_group_members_options(
option_a: UserPillData | UserGroupPillData,
option_b: UserPillData | UserGroupPillData,
): number {
@@ -890,7 +885,7 @@ export function compare_stream_setting_options(
return 1;
}
export let sort_stream_setting_options = ({
export let sort_stream_or_group_members_options = ({
users,
query,
groups,
@@ -899,18 +894,18 @@ export let sort_stream_setting_options = ({
query: string;
groups: UserGroupPillData[];
}): (UserPillData | UserGroupPillData)[] =>
sort_setting_options({
sort_users_and_groups_options({
users,
query,
groups,
compare_setting_options: compare_stream_setting_options,
compare_options: compare_stream_or_group_members_options,
target_group: undefined,
});
export function rewire_sort_stream_setting_options(
value: typeof sort_stream_setting_options,
export function rewire_sort_stream_or_group_members_options(
value: typeof sort_stream_or_group_members_options,
): void {
sort_stream_setting_options = value;
sort_stream_or_group_members_options = value;
}
type SlashCommand = {

14
web/tests/pill_typeahead.test.cjs
View File

@@ -30,7 +30,7 @@ set_realm(realm);
let sort_recipients_called = false;
let sort_streams_called = false;
let sort_group_setting_options_called = false;
let sort_stream_setting_options_called = false;
let sort_stream_or_group_members_options_called = false;
const $fake_rendered_person = $.create("fake-rendered-person");
const $fake_rendered_stream = $.create("fake-rendered-stream");
const $fake_rendered_group = $.create("fake-rendered-group");
@@ -42,8 +42,8 @@ function override_typeahead_helper(override_rewire) {
override_rewire(typeahead_helper, "sort_streams", () => {
sort_streams_called = true;
});
override_rewire(typeahead_helper, "sort_stream_setting_options", ({users}) => {
sort_stream_setting_options_called = true;
override_rewire(typeahead_helper, "sort_stream_or_group_members_options", ({users}) => {
sort_stream_or_group_members_options_called = true;
return users;
});
}
@@ -528,17 +528,17 @@ run_test("set_up_combined", ({mock_template, override, override_rewire}) => {
}
if (opts.user_group) {
sort_recipients_called = false;
sort_stream_setting_options_called = false;
sort_stream_or_group_members_options_called = false;
config.sorter([testers_item], group_query);
assert.ok(!sort_recipients_called);
assert.ok(sort_stream_setting_options_called);
assert.ok(sort_stream_or_group_members_options_called);
}
if (opts.user) {
sort_recipients_called = false;
sort_stream_setting_options_called = false;
sort_stream_or_group_members_options_called = false;
config.sorter([me_item], person_query);
assert.ok(!sort_recipients_called);
assert.ok(sort_stream_setting_options_called);
assert.ok(sort_stream_or_group_members_options_called);
}
})();

75
web/tests/typeahead_helper.test.cjs
View File

@@ -1281,14 +1281,14 @@ test("compare_group_setting_options", () => {
assert.equal(th.compare_group_setting_options(b_user_1_item, b_user_1_item, bob_group), 0);
});
test("sort_stream_setting_options", ({override_rewire}) => {
function get_stream_setting_typeahead_result(query) {
test("sort_stream_or_group_members_options", ({override_rewire}) => {
function get_stream_or_group_members_typeahead_result(query) {
const users = people.get_realm_active_human_users().map((user) => ({type: "user", user}));
const groups = user_groups.get_all_realm_user_groups().map((group) => ({
type: "user_group",
...group,
}));
const result = th.sort_stream_setting_options({
const result = th.sort_stream_or_group_members_options({
users,
query,
groups,
@@ -1302,7 +1302,7 @@ test("sort_stream_setting_options", ({override_rewire}) => {
});
}
assert.deepEqual(get_stream_setting_typeahead_result("Bo"), [
assert.deepEqual(get_stream_or_group_members_typeahead_result("Bo"), [
bob_group.name,
second_bob_group.name,
b_user_1.full_name,
@@ -1316,7 +1316,7 @@ test("sort_stream_setting_options", ({override_rewire}) => {
everyone_group.name,
]);
assert.deepEqual(get_stream_setting_typeahead_result("bo"), [
assert.deepEqual(get_stream_or_group_members_typeahead_result("bo"), [
bob_group.name,
second_bob_group.name,
b_user_1.full_name,
@@ -1330,7 +1330,7 @@ test("sort_stream_setting_options", ({override_rewire}) => {
everyone_group.name,
]);
assert.deepEqual(get_stream_setting_typeahead_result("Z"), [
assert.deepEqual(get_stream_or_group_members_typeahead_result("Z"), [
zman.full_name,
admins_group.name,
a_user.full_name,
@@ -1344,7 +1344,7 @@ test("sort_stream_setting_options", ({override_rewire}) => {
everyone_group.name,
]);
assert.deepEqual(get_stream_setting_typeahead_result("me"), [
assert.deepEqual(get_stream_or_group_members_typeahead_result("me"), [
members_group.name,
admins_group.name,
bob_group.name,
@@ -1358,7 +1358,7 @@ test("sort_stream_setting_options", ({override_rewire}) => {
everyone_group.name,
]);
assert.deepEqual(get_stream_setting_typeahead_result("ever"), [
assert.deepEqual(get_stream_or_group_members_typeahead_result("ever"), [
members_group.name,
everyone_group.name,
admins_group.name,
@@ -1372,7 +1372,7 @@ test("sort_stream_setting_options", ({override_rewire}) => {
bob_system_group.name,
]);
assert.deepEqual(get_stream_setting_typeahead_result("translated: members"), [
assert.deepEqual(get_stream_or_group_members_typeahead_result("translated: members"), [
members_group.name,
admins_group.name,
bob_group.name,
@@ -1387,7 +1387,7 @@ test("sort_stream_setting_options", ({override_rewire}) => {
]);
override_rewire(bootstrap_typeahead, "MAX_ITEMS", 6);
assert.deepEqual(get_stream_setting_typeahead_result("Bo"), [
assert.deepEqual(get_stream_or_group_members_typeahead_result("Bo"), [
bob_group.name,
second_bob_group.name,
b_user_1.full_name,
@@ -1397,33 +1397,54 @@ test("sort_stream_setting_options", ({override_rewire}) => {
]);
});
test("compare_stream_setting_options", () => {
test("compare_stream_or_group_members_options", () => {
// Non system user group has higher priority than user.
assert.equal(th.compare_stream_setting_options(a_user_item, bob_group_item), 1);
assert.equal(th.compare_stream_setting_options(bob_group_item, a_user_item), -1);
assert.equal(th.compare_stream_or_group_members_options(a_user_item, bob_group_item), 1);
assert.equal(th.compare_stream_or_group_members_options(bob_group_item, a_user_item), -1);
// User has higher priority than non `role:members` system user group.
assert.equal(th.compare_stream_setting_options(a_user_item, bob_system_group_item), -1);
assert.equal(th.compare_stream_setting_options(bob_system_group_item, a_user_item), 1);
assert.equal(
th.compare_stream_or_group_members_options(a_user_item, bob_system_group_item),
-1,
);
assert.equal(th.compare_stream_or_group_members_options(bob_system_group_item, a_user_item), 1);
// System user group has lower priority than other user groups.
assert.equal(th.compare_stream_setting_options(bob_group_item, bob_system_group_item), -1);
assert.equal(th.compare_stream_setting_options(bob_system_group_item, bob_group_item), 1);
assert.equal(th.compare_stream_setting_options(admins_group_item, bob_system_group_item), -1);
assert.equal(
th.compare_stream_or_group_members_options(bob_group_item, bob_system_group_item),
-1,
);
assert.equal(
th.compare_stream_or_group_members_options(bob_system_group_item, bob_group_item),
1,
);
assert.equal(
th.compare_stream_or_group_members_options(admins_group_item, bob_system_group_item),
-1,
);
// Members group always takes priority against any other group.
assert.equal(th.compare_stream_setting_options(bob_group_item, members_group_item), 1);
assert.equal(th.compare_stream_setting_options(members_group_item, bob_group_item), -1);
assert.equal(th.compare_stream_setting_options(admins_group_item, members_group_item), 1);
assert.equal(th.compare_stream_setting_options(members_group_item, admins_group_item), -1);
assert.equal(th.compare_stream_or_group_members_options(bob_group_item, members_group_item), 1);
assert.equal(
th.compare_stream_or_group_members_options(members_group_item, bob_group_item),
-1,
);
assert.equal(
th.compare_stream_or_group_members_options(admins_group_item, members_group_item),
1,
);
assert.equal(
th.compare_stream_or_group_members_options(members_group_item, admins_group_item),
-1,
);
// In case both groups are not system groups, alphabetical order is used to decide priority.
assert.equal(th.compare_stream_setting_options(bob_group_item, admins_group_item), 1);
assert.equal(th.compare_stream_setting_options(admins_group_item, bob_group_item), -1);
assert.equal(th.compare_stream_or_group_members_options(bob_group_item, admins_group_item), 1);
assert.equal(th.compare_stream_or_group_members_options(admins_group_item, bob_group_item), -1);
// Use alphabetical order to compare two users.
assert.equal(th.compare_stream_setting_options(b_user_1_item, b_user_2_item), -1);
assert.equal(th.compare_stream_setting_options(b_user_2_item, b_user_1_item), 1);
assert.equal(th.compare_stream_or_group_members_options(b_user_1_item, b_user_2_item), -1);
assert.equal(th.compare_stream_or_group_members_options(b_user_2_item, b_user_1_item), 1);
// Get coverage for case where two users have same names. Original order is preserved
// in such cases.
assert.equal(th.compare_stream_setting_options(b_user_1_item, b_user_1_item), 0);
assert.equal(th.compare_stream_or_group_members_options(b_user_1_item, b_user_1_item), 0);
});