refactor: Make bot owner hovers more robust.

This is code simplification motivated
by a recent bug that we fixed with some
server changes, but which was really
caused in some sense by our client code
using an overly finicky
condition to check falsiness.

For cross-realm bots, the value of
`user.bot_owner_id` may be `null`, or it
may simply be `undefined`, depending
on whether the server passes `None`
or simply omits the field.

We don't want out client code to be
coupled to that rather arbitrary
decision.

We were doing a `!== null` check instead
of checking for falsiness, which led to
blueslip errors in the past.  Because a
bot owner id could be plausibly 0, a falsiness
check would be brittle in a different way.

Now we avoid that ugliness by calling
`get_bot_owner_user`, which either returns
an object or `undefined`.

And then the caller can just do a concise
check for whether `bot_owner` exists.

And we also fix up the crufty code that
was putting `bot_owner_full_name` on to
the object instead of using a local.

We have a bug report for this again, although
it might be on an old branch.

Fixes #13621.

frontend_tests/node_tests/people.js

@@ -18,6 +18,14 @@ function set_email_visibility(code) {
 
 set_email_visibility(admins_only);
 
+const welcome_bot = {
+    email: 'welcome-bot@example.com',
+    user_id: 4,
+    full_name: 'Welcome Bot',
+    is_bot: true,
+    // cross realm bots have no owner
+};
+
 const me = {
     email: 'me@example.com',
     user_id: 30,
@@ -112,10 +120,26 @@ run_test('basics', () => {
         user_id: 35,
         full_name: 'Bot Botson',
         is_bot: true,
+        bot_owner_id: isaac.user_id,
     };
     people.add(bot_botson);
     assert.equal(people.is_active_user_for_popover(bot_botson.user_id), true);
 
+    assert.equal(
+        people.get_bot_owner_user(bot_botson).full_name,
+        'Isaac Newton'
+    );
+
+    // Add our cross-realm bot.  It won't add to our human
+    // count, and it has no owner.
+    people._add_user(welcome_bot);
+    assert.equal(people.get_bot_owner_user(welcome_bot), undefined);
+    assert.equal(people.get_active_human_count(), 1);
+    assert.equal(
+        people.get_by_email(welcome_bot.email).full_name,
+        'Welcome Bot'
+    );
+
     // get_realm_users() will include our active bot,
     // but will exclude isaac (who is deactivated)
     assert.deepEqual(

static/js/buddy_data.js

@@ -218,12 +218,13 @@ exports.get_title_data = function (user_ids_string, is_group) {
     const person = people.get_by_user_id(user_id);
 
     if (person.is_bot) {
-        // Bot has an owner.
-        if (person.bot_owner_id !== null) {
-            person.bot_owner_full_name = people.get_by_user_id(
-                person.bot_owner_id).full_name;
+        const bot_owner = people.get_bot_owner_user(person);
 
-            const bot_owner_name = i18n.t('Owner: __name__', {name: person.bot_owner_full_name});
+        if (bot_owner) {
+            const bot_owner_name = i18n.t(
+                'Owner: __name__',
+                {name: bot_owner.full_name}
+            );
 
             return {
                 first_line: person.full_name,

static/js/people.js

@@ -42,6 +42,7 @@ function split_to_ints(lst) {
     return lst.split(',').map(s => parseInt(s, 10));
 }
 
+
 exports.get_by_user_id = function (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);
@@ -67,6 +68,17 @@ exports.get_by_email = function (email) {
     return person;
 };
 
+exports.get_bot_owner_user = function (user) {
+    const owner_id = user.bot_owner_id;
+
+    if (owner_id === undefined || owner_id === null) {
+        // This is probably a cross-realm bot.
+        return;
+    }
+
+    return exports.get_by_user_id(owner_id);
+};
+
 exports.id_matches_email_operand = function (user_id, email) {
     const person = exports.get_by_email(email);