mirror of https://github.com/zulip/zulip.git
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:
parent
5ecd0cecf4
commit
b21f533af3
|
@ -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}) => {
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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();
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue