diff --git a/web/src/buddy_data.js b/web/src/buddy_data.js index d1f071432d..12a9247b3e 100644 --- a/web/src/buddy_data.js +++ b/web/src/buddy_data.js @@ -75,8 +75,8 @@ export function compare_function(a, b) { } // Sort equivalent direct message names alphabetically - const person_a = people.get_by_user_id(a); - const person_b = people.get_by_user_id(b); + const person_a = people.maybe_get_user_by_id(a); + const person_b = people.maybe_get_user_by_id(b); const full_name_a = person_a ? person_a.full_name : ""; const full_name_b = person_b ? person_b.full_name : ""; @@ -253,7 +253,7 @@ function filter_user_ids(user_filter_text, user_ids) { // This first filter is for whether the user is eligible to be // displayed in the right sidebar at all. user_ids = user_ids.filter((user_id) => { - const person = people.get_by_user_id(user_id); + const person = people.maybe_get_user_by_id(user_id); if (!person) { blueslip.warn("Got user_id in presence but not people: " + user_id); diff --git a/web/src/compose_validate.js b/web/src/compose_validate.js index e779733693..3aa6e5a615 100644 --- a/web/src/compose_validate.js +++ b/web/src/compose_validate.js @@ -39,7 +39,7 @@ export function needs_subscribe_warning(user_id, stream_id) { // We expect the caller to already have verified that we're // sending to a valid stream and trying to mention the user. - const user = people.get_by_user_id(user_id); + const user = people.maybe_get_user_by_id(user_id); if (!user) { return false; diff --git a/web/src/message_helper.js b/web/src/message_helper.js index bdeffa946f..4ac7cbc91c 100644 --- a/web/src/message_helper.js +++ b/web/src/message_helper.js @@ -29,7 +29,7 @@ export function process_new_message(message) { people.extract_people_from_message(message); people.maybe_incr_recipient_count(message); - const sender = people.get_by_user_id(message.sender_id); + const sender = people.maybe_get_user_by_id(message.sender_id); if (sender) { message.sender_full_name = sender.full_name; message.sender_email = sender.email; diff --git a/web/src/message_store.js b/web/src/message_store.js index 2961b17f08..eaeeef6e69 100644 --- a/web/src/message_store.js +++ b/web/src/message_store.js @@ -39,7 +39,7 @@ export function get_pm_emails(message) { const user_ids = people.pm_with_user_ids(message); const emails = user_ids .map((user_id) => { - const person = people.get_by_user_id(user_id); + const person = people.maybe_get_user_by_id(user_id); if (!person) { blueslip.error("Unknown user id", {user_id}); return "?"; diff --git a/web/src/peer_data.js b/web/src/peer_data.js index ab85c2e4ca..bf37e07a23 100644 --- a/web/src/peer_data.js +++ b/web/src/peer_data.js @@ -98,7 +98,7 @@ export function add_subscriber(stream_id, user_id) { // If stream_id/user_id are unknown to us, we will // still track it, but we will warn. const subscribers = get_user_set(stream_id); - const person = people.get_by_user_id(user_id); + const person = people.maybe_get_user_by_id(user_id); if (person === undefined) { blueslip.warn("We tried to add invalid subscriber: " + user_id); } diff --git a/web/src/people.js b/web/src/people.js index 19534aed68..bc3220c7d3 100644 --- a/web/src/people.js +++ b/web/src/people.js @@ -1,5 +1,6 @@ import md5 from "blueimp-md5"; import {format, utcToZonedTime} from "date-fns-tz"; +import assert from "minimalistic-assert"; import * as typeahead from "../shared/src/typeahead"; @@ -58,9 +59,17 @@ export function get_users_from_ids(user_ids) { return user_ids.map((user_id) => get_by_user_id(user_id)); } -export function get_by_user_id(user_id, ignore_missing) { - if (!people_by_user_id_dict.has(user_id) && !ignore_missing) { - blueslip.error("Unknown user_id in get_by_user_id", {user_id}); +// Use this function only when you are sure that user_id is valid. +export function get_by_user_id(user_id) { + const person = people_by_user_id_dict.get(user_id); + assert(person, `Unknown user_id in get_by_user_id: ${user_id}`); + return person; +} + +// This is type unsafe version of get_by_user_id for the callers that expects undefined values. +export function maybe_get_user_by_id(user_id) { + if (!people_by_user_id_dict.has(user_id)) { + blueslip.error("Unknown user_id in maybe_get_user_by_id", {user_id}); return undefined; } return people_by_user_id_dict.get(user_id); @@ -347,7 +356,7 @@ export function get_full_names_for_poll_option(user_ids) { } export function get_display_full_name(user_id) { - const person = get_by_user_id(user_id); + const person = maybe_get_user_by_id(user_id); if (!person) { blueslip.error("Unknown user id", {user_id}); return "?"; @@ -528,7 +537,7 @@ export function pm_with_url(message) { if (user_ids.length > 1) { suffix = "group"; } else { - const person = get_by_user_id(user_ids[0]); + const person = maybe_get_user_by_id(user_ids[0]); if (person && person.full_name) { suffix = person.full_name.replaceAll(/[ "%/<>`\p{C}]+/gu, "-"); } else { @@ -772,7 +781,7 @@ export function small_avatar_url(message) { // We should always have message.sender_id, except for in the // tutorial, where it's ok to fall back to the URL in the fake // messages. - person = get_by_user_id(message.sender_id); + person = maybe_get_user_by_id(message.sender_id); } // The first time we encounter a sender in a message, we may diff --git a/web/src/popovers.js b/web/src/popovers.js index 8313c1e9c7..6f9144767e 100644 --- a/web/src/popovers.js +++ b/web/src/popovers.js @@ -450,7 +450,7 @@ function get_user_info_popover_manage_menu_items() { function fetch_group_members(member_ids) { return member_ids - .map((m) => people.get_by_user_id(m)) + .map((m) => people.maybe_get_user_by_id(m)) .filter((m) => m !== undefined) .map((p) => ({ ...p, diff --git a/web/src/presence.js b/web/src/presence.js index 58c95fa638..6b240e69a2 100644 --- a/web/src/presence.js +++ b/web/src/presence.js @@ -175,7 +175,7 @@ export function set_info(presences, server_timestamp) { // reload_state.is_in_progress, because races where presence // returns data on users not yet received via the server_events // system are common in both situations. - const person = people.get_by_user_id(user_id, true); + const person = people.maybe_get_user_by_id(user_id); if (person === undefined) { if (!(watchdog.suspects_user_is_offline() || reload_state.is_in_progress())) { // If we're online, and we get a user who we don't diff --git a/web/src/rendered_markdown.js b/web/src/rendered_markdown.js index df05aad7d7..764ebe3df4 100644 --- a/web/src/rendered_markdown.js +++ b/web/src/rendered_markdown.js @@ -107,7 +107,7 @@ export const update_elements = ($content) => { // mention text to show the user's current name, // assuming that you're not searching for text // inside the highlight. - const person = people.get_by_user_id(user_id, true); + const person = people.maybe_get_user_by_id(user_id); if (person !== undefined) { // Note that person might be undefined in some // unpleasant corner cases involving data import. diff --git a/web/src/settings_bots.js b/web/src/settings_bots.js index cb07a7fe4a..cfb4b4d03f 100644 --- a/web/src/settings_bots.js +++ b/web/src/settings_bots.js @@ -340,7 +340,7 @@ export function confirm_bot_deactivation(bot_id, handle_confirm, loading_spinner } export function show_edit_bot_info_modal(user_id, from_user_info_popover) { - const bot = people.get_by_user_id(user_id); + const bot = people.maybe_get_user_by_id(user_id); const owner_id = bot_data.get(user_id).owner_id; const owner_full_name = people.get_full_name(owner_id); diff --git a/web/src/settings_emoji.js b/web/src/settings_emoji.js index 97acc96ca5..fae0bd5e4e 100644 --- a/web/src/settings_emoji.js +++ b/web/src/settings_emoji.js @@ -102,7 +102,7 @@ export function populate_emoji() { for (const emoji of Object.values(emoji_data)) { // Add people.js data for the user here. if (emoji.author_id !== null) { - emoji.author = people.get_by_user_id(emoji.author_id); + emoji.author = people.maybe_get_user_by_id(emoji.author_id); } else { emoji.author = null; } diff --git a/web/src/settings_users.js b/web/src/settings_users.js index 93408538d3..170bebd769 100644 --- a/web/src/settings_users.js +++ b/web/src/settings_users.js @@ -191,7 +191,7 @@ function bot_owner_full_name(owner_id) { return undefined; } - const bot_owner = people.get_by_user_id(owner_id); + const bot_owner = people.maybe_get_user_by_id(owner_id); if (!bot_owner) { return undefined; } @@ -200,7 +200,7 @@ function bot_owner_full_name(owner_id) { } function bot_info(bot_user_id) { - const bot_user = people.get_by_user_id(bot_user_id); + const bot_user = people.maybe_get_user_by_id(bot_user_id); if (!bot_user) { return undefined; @@ -616,7 +616,7 @@ function handle_reactivation($tbody) { } export function show_edit_user_info_modal(user_id, from_user_info_popover) { - const person = people.get_by_user_id(user_id); + const person = people.maybe_get_user_by_id(user_id); if (!person) { return; diff --git a/web/src/stream_create_subscribers_data.js b/web/src/stream_create_subscribers_data.js index 33648755f4..53e4a7f03e 100644 --- a/web/src/stream_create_subscribers_data.js +++ b/web/src/stream_create_subscribers_data.js @@ -40,7 +40,7 @@ export function must_be_subscribed(user_id) { export function add_user_ids(user_ids) { for (const user_id of user_ids) { if (!user_id_set.has(user_id)) { - const user = people.get_by_user_id(user_id); + const user = people.maybe_get_user_by_id(user_id); if (user) { user_id_set.add(user_id); } diff --git a/web/src/user_events.js b/web/src/user_events.js index ca9af2a815..caea60b91b 100644 --- a/web/src/user_events.js +++ b/web/src/user_events.js @@ -22,7 +22,7 @@ import * as settings_streams from "./settings_streams"; import * as settings_users from "./settings_users"; export const update_person = function update(person) { - const person_obj = people.get_by_user_id(person.user_id); + const person_obj = people.maybe_get_user_by_id(person.user_id); if (!person_obj) { blueslip.error("Got update_person event for unexpected user", {user_id: person.user_id}); diff --git a/web/src/user_group_create_members_data.js b/web/src/user_group_create_members_data.js index 66e34973d4..c24a871548 100644 --- a/web/src/user_group_create_members_data.js +++ b/web/src/user_group_create_members_data.js @@ -36,7 +36,7 @@ export function get_potential_members() { export function add_user_ids(user_ids) { for (const user_id of user_ids) { if (!user_id_set.has(user_id)) { - const user = people.get_by_user_id(user_id); + const user = people.maybe_get_user_by_id(user_id); if (user) { user_id_set.add(user_id); } diff --git a/web/tests/buddy_data.test.js b/web/tests/buddy_data.test.js index f85fb47be6..e9bec489f5 100644 --- a/web/tests/buddy_data.test.js +++ b/web/tests/buddy_data.test.js @@ -549,7 +549,7 @@ test("get_items_for_users", () => { test("error handling", () => { presence.presence_info.set(42, {status: "active"}); - blueslip.expect("error", "Unknown user_id in get_by_user_id"); + blueslip.expect("error", "Unknown user_id in maybe_get_user_by_id"); blueslip.expect("warn", "Got user_id in presence but not people: 42"); buddy_data.get_filtered_and_sorted_user_ids(); }); diff --git a/web/tests/compose_validate.test.js b/web/tests/compose_validate.test.js index cbc6cba280..15467b7140 100644 --- a/web/tests/compose_validate.test.js +++ b/web/tests/compose_validate.test.js @@ -616,7 +616,7 @@ test_ui("needs_subscribe_warning", () => { stream_data.add_sub(sub); peer_data.set_subscribers(sub.stream_id, [bob.user_id, me.user_id]); - blueslip.expect("error", "Unknown user_id in get_by_user_id"); + blueslip.expect("error", "Unknown user_id in maybe_get_user_by_id"); // Test with an invalid user id. assert.equal(compose_validate.needs_subscribe_warning(invalid_user_id, sub.stream_id), false); diff --git a/web/tests/message_store.test.js b/web/tests/message_store.test.js index f3b7771d37..32238cb2bf 100644 --- a/web/tests/message_store.test.js +++ b/web/tests/message_store.test.js @@ -212,7 +212,7 @@ test("errors", ({disallow_rewire}) => { display_recipient: [{id: 92714}], }; - blueslip.expect("error", "Unknown user_id in get_by_user_id", 2); + blueslip.expect("error", "Unknown user_id in maybe_get_user_by_id", 2); blueslip.expect("error", "Unknown user id", 2); // From person.js // Expect each to throw two blueslip errors diff --git a/web/tests/peer_data.test.js b/web/tests/peer_data.test.js index 894226ca18..a92704de1b 100644 --- a/web/tests/peer_data.test.js +++ b/web/tests/peer_data.test.js @@ -204,7 +204,7 @@ test("subscribers", () => { blueslip.reset(); // Verify that we don't crash for a bad user id. - blueslip.expect("error", "Unknown user_id in get_by_user_id"); + blueslip.expect("error", "Unknown user_id in maybe_get_user_by_id"); blueslip.expect("warn", "We tried to add invalid subscriber: 88888"); peer_data.add_subscriber(stream_id, 88888); blueslip.reset(); diff --git a/web/tests/people.test.js b/web/tests/people.test.js index 50a849df8a..656a259f82 100644 --- a/web/tests/people.test.js +++ b/web/tests/people.test.js @@ -795,7 +795,7 @@ test_people("message_methods", () => { "https://secure.gravatar.com/avatar/6dbdd7946b58d8b11351fcb27e5cdd55?d=identicon&s=50", ); - blueslip.expect("error", "Unknown user_id in get_by_user_id"); + blueslip.expect("error", "Unknown user_id in maybe_get_user_by_id"); message = { avatar_url: undefined, sender_email: "foo@example.com", diff --git a/web/tests/people_errors.test.js b/web/tests/people_errors.test.js index 03e76ec063..597528cab2 100644 --- a/web/tests/people_errors.test.js +++ b/web/tests/people_errors.test.js @@ -101,7 +101,7 @@ run_test("blueslip", () => { const reply_to = people.pm_reply_to(message); assert.ok(reply_to.includes("?")); - blueslip.expect("error", "Unknown user_id in get_by_user_id"); + blueslip.expect("error", "Unknown user_id in maybe_get_user_by_id"); blueslip.expect("error", "Unknown people in message"); const url = people.pm_with_url({type: "private", display_recipient: [{id: 42}]}); assert.equal(url.indexOf("unk"), url.length - 3); diff --git a/web/tests/presence.test.js b/web/tests/presence.test.js index 8fb2c5362f..89745a0e18 100644 --- a/web/tests/presence.test.js +++ b/web/tests/presence.test.js @@ -95,6 +95,7 @@ test("unknown user", ({override}) => { const presences = {}; presences[unknown_user_id.toString()] = "does-not-matter"; + blueslip.expect("error", "Unknown user_id in maybe_get_user_by_id", 3); blueslip.expect("error", "Unknown user ID in presence data"); presence.set_info(presences, now); diff --git a/web/tests/user_events.test.js b/web/tests/user_events.test.js index 3172bb5880..33ce685b8e 100644 --- a/web/tests/user_events.test.js +++ b/web/tests/user_events.test.js @@ -217,7 +217,7 @@ run_test("updates", () => { assert.ok(person.timezone); blueslip.expect("error", "Got update_person event for unexpected user"); - blueslip.expect("error", "Unknown user_id in get_by_user_id"); + blueslip.expect("error", "Unknown user_id in maybe_get_user_by_id"); assert.ok(!user_events.update_person({user_id: 29, full_name: "Sir Isaac Newton"})); me.profile_data = {};