fix(lists): make the server and time filters actually filter

AGENTS.md

@@ -5019,6 +5019,7 @@ the spec, target a 1920px viewport, not 1440px.
 | Wrap a public-list legacy advanced-search form (banlist / commslist) in a default-collapsed `<details class="filters-details">` disclosure | `web/themes/default/page_bans.tpl` + `web/themes/default/page_comms.tpl` (#1315). The same `.filters-details` rules in `web/themes/default/css/theme.css` cover the chrome (summary chevron, `[open]` state, hover bg, focus ring, count badge); the public-list usage adds a `.filters-details__body > form.card` rule that suppresses the inner card framing because the disclosure body wraps a `{load_template file="admin.bans.search"}` (or `…comms.search`) — i.e. a sibling page-render that emits its own `<form class="card">`. The View DTO (`Sbpp\View\BanListView` / `Sbpp\View\CommsListView`) carries a `bool $is_advanced_search_open` set by the page handler from `isset($_GET['advSearch']) && (string) $_GET['advSearch'] !== ''` so the legacy `?advSearch=…&advType=…` URL shim auto-opens the disclosure. Bare `?p=banlist` / `?p=commslist` and simple-bar filters (`?searchText=` / `?server=` / `?time=`) leave it closed so the unfiltered list reaches above the fold. Selectors anchor on `[data-testid="banlist-advsearch-disclosure"]` / `[data-testid="commslist-advsearch-disclosure"]` (the `<details>`) + the `…-toggle` (the `<summary>`) + the `…-active` count badge. The same `.filters-details` chrome was introduced for admin-admins by #1303 — both surfaces share the CSS so a future tweak is single-source. |
 | Surface admin-authored per-ban / per-comm comments inline on the public banlist + commslist (`<details data-testid="ban-comments-inline">` / `comm-comments-inline`) | `web/themes/default/page_bans.tpl` + `web/themes/default/page_comms.tpl` (#BANLIST-COMMENTS). Both desktop tables wrap the player-name anchor + comment chip in `.ban-player-cell__head` so the collapsed chip sits on the SAME line as the name (icon + tabular-nums count only; `title` / `aria-label` carry "N comment(s)"). The name uses `.ban-player-cell__name` (ellipsis truncate) so the chip never wraps alone; opened details take `flex-basis: 100%` and the body lists each comment below. CSS lives under `.ban-comments-inline*` in `theme.css`. The `nofilter` on comment text is load-bearing (`encodePreservingBr` + URL-wrap). Mobile cards keep a non-interactive count indicator (`ban-comments-count-mobile`). Gate: `$view_comments`. **#1500** also hides comment author/editor when `banlist.hideadminname` is on. Add / Edit on banlist and commslist open the player drawer composer (`data-comment-compose` + `[data-testid="drawer-comment-form"]` in `theme.js`). After add/edit in the drawer, `loadDrawer` patches the matching inline disclosure (chip count + list items) and the mobile ban-card count chip from the detail envelope. Delete on the banlist / commslist (table or drawer) does **not** reload: `comment-actions.js` emits `sbpp:comment-deleted` and `theme.js` removes the `<li>` in place so scroll, pagination, and open disclosures survive. Protests / submissions use an inline card composer (`data-action="comment-compose"` + `[data-comment-composer]` in `comment-actions.js`). `?comment=` is not an editor: the list still renders. Regression: `BanlistCommentsVisibilityTest` + `banlist-comments-visibility.spec.ts`. |
 | Show how many punishments a player has on record from the public banlist / commslist rows (repeat-offender history chip) | Handlers set `history_total` + `history_url` per row (`page.banlist.php`: `history_count` subquery, Steam ID for Steam bans, IP for IP bans; `page.commslist.php`: also `history_mutes` / `history_gags` / `history_silences` from the `mute_count` / `gag_count` / `silence_count` subqueries, raw counts, row included, plus a `history_label` tooltip string; type 3 silences count because forks / imports store them as one row). `page_bans.tpl` / `page_comms.tpl` paint `.history-chip` (theme.css, warning tint) next to the player name only when the total is above 1: a link to `?p=<list>&searchText=<identity>` on desktop (`ban-history-chip` / `comm-history-chip`), a static span inside the mobile drawer-trigger card (`*-history-chip-mobile`). IP-type ban rows get `history_url = ''` (span, no link) when `banlist.hideplayerips` hides IPs from the viewer, so the search URL can't leak the IP. Templates guard with `isset($row.history_total)` for callers that build rows by hand. Regression: `web/tests/integration/PunishmentHistoryChipTest.php`. |
+| Filter the public banlist / commslist by server or time range (`?server=<sid>` / `?time=1d\|7d\|30d` from the filter bar selects) | `$publicFilterClauses` / `$publicFilterAnd` / `$publicFilterWheren` / `$publicFilterArgs` / `$publicFilterLink` near the top of `web/pages/page.banlist.php` (`BA.*`, #1226) and `web/pages/page.commslist.php` (`CO.*`, also carries the `?type=` chip). Every query branch (search / no search / advSearch) appends `$publicFilterAnd` when a WHERE is already open and `$publicFilterWheren` otherwise; `$publicFilterLink` rides `$searchlink` so pagination and the Hide-inactive toggle keep the filter. The filter form always submits `searchText=` (empty), so Apply always lands in the search branch: its `OR` group MUST stay parenthesised, or the AND predicates bind only the last arm and every filter turns into a no-op. Regression: `web/tests/integration/PublicListFilterBarTest.php`. |
 | Filter the public banlist by ban state (`?p=banlist&state=permanent\|active\|expired\|unbanned`) — server-side rowset narrowing for a chip strip | `web/pages/page.banlist.php` (#1352). The `?state=` value is allowlisted (`['permanent','active','expired','unbanned']`, anything else falls through to "All"), composed into the existing `$publicFilterAnd` / `$publicFilterWheren` SQL fragments via the per-state predicates documented inline. **Symmetric defensive shape across the four arms**: `permanent` / `active` BOTH require `RemovedOn IS NULL` (otherwise a pre-2.0 admin-lifted row with `ends > now` would match BOTH `?state=active` AND `?state=unbanned`); `expired` carries THREE arms (`RemoveType = 'E'` post-migration shape + `RemoveType IS NULL AND length > 0 AND ends < now AND RemovedOn IS NULL` for pre-475 installs the prune writer never touched + `RemoveType IS NULL AND RemovedOn IS NOT NULL AND length > 0 AND (RemovedBy IS NULL OR RemovedBy = 0)` for the fork-divergence shape `810.php` pass 2 backfills); `unbanned` carries TWO arms (`RemoveType IN ('D', 'U')` post-migration shape + `RemovedOn IS NOT NULL AND RemoveType IS NULL AND RemovedBy IS NOT NULL AND RemovedBy > 0` for pre-2.0 admin-lifts that `810.php` pass 1 backfills). The `RemovedBy > 0` vs `RemovedBy IS NULL OR = 0` split between unbanned / expired is the **load-bearing distinction** — natural expiry sets `RemovedBy = 0` (PruneBans) or NULL (pre-475); admin lifts set `RemovedBy = <aid>`. The View DTO (`Sbpp\View\BanListView`) carries `string $active_state` + `string $chip_base_link`; the chip strip in `web/themes/default/page_bans.tpl` renders as real `<a href="index.php?p=banlist{$chip_base_link}&state=…">` anchors with `aria-current="true"` (NOT `aria-pressed` — only valid on role=button; axe rejects it on `<a>` as `aria-allowed-attr`) and `data-active="true"` server-rendered on the matching chip — never `<button onclick=…>` (the legacy `web/scripts/banlist.js applyStateFilter` row-hide layer was a client-side filter on top of server-side pagination, so on installs with thousands of bans the unbanned chip silently rendered an empty page 1). The "Hide inactive" toggle is suppressed in the template when `$active_state !== ''` so the two predicates (`hideinactive` is `RemoveType IS NULL`; `?state=expired` / `unbanned` ask for the OPPOSITE) don't visually fight. Pagination URLs preserve `&state=` via `$stateFilterLink`; the per-row state classifier in the same file mirrors the SQL filter (a row pulled in by `?state=unbanned` renders the Unbanned pill, never the legacy mis-classified "Active" pill). The JSON API parity surface is `api_bans_detail` + `api_bans_player_history` in `web/api/handlers/bans.php` — same `isPre2AdminLift` defensive branch so the drawer's detail view + the history pane don't visibly contradict the SQL filter. Regression guards: `web/tests/integration/BanListStateFilterTest.php` (per-state row inclusion / exclusion + chip render contract + suppressed Hide-inactive toggle + symmetric `expired` arm 3 contract), `web/tests/integration/UpdaterBackfillRemoveTypeTest.php` (the migration's idempotency + two-pass cross-contamination contract), `web/tests/api/BansTest.php::testDetailReportsUnbannedForPre2AdminLiftWithRemoveTypeNull` (API parity), `web/tests/e2e/specs/responsive/banlist.spec.ts` + `web/tests/e2e/specs/responsive/filters.spec.ts` (mobile chip click navigates + asserts `aria-current` rather than `history.replaceState` + `aria-pressed`'ing). |
 | 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`). |
 | 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. |

CHANGELOG.md

@@ -10,6 +10,18 @@ Legend:
 ? = Other stuff
 ```
 
+(29/10/26): Version 2.2.8-srcdslab
+-----------------------
+```
+01. ! Ban list: the server and time filters (and the status chips /
+        Hide inactive) did nothing after clicking Apply; the search
+        clauses now wrap in parentheses so every filter applies
+02. ! Comm blocks list: the server and time filters were never applied
+        to the query; they now narrow the list and survive pagination
+03. ! Comm blocks list: Hide inactive combined with an advanced search
+        the viewer can't run no longer fails with a SQL error
+```
+
 (26/10/26): Version 2.2.5-srcdslab (**Run updater required**)
 -----------------------
 ```

web/pages/page.banlist.php

@@ -534,6 +534,14 @@
         $search_array[] = $search;
     }
 
+    // Wrap the OR-search clauses in parens before appending the AND
+    // predicates (hideinactive, server, time, state). Without them SQL
+    // precedence parses `WHERE A OR B OR C OR D AND E` as
+    // `WHERE A OR B OR C OR (D AND E)`, so the filters only narrowed
+    // the reason arm. The filter bar always submits `searchText=`
+    // (empty, so every arm matches), which made server / time / state
+    // / hideinactive no-ops after clicking Apply. Same fix #1274 made
+    // in page.commslist.php.
     $res = $GLOBALS['PDO']->query("SELECT BA.bid ban_id, BA.type, BA.ip ban_ip, BA.authid, BA.name player_name, created ban_created, ends ban_ends, length ban_length, reason ban_reason, BA.ureason unban_reason, BA.aid, AD.gid AS gid, adminIp, BA.sid ban_server, country ban_country, RemovedOn, RemovedBy, RemoveType row_type,
 			SE.ip server_ip, COALESCE(NULLIF(BA.admin_name, ''), AD.user) AS admin_name, AD.gid, MO.icon as mod_icon,
 			CAST(MID(BA.authid, 9, 1) AS UNSIGNED) + CAST('76561197960265728' AS UNSIGNED) + CAST(MID(BA.authid, 11, 10) * 2 AS UNSIGNED) AS community_id,
@@ -543,7 +551,7 @@
   LEFT JOIN `:prefix_servers` AS SE ON SE.sid = BA.sid
   LEFT JOIN `:prefix_mods` AS MO on SE.modid = MO.mid
   LEFT JOIN `:prefix_admins` AS AD ON BA.aid = AD.aid
-      WHERE " . $search_ips . $authidClause . " or BA.name LIKE ? or BA.reason LIKE ?" . $hideinactive . $publicFilterAnd . "
+      WHERE (" . $search_ips . $authidClause . " or BA.name LIKE ? or BA.reason LIKE ?)" . $hideinactive . $publicFilterAnd . "
    ORDER BY BA.created DESC
    LIMIT ?,?")->resultset(array_merge(
         $search_array,
@@ -553,7 +561,7 @@
     ));
 
 
-    $res_count  = $GLOBALS['PDO']->query("SELECT count(BA.bid) AS cnt FROM `:prefix_bans` AS BA WHERE " . $search_ips . $authidClause . " OR BA.name LIKE ? OR BA.reason LIKE ?" . $hideinactive . $publicFilterAnd)->resultset(array_merge(
+    $res_count  = $GLOBALS['PDO']->query("SELECT count(BA.bid) AS cnt FROM `:prefix_bans` AS BA WHERE (" . $search_ips . $authidClause . " OR BA.name LIKE ? OR BA.reason LIKE ?)" . $hideinactive . $publicFilterAnd)->resultset(array_merge(
         $search_array,
         [$authidParam, $search, $search],
         $publicFilterArgs