From f6b11760457877d5e9d83bb0cc5145de6826906f Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Sun, 10 May 2020 10:52:27 +0000 Subject: [PATCH] bot settings: Use `get_item` in the list widget. Our `list_render` list widget gives us the option to use ids as our "list" and then hydrate that list on-demand with an `opts.get_item` function. We now use that for the bots list, passing in `bot_info` as that option. And, importantly, we are now actually hydrating the bot data from `bot_data.js` data structures, and not `/json/bots`. Using the `get_item` scheme has a couple benefits: - Our sort functions are based on the actual items that we use to build the template, so there's a bit less code duplication. (Generally, the data that we pass in to the template is "finalized" in some sense, such as the bot owner name.) - We are less likely to display stale data. - We are less likely to wire up filters to intermediate data elements that are not actually displayed to users (think of email vs. delivery_email). We do rely on `get_item` (i.e. `bot_info`) to be inexpensive, which it should be. Note that we haven't completely decoupled ourselves from `/json/bots`, which we still use as our source for bot user_ids. We will fix that in the next commit. --- static/js/settings_users.js | 48 +++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/static/js/settings_users.js b/static/js/settings_users.js index 2863ddddea..ea36679eed 100644 --- a/static/js/settings_users.js +++ b/static/js/settings_users.js @@ -28,6 +28,14 @@ function sort_email(a, b) { ); } +function sort_bot_email(a, b) { + function email(bot) { + return (bot.display_email || '').toLowerCase(); + } + + return compare_a_b(email(a), email(b)); +} + function sort_role(a, b) { function role(user) { if (user.is_admin) { return 0; } @@ -38,18 +46,8 @@ function sort_role(a, b) { } function sort_bot_owner(a, b) { - function owner_name(item) { - const owner = people.get_bot_owner_user(item); - - if (!owner) { - return ''; - } - - if (!owner.full_name) { - return ''; - } - - return owner.full_name.toLowerCase(); + function owner_name(bot) { + return (bot.bot_owner_full_name || '').toLowerCase(); } return compare_a_b( @@ -167,8 +165,14 @@ function bot_owner_full_name(owner_id) { return bot_owner.full_name; } -function bot_info(bot_user) { - const owner_id = bot_user.bot_owner_id; +function bot_info(bot_user_id) { + const bot_user = bot_data.get(bot_user_id); + + if (!bot_user) { + return; + } + + const owner_id = bot_user.owner_id; const info = {}; @@ -235,24 +239,26 @@ function human_info(person) { section.bots.create_table = (bots) => { const $bots_table = $("#admin_bots_table"); - list_render.create($bots_table, bots, { + const bot_user_ids = bots.map(b => b.user_id); + list_render.create($bots_table, bot_user_ids, { name: "admin_bot_list", - modifier: function (bot_user) { - const info = bot_info(bot_user); - return render_admin_user_list(info); - }, + get_item: bot_info, + modifier: render_admin_user_list, filter: { element: $bots_table.closest(".settings-section").find(".search"), predicate: function (item, value) { + if (!item) { + return false; + } return item.full_name.toLowerCase().includes(value) || - item.email.toLowerCase().includes(value); + item.display_email.toLowerCase().includes(value); }, onupdate: reset_scrollbar($bots_table), }, parent_container: $("#admin-bot-list").expectOne(), init_sort: ['alphabetic', 'full_name'], sort_fields: { - email: sort_email, + email: sort_bot_email, bot_owner: sort_bot_owner, }, });