Skip to content

Commit 101d343

Browse files
committed
Fix #1302: restore IP column on the public banlist for admins
The v2.0 redesign of `web/themes/default/page_bans.tpl` dropped the per-row IP column entirely — even an `ADMIN_OWNER` user lost the at-a-glance IP that v1.x rendered (gated on `is_admin()`). The page handler (`web/pages/page.banlist.php`) was already computing `$hideplayerips = Config::getBool('banlist.hideplayerips') && !$userbank->is_admin()` correctly, and `BanListView` was already exposing `hideplayerips` + the per-row `ban_ip_raw` field, so the fix is template-only: - Reintroduce the IP `<th>` and per-row `<td>` on the desktop table, gated on `{if !$hideplayerips}` (mirrors the existing `{if !$hideadminname}` admin-name guard). `data-testid="ban-ip"` hooks the cell for E2E specs. - Mirror the same gate on the mobile `.ban-cards` block, surfacing the IP under the SteamID line. `data-testid="ban-ip-mobile"` for the responsive E2E. - Bump the empty-state `colspan` from 9 to 10 to cover the maximum render (Player, SteamID, IP, Reason, Server, Admin, Length, Banned, Status, Actions). The wider colspan is harmless when conditional columns hide. - Add `.col-ip` to the existing `white-space: nowrap` rule in `theme.css` so IPv4 strings don't wrap. - Drop `$hideplayerips` from the EOF manifest now that it's referenced for real in the template body. Regression coverage: - `web/tests/integration/BanListIpColumnTest.php` renders `BanListView` end-to-end through Smarty and asserts on the HTML in both directions (visible at `hideplayerips=false`, fully suppressed at `hideplayerips=true`) plus the empty-state `colspan="10"` invariant. - `web/tests/e2e/specs/flows/banlist-ip-column.spec.ts` closes the integration gap end-to-end: the seeded `admin/admin` storage state seeds an IP-bearing ban via `bans.add` and asserts the IP renders in the live desktop `<table>` (chromium project) AND the mobile `.ban-cards` block (mobile-chromium project).
1 parent a9bd72f commit 101d343

4 files changed

Lines changed: 459 additions & 2 deletions

File tree

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
/**
2+
* Flow: public banlist surfaces the IP column to admins (#1302).
3+
*
4+
* The v2.0 redesign of `web/themes/default/page_bans.tpl` dropped
5+
* the per-row IP column entirely — even an `ADMIN_OWNER` user lost
6+
* the at-a-glance IP that v1.x rendered (gated on `is_admin()`).
7+
*
8+
* The page handler (`web/pages/page.banlist.php`) was already
9+
* computing `$hideplayerips = Config::getBool('banlist.hideplayerips')
10+
* && !$userbank->is_admin()` correctly, and `BanListView` already
11+
* carried the per-row `ban_ip_raw` field. Only the template was
12+
* missing — `BanListIpColumnTest` covers the template-level
13+
* regression in both directions (visible + suppressed). This spec
14+
* closes the integration gap end-to-end: the seeded `admin/admin`
15+
* user (storage state from `fixtures/global-setup.ts` —
16+
* `is_admin()` = true via `ALL_WEB`) seeds an IP-bearing ban
17+
* through `bans.add` and asserts the IP renders in the live
18+
* desktop `<table>` AND the mobile `.ban-cards` block.
19+
*
20+
* Project gating
21+
* --------------
22+
* Runs on BOTH desktop-chromium and mobile-chromium so we cover
23+
* the desktop `<table>` testid (`ban-ip`) and the mobile card
24+
* mirror (`ban-ip-mobile`). The describe block branches on
25+
* `isMobile` to assert the right surface per project — keeps the
26+
* regression locked at every viewport.
27+
*/
28+
29+
import type { Page } from '@playwright/test';
30+
import { expect, test } from '../../fixtures/auth.ts';
31+
32+
const SEED_STEAM = 'STEAM_0:1:1302001';
33+
const SEED_NICK = 'e2e-ip-col';
34+
const SEED_IP = '203.0.113.42';
35+
36+
/**
37+
* Seed a Steam ban WITH an IP via `bans.add` and tolerate the
38+
* `already_banned` collision (the panel runs against the dev
39+
* `sourcebans` DB, which persists across `playwright test`
40+
* invocations — see `responsive/banlist.spec.ts`'s `seedBan`
41+
* for the per-DB rationale).
42+
*
43+
* The shared `seedBanViaApi` fixture hardcodes `ip: ''`, so
44+
* we drive `bans.add` directly with the IP set. The handler
45+
* persists `:prefix_bans.ip = '203.0.113.42'` which the page
46+
* handler later reads into the per-row `ban_ip_raw` field.
47+
*/
48+
async function seedBanWithIp(page: Page): Promise<void> {
49+
await page.goto('/');
50+
51+
const env = await page.evaluate(
52+
async (args) => {
53+
const w = window as unknown as {
54+
sb: {
55+
api: {
56+
call: (
57+
action: string,
58+
params: Record<string, unknown>,
59+
) => Promise<{
60+
ok: boolean;
61+
data?: { bid?: number };
62+
error?: { code: string; message: string };
63+
}>;
64+
};
65+
};
66+
Actions: Record<string, string>;
67+
};
68+
return await w.sb.api.call(w.Actions.BansAdd, {
69+
nickname: args.nickname,
70+
steam: args.steam,
71+
type: 0,
72+
ip: args.ip,
73+
length: 0,
74+
reason: 'e2e: banlist IP column seed (#1302)',
75+
});
76+
},
77+
{ nickname: SEED_NICK, steam: SEED_STEAM, ip: SEED_IP },
78+
);
79+
80+
if (env && env.ok === false && env.error?.code !== 'already_banned') {
81+
throw new Error(`bans.add seed failed: ${JSON.stringify(env)}`);
82+
}
83+
}
84+
85+
test.describe('flow: banlist IP column visible to admin (#1302)', () => {
86+
test('IP column renders for the admin caller on the public banlist', async ({
87+
page,
88+
isMobile,
89+
}) => {
90+
await seedBanWithIp(page);
91+
await page.goto('/index.php?p=banlist');
92+
93+
if (isMobile) {
94+
// Mobile chrome is `.ban-cards`, not `<table>` (theme.css
95+
// L266: `.table { display: none }` at <=768px). The IP
96+
// line lives directly under the SteamID line on each
97+
// card, gated on `{if !$hideplayerips && $ban.ban_ip_raw
98+
// != ''}`.
99+
const cards = page.locator('#banlist-root .ban-cards');
100+
await expect(cards).toBeVisible();
101+
102+
const ipLine = page
103+
.locator('[data-testid="ban-ip-mobile"]')
104+
.filter({ hasText: SEED_IP });
105+
await expect(ipLine).toBeVisible();
106+
} else {
107+
// Desktop column header: visible "IP" copy + `.col-ip`
108+
// hook. Anchor on both so a future refactor that reuses
109+
// the class for an unrelated column still trips the
110+
// assertion.
111+
const table = page.locator('#banlist-root .table');
112+
await expect(table).toBeVisible();
113+
const header = table.locator('th.col-ip');
114+
await expect(header).toBeVisible();
115+
await expect(header).toHaveText(/^IP$/);
116+
117+
// Per-row IP cell — testid contract for any future
118+
// filter / styling work.
119+
const seededRow = table
120+
.locator('[data-testid="ban-row"]')
121+
.filter({ hasText: SEED_STEAM });
122+
await expect(seededRow).toBeVisible();
123+
const ipCell = seededRow.locator('[data-testid="ban-ip"]');
124+
await expect(ipCell).toBeVisible();
125+
await expect(ipCell).toContainText(SEED_IP);
126+
}
127+
});
128+
});

0 commit comments

Comments
 (0)