settings_users: Add role filter to user list.

Fixes #18617.
- The role filter is added to both active and deactivated user
list. The original plan was to introduce a multi select checkbox
dropdown for the roles, but since that component is not ready yet,
we will use the dropdown component for the first iteration.
- To accomodate multiple filters, we have used an approach
similar to the one in recent_view_ui.js where we use dropdown
callback function and on("input") event on text search to set the
filter value in memory. The predicate functions uses these filters.
- We have also changed the default message when there are no
users to show to reflect `filters` instead of `current filter`.
This commit is contained in:
Shubham Padia
2024-04-05 22:41:54 +05:30
committed by Tim Abbott
parent 6c2535fe46
commit 3f6ceba39a
7 changed files with 209 additions and 45 deletions

View File

@@ -226,6 +226,9 @@ export function build_page() {
automatically_unmute_topics_in_muted_streams_policy_values: automatically_unmute_topics_in_muted_streams_policy_values:
settings_config.automatically_follow_or_unmute_topics_policy_values, settings_config.automatically_follow_or_unmute_topics_policy_values,
realm_enable_guest_user_indicator: realm.realm_enable_guest_user_indicator, realm_enable_guest_user_indicator: realm.realm_enable_guest_user_indicator,
active_user_list_dropdown_widget_name: settings_users.active_user_list_dropdown_widget_name,
deactivated_user_list_dropdown_widget_name:
settings_users.deactivated_user_list_dropdown_widget_name,
}; };
if (options.realm_logo_source !== "D" && options.realm_night_logo_source === "D") { if (options.realm_logo_source !== "D" && options.realm_night_logo_source === "D") {

View File

@@ -1595,9 +1595,25 @@ export function matches_user_settings_search(person: User, value: string): boole
return safe_lower(person.full_name).includes(value) || safe_lower(email).includes(value); return safe_lower(person.full_name).includes(value) || safe_lower(email).includes(value);
} }
export function filter_for_user_settings_search(persons: User[], query: string): User[] { function matches_user_settings_role(person: User, role_code: number): boolean {
if (role_code === 0 || role_code === person.role) {
return true;
}
return false;
}
type SettingsUsersFilterQuery = {
text_search: string;
role_code: number;
};
export function predicate_for_user_settings_filters(
person: User,
query: SettingsUsersFilterQuery,
): boolean {
/* /*
TODO: For large realms, we can optimize this a couple TODO: For text_search:
For large realms, we can optimize this a couple
different ways. For realms that don't show different ways. For realms that don't show
emails, we can make a simpler filter predicate emails, we can make a simpler filter predicate
that works solely with full names. And we can that works solely with full names. And we can
@@ -1607,7 +1623,10 @@ export function filter_for_user_settings_search(persons: User[], query: string):
See #13554 for more context. See #13554 for more context.
*/ */
return persons.filter((person) => matches_user_settings_search(person, query)); return (
matches_user_settings_search(person, query.text_search) &&
matches_user_settings_role(person, query.role_code)
);
} }
export function maybe_incr_recipient_count( export function maybe_incr_recipient_count(

View File

@@ -6,6 +6,7 @@ import * as blueslip from "./blueslip";
import * as browser_history from "./browser_history"; import * as browser_history from "./browser_history";
import * as channel from "./channel"; import * as channel from "./channel";
import * as dialog_widget from "./dialog_widget"; import * as dialog_widget from "./dialog_widget";
import * as dropdown_widget from "./dropdown_widget";
import {$t} from "./i18n"; import {$t} from "./i18n";
import * as ListWidget from "./list_widget"; import * as ListWidget from "./list_widget";
import * as loading from "./loading"; import * as loading from "./loading";
@@ -13,6 +14,7 @@ import * as people from "./people";
import * as presence from "./presence"; import * as presence from "./presence";
import * as scroll_util from "./scroll_util"; import * as scroll_util from "./scroll_util";
import * as settings_bots from "./settings_bots"; import * as settings_bots from "./settings_bots";
import * as settings_config from "./settings_config";
import * as settings_data from "./settings_data"; import * as settings_data from "./settings_data";
import {current_user} from "./state_data"; import {current_user} from "./state_data";
import * as timerender from "./timerender"; import * as timerender from "./timerender";
@@ -20,9 +22,26 @@ import * as user_deactivation_ui from "./user_deactivation_ui";
import * as user_profile from "./user_profile"; import * as user_profile from "./user_profile";
import * as user_sort from "./user_sort"; import * as user_sort from "./user_sort";
export const active_user_list_dropdown_widget_name = "active_user_list_select_user_role";
export const deactivated_user_list_dropdown_widget_name = "deactivated_user_list_select_user_role";
const section = { const section = {
active: {}, active: {
deactivated: {}, dropdown_widget_name: active_user_list_dropdown_widget_name,
filters: {
text_search: "",
// 0 role_code signifies All roles for our filter.
role_code: 0,
},
},
deactivated: {
dropdown_widget_name: deactivated_user_list_dropdown_widget_name,
filters: {
text_search: "",
// 0 role_code signifies All roles for our filter.
role_code: 0,
},
},
bots: {}, bots: {},
}; };
@@ -94,6 +113,52 @@ function failed_listing_users() {
blueslip.error("Error while listing users for user_id", {user_id}); blueslip.error("Error while listing users for user_id", {user_id});
} }
function add_value_to_filters(section, key, value) {
section.filters[key] = value;
// This hard_redraw will rerun the relevant predicate function
// and in turn apply the new filters.
section.list_widget.hard_redraw();
}
function role_selected_handler(event, dropdown, widget) {
event.preventDefault();
event.stopPropagation();
const role_code = Number($(event.currentTarget).attr("data-unique-id"));
if (widget.widget_name === section.active.dropdown_widget_name) {
add_value_to_filters(section.active, "role_code", role_code);
} else if (widget.widget_name === section.deactivated.dropdown_widget_name) {
add_value_to_filters(section.deactivated, "role_code", role_code);
}
dropdown.hide();
widget.render();
}
function get_roles() {
return [
{unique_id: "0", name: $t({defaultMessage: "All roles"})},
...Object.values(settings_config.user_role_values).map((user_role_value) => ({
unique_id: user_role_value.code.toString(),
name: user_role_value.description,
})),
];
}
function create_role_filter_dropdown($events_container, widget_name) {
new dropdown_widget.DropdownWidget({
widget_name,
get_options: get_roles,
$events_container,
item_click_callback: role_selected_handler,
default_id: "0",
tippy_props: {
placement: "bottom-start",
offset: [0, 0],
},
}).setup();
}
function populate_users() { function populate_users() {
const active_user_ids = people.get_realm_active_human_user_ids(); const active_user_ids = people.get_realm_active_human_user_ids();
const deactivated_user_ids = people.get_non_active_human_ids(); const deactivated_user_ids = people.get_non_active_human_ids();
@@ -104,6 +169,11 @@ function populate_users() {
section.active.create_table(active_user_ids); section.active.create_table(active_user_ids);
section.deactivated.create_table(deactivated_user_ids); section.deactivated.create_table(deactivated_user_ids);
create_role_filter_dropdown($("#admin-user-list"), section.active.dropdown_widget_name);
create_role_filter_dropdown(
$("#admin-deactivated-users-list"),
section.deactivated.dropdown_widget_name,
);
} }
function reset_scrollbar($sel) { function reset_scrollbar($sel) {
@@ -244,7 +314,7 @@ section.bots.create_table = () => {
section.active.create_table = (active_users) => { section.active.create_table = (active_users) => {
const $users_table = $("#admin_users_table"); const $users_table = $("#admin_users_table");
ListWidget.create($users_table, active_users, { section.active.list_widget = ListWidget.create($users_table, active_users, {
name: "users_table_list", name: "users_table_list",
get_item: people.get_by_user_id, get_item: people.get_by_user_id,
modifier_html(item) { modifier_html(item) {
@@ -252,8 +322,9 @@ section.active.create_table = (active_users) => {
return render_admin_user_list(info); return render_admin_user_list(info);
}, },
filter: { filter: {
$element: $users_table.closest(".settings-section").find(".search"), predicate(person) {
filterer: people.filter_for_user_settings_search, return people.predicate_for_user_settings_filters(person, section.active.filters);
},
onupdate: reset_scrollbar($users_table), onupdate: reset_scrollbar($users_table),
}, },
$parent_container: $("#admin-user-list").expectOne(), $parent_container: $("#admin-user-list").expectOne(),
@@ -274,28 +345,36 @@ section.active.create_table = (active_users) => {
section.deactivated.create_table = (deactivated_users) => { section.deactivated.create_table = (deactivated_users) => {
const $deactivated_users_table = $("#admin_deactivated_users_table"); const $deactivated_users_table = $("#admin_deactivated_users_table");
ListWidget.create($deactivated_users_table, deactivated_users, { section.deactivated.list_widget = ListWidget.create(
name: "deactivated_users_table_list", $deactivated_users_table,
get_item: people.get_by_user_id, deactivated_users,
modifier_html(item) { {
const info = human_info(item); name: "deactivated_users_table_list",
return render_admin_user_list(info); get_item: people.get_by_user_id,
modifier_html(item) {
const info = human_info(item);
return render_admin_user_list(info);
},
filter: {
predicate(person) {
return people.predicate_for_user_settings_filters(
person,
section.deactivated.filters,
);
},
onupdate: reset_scrollbar($deactivated_users_table),
},
$parent_container: $("#admin-deactivated-users-list").expectOne(),
init_sort: "full_name_alphabetic",
sort_fields: {
email: user_sort.sort_email,
role: user_sort.sort_role,
id: user_sort.sort_user_id,
...ListWidget.generic_sort_functions("alphabetic", ["full_name"]),
},
$simplebar_container: $("#admin-deactivated-users-list .progressive-table-wrapper"),
}, },
filter: { );
$element: $deactivated_users_table.closest(".settings-section").find(".search"),
filterer: people.filter_for_user_settings_search,
onupdate: reset_scrollbar($deactivated_users_table),
},
$parent_container: $("#admin-deactivated-users-list").expectOne(),
init_sort: "full_name_alphabetic",
sort_fields: {
email: user_sort.sort_email,
role: user_sort.sort_role,
id: user_sort.sort_user_id,
...ListWidget.generic_sort_functions("alphabetic", ["full_name"]),
},
$simplebar_container: $("#admin-deactivated-users-list .progressive-table-wrapper"),
});
loading.destroy_indicator($("#admin_page_deactivated_users_loading_indicator")); loading.destroy_indicator($("#admin_page_deactivated_users_loading_indicator"));
$("#admin_deactivated_users_table").show(); $("#admin_deactivated_users_table").show();
@@ -437,9 +516,23 @@ function handle_edit_form($tbody) {
}); });
} }
function handle_filter_change($tbody, section) {
// This duplicates the built-in search filter live-update logic in
// ListWidget for the input.list_widget_filter event type, but we
// can't use that, because we're also filtering on Role with our
// custom predicate.
$tbody
.closest(".settings-section")
.find(".search")
.on("input.list_widget_filter", function () {
add_value_to_filters(section, "text_search", this.value.toLocaleLowerCase());
});
}
section.active.handle_events = () => { section.active.handle_events = () => {
const $tbody = $("#admin_users_table").expectOne(); const $tbody = $("#admin_users_table").expectOne();
handle_filter_change($tbody, section.active);
handle_deactivation($tbody); handle_deactivation($tbody);
handle_reactivation($tbody); handle_reactivation($tbody);
handle_edit_form($tbody); handle_edit_form($tbody);
@@ -448,6 +541,7 @@ section.active.handle_events = () => {
section.deactivated.handle_events = () => { section.deactivated.handle_events = () => {
const $tbody = $("#admin_deactivated_users_table").expectOne(); const $tbody = $("#admin_deactivated_users_table").expectOne();
handle_filter_change($tbody, section.deactivated);
handle_deactivation($tbody); handle_deactivation($tbody);
handle_reactivation($tbody); handle_reactivation($tbody);
handle_edit_form($tbody); handle_edit_form($tbody);

View File

@@ -1886,7 +1886,26 @@ $option_title_width: 180px;
display: inline-block; display: inline-block;
} }
.user_filters {
display: flex;
float: right;
flex-wrap: wrap;
& .dropdown-widget-button {
height: 30px;
max-width: 160px;
margin-right: 12px;
margin-top: 12px;
}
}
& input.search { & input.search {
/* Without explicitly mentioning the height, it used be just a little
bit short of 20px, since we need to maintain the same height with
.dropdown-widget-button, we need to explicitly set the height here.
We could set the height inside .user_filters but that would make
the height a tiny bit inconsistent on pages without .user_filters. */
height: 20px;
float: right; float: right;
font-size: 1em; font-size: 1em;
max-width: 160px; max-width: 160px;

View File

@@ -6,7 +6,10 @@
{{> ../help_link_widget link="/help/deactivate-or-reactivate-a-user" }} {{> ../help_link_widget link="/help/deactivate-or-reactivate-a-user" }}
</h3> </h3>
<div class="alert-notification" id="deactivated-user-field-status"></div> <div class="alert-notification" id="deactivated-user-field-status"></div>
<input type="text" class="search filter_text_input" placeholder="{{t 'Filter deactivated users' }}" aria-label="{{t 'Filter deactivated users' }}"/> <div class="user_filters">
{{> ../dropdown_widget widget_name=deactivated_user_list_dropdown_widget_name}}
<input type="text" class="search filter_text_input" placeholder="{{t 'Filter deactivated users' }}" aria-label="{{t 'Filter deactivated users' }}"/>
</div>
</div> </div>
<div class="progressive-table-wrapper" data-simplebar> <div class="progressive-table-wrapper" data-simplebar>
@@ -20,7 +23,7 @@
{{/if}} {{/if}}
</thead> </thead>
<tbody id="admin_deactivated_users_table" class="admin_user_table" <tbody id="admin_deactivated_users_table" class="admin_user_table"
data-empty="{{t 'There are no deactivated users.' }}" data-search-results-empty="{{t 'No users match your current filter.' }}"></tbody> data-empty="{{t 'There are no deactivated users.' }}" data-search-results-empty="{{t 'No users match your filters.' }}"></tbody>
</table> </table>
</div> </div>
<div id="admin_page_deactivated_users_loading_indicator"></div> <div id="admin_page_deactivated_users_loading_indicator"></div>

View File

@@ -5,7 +5,10 @@
<div class="settings_panel_list_header"> <div class="settings_panel_list_header">
<h3>{{t "Users"}}</h3> <h3>{{t "Users"}}</h3>
<div class="alert-notification" id="user-field-status"></div> <div class="alert-notification" id="user-field-status"></div>
<input type="text" class="search filter_text_input" placeholder="{{t 'Filter users' }}" aria-label="{{t 'Filter users' }}"/> <div class="user_filters">
{{> ../dropdown_widget widget_name=active_user_list_dropdown_widget_name}}
<input type="text" class="search filter_text_input" placeholder="{{t 'Filter users' }}" aria-label="{{t 'Filter users' }}"/>
</div>
</div> </div>
<div class="progressive-table-wrapper" data-simplebar> <div class="progressive-table-wrapper" data-simplebar>
@@ -20,7 +23,7 @@
{{/if}} {{/if}}
</thead> </thead>
<tbody id="admin_users_table" class="admin_user_table" <tbody id="admin_users_table" class="admin_user_table"
data-empty="{{t 'No users match your current filter.' }}"></tbody> data-empty="{{t 'No users match your filters.' }}"></tbody>
</table> </table>
</div> </div>
<div id="admin_page_users_loading_indicator"></div> <div id="admin_page_users_loading_indicator"></div>

View File

@@ -1232,7 +1232,7 @@ test_people("initialize", () => {
assert.equal(page_params.realm_non_active_users, undefined); assert.equal(page_params.realm_non_active_users, undefined);
}); });
test_people("filter_for_user_settings_search", () => { test_people("predicate_for_user_settings_filters", () => {
/* /*
This function calls matches_user_settings_search, This function calls matches_user_settings_search,
so that is where we do more thorough testing. so that is where we do more thorough testing.
@@ -1240,18 +1240,41 @@ test_people("filter_for_user_settings_search", () => {
*/ */
current_user.is_admin = false; current_user.is_admin = false;
const fred_smith = {full_name: "Fred Smith"}; const fred_smith = {full_name: "Fred Smith", role: 100};
const alice_lee = {full_name: "Alice Lee"};
const jenny_franklin = {full_name: "Jenny Franklin"};
const persons = [fred_smith, alice_lee, jenny_franklin]; // Test only when text_search filter is true
assert.equal(
assert.deepEqual(people.filter_for_user_settings_search(persons, "fr"), [ people.predicate_for_user_settings_filters(fred_smith, {text_search: "fr", role_code: 0}),
fred_smith, true,
jenny_franklin, );
]); // Test only when role_code filter is true
assert.equal(
assert.deepEqual(people.filter_for_user_settings_search(persons, "le"), [alice_lee]); people.predicate_for_user_settings_filters(fred_smith, {text_search: "", role_code: 100}),
true,
);
// Test only when text_search filter is false
assert.equal(
people.predicate_for_user_settings_filters(fred_smith, {text_search: "ab", role_code: 0}),
false,
);
// Test only when role_code filter is false
assert.equal(
people.predicate_for_user_settings_filters(fred_smith, {text_search: "", role_code: 200}),
false,
);
// Test when both text_search and role_code filter are true
assert.equal(
people.predicate_for_user_settings_filters(fred_smith, {
text_search: "smi",
role_code: 100,
}),
true,
);
// Test when both text_search and role_code filter are false
assert.equal(
people.predicate_for_user_settings_filters(fred_smith, {text_search: "de", role_code: 300}),
false,
);
}); });
test_people("matches_user_settings_search", () => { test_people("matches_user_settings_search", () => {