people: Make `get_by_user_id` type-safe.

We should make `get_by_user_id` type-safe in the sense that it's caller
should not expect undefined values and hence it's caller should not
need to validate the user ids.

To do this I extracted a method `maybe_get_user_by_id` which is
type-unsafe version of this function which will allow undefined values
and will behave exactly the same as the previous version. So the callers
which expects the undefined values from this function should use this
new function `maybe_get_user_by_id`.

Probably about half of these callers are implicitly iterating through
all users in the people.js data set and thus by construction should
never fail, but it's simpler to just convert all existing callers than
do that audit.

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_a = people.maybe_get_user_by_id(a);
-    const person_b = people.get_by_user_id(b);
+    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);

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;

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;

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 "?";

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);
     }

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) {
+// Use this function only when you are sure that user_id is valid.
-    if (!people_by_user_id_dict.has(user_id) && !ignore_missing) {
+export function get_by_user_id(user_id) {
-        blueslip.error("Unknown user_id in 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

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,

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

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.

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);
 

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;
         }

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;

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);
             }

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});

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);
             }

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();
 });

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);
 

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

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();

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",

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);

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);
 

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 = {};