popovers: Fix is-active checks for popovers.

We were incorrectly reporting active bots as non-active in
popovers, and we had no test coverage for cross-realm bots.

We also rename the function to is_active_user_for_popover,
since the old name, realm_user_is_active_human_or_bot, suggested
the wrong semantics for cross-realm bots.

Last but not least, we only do a blueslip warning if a user id
is not found.  When lookups fail, we are pretty confident that
the user is not active, so an error is overkill.  We can change
that as part of issue #7120.

Fixes #7153

frontend_tests/node_tests/people.js

@@ -66,7 +66,7 @@ initialize();
 
     var active_user_ids = people.get_active_user_ids();
     assert.deepEqual(active_user_ids, [isaac.user_id]);
-    assert.equal(people.realm_user_is_active_human_or_bot(isaac.user_id), true);
+    assert.equal(people.is_active_user_for_popover(isaac.user_id), true);
     assert(people.is_valid_email_for_compose(isaac.email));
 
     // Now deactivate isaac
@@ -74,7 +74,7 @@ initialize();
     person = people.get_active_user_for_email(email);
     assert(!person);
     assert.equal(people.get_realm_count(), 0);
-    assert.equal(people.realm_user_is_active_human_or_bot(isaac.user_id), false);
+    assert.equal(people.is_active_user_for_popover(isaac.user_id), false);
     assert.equal(people.is_valid_email_for_compose(isaac.email), false);
 
     var bot_botson = {
@@ -83,11 +83,17 @@ initialize();
         full_name: 'Bot Botson',
         is_bot: true,
     };
-    people.add(bot_botson);
+    people.add_in_realm(bot_botson);
-    assert.equal(people.realm_user_is_active_human_or_bot(bot_botson.user_id), true);
+    assert.equal(people.is_active_user_for_popover(bot_botson.user_id), true);
 
-    // Invalid user ID just returns false
+    // Invalid user ID returns false and warns.
-    assert.equal(people.realm_user_is_active_human_or_bot(123412), false);
+    var message;
+    blueslip.warn = function (msg) {
+        message = msg;
+    };
+
+    assert.equal(people.is_active_user_for_popover(123412), false);
+    assert.equal(message, 'Unexpectedly invalid user_id in user popover query: 123412');
 
     // We can still get their info for non-realm needs.
     person = people.get_by_email(email);
@@ -176,6 +182,8 @@ initialize();
     assert.equal(person.user_id, 42);
 }());
 
+initialize();
+
 (function test_get_rest_of_realm() {
     var alice1 = {
         email: 'alice1@example.com',
@@ -655,6 +663,7 @@ initialize();
     people.initialize();
 
     assert.equal(people.get_active_user_for_email('alice@example.com').full_name, 'Alice');
+    assert.equal(people.is_active_user_for_popover(17), true);
     assert(people.is_cross_realm_email('bot@example.com'));
     assert(people.is_valid_email_for_compose('bot@example.com'));
     assert(people.is_valid_email_for_compose('alice@example.com'));

static/js/people.js

@@ -545,19 +545,24 @@ exports.get_active_user_for_email = function (email) {
     return active_user_dict.get(person.user_id);
 };
 
-exports.realm_user_is_active_human_or_bot = function (id) {
+exports.is_active_user_for_popover = function (user_id) {
-    if (active_user_dict.get(id) !== undefined) {
+    // For popover menus, we include cross-realm bots as active
+    // users.
+
+    if (cross_realm_dict.get(user_id)) {
         return true;
     }
-    // TODO: Technically, we should probably treat deactivated bots
+    if (active_user_dict.has(user_id)) {
-    // like deactivated users here.  But we don't have the data to do
+        return true;
-    // that.  See #7153 for notes on fixing this.
-    var person = exports.get_person_from_user_id(id);
-    if (person === undefined) {
-        blueslip.error("Unexpectedly invalid user ID in user popover query " + id);
-        return false;
     }
-    return !!person.is_bot;
+
+    // TODO: We can report errors here once we start loading
+    //       deactivated users at page-load time. For now just warn.
+    if (!people_by_user_id_dict.has(user_id)) {
+        blueslip.warn("Unexpectedly invalid user_id in user popover query: " + user_id);
+    }
+
+    return false;
 };
 
 exports.get_all_persons = function () {

static/js/popovers.js

@@ -100,7 +100,7 @@ function show_user_info_popover(element, user, message) {
             sent_by_uri: narrow.by_sender_uri(user.email),
             narrowed: narrow_state.active(),
             private_message_class: "respond_personal_button",
-            is_active: people.realm_user_is_active_human_or_bot(user.user_id),
+            is_active: people.is_active_user_for_popover(user.user_id),
         };
 
         var ypos = elt.offset().top;
@@ -448,7 +448,7 @@ exports.register_click_handlers = function () {
             pm_with_uri: narrow.pm_with_uri(user_email),
             sent_by_uri: narrow.by_sender_uri(user_email),
             private_message_class: "compose_private_message",
-            is_active: people.realm_user_is_active_human_or_bot(user_id),
+            is_active: people.is_active_user_for_popover(user_id),
         };
 
         target.popover({