server_events_dispatch: Update bots list only if needed.

After b21f533af, we now update the organization bots
list on receiving realm_user events since non-admins
can also see bots not owned by them in the list.

But the functions to update bot list should be called
only for bot users and not for others, otherwise it
results in an error.

This can be reproduced by first opening the organization
bots list and then just updating the name of the user.
Calling `redraw_bots_list` on receiving "realm_user/add"
event for non bot users will not raise any error but
still we avoid redrawing the whole list when not required.

This commit fixes the code to call functions to update
bot list only when the event is received for a bot.

web/src/people.js

@@ -678,6 +678,11 @@ export function sender_is_guest(message) {
     return false;
 }
 
+export function user_is_bot(user_id) {
+    const user = get_by_user_id(user_id);
+    return user.is_bot;
+}
+
 function gravatar_url_for_email(email) {
     const hash = md5(email.toLowerCase());
     const avatar_url = "https://secure.gravatar.com/avatar/" + hash + "?d=identicon";

web/src/server_events_dispatch.js

@@ -435,7 +435,9 @@ export function dispatch_normal_event(event) {
                 case "add":
                     people.add_active_user(event.person);
                     settings_account.maybe_update_deactivate_account_button();
+                    if (event.person.is_bot) {
                         settings_users.redraw_bots_list();
+                    }
                     break;
                 case "remove":
                     people.deactivate(event.person);
@@ -443,12 +445,16 @@ export function dispatch_normal_event(event) {
                     settings_users.update_view_on_deactivate(event.person.user_id);
                     buddy_list.maybe_remove_key({key: event.person.user_id});
                     settings_account.maybe_update_deactivate_account_button();
+                    if (people.user_is_bot(event.person.user_id)) {
                         settings_users.update_bot_data(event.person.user_id);
+                    }
                     break;
                 case "update":
                     user_events.update_person(event.person);
                     settings_account.maybe_update_deactivate_account_button();
+                    if (people.user_is_bot(event.person.user_id)) {
                         settings_users.update_bot_data(event.person.user_id);
+                    }
                     break;
                 default:
                     blueslip.error("Unexpected event type realm_user/" + event.op);

web/tests/dispatch.test.js

@@ -661,14 +661,11 @@ run_test("realm_domains", ({override}) => {
 run_test("realm_user", ({override}) => {
     override(settings_account, "maybe_update_deactivate_account_button", noop);
     let event = event_fixtures.realm_user__add;
-    const add_admin_stub = make_stub();
-    override(settings_users, "redraw_bots_list", add_admin_stub.f);
     dispatch({...event});
     const added_person = people.get_by_user_id(event.person.user_id);
     // sanity check a few individual fields
     assert.equal(added_person.full_name, "Test User");
     assert.equal(added_person.timezone, "America/New_York");
-    assert.equal(add_admin_stub.num_calls, 1);
 
     // ...but really the whole struct gets copied without any
     // manipulation
@@ -676,30 +673,43 @@ run_test("realm_user", ({override}) => {
 
     assert.ok(people.is_active_user_for_popover(event.person.user_id));
 
-    const remove_admin_stub = make_stub();
     event = event_fixtures.realm_user__remove;
     override(stream_events, "remove_deactivated_user_from_all_streams", noop);
     override(settings_users, "update_view_on_deactivate", noop);
-    override(settings_users, "update_bot_data", remove_admin_stub.f);
     dispatch(event);
 
     // We don't actually remove the person, we just deactivate them.
     const removed_person = people.get_by_user_id(event.person.user_id);
     assert.equal(removed_person.full_name, "Test User");
     assert.ok(!people.is_active_user_for_popover(event.person.user_id));
-    assert.equal(remove_admin_stub.num_calls, 1);
 
     event = event_fixtures.realm_user__update;
     const stub = make_stub();
-    const update_admin_stub = make_stub();
     override(user_events, "update_person", stub.f);
-    override(settings_users, "update_bot_data", update_admin_stub.f);
     dispatch(event);
     assert.equal(stub.num_calls, 1);
-    assert.equal(update_admin_stub.num_calls, 1);
     let args = stub.get_args("person");
     assert_same(args.person, event.person);
-    args = update_admin_stub.get_args("update_user_id", "update_bot_data");
+
+    // Test bot related functions are being called.
+    const add_bot_stub = make_stub();
+    event = event_fixtures.realm_user__add_bot;
+    override(settings_users, "redraw_bots_list", add_bot_stub.f);
+    dispatch({...event});
+    assert.equal(add_bot_stub.num_calls, 1);
+
+    const remove_bot_stub = make_stub();
+    event = event_fixtures.realm_user__remove;
+    override(settings_users, "update_bot_data", remove_bot_stub.f);
+    dispatch(event);
+    assert.equal(remove_bot_stub.num_calls, 1);
+
+    const update_bot_stub = make_stub();
+    event = event_fixtures.realm_user__update;
+    override(settings_users, "update_bot_data", update_bot_stub.f);
+    dispatch(event);
+    assert.equal(update_bot_stub.num_calls, 1);
+    args = update_bot_stub.get_args("update_user_id", "update_bot_data");
     assert_same(args.update_user_id, event.person.user_id);
 });
 

web/tests/example4.test.js

@@ -68,6 +68,7 @@ const bob = {
     email: "bob@example.com",
     user_id: 33,
     full_name: "Bob Roberts",
+    is_bot: true,
 };
 
 run_test("add users with event", ({override}) => {
@@ -117,6 +118,7 @@ run_test("update user with event", ({override}) => {
         email: "bob@example.com",
         user_id: bob.user_id,
         full_name: "The Artist Formerly Known as Bob",
+        is_bot: true,
     };
 
     const event = {

web/tests/lib/events.js

@@ -544,6 +544,27 @@ exports.fixtures = {
         },
     },
 
+    realm_user__add_bot: {
+        type: "realm_user",
+        op: "add",
+        person: {
+            ...test_user,
+            avatar_url: "/some/path/to/avatar",
+            avatar_version: 1,
+            is_admin: false,
+            is_active: true,
+            is_owner: false,
+            is_billing_admin: false,
+            role: 400,
+            is_bot: true,
+            is_guest: false,
+            profile_data: {},
+            timezone: "America/New_York",
+            date_joined: "2020-01-01",
+            delivery_email: "test-delivery@example.com",
+        },
+    },
+
     realm_user__remove: {
         type: "realm_user",
         op: "remove",