user_pill: Use user_id to get user info instead of email.

This commit is contained in:
Aman Agrawal
2025-06-12 10:45:10 +05:30
committed by Tim Abbott
parent 76313f1753
commit b77ecca1a3
8 changed files with 58 additions and 84 deletions

View File

@@ -79,7 +79,7 @@ export function create_item_from_text(
return undefined;
}
return user_pill.create_item_from_email(text, current_items);
return user_pill.create_item_from_user_id(text, current_items);
}
export function create({

View File

@@ -19,7 +19,7 @@ export function create_item_from_text(
const funcs = [
stream_pill.create_item_from_stream_name,
user_group_pill.create_item_from_group_name,
user_pill.create_item_from_email,
user_pill.create_item_from_user_id,
];
for (const func of funcs) {
const item = func(text, current_items);
@@ -40,7 +40,7 @@ export function get_text_from_item(item: CombinedPill): string {
text = user_group_pill.get_group_name_from_item(item);
break;
case "user":
text = user_pill.get_email_from_item(item);
text = user_pill.get_unique_full_name_from_item(item);
break;
}
return text;

View File

@@ -20,8 +20,8 @@ export function initialize_pill(): UserPillWidget {
const pill = input_pill.create({
$container,
pill_config,
create_item_from_text: user_pill.create_item_from_email,
get_text_from_item: user_pill.get_email_from_item,
create_item_from_text: user_pill.create_item_from_user_id,
get_text_from_item: user_pill.get_unique_full_name_from_item,
get_display_value_from_item: user_pill.get_display_value_from_item,
generate_pill_html: (item: UserPill) => user_pill.generate_pill_html(item, true),
});
@@ -66,7 +66,13 @@ export function set_from_typeahead(person: User): void {
export function set_from_emails(value: string): void {
// value is something like "alice@example.com,bob@example.com"
clear();
widget.appendValue(value);
if (value === "") {
return;
}
const user_ids_string = people.emails_strings_to_user_ids_string(value);
if (user_ids_string) {
widget.appendValue(user_ids_string);
}
}
export function set_from_user_ids(value: number[]): void {

View File

@@ -56,7 +56,7 @@ function check_user_allowed_for_setting(
return true;
}
const user = people.get_by_email(user_item.email);
const user = people.maybe_get_user_by_id(user_item.user_id, true);
return user !== undefined && !user.is_guest;
}
@@ -79,7 +79,7 @@ export function create_item_from_text(
return undefined;
}
const user_item = user_pill.create_item_from_email(text, current_items);
const user_item = user_pill.create_item_from_user_id(text, current_items);
if (user_item) {
if (check_user_allowed_for_setting(user_item, setting_name, setting_type)) {
return user_item;
@@ -97,7 +97,7 @@ export function get_text_from_item(item: GroupSettingPill): string {
text = user_group_pill.get_group_name_from_item(item);
break;
case "user":
text = user_pill.get_email_from_item(item);
text = user_pill.get_unique_full_name_from_item(item);
break;
}
return text;

View File

@@ -117,14 +117,10 @@ export function get_users_from_ids(user_ids: number[]): User[] {
}
// Use this function only when you are sure that user_id is valid.
export let get_by_user_id = (user_id: number): User => {
export function get_by_user_id(user_id: number): User {
const person = people_by_user_id_dict.get(user_id);
assert(person, `Unknown user_id in get_by_user_id: ${user_id}`);
return person;
};
export function rewire_get_by_user_id(value: typeof get_by_user_id): void {
get_by_user_id = value;
}
// This is type unsafe version of get_by_user_id for the callers that expects undefined values.
@@ -155,7 +151,7 @@ export function validate_user_ids(user_ids: number[]): number[] {
return good_ids;
}
export let get_by_email = (email: string): User | undefined => {
export function get_by_email(email: string): User | undefined {
const person = people_dict.get(email);
if (!person) {
@@ -169,10 +165,6 @@ export let get_by_email = (email: string): User | undefined => {
}
return person;
};
export function rewire_get_by_email(value: typeof get_by_email): void {
get_by_email = value;
}
export function get_bot_owner_user(user: User & {is_bot: true}): User | undefined {
@@ -1492,6 +1484,14 @@ export function get_from_unique_full_name(query: string): User | undefined {
return undefined;
}
export function get_unique_full_name(full_name: string, user_id: number): string {
let unique_full_name = full_name;
if (is_duplicate_full_name(full_name)) {
unique_full_name += `|${user_id}`;
}
return unique_full_name;
}
export function get_mention_syntax(full_name: string, user_id?: number, silent = false): string {
let mention = "";
if (silent) {

View File

@@ -35,15 +35,13 @@ export type UserPillWidget = InputPillContainer<UserPill>;
export type UserPillData = {type: "user"; user: User};
export function create_item_from_email(
email: string,
export function create_item_from_user_id(
user_id: string,
current_items: CombinedPill[],
pill_config?: InputPillConfig,
): UserPill | undefined {
const user = people.get_by_email(email);
const user = people.maybe_get_user_by_id(Number(user_id), true);
if (!user) {
// The email is not allowed, so return.
return undefined;
}
@@ -84,8 +82,8 @@ export function create_item_from_email(
return item;
}
export function get_email_from_item(item: UserPill): string {
return item.email;
export function get_unique_full_name_from_item(item: UserPill): string {
return people.get_unique_full_name(item.full_name, item.user_id);
}
export function append_person(
@@ -216,8 +214,8 @@ export function create_pills(
const pills = input_pill.create({
$container: $pill_container,
pill_config,
create_item_from_text: create_item_from_email,
get_text_from_item: get_email_from_item,
create_item_from_text: create_item_from_user_id,
get_text_from_item: get_unique_full_name_from_item,
get_display_value_from_item,
generate_pill_html,
});

View File

@@ -18,7 +18,7 @@ let pills = {
pill: {},
};
run_test("pills", ({override, override_rewire}) => {
run_test("pills", ({override}) => {
const me = {
email: "me@example.com",
user_id: 30,
@@ -81,49 +81,18 @@ run_test("pills", ({override, override_rewire}) => {
let appendValue_called;
pills.appendValue = function (value) {
appendValue_called = true;
assert.equal(value, "othello@example.com");
assert.equal(value, othello.user_id.toString());
this.appendValidatedData(othello);
};
let get_by_email_called = false;
override_rewire(people, "get_by_email", (user_email) => {
get_by_email_called = true;
switch (user_email) {
case iago.email:
return iago;
case othello.email:
return othello;
/* istanbul ignore next */
default:
throw new Error(`Unknown user email ${user_email}`);
}
});
let get_by_user_id_called = false;
override_rewire(people, "get_by_user_id", (id) => {
get_by_user_id_called = true;
switch (id) {
case othello.user_id:
return othello;
case hamlet.user_id:
return hamlet;
/* istanbul ignore next */
default:
throw new Error(`Unknown user ID ${id}`);
}
});
function test_create_item(handler) {
(function test_rejection_path() {
const item = handler(othello.email, pills.items());
assert.ok(get_by_email_called);
const item = handler(othello.user_id, pills.items());
assert.equal(item, undefined);
})();
(function test_success_path() {
get_by_email_called = false;
const res = handler(iago.email, pills.items());
assert.ok(get_by_email_called);
const res = handler(iago.user_id, pills.items());
assert.equal(typeof res, "object");
assert.equal(res.user_id, iago.user_id);
assert.equal(res.full_name, iago.full_name);
@@ -131,9 +100,7 @@ run_test("pills", ({override, override_rewire}) => {
(function test_deactivated_pill() {
people.deactivate(iago);
get_by_email_called = false;
const res = handler(iago.email, pills.items());
assert.ok(get_by_email_called);
const res = handler(iago.user_id, pills.items());
assert.equal(typeof res, "object");
assert.equal(res.user_id, iago.user_id);
assert.equal(res.full_name, iago.full_name);
@@ -193,7 +160,6 @@ run_test("pills", ({override, override_rewire}) => {
compose_pm_pill.set_from_emails("othello@example.com");
assert.ok(compose_pm_pill.widget);
assert.ok(get_by_user_id_called);
assert.ok(pills_cleared);
assert.ok(appendValue_called);
assert.ok(text_cleared);

View File

@@ -27,11 +27,10 @@ const isaac = {
full_name: "Isaac Newton",
};
const bogus_item = {
email: "bogus@example.com",
type: "user",
full_name: undefined,
// status_emoji_info: undefined,
const isaac_duplicate = {
email: "isaac_duplicate@example.com",
user_id: 102102,
full_name: "Isaac Newton",
};
const isaac_item = {
@@ -73,29 +72,34 @@ function test(label, f) {
}
test("create_item", ({override}) => {
function test_create_item(email, current_items, expected_item, pill_config) {
const item = user_pill.create_item_from_email(email, current_items, pill_config);
function test_create_item(user_id, current_items, expected_item, pill_config) {
const item = user_pill.create_item_from_user_id(user_id, current_items, pill_config);
assert.deepEqual(item, expected_item);
}
settings_data.user_can_access_all_other_users = () => false;
test_create_item("bogus@example.com", [bogus_item], undefined);
test_create_item("bogus@example.com", [], undefined);
test_create_item("isaac@example.com", [], isaac_item);
test_create_item("isaac@example.com", [isaac_item], undefined);
test_create_item(isaac_item.user_id.toString(), [], isaac_item);
test_create_item(isaac_item.user_id.toString(), [isaac_item], undefined);
override(realm, "realm_bot_domain", "example.com");
people.add_inaccessible_user(inaccessible_user_id);
test_create_item("user103@example.com", [], undefined, {exclude_inaccessible_users: true});
test_create_item("user103@example.com", [], inaccessible_user_item, {
test_create_item(inaccessible_user_id.toString(), [], undefined, {
exclude_inaccessible_users: true,
});
test_create_item(inaccessible_user_id.toString(), [], inaccessible_user_item, {
exclude_inaccessible_users: false,
});
});
test("get_email", () => {
assert.equal(user_pill.get_email_from_item({email: "foo@example.com"}), "foo@example.com");
test("get_unique_full_name_from_item", () => {
people.add_active_user(isaac);
people.add_active_user(isaac_duplicate);
assert.equal(
user_pill.get_unique_full_name_from_item({user_id: 1, full_name: isaac.full_name}),
"Isaac Newton|1",
);
});
test("append", () => {
@@ -130,14 +134,14 @@ test("append", () => {
});
test("get_items", () => {
const items = [isaac_item, bogus_item];
const items = [isaac_item];
pill_widget.items = () => items;
assert.deepEqual(user_pill.get_user_ids(pill_widget), [isaac.user_id]);
});
test("typeahead", () => {
const items = [isaac_item, bogus_item];
const items = [isaac_item];
pill_widget.items = () => items;
// Both alice and isaac are in our realm, but isaac will be