Skip to content

Improve Admin Management UX: permissions, search, table, tooltips, and mobile fixes #1532

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
maxijabase wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Improve Admin Management UX: permissions, search, table, tooltips, and mobile fixes #1532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c6503d0
fix(admin): "Edit Admins" permission not working
maxijabase Jul 28, 2026
43e0ae9
revive tooltip UI component
maxijabase Jul 28, 2026
ca7068d
fix tables disappearing in mobile
maxijabase Jul 28, 2026
bcc840d
refactor admins table, fix styles
maxijabase Jul 28, 2026
4ba2c3e
simplify admins search filters and multi-select
maxijabase Jul 28, 2026
123c5f8
improve edit-admin permissions chrome and layout
maxijabase Jul 29, 2026
c30a391
fix(overrides): stack add-override fields on mobile
maxijabase Jul 29, 2026
13db66d
drop always-true password gate after edit access check
maxijabase Jul 29, 2026
02f75e9
fix(admins): keep override form tables visible on mobile
maxijabase Jul 30, 2026
dad63a3
extend seeder data
maxijabase Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 10 additions & 23 deletions web/includes/View/AdminAdminsSearchView.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,11 @@
* `{load_template file="admin.admins.search"}` Smarty plugin.
*
* The form submits as a plain `GET` to `?p=admin&c=admins` with one
* parameter per populated filter (`name`, `name_match`, `steamid`,
* `steam_match`, `admemail`, `admemail_match`, `webgroup`,
* `srvadmgroup`, `srvgroup`, `admwebflag[]`, `admsrvflag[]`,
* `server`). admin.admins.php AND-combines every non-empty filter —
* see #1207 ADM-4. No CSRF field — search is read-only.
*
* `name_match` / `admemail_match` were added in #1231 so Login and
* E-mail can be flipped between exact / partial mode the way SteamID
* already could; defaults are partial ('1') to preserve pre-#1231
* substring behaviour for legacy URLs.
* parameter per populated filter (`name`, `steamid`, `admemail`,
* `webgroup`, `srvadmgroup`, `srvgroup`, `admwebflag[]`, `admsrvflag[]`,
* `server`). Text filters are always partial (`LIKE %…%`).
* admin.admins.php AND-combines every non-empty filter. No CSRF
* field — search is read-only.
*
* `$active_filter_*` mirror the corresponding $_GET keys so the
* template can pre-fill the form without splattering
Expand All @@ -40,14 +35,11 @@
* given $_GET shape came from a modern submit, a legacy
* `advType=…&advSearch=…` URL, or nothing at all.
*
* #1303 — collapsible disclosure
* ------------------------------
* The form is wrapped in a `<details class="card filters-details">`
* default-collapsed disclosure so the unfiltered admin list paints
* above the fold. `$has_active_filters` (derived from the nine
* `active_filter_*` value slots — match-mode toggles don't count
* because they always carry a default) drives the `[open]` attribute,
* so any post-submit page paints with the form expanded. The chrome
* above the fold. `$has_active_filters` (derived from the
* `active_filter_*` value slots) drives the `[open]` attribute, so
* any post-submit page paints with the form expanded. The chrome
* mirrors `core/admin_sidebar.tpl`'s mobile `<details open>` pattern
* (chevron + label + `prefers-reduced-motion: reduce` override). The
* count badge ("Filters · N active") rides `$active_filter_count`.
Expand Down Expand Up @@ -88,10 +80,8 @@ final class AdminAdminsSearchView extends View
* `admsrvflag[]` values.
* @param int $active_filter_count Number of non-empty filter
* value slots — drives the `<summary>` count badge ("Filters
* · N active") and `$has_active_filters`. Match-mode toggles
* (`name_match` / `steam_match` / `admemail_match`) are NOT
* counted: they always carry a default ('0' or '1') and only
* refine the matching filter, they don't filter on their own.
* · N active") and `$has_active_filters`. Empty multi-select
* arrays count as zero.
* @param bool $has_active_filters Convenience boolean derived from
* `$active_filter_count > 0`. The template uses it to decide
* whether the disclosure paints `<details open>` (post-submit
Expand All @@ -107,11 +97,8 @@ public function __construct(
public readonly array $admwebflag_list,
public readonly array $admsrvflag_list,
public readonly string $active_filter_name = '',
public readonly string $active_filter_name_match = '1',
public readonly string $active_filter_steamid = '',
public readonly string $active_filter_steam_match = '0',
public readonly string $active_filter_admemail = '',
public readonly string $active_filter_admemail_match = '1',
public readonly string $active_filter_webgroup = '',
public readonly string $active_filter_srvadmgroup = '',
public readonly string $active_filter_srvgroup = '',
Expand Down
5 changes: 3 additions & 2 deletions web/includes/View/EditAdminDetailsView.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@
* The page handler (`admin.edit.admindetails.php`) gates entry on
* `ADMIN_OWNER | ADMIN_EDIT_ADMINS` (or self-edit) before reaching the
* template, so the View doesn't carry its own access boolean. `$change_pass`
* is a per-request capability flag from the handler — true when the current
* user is allowed to set the target admin's password (root or self).
* is always true here: anyone allowed to open this page may also set
* the target's password. Edit-admins callers cannot open owner targets;
* that block lives in the page handler.
*
* The property set is intentionally identical to the legacy handler's
* `$theme->assign(...)` calls so the existing `$theme->display(...)` path
Expand Down
12 changes: 7 additions & 5 deletions web/includes/system-functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,12 @@
* still build link strings server-side.
*
* NOTE: the `$tooltip`-bearing arm picks up the `tip` / `perm` CSS
* class (legacy default theme); the bare arm has no class. The HTML
* is whitespace-padded between the opening and closing tag for
* legacy-template compatibility — the v1.x consumer relied on the
* leading + trailing space when concatenating links inline.
* class (legacy default theme) and emits `data-tooltip` for the
* themed tip in `sb.js` (not the native `title=` bubble). The bare
* arm has no class. The HTML is whitespace-padded between the
* opening and closing tag for legacy-template compatibility — the
* v1.x consumer relied on the leading + trailing space when
* concatenating links inline.
*/
function CreateLinkR(string $title, string $url, string $tooltip = '', string $target = '_self', bool $wide = false, string $onclick = ''): string
{
Expand All @@ -50,7 +52,7 @@ function CreateLinkR(string $title, string $url, string $tooltip = '', string $t
];
if ($hasTooltip) {
$attrs['class'] = $wide ? 'perm' : 'tip';
$attrs['title'] = $tooltip;
$attrs['data-tooltip'] = $tooltip;
} else {
$attrs['onclick'] = $onclick;
}
Expand Down
99 changes: 41 additions & 58 deletions web/pages/admin.admins.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,12 @@
*/

/** @var bool $canListAdmins */
$canListAdmins = $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::ListAdmins));
$canListAdmins = $userbank->HasAccess(WebPermission::mask(
WebPermission::Owner,
WebPermission::ListAdmins,
WebPermission::EditAdmins,
WebPermission::DeleteAdmins,
));
/** @var bool $canAddAdmins */
$canAddAdmins = $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::AddAdmins));
/** @var bool $canEditAdmins */
Expand All @@ -79,7 +84,7 @@
[
'slug' => 'admins',
'name' => 'Admins',
'permission' => ADMIN_OWNER | ADMIN_LIST_ADMINS,
'permission' => ADMIN_OWNER | ADMIN_LIST_ADMINS | ADMIN_EDIT_ADMINS | ADMIN_DELETE_ADMINS,
'url' => 'index.php?p=admin&c=admins&section=admins',
'icon' => 'users',
],
Expand Down Expand Up @@ -195,11 +200,12 @@
* combined filter form and AND the populated filters server-side.
*
* The new wire format reads each filter from its own query parameter
* (`name`, `steamid`, `steam_match`, `admemail`, `webgroup`,
* (`name`, `steamid`, `admemail`, `webgroup`,
* `srvadmgroup`, `srvgroup`, `admwebflag[]`, `admsrvflag[]`, `server`)
* so a single GET submit carries the full filter snapshot. URL-shareable
* searches are preserved by the legacy-shim block below: any incoming
* `?advType=…&advSearch=…` is translated into the new shape so old
* so a single GET submit carries the full filter snapshot. Text filters
* always substring-match (`LIKE %…%`). URL-shareable searches are
* preserved by the legacy-shim block below: any incoming
* `?advType=…&advSearch=…` is translated into the modern shape so old
* bookmarks and cross-page links keep working.
*
* Server-side filters are AND-combined: a request with two non-empty
Expand Down Expand Up @@ -238,10 +244,9 @@
case 'steam':
// The legacy form distinguished exact (`steamid`) from
// partial (`steam`) matches as two distinct advTypes. The
// modern form folds both onto `steamid` + `steam_match`.
// modern form folds both onto `steamid` (always partial).
if (!isset($_GET['steamid']) || $_GET['steamid'] === '') {
$_GET['steamid'] = $legacyValue;
$_GET['steam_match'] = '1';
$_GET['steamid'] = $legacyValue;
}
break;
case 'admwebflag':
Expand All @@ -253,52 +258,26 @@
}
}

// 1) Login name (exact or partial against ADM.user).
// `name_match` was added in #1231; default is partial ('1') so
// pre-#1231 URLs (`?name=alice` with no name_match) keep their
// substring semantics. `0` flips to exact.
// 1) Login name (partial against ADM.user).
if (!empty($_GET['name']) && is_string($_GET['name'])) {
$partialName = !isset($_GET['name_match']) || (string) $_GET['name_match'] !== '0';
if ($partialName) {
$where .= " AND ADM.user LIKE ?";
$whereParams[] = '%' . $_GET['name'] . '%';
} else {
$where .= " AND ADM.user = ?";
$whereParams[] = $_GET['name'];
}
$activeFilters['name'] = (string) $_GET['name'];
$activeFilters['name_match'] = $partialName ? '1' : '0';
$where .= " AND ADM.user LIKE ?";
$whereParams[] = '%' . $_GET['name'] . '%';
$activeFilters['name'] = (string) $_GET['name'];
}

// 2) Steam ID (exact or partial against ADM.authid).
// 2) Steam ID (partial against ADM.authid).
if (!empty($_GET['steamid']) && is_string($_GET['steamid'])) {
$partial = isset($_GET['steam_match']) && (string) $_GET['steam_match'] === '1';
if ($partial) {
$where .= " AND ADM.authid LIKE ?";
$whereParams[] = '%' . $_GET['steamid'] . '%';
} else {
$where .= " AND ADM.authid = ?";
$whereParams[] = $_GET['steamid'];
}
$activeFilters['steamid'] = (string) $_GET['steamid'];
$activeFilters['steam_match'] = $partial ? '1' : '0';
$where .= " AND ADM.authid LIKE ?";
$whereParams[] = '%' . $_GET['steamid'] . '%';
$activeFilters['steamid'] = (string) $_GET['steamid'];
}

// 3) E-mail (exact or partial; `admemail_match` was added in #1231,
// same default-partial shape as `name_match`). Gated on the same
// flag the search box gates the input field on so URL forgery
// can't bypass the visibility gate.
// 3) E-mail (partial). Gated on the same flag the search box gates
// the input field on so URL forgery can't bypass the visibility gate.
if (!empty($_GET['admemail']) && is_string($_GET['admemail']) && $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::EditAdmins))) {
$partialEmail = !isset($_GET['admemail_match']) || (string) $_GET['admemail_match'] !== '0';
if ($partialEmail) {
$where .= " AND ADM.email LIKE ?";
$whereParams[] = '%' . $_GET['admemail'] . '%';
} else {
$where .= " AND ADM.email = ?";
$whereParams[] = $_GET['admemail'];
}
$activeFilters['admemail'] = (string) $_GET['admemail'];
$activeFilters['admemail_match'] = $partialEmail ? '1' : '0';
$where .= " AND ADM.email LIKE ?";
$whereParams[] = '%' . $_GET['admemail'] . '%';
$activeFilters['admemail'] = (string) $_GET['admemail'];
}

// 4) Web group (`:prefix_groups.gid` -> `:prefix_admins.gid`).
Expand Down Expand Up @@ -360,7 +339,9 @@
}
}

// 8) Server permission flags (multi).
// 8) Server permission flags (multi). SM_* constants are single-char
// strings (`SM_ROOT` = `z`); pass them to HasAccess as strings so the
// srv_flags path runs. SM_ROOT implies every other server flag.
$rawSrvFlags = $_GET['admsrvflag'] ?? null;
if (is_string($rawSrvFlags)) {
$rawSrvFlags = explode(',', $rawSrvFlags);
Expand All @@ -369,27 +350,29 @@
/** @var list<string> $srvFlagNames */
$srvFlagNames = [];
foreach ($rawSrvFlags as $candidate) {
if (is_string($candidate) && preg_match('/^SM_[A-Z_]+$/', $candidate) && defined($candidate)) {
if (is_string($candidate) && preg_match('/^SM_[A-Z0-9_]+$/', $candidate) && defined($candidate)) {
$srvFlagNames[] = $candidate;
}
}
if (!empty($srvFlagNames)) {
$flagBits = array_map(fn(string $name): int => (int) constant($name), $srvFlagNames);
$alladmins = $GLOBALS['PDO']->query("SELECT aid, authid FROM `:prefix_admins` WHERE aid > 0")->resultset();
/** @var list<string> $flagChars */
$flagChars = array_map(fn(string $name): string => (string) constant($name), $srvFlagNames);
$alladmins = $GLOBALS['PDO']->query("SELECT aid FROM `:prefix_admins` WHERE aid > 0")->resultset();
$accessAids = [];
foreach ($alladmins as $row) {
$aid = (int) $row['aid'];
$matched = false;
foreach ($flagBits as $fla) {
if ($userbank->HasAccess($fla, $row['authid'])) {
foreach ($flagChars as $fla) {
if ($userbank->HasAccess($fla, $aid)) {
$matched = true;
break;
}
}
if (!$matched && $userbank->HasAccess(SM_ROOT, $row['authid'])) {
if (!$matched && $userbank->HasAccess(SM_ROOT, $aid)) {
$matched = true;
}
if ($matched) {
$accessAids[] = (int) $row['aid'];
$accessAids[] = $aid;
}
}
if (empty($accessAids)) {
Expand Down Expand Up @@ -470,10 +453,10 @@
$admin['web_group'] = $userbank->GetProperty("group_name", $admin['aid']);
$admin['server_group'] = $userbank->GetProperty("srv_groups", $admin['aid']);
if (empty($admin['web_group']) || $admin['web_group'] == " ") {
$admin['web_group'] = "No Group/Individual Permissions";
$admin['web_group'] = "No groups";
}
if (empty($admin['server_group']) || $admin['server_group'] == " ") {
$admin['server_group'] = "No Group/Individual Permissions";
$admin['server_group'] = "No groups";
}
$GLOBALS['PDO']->query("SELECT count(authid) AS num FROM `:prefix_bans` WHERE aid = :aid");
$GLOBALS['PDO']->bind(':aid', $admin['aid']);
Expand Down
38 changes: 11 additions & 27 deletions web/pages/admin.admins.search.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,19 +158,12 @@
$activeSrvFlags = [];
if (is_array($rawSrvFlag)) {
foreach ($rawSrvFlag as $f) {
if (is_string($f) && preg_match('/^SM_[A-Z_]+$/', $f)) {
if (is_string($f) && preg_match('/^SM_[A-Z0-9_]+$/', $f)) {
$activeSrvFlags[] = $f;
}
}
}

// Match-mode defaults differ per filter (#1231):
// - steam_match defaults to '0' (exact) — typical SteamID
// queries are "find this one admin by their full ID".
// - name_match / admemail_match default to '1' (partial) so
// pre-#1231 URLs (`?name=alice`) keep their substring
// behaviour. Adding the toggle widens the UI without
// regressing the default.
$activeFilterName = is_string($_GET['name'] ?? null) ? (string) $_GET['name'] : '';
$activeFilterSteamid = is_string($_GET['steamid'] ?? null) ? (string) $_GET['steamid'] : '';
$activeFilterAdmemail = is_string($_GET['admemail'] ?? null) ? (string) $_GET['admemail'] : '';
Expand All @@ -179,26 +172,20 @@
$activeFilterSrvgroup = is_scalar($_GET['srvgroup'] ?? null) ? (string) $_GET['srvgroup'] : '';
$activeFilterServer = is_scalar($_GET['server'] ?? null) ? (string) $_GET['server'] : '';

// #1303 — the `admemail` filter is permission-gated by
// `$can_editadmin` in both the rendering template AND the page
// handler (`admin.admins.php` ignores `?admemail=` from a user without
// `EditAdmins | Owner`). For URL-forgery cases where a non-admin
// passes `?admemail=foo`, the input is hidden in the form and the
// server narrows nothing; the count must mirror that — otherwise the
// "N active" badge would say "1 active" while every visible filter
// row reads empty. Mirror the gate locally so the count stays an
// honest summary of what the visible form actually filters on.
// The `admemail` filter is permission-gated by `$can_editadmin` in
// both the rendering template AND the page handler (`admin.admins.php`
// ignores `?admemail=` from a user without `EditAdmins | Owner`). For
// URL-forgery cases where a non-admin passes `?admemail=foo`, the
// input is hidden in the form and the server narrows nothing; the
// count must mirror that — otherwise the "N active" badge would say
// "1 active" while every visible filter row reads empty.
$canFilterByEmail = $userbank->HasAccess(WebPermission::mask(WebPermission::EditAdmins, WebPermission::Owner));

// #1303 — count populated filter slots so the disclosure can paint a
// "Filters · N active" badge on the <summary> and auto-expand on
// post-submit. Match-mode selects (`name_match` / `steam_match` /
// `admemail_match`) deliberately don't count: they always carry a
// default ('0' or '1') and only refine the matching filter, they
// don't filter on their own. Empty multi-select arrays count as zero
// even though the array itself "exists" — the user hasn't picked a
// permission. The `admemail` slot only counts when the user can
// actually filter by it (see `$canFilterByEmail` above).
// post-submit. Empty multi-select arrays count as zero even though
// the array itself "exists". The `admemail` slot only counts when
// the user can actually filter by it (see `$canFilterByEmail`).
$activeFilterCount =
($activeFilterName !== '' ? 1 : 0)
+ ($activeFilterSteamid !== '' ? 1 : 0)
Expand All @@ -220,11 +207,8 @@
admwebflag_list: $webflag,
admsrvflag_list: $serverflag,
active_filter_name: $activeFilterName,
active_filter_name_match: is_scalar($_GET['name_match'] ?? null) ? (string) $_GET['name_match'] : '1',
active_filter_steamid: $activeFilterSteamid,
active_filter_steam_match: is_scalar($_GET['steam_match'] ?? null) ? (string) $_GET['steam_match'] : '0',
active_filter_admemail: $activeFilterAdmemail,
active_filter_admemail_match: is_scalar($_GET['admemail_match'] ?? null) ? (string) $_GET['admemail_match'] : '1',
active_filter_webgroup: $activeFilterWebgroup,
active_filter_srvadmgroup: $activeFilterSrvadmgroup,
active_filter_srvgroup: $activeFilterSrvgroup,
Expand Down
Loading
Loading