settings_users: Show all bots in organization settings.

We intended to show all the bots in the bots organization settings for
non-admin users as well. This switches from bot_data.all_user_ids() to
people.get_bot_ids() to get a full set of ids for all the bots in the
organization.

Because the source of data changes, "realm_user" instead of "realm_bot"
triggers the update of the bots list.

The code example (example4) is updated since we incorporate a side
effect into "realm_user"'s "add" op.

Note that while "realm_user" does not have a "delete" op, we still stop
redrawing bots on "relam_bot"'s "delete" op, because "delete" was only
triggered when the bot owner changes, the bot does not disappear from
the list of all bots.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This commit is contained in:
Zixuan James Li 2023-01-28 21:00:46 -05:00 committed by Tim Abbott
parent 5ecd0cecf4
commit b21f533af3
4 changed files with 30 additions and 32 deletions

View File

@ -505,14 +505,11 @@ run_test("realm settings", ({override}) => {
run_test("realm_bot add", ({override}) => {
const event = event_fixtures.realm_bot__add;
const bot_stub = make_stub();
const admin_stub = make_stub();
override(bot_data, "add", bot_stub.f);
override(settings_bots, "render_bots", () => {});
override(settings_users, "redraw_bots_list", admin_stub.f);
dispatch(event);
assert.equal(bot_stub.num_calls, 1);
assert.equal(admin_stub.num_calls, 1);
const args = bot_stub.get_args("bot");
assert_same(args.bot, event.bot);
});
@ -520,14 +517,11 @@ run_test("realm_bot add", ({override}) => {
run_test("realm_bot remove", ({override}) => {
const event = event_fixtures.realm_bot__remove;
const bot_stub = make_stub();
const admin_stub = make_stub();
override(bot_data, "deactivate", bot_stub.f);
override(settings_bots, "render_bots", () => {});
override(settings_users, "update_bot_data", admin_stub.f);
dispatch(event);
assert.equal(bot_stub.num_calls, 1);
assert.equal(admin_stub.num_calls, 1);
const args = bot_stub.get_args("user_id");
assert_same(args.user_id, event.bot.user_id);
});
@ -535,37 +529,27 @@ run_test("realm_bot remove", ({override}) => {
run_test("realm_bot delete", ({override}) => {
const event = event_fixtures.realm_bot__delete;
const bot_stub = make_stub();
const admin_stub = make_stub();
override(bot_data, "del", bot_stub.f);
override(settings_bots, "render_bots", () => {});
override(settings_users, "redraw_bots_list", admin_stub.f);
dispatch(event);
assert.equal(bot_stub.num_calls, 1);
const args = bot_stub.get_args("user_id");
assert_same(args.user_id, event.bot.user_id);
assert.equal(admin_stub.num_calls, 1);
});
run_test("realm_bot update", ({override}) => {
const event = event_fixtures.realm_bot__update;
const bot_stub = make_stub();
const admin_stub = make_stub();
override(bot_data, "update", bot_stub.f);
override(settings_bots, "render_bots", () => {});
override(settings_users, "update_bot_data", admin_stub.f);
dispatch(event);
assert.equal(bot_stub.num_calls, 1);
assert.equal(admin_stub.num_calls, 1);
let args = bot_stub.get_args("user_id", "bot");
const args = bot_stub.get_args("user_id", "bot");
assert_same(args.user_id, event.bot.user_id);
assert_same(args.bot, event.bot);
args = admin_stub.get_args("update_user_id", "update_bot_data");
assert_same(args.update_user_id, event.bot.user_id);
});
run_test("realm_emoji", ({override}) => {
@ -643,11 +627,14 @@ 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
@ -655,23 +642,31 @@ 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);
const args = stub.get_args("person");
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");
assert_same(args.update_user_id, event.person.user_id);
});
run_test("restart", ({override}) => {

View File

@ -70,7 +70,7 @@ const bob = {
full_name: "Bob Roberts",
};
run_test("add users with event", () => {
run_test("add users with event", ({override}) => {
people.init();
const event = {
@ -81,6 +81,9 @@ run_test("add users with event", () => {
assert.ok(!people.is_known_user_id(bob.user_id));
// We need to override a stub here before dispatching the event.
// Keep reading to see how overriding works!
override(settings_users, "redraw_bots_list", () => {});
// Let's simulate dispatching our event!
server_events_dispatch.dispatch_normal_event(event);
@ -91,13 +94,13 @@ run_test("add users with event", () => {
/*
It's actually a little surprising that adding a user does
not have side effects beyond the people object. I guess
we don't immediately update the buddy list, but that's
not have side effects beyond the people object and the bots list.
I guess we don't immediately update the buddy list, but that's
because the buddy list gets updated on the next server
fetch.
Let's try an update next. To make this work, we will want
to override some of our stubs.
to override some more of our stubs.
This is where we see a little extra benefit from the
run_test wrapper. It passes us in an object that we
@ -131,6 +134,7 @@ run_test("update user with event", ({override}) => {
override(message_live_update, "update_user_full_name", () => {});
override(pm_list, "update_private_messages", () => {});
override(settings_users, "update_user_data", () => {});
override(settings_users, "update_bot_data", () => {});
// Dispatch the realm_user/update event, which will update
// data structures and have other side effects that are

View File

@ -324,23 +324,19 @@ export function dispatch_normal_event(event) {
case "add":
bot_data.add(event.bot);
settings_bots.render_bots();
settings_users.redraw_bots_list();
break;
case "remove":
bot_data.deactivate(event.bot.user_id);
event.bot.is_active = false;
settings_bots.render_bots();
settings_users.update_bot_data(event.bot.user_id);
break;
case "delete":
bot_data.del(event.bot.user_id);
settings_bots.render_bots();
settings_users.redraw_bots_list();
break;
case "update":
bot_data.update(event.bot.user_id, event.bot);
settings_bots.render_bots();
settings_users.update_bot_data(event.bot.user_id);
break;
default:
blueslip.error("Unexpected event type realm_bot/" + event.op);
@ -430,6 +426,7 @@ export function dispatch_normal_event(event) {
case "add":
people.add_active_user(event.person);
settings_account.maybe_update_deactivate_account_button();
settings_users.redraw_bots_list();
break;
case "remove":
people.deactivate(event.person);
@ -437,10 +434,12 @@ 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();
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();
settings_users.update_bot_data(event.person.user_id);
break;
default:
blueslip.error("Unexpected event type realm_user/" + event.op);

View File

@ -177,19 +177,19 @@ function bot_owner_full_name(owner_id) {
}
function bot_info(bot_user_id) {
const bot_user = bot_data.get(bot_user_id);
const bot_user = people.get_by_user_id(bot_user_id);
if (!bot_user) {
return undefined;
}
const owner_id = bot_user.owner_id;
const owner_id = bot_user.bot_owner_id;
const info = {};
info.is_bot = true;
info.role = people.get_by_user_id(bot_user_id).role;
info.is_active = bot_user.is_active;
info.role = bot_user.role;
info.is_active = people.is_person_active(bot_user.user_id);
info.user_id = bot_user.user_id;
info.full_name = bot_user.full_name;
info.bot_owner_id = owner_id;
@ -254,7 +254,7 @@ section.bots.create_table = () => {
loading.make_indicator($("#admin_page_bots_loading_indicator"), {text: "Loading..."});
const $bots_table = $("#admin_bots_table");
$bots_table.hide();
const bot_user_ids = bot_data.all_user_ids();
const bot_user_ids = people.get_bot_ids();
bot_list_widget = ListWidget.create($bots_table, bot_user_ids, {
name: "admin_bot_list",
@ -378,7 +378,7 @@ export function redraw_bots_list() {
// In order to properly redraw after a user may have been added,
// we need to update the bot_list_widget with the new set of bot
// user IDs to display.
const bot_user_ids = bot_data.all_user_ids();
const bot_user_ids = people.get_bot_ids();
bot_list_widget.replace_list_data(bot_user_ids);
bot_list_widget.hard_redraw();
}