Skip to content

Commit 5fba092

Browse files
authored
feat(admin-admins): hydrate per-server access list with live hostnames (#1405) (#1412)
* feat(admin-admins): hydrate per-server access list with live hostnames (#1405) Closes #1405. The Add Admin per-server access checkbox grid was painting bare IP:port placeholders on every row; the v1.x `LoadServerHost(...)` async-hostname feeder that used to replace them was deleted with sourcebans.js at #1123 D1, leaving the rows static for two minor versions. Wire the rows onto the existing `server-tile-hydrate.js` helper + `Sbpp\Servers\SourceQueryCache`, mirroring the dashboard Servers widget's minimal-integration shape. Refs #1404 (cleanup that dropped the dead script blob). * chore(server-tile-hydrate): update docblock + drop redundant {escape} filters (review) Adversarial reviewer findings on PR #1412 (issue #1405): 1. server-tile-hydrate.js's docblock listed only two consuming surfaces; post-#1375 the dashboard widget joined and post-#1412 the Add Admin grid is the fourth consumer. Updated the docblock to enumerate all four with their per-surface testid count so the feature-detection contract is discoverable from the file. Structure mirrors the AGENTS.md "Where to find what" → "Hydrate server-tile cards…" row so the prose source of truth and the in-file docblock stay aligned. 2. page_admin_admins_add.tpl used {$server.ip|escape}:{$server.port|escape} for both the data-fallback attribute and the inner host span; Smarty's global setEscapeHtml(true) (web/init.php) double-escapes the value. Dropped the four explicit filters to match the canonical sibling page_dashboard.tpl shape. IP/port have no HTML-special chars in valid input so the change is cosmetic, but it eliminates sibling-surface drift. AddAdminServerHostHydrationTest::testRowsCarryHostnameSlotWithFallback pinned the literal-Smarty-interpolation regex with the {escape} filters baked in, so it's adjusted in the same commit to the new filter-less shape; the test docblock now documents why no explicit |escape lives here.
1 parent 9085775 commit 5fba092

6 files changed

Lines changed: 905 additions & 27 deletions

File tree

‎AGENTS.md‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2298,13 +2298,17 @@ contacting every contributor individually.
22982298
group on every page load with no visible effect (the static `<span
22992299
id="saSID">IP:port</span>` fallback / the literal "Servers populate
23002300
via the legacy LoadServerHostPlayersList hook." placeholder masked
2301-
the symptom). Both went at #1404 along with the orphan
2301+
the symptom). Both went at #1404 along with the orphan
23022302
`AdminAdminsAddView::server_script` ctor param and the
23032303
`<div id="servers_{gid}">` hydration slot; `DeadJsCallSitesTest` is
23042304
the per-file forbidden-substring gate so a fork pasting back the
2305-
v1.x shape gets caught at PR time. Hydration follow-ups tracked at
2306-
#1405 (Add Admin per-server access list) / #1406 (Server Groups
2307-
per-group server cards).
2305+
v1.x shape gets caught at PR time. Hydration follow-up for the
2306+
Add Admin per-server access list LANDED at #1405 — it wires the
2307+
per-server checkbox grid onto the shared
2308+
`web/scripts/server-tile-hydrate.js` helper (the same one driving
2309+
the dashboard Servers widget); no new View property, no new JSON
2310+
action, no new cache layer. Sister surface still tracked: #1406
2311+
(Server Groups per-group server cards).
23082312
- Raw `<script>ShowBox(...)</script>` (or `<script>showBox(...)</script>` /
23092313
`<script type="text/javascript">ShowBox(...)`) blobs echoed from a
23102314
PHP page handler → use `\Sbpp\View\Toast::emit($kind, $title,
@@ -3065,7 +3069,7 @@ contacting every contributor individually.
30653069
| Edit the player-detail drawer (open trigger, tabs, panes, lazy loaders) | `web/themes/default/js/theme.js` (`renderDrawerBody` / `loadPaneIfNeeded`). The drawer handles two focal kinds via `drawerKind` (`'ban'` / `'comm'`): the bans list ships `data-drawer-bid` / `data-drawer-href` on row anchors, the comms list ships `data-drawer-cid`. `loadDrawer({kind, id})` dispatches to `Actions.BansDetail` (bid → `bans.detail`) or `Actions.CommsDetail` (cid → `comms.detail`) and stamps the response into `drawerDetail`. `loadPaneIfNeeded` then keys lazy panes off the focal kind: bans-focal History sends `{bid}` (handler excludes the focal via `BA.bid <> ?`); comm-focal History sends `{authid: drawerDetail.player.steam_id}` (no focal to exclude — different table); bans-focal Comms sends `{bid}` (resolves to authid; no comm to exclude); comm-focal Comms sends `{cid}` (handler excludes the focal cid via `C.bid <> ?` — sister contract to the bans-focal History exclusion so the Overview pane and the Comms tab don't render the same record twice). The Notes tab is admin-only and shared across both focal kinds (keys off `player.steam_id`). |
30663070
| Add a comms-list player drawer parity surface (mirror the banlist's `data-drawer-href` row anchor with a comm-focal equivalent) | The desktop `<tr data-testid="comm-row">` and mobile `<div data-testid="comm-card">` rows in `web/themes/default/page_comms.tpl` carry a player-name anchor with `data-drawer-cid="{$comm.cid}"` + `data-testid="drawer-trigger"`. The `href` falls back to a useful no-JS surface (`?p=commslist&id=…` desktop / `?p=commslist&searchText=…` mobile) so the affordance still leads somewhere when JS is off / `theme.js` is stripped by a third-party theme. The drawer JS (`theme.js`'s document `click` delegate at `[data-drawer-bid], [data-drawer-cid], [data-drawer-href]`) routes the click through `keyFromTrigger(trigger)` which returns `{kind: 'comm', id}` for cid triggers; downstream `loadDrawer` dispatches `Actions.CommsDetail` and the renderer branches on `drawerKind === 'comm'` for the header chip ("Comm #N") and the Overview pane's focal-block grid (`[data-testid="drawer-block"]` with `Type` / `Reason` / `Started` / `Ends` rows — vs `[data-testid="drawer-ban"]` on the bans-focal path). The handler is `api_comms_detail` in `web/api/handlers/comms.php` (sister to `api_bans_detail`, same envelope shape modulo `cid` instead of `bid` / `block` instead of `ban` / `'unmuted'` instead of `'unbanned'` in the state vocab — both `api_comms_detail` AND `api_comms_player_history` use `'unmuted'` for `RemoveType IN ('U', 'D')` rows so the drawer's Overview pane and Comms tab don't render contradictory state labels for the same player). Public action; field-level hide-* gating mirrors `bans.detail`. Pill CSS lives next to `.pill--unbanned` in `theme.css` (`.pill--unmuted` carries the same success-bg + emerald colour treatment because admin-lifted is admin-lifted regardless of the focal kind). The drawer JS's `stateLabel()` switch in `theme.js` carries the matching `'unmuted' → 'Unmuted'` arm. Regression guards: `web/tests/api/CommsTest.php` (snapshot + state vocab + lifted-block branch + permanent-block branch + `comms.player_history` cid path with focal exclusion + 404 on unknown cid + lone-focal empty-feed shape) and `web/tests/e2e/specs/flows/ui/comms-drawer.spec.ts` (desktop) + `web/tests/e2e/specs/responsive/drawer.spec.ts` (mobile — clicking a `.ban-cards [data-testid="drawer-trigger"]` opens the comm-focal drawer, header reads "Comm #N", Type row in Overview pane). The desktop spec mirrors `player-drawer.spec.ts`'s isolation strategy — NO `truncateE2eDb` between tests, unique authids per (subtest × project × worker), and `seedCommOrAccept` / `seedBanOrAccept` helpers that tolerate `already_blocked` / `already_banned` so a Playwright retry on the same worker reuses the existing row. Adding a per-test truncate would only widen the cross-file race window where a concurrent worker's API call lands during another worker's truncate→reseed gap and gets a `forbidden` cascade; the comms-drawer tests are read-shaped (open the drawer, assert the chrome) so authid-namespacing is enough. |
30673071
| Render the per-server map thumbnail in the expanded public server card | `web/themes/default/page_servers.tpl` (`<img data-testid="server-map-img" hidden>` slot inside `[data-testid="server-players-panel"]`) + `web/scripts/server-tile-hydrate.js`'s `applyData()` (patches `src` from `r.data.mapimg`, toggles `hidden` on `load` / `error`). The lookup is feature-detected via the testid so the admin Server Management list (which does NOT ship the slot) silently no-ops. The URL itself comes from global helper `\GetMapImage()` in `web/includes/system-functions.php` (falls back to `images/maps/nomap.jpg` when the file is missing); the bundled `nomap.jpg` placeholder ships under `web/images/maps/`. The slot must default to `hidden` and stay hidden on the `error` branch — fork installs without `nomap.jpg` would otherwise paint a broken-image icon. Sizing (#1375): the inline style is `display:block;width:100%;max-width:340px;height:auto;margin:0 auto 0.5rem` — `max-width: 340px` matches the natural source width of the bundled `*.jpg` thumbnails (340×255, ~4:3) so the box never upscales and never exceeds the source dimensions; `height: auto` derives the proportional height from the rendered width so the rendered box matches the source aspect ratio exactly. Pre-#1375 the slot ran `width:100%;max-height:140px;object-fit:cover` which clamped the box to a ~2.86:1 strip on a 28rem card and `object-fit:cover` cropped the middle horizontal band of a 4:3 source — operators perceived the result as "stretched horizontally". Don't reintroduce `max-height` or `object-fit:cover` here; let `height: auto` carry the proportional sizing. Regression guards: `web/tests/integration/ServerMapImageRenderTest.php` (template ships the slot + helper carries the wiring + handler still emits `mapimg`, AND `testMapImgSlotPreservesNaturalAspectRatio` pins the new `max-width: 340px` / `height: auto` shape + the absence of `max-height` / `object-fit`) + `web/tests/e2e/specs/flows/server-map-thumbnail.spec.ts` (runtime visibility under success / 404 / connect-error). #1312 restored this surface after the #1123 D1 redesign dropped the legacy `<img id="mapimg_{$server.sid}">`; #1313 moved the wiring out of the inline `<script>` block into the shared helper; #1375 fixed the squashed aspect ratio. |
3068-
| Hydrate server-tile cards with live A2S data (status pill / map / players / hostname / refresh) on the public servers list, the admin Server Management list, AND the dashboard's Servers widget | `web/scripts/server-tile-hydrate.js` (`window.SBPP.hydrateServerTiles`) — auto-runs on first paint for every container marked `data-server-hydrate="auto"`. Consumed by `web/themes/default/page_servers.tpl` (public, full chrome), `web/themes/default/page_admin_servers_list.tpl` (admin, #1313 — same chrome minus the map thumbnail + players panel), and `web/themes/default/page_dashboard.tpl` (dashboard Servers widget, #1375 — hostname slot only, every other testid hook deliberately omitted). Selector contract per tile: `[data-testid="server-tile"]` outer card + `data-id="<sid>"` + `[data-testid="server-{status,map,players,host}"]` cells + optional `[data-testid="server-{refresh,toggle}"]` / `[data-players-bar]` / `[data-testid="server-players-panel"]` / `[data-testid="server-map-img"]` (every cell beyond `data-id` is feature-detected; the dashboard widget ships only `[data-testid="server-host"]` and the helper no-ops every other branch). Disabled tiles carry `data-server-skip="1"` so the helper leaves them at the server-rendered placeholder. Never copy-paste the hydration code into a new template — wire the testids and `data-server-hydrate="auto"` instead and the helper picks the surface up automatically. The dashboard widget is the canonical reference for "minimal-testid integration": it ships ONE optional testid + the outer `[data-testid="server-tile"]` + `data-id="<sid>"`, and the helper still hydrates the hostname per the same `Actions.ServersHostPlayers` round-trip. The per-surface truncation knob (`data-trunchostname="<n>"` on the wrapping container) is the right place to dial the hostname length per surface — `70` on the full-width public + admin cards, `40` on the cramped dashboard widget so a long hostname doesn't trip `truncate`'s ellipsis prematurely. |
3072+
| Hydrate server-tile cards with live A2S data (status pill / map / players / hostname / refresh) on the public servers list, the admin Server Management list, the dashboard's Servers widget, AND the Add Admin per-server access checkbox grid | `web/scripts/server-tile-hydrate.js` (`window.SBPP.hydrateServerTiles`) — auto-runs on first paint for every container marked `data-server-hydrate="auto"`. Consumed by `web/themes/default/page_servers.tpl` (public, full chrome), `web/themes/default/page_admin_servers_list.tpl` (admin, #1313 — same chrome minus the map thumbnail + players panel), `web/themes/default/page_dashboard.tpl` (dashboard Servers widget, #1375 — hostname slot only, every other testid hook deliberately omitted), and `web/themes/default/page_admin_admins_add.tpl` (Add Admin per-server access checkbox grid, #1405 — same minimal-testid shape as the dashboard widget). Selector contract per tile: `[data-testid="server-tile"]` outer card + `data-id="<sid>"` + `[data-testid="server-{status,map,players,host}"]` cells + optional `[data-testid="server-{refresh,toggle}"]` / `[data-players-bar]` / `[data-testid="server-players-panel"]` / `[data-testid="server-map-img"]` (every cell beyond `data-id` is feature-detected; the dashboard widget + the Add Admin grid both ship ONLY `[data-testid="server-host"]` and the helper no-ops every other branch). Disabled tiles carry `data-server-skip="1"` so the helper leaves them at the server-rendered placeholder. Never copy-paste the hydration code into a new template — wire the testids and `data-server-hydrate="auto"` instead and the helper picks the surface up automatically. The dashboard widget + the Add Admin grid are the canonical references for "minimal-testid integration": each ships ONE optional testid + the outer `[data-testid="server-tile"]` + `data-id="<sid>"`, and the helper still hydrates the hostname per the same `Actions.ServersHostPlayers` round-trip. The per-surface truncation knob (`data-trunchostname="<n>"` on the wrapping container) is the right place to dial the hostname length per surface — `70` on the full-width public + admin cards, `40` on the cramped dashboard widget AND the Add Admin grid's ~18rem checkbox columns so a long hostname doesn't trip `truncate`'s ellipsis prematurely. The Add Admin grid restored hostname hydration that was silently broken since v2.0.0 / #1123 D1 — the legacy v1.4.11 `<script>LoadServerHost('SID', …)</script>` per-row feeder was deleted with `sourcebans.js`, raising one `ReferenceError` per configured server on every page load. Sister cleanup #1404 dropped the dead feeder + the orphan `$server_script` View property; #1405 is the additive replacement, no new View property + no new JSON action. |
30693073
| Tune the server card grid layout (column min-width, mobile single-column collapse) shared between the public + admin Server Management lists | `.servers-grid` rule in `web/themes/default/css/theme.css` (#1316). Single-source `repeat(auto-fill, minmax(28rem, 1fr))` — both `page_servers.tpl` and `page_admin_servers_list.tpl` apply the class to their grid container instead of an inline `style="grid-template-columns:..."`. The 28rem (448px) min replaced the pre-#1316 20rem (320px) min that packed cards into ~340px columns even on a 31" 4K monitor (both pages cap their content area at 1400px via `.page-section` / inline `max-width:1400px`, so wider viewports got zero benefit from the wider screen — that's the bug #1316 fixed). With the 1rem grid gap factored in: 1280px laptop ≈ 2 cols × ~488px each; 1400px+ desktop ≈ 2 cols × ~668px each. At <=768px the sibling `@media` rule collapses to `minmax(0, 1fr)` (NOT bare `1fr`, which would inflate the track to the card's `truncate`-nowrap min-content and overflow the viewport) so a phone-portrait viewport never overflows horizontally. Don't reach for a different column-min on a per-template basis — the class is the unified knob; theme forks override the rule wholesale. Regression guard: `web/tests/e2e/specs/responsive/server-cards.spec.ts` walks four desktop viewports (1280/1920/2560/3840) plus iPhone-13-like 390px and asserts both surfaces apply the class, the card width floor is ≥28rem, and the mobile collapse holds. |
30703074
| Edit the command palette (icon-only trigger, ⌘K binding, result rows, kbd hints, Ctrl+Enter copy) | `web/themes/default/js/theme.js` (`openPalette` / `closePalette` / `renderPaletteResults` / `applyPlatformHints` / `handlePaletteCopyShortcut`) + `core/title.tpl` (the `.topbar__search` icon button) + the `.palette__row*` rules in `web/themes/default/css/theme.css`. Player rows carry `data-drawer-bid="<bid>"` (bare Enter / click → `loadDrawer`, palette closes itself) + `data-steamid="<steam>"` (`Ctrl/Cmd+Enter` → `navigator.clipboard.writeText` + `showToast`). The kbd glyphs are server-rendered in non-Mac form (`Enter`, `Ctrl`); `applyPlatformHints` swaps `[data-enterkey]` → ⏎ and `[data-modkey]` → ⌘ on Mac/iOS at boot and after every render (#1184, #1207 DET-2). |
30713075
| Add or edit a palette "Navigate" entry (the icon-label-href rows the palette renders alongside player results) | `web/includes/View/PaletteActions.php` (`Sbpp\View\PaletteActions::for($userbank)` — catalog + filter). The catalog's `entries()` method declares each entry as `{icon, label, href, permission, config?}`; `for()` drops entries the user can't reach (admin entries gated via `HasAccess` with `ADMIN_OWNER` OR'd in; public entries optionally gated on a `config.enable*` toggle) and emits the public `{icon, label, href}` triple. The filtered list is JSON-encoded by `web/pages/core/footer.php` (with `JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT` so the content can never escape its `<script>` wrapper) and emitted by `core/footer.tpl` inside `<script type="application/json" id="palette-actions" data-testid="palette-actions">`. `theme.js`'s `loadNavItems()` reads + `JSON.parse`s the blob at boot. Pre-#1304 the entry list was a hardcoded `NAV_ITEMS` array in `theme.js` with no permission check, leaking admin entries to logged-out + partial-permission users; the regression guard is `web/tests/integration/PaletteActionsTest.php` (server-side filter) plus `web/tests/e2e/specs/flows/ui/command-palette-permissions.spec.ts` (end-to-end blob → DOM contract). |

‎web/pages/admin.admins.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,17 @@
135135
// `ReferenceError: LoadServerHost is not defined` once per
136136
// configured server with no visible effect (the `sa<SID>` <span>
137137
// already renders bare `IP:port` text from `$server_list`).
138-
// Hydration to per-server hostnames is the next step — tracked as
139-
// a follow-up ticket; see AGENTS.md "Anti-patterns" for the
140-
// matching `LoadServerHost` entry.
138+
//
139+
// #1405 — the additive replacement: the template's per-server rows
140+
// now ride the shared `web/scripts/server-tile-hydrate.js` helper
141+
// (the same one driving the public servers list, admin Server
142+
// Management list, and dashboard Servers widget). The View DTO
143+
// does NOT carry a new property — hydration is purely client-side
144+
// off the existing `$server_list` rows (each row already exposes
145+
// `sid` / `ip` / `port`, which is all the helper needs to fire
146+
// `Actions.ServersHostPlayers` per row and patch the live hostname
147+
// into the `[data-testid="server-host"]` slot). See AGENTS.md
148+
// "Hydrate server-tile cards…" for the per-surface contract.
141149
$server_list = [];
142150
foreach ($servers as $server) {
143151
$info['sid'] = $server['sid'];

0 commit comments

Comments
 (0)