Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

user settings: Separate code for bot form. 

When editing a bot, there are only two fields
that are similar to humans--full name and
email--which are trivial.

Before this commit we used a single codepath
to build the human form and the bot form.

Now we have two simple codepaths.

The tricky nature of the code had already led
to ugly things for the bot codepath that
fortunately weren't user facing, but which
were distracting:

    - For bots we would needlessly set things
      like is_admin, is_guest in the template
      data.

    - For bots we would needlessly try to update
      custom profile fields.

The code that differs between bots and humans
is nontrivial, and the code was both hard to read
and hard to improve:

    - Humans don't have bot owners.

    - Bots don't have custom profile fields.

The bot-owner code is nontrivial for performance
reasons.  In a big realm there are tens of thousands
of potential bot owners.  We avoid the most egregious
performance problems (i.e we don't have multiple
copies of the dropdown), but we may still want
to refine that (at least adding a spinner).

The custom-profile-fields code is nontrivial due
to the dynamic nature of custom profile fields,
which can bring in specialized widgets like
pill fields.

Now each form corresponds to a single endpoint:

    * human -> /json/users
    * bot -> /json/bots

Before we had a lot of conditional logic in
the template, the code to build to views, and
the code to submit the data.  Now everything is
much flatter.

The human code is still a bit messy (more work
coming on that), but the bot code is fairly
pristine.  All three components of the bot code
fit on a page, and there are no conditionals:

    - admin_bot_form.hbs
    - open_bot_form
    - handle_bot_form

We may want to grow out the bot code a bit
to allow admins to do more things, such as
adding services, and this will be easier now.
It would also be easier for us now to share
widgets with the per-user bot settings.

Note that the form for editing human data will
continue to be invoked from two panels:

    - Users
    - Deactivated users

There are some minor differences between
users and deactivated users, but the shape of
the data is the same for both, so that's still
all one codepath.

We eliminate `reset_edit_user` here, since
it was never used.

One nice thing about these forms was that they
had very little custom CSS attached to them
(at form-level specificity), and it turned out
all the custom CSS was for the human-specific
form.
This commit is contained in:
Steve Howell 2020-05-09 10:13:03 +00:00 committed by Tim Abbott
parent f2ee1a1a65
commit d5cadbcec2
4 changed files with 118 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

132
static/js/settings_users.js
View File

@ -1,7 +1,8 @@
const settings_data = require("./settings_data");
const render_admin_user_list = require("../templates/admin_user_list.hbs");
const render_bot_owner_select = require("../templates/bot_owner_select.hbs");
const render_user_info_form_modal = require('../templates/user_info_form_modal.hbs');
const render_admin_human_form = require('../templates/admin_human_form.hbs');
const render_admin_bot_form = require('../templates/admin_bot_form.hbs');
const meta = {
loaded: false,
@ -325,31 +326,41 @@ exports.set_up = function () {
});
};
function open_user_info_form_modal(person) {
const html = render_user_info_form_modal({
function open_human_form(person) {
const html = render_admin_human_form({
user_id: person.user_id,
email: person.email,
full_name: people.get_full_name(person.user_id),
is_admin: person.is_admin,
is_guest: person.is_guest,
is_member: !person.is_admin && !person.is_guest,
is_bot: person.is_bot,
});
const user_info_form_modal = $(html);
const div = $(html);
const modal_container = $('#user-info-form-modal-container');
modal_container.empty().append(user_info_form_modal);
overlays.open_modal('#user-info-form-modal');
modal_container.empty().append(div);
overlays.open_modal('#admin-human-form');
if (person.is_bot) {
// Dynamically add the owner select control in order to
// avoid performance issues in case of large number of users.
const users_list = people.get_active_humans();
const owner_select = $(render_bot_owner_select({users_list: users_list}));
owner_select.val(bot_data.get(person.user_id).owner || "");
modal_container.find(".edit_bot_owner_container").append(owner_select);
}
return div;
}
return user_info_form_modal;
function open_bot_form(person) {
const html = render_admin_bot_form({
user_id: person.user_id,
email: person.email,
full_name: people.get_full_name(person.user_id),
});
const div = $(html);
const modal_container = $('#user-info-form-modal-container');
modal_container.empty().append(div);
overlays.open_modal('#admin-bot-form');
// NOTE: building `users_list` is quite expensive!
const users_list = people.get_active_humans();
const owner_select = $(render_bot_owner_select({users_list: users_list}));
owner_select.val(bot_data.get(person.user_id).owner || "");
modal_container.find(".edit_bot_owner_container").append(owner_select);
return div;
}
function handle_deactivation() {
@ -451,8 +462,8 @@ function handle_bot_owner_profile() {
});
}
function handle_user_form() {
$(".admin_user_table, .admin_bot_table").on("click", ".open-user-form", function (e) {
function handle_human_form() {
$(".admin_user_table").on("click", ".open-user-form", function (e) {
const user_id = parseInt($(e.currentTarget).attr("data-user-id"), 10);
const person = people.get_by_user_id(user_id);
@ -460,8 +471,8 @@ function handle_user_form() {
return;
}
const user_info_form_modal = open_user_info_form_modal(person);
const element = "#user-info-form-modal .custom-profile-field-form";
const modal = open_human_form(person);
const element = "#admin-human-form .custom-profile-field-form";
$(element).html("");
settings_account.append_custom_profile_fields(element, user_id);
settings_account.initialize_custom_date_type_fields(element);
@ -469,29 +480,15 @@ function handle_user_form() {
user_id,
true, false);
let url;
let data;
const full_name = user_info_form_modal.find("input[name='full_name']");
user_info_form_modal.find('.submit_user_info_change').on("click", function (e) {
modal.find('.submit_human_change').on("click", function (e) {
e.preventDefault();
e.stopPropagation();
const user_role_select_value = user_info_form_modal.find('#user-role-select').val();
const admin_status = get_status_field();
if (person.is_bot) {
url = "/json/bots/" + encodeURIComponent(user_id);
data = {
full_name: full_name.val(),
};
const owner_select_value = user_info_form_modal.find('.bot_owner_select').val();
if (owner_select_value) {
data.bot_owner_id = people.get_by_email(owner_select_value).user_id;
}
} else {
function get_profile_data() {
const new_profile_data = [];
$("#user-info-form-modal .custom_user_field_value").each(function () {
$("#admin-human-form .custom_user_field_value").each(function () {
// Remove duplicate datepicker input element generated flatpicker library
if (!$(this).hasClass("form-control")) {
new_profile_data.push({
@ -511,17 +508,57 @@ function handle_user_form() {
}
}
url = "/json/users/" + encodeURIComponent(user_id);
data = {
full_name: JSON.stringify(full_name.val()),
is_admin: JSON.stringify(user_role_select_value === 'admin'),
is_guest: JSON.stringify(user_role_select_value === 'guest'),
profile_data: JSON.stringify(new_profile_data),
};
return new_profile_data;
}
const user_role_select_value = modal.find('#user-role-select').val();
const full_name = modal.find("input[name='full_name']");
const profile_data = get_profile_data();
const url = "/json/users/" + encodeURIComponent(user_id);
const data = {
full_name: JSON.stringify(full_name.val()),
is_admin: JSON.stringify(user_role_select_value === 'admin'),
is_guest: JSON.stringify(user_role_select_value === 'guest'),
profile_data: JSON.stringify(profile_data),
};
settings_ui.do_settings_change(channel.patch, url, data, admin_status);
overlays.close_modal('#admin-human-form');
});
});
}
function handle_bot_form() {
$(".admin_bot_table").on("click", ".open-user-form", function (e) {
const user_id = parseInt($(e.currentTarget).attr("data-user-id"), 10);
const bot = people.get_by_user_id(user_id);
if (!bot) {
return;
}
const modal = open_bot_form(bot);
modal.find('.submit_bot_change').on("click", function (e) {
e.preventDefault();
e.stopPropagation();
const full_name = modal.find("input[name='full_name']");
const admin_status = get_status_field();
const url = "/json/bots/" + encodeURIComponent(user_id);
const data = {
full_name: full_name.val(),
};
const owner_select_value = modal.find('.bot_owner_select').val();
if (owner_select_value) {
data.bot_owner_id = people.get_by_email(owner_select_value).user_id;
}
settings_ui.do_settings_change(channel.patch, url, data, admin_status);
overlays.close_modal('#user-info-form-modal');
overlays.close_modal('#admin-bot-form');
});
});
}
@ -532,10 +569,11 @@ exports.on_load_success = function (realm_people_data) {
populate_users(realm_people_data);
handle_deactivation();
handle_reactivation();
handle_user_form();
handle_human_form();
handle_bot_owner_profile();
handle_bot_deactivation();
handle_bot_form();
};
window.settings_users = exports;

2
static/styles/settings.scss
View File

@ -1566,7 +1566,7 @@ input[type=checkbox].inline-block {
}
#account-settings,
#user-info-form-modal {
#admin-human-form {
.user-role button {
cursor: default;
}

28
static/templates/admin_bot_form.hbs Normal file
View File

@ -0,0 +1,28 @@
<div id="admin-bot-form" class="modal modal-bg hide fade" tabindex="-1" role="dialog" aria-labelledby="admin-bot-form-label" aria-hidden="true">
<div class="modal-header">
<button type="button" class="close" data-dismiss="modal" aria-label="{{t 'Close' }}"><span aria-hidden="true">&times;</span></button>
<h3 id="admin-bot-form-label">{{t "Change bot info and owner" }}</h3>
</div>
<div class="modal-body">
<div id="user-name-form" data-user-id="{{user_id}}">
<form class="form-horizontal name-setting">
<input type="hidden" name="is_full_name" value="true" />
<div class="input-group name_change_container">
<label for="full_name">{{t "Full name" }}</label>
<input type="text" autocomplete="off" name="full_name" value="{{ full_name }}" />
</div>
<div class="input-group email_change_container">
<label for="email">{{t "Email" }}</label>
<input type="text" autocomplete="off" name="email" value="{{ email }}" readonly/>
</div>
<div class="input-group edit_bot_owner_container">
<label for="bot_owner_select">{{t "Owner" }}</label>
</div>
</form>
</div>
</div>
<div class="modal-footer">
<button type="submit" class="button rounded sea-green submit_bot_change">{{t 'Save changes' }}</button>
<button type="button" class="button rounded" data-dismiss="modal">{{t "Cancel" }}</button>
</div>
</div>

18
static/templates/user_info_form_modal.hbs → static/templates/admin_human_form.hbs
View File

@ -1,11 +1,7 @@
<div id="user-info-form-modal" class="modal modal-bg hide fade" tabindex="-1" role="dialog" aria-labelledby="user-info-form-modal-label" aria-hidden="true">
<div id="admin-human-form" class="modal modal-bg hide fade" tabindex="-1" role="dialog" aria-labelledby="admin-human-form-label" aria-hidden="true">
<div class="modal-header">
<button type="button" class="close" data-dismiss="modal" aria-label="{{t 'Close' }}"><span aria-hidden="true">&times;</span></button>
{{#if is_bot}}
<h3 id="user-info-form-modal-label">{{t "Change bot info and owner" }}</h3>
{{else}}
<h3 id="user-info-form-modal-label">{{t "Change user info and roles" }}</h3>
{{/if}}
<h3 id="admin-human-form-label">{{t "Change user info and roles" }}</h3>
</div>
<div class="modal-body">
<div id="user-name-form" data-user-id="{{user_id}}">
@ -19,11 +15,6 @@
<label for="email">{{t "Email" }}</label>
<input type="text" autocomplete="off" name="email" value="{{ email }}" readonly/>
</div>
{{#if is_bot}}
<div class="input-group edit_bot_owner_container">
<label for="bot_owner_select">{{t "Owner" }}</label>
</div>
{{else}}
<div class="input-group">
<label class="input-label" for="user-role-select">{{t 'User role' }}</label>
<select name="user-role-select" id="user-role-select">
@ -33,12 +24,11 @@
</select>
</div>
<div class="custom-profile-field-form"></div>
{{/if}}
</form>
</div>
</div>
<div class="modal-footer">
<button type="submit" class="button rounded sea-green submit_user_info_change">{{t 'Save changes' }}</button>
<button type="button" class="button rounded reset_edit_user" data-dismiss="modal">{{t "Cancel" }}</button>
<button type="submit" class="button rounded sea-green submit_human_change">{{t 'Save changes' }}</button>
<button type="button" class="button rounded" data-dismiss="modal">{{t "Cancel" }}</button>
</div>
</div>