From 74555ea2a6e390a6421cecd4ca0644b693411bc8 Mon Sep 17 00:00:00 2001 From: rumblefrog Date: Sat, 4 Jul 2026 19:03:25 -0400 Subject: [PATCH] fix(admin-servers): surface copyable Server ID on server cards (#1504) The v2.0 card-grid rewrite of the admin Servers list dropped the numeric Server ID from view, but the SourceMod plugin's sourcebans.cfg and the setup docs both tell operators to find it "in the admin panel -> servers". Add a labelled, copyable "Server ID" row to each server card so operators can wire the plugin's ServerID field without guessing. - page_admin_servers_list.tpl: labelled Server ID row + copy button (rides the existing document-level [data-copy] delegate in theme.js) - docs: point adding-server.md + quickstart.mdx at the card's Server ID (was a nonexistent "ID column"); fix the admin nav label (Servers, not Server Settings) - tests: PHPUnit markup contract (enabled + disabled tiles) + E2E render + critical-a11y coverage of the populated card --- .../docs/getting-started/quickstart.mdx | 7 +- docs/src/content/docs/setup/adding-server.md | 11 +- web/tests/e2e/pages/admin/AdminServers.ts | 17 +++ .../e2e/specs/flows/admin-server-id.spec.ts | 105 ++++++++++++++++++ .../AdminServersListHydrationTest.php | 48 ++++++++ .../default/page_admin_servers_list.tpl | 24 +++- 6 files changed, 203 insertions(+), 9 deletions(-) create mode 100644 web/tests/e2e/specs/flows/admin-server-id.spec.ts diff --git a/docs/src/content/docs/getting-started/quickstart.mdx b/docs/src/content/docs/getting-started/quickstart.mdx index 9aba3bf1d..d4dd7db6d 100644 --- a/docs/src/content/docs/getting-started/quickstart.mdx +++ b/docs/src/content/docs/getting-started/quickstart.mdx @@ -208,10 +208,11 @@ hijack your panel. ## Step 4 — Register your first game server -In the panel, navigate to **Admin Panel → Server Settings → Add new +In the panel, navigate to **Admin Panel → Servers → Add new server**. Fill in the form (game, RCON password, IP, port), click -**Add Server**, and note the **`ID` column** for the new row. That -number is the `ServerID` the plugin needs in the next step. +**Add Server**, and note the **`Server ID`** shown on the new +server's card. That number is the `ServerID` the plugin needs in the +next step. Full walkthrough with screenshots: [Adding a server](/setup/adding-server/). diff --git a/docs/src/content/docs/setup/adding-server.md b/docs/src/content/docs/setup/adding-server.md index 7fbc0edde..b9356ba00 100644 --- a/docs/src/content/docs/setup/adding-server.md +++ b/docs/src/content/docs/setup/adding-server.md @@ -32,7 +32,7 @@ a server without RCON access. 1. Sign into the web panel as an admin with the **Add server** permission (owners have this by default). -2. Navigate to **Admin Panel → Server Settings → Add new server**. +2. Navigate to **Admin Panel → Servers → Add new server**. 3. Fill in: @@ -43,9 +43,10 @@ a server without RCON access. 4. Click **Add server**. -5. After the page reloads, find the new row in the server list and - **note the `ID` column**. That number is the `ServerID` you'll - write into the SourceMod plugin's config in the next step. +5. After the page reloads, find the new server's card in the list and + **note its `Server ID`**. That number is the `ServerID` you'll + write into the SourceMod plugin's config in the next step. Use the + copy button next to it to grab the value. The panel attempts an RCON connection right away to validate the details. If it can't reach the server, you'll see a warning, but @@ -95,7 +96,7 @@ If it doesn't: :::tip Adding a second or third server later? Repeat the same two halves (panel registration + plugin `ServerID` edit) on each new server. -Each server gets its own row in the panel and its own `ServerID`. +Each server gets its own card in the panel and its own `ServerID`. The `databases.cfg` section stays the same across all servers as long as they share the same database. ::: diff --git a/web/tests/e2e/pages/admin/AdminServers.ts b/web/tests/e2e/pages/admin/AdminServers.ts index d246f5228..da056ca56 100644 --- a/web/tests/e2e/pages/admin/AdminServers.ts +++ b/web/tests/e2e/pages/admin/AdminServers.ts @@ -50,6 +50,23 @@ export class AdminServersPage extends BasePage { return this.page.locator('script[src$="server-tile-hydrate.js"]'); } + /** + * The tile for a specific server, keyed on the stable + * `data-id=""` hook the hydration helper also routes on. + */ + tile(sid: number): Locator { + return this.page.locator(`[data-testid="server-tile"][data-id="${sid}"]`); + } + + /** + * The "Server ID" `
` inside a tile (#1504) — carries the + * numeric value span + the copy button. Operators need this to + * wire the SourceMod plugin's `sourcebans.cfg` `ServerID` field. + */ + serverIdRow(sid: number): Locator { + return this.tile(sid).locator('[data-testid="server-id"]'); + } + async goto(): Promise { await super.goto(this.path); } diff --git a/web/tests/e2e/specs/flows/admin-server-id.spec.ts b/web/tests/e2e/specs/flows/admin-server-id.spec.ts new file mode 100644 index 000000000..1a2112445 --- /dev/null +++ b/web/tests/e2e/specs/flows/admin-server-id.spec.ts @@ -0,0 +1,105 @@ +/** + * Flow: the admin Server Management list surfaces each server's numeric + * Server ID on its card (#1504). + * + * Background + * ---------- + * The SourceMod plugin's `sourcebans.cfg` carries a `ServerID` field + * whose inline comment reads "Check in the admin panel -> servers to + * find the ID of this server", and the setup docs tell operators to + * note the ID after adding a server. But the v2.0 card-grid rewrite of + * `page_admin_servers_list.tpl` never printed the sid anywhere the + * operator could read it (only inside RCON/Edit/Admins hrefs and the + * no-icon fallback glyph), so operators had no way to find the value + * the plugin needs. The reporter (issue #1504) hit exactly this wall. + * + * This spec seeds a server through the JSON API and asserts the tile + * renders a labelled, copyable "Server ID" row showing the raw sid. + * The clipboard round-trip itself is already exercised by the shared + * [data-copy] delegate spec (`flows/ui/copy-buttons.spec.ts`), so this + * spec locks the render contract only. + * + * It also runs the critical-axe gate on the POPULATED card. The + * sibling smoke spec (`smoke/admin/servers.spec.ts`) deliberately + * never seeds a row, so its axe pass only covers the empty state — + * the new icon-only copy button + Server ID row only exist once a + * tile renders, and this is the only spec that puts axe on them. + * + * Selectors are `data-testid` per #1123. Single-project (chromium) + * because it drives `truncateE2eDb()` — same DB-isolation rationale as + * `server-refresh-debounce.spec.ts` (see that file's comment for the + * truncate-vs-Apache race against the shared `sourcebans_e2e` DB). + */ + +import { expect, test } from '../../fixtures/auth.ts'; +import { expectNoCriticalA11y } from '../../fixtures/axe.ts'; +import { truncateE2eDb } from '../../fixtures/db.ts'; +import { AdminServersPage } from '../../pages/admin/AdminServers.ts'; + +test.describe('flow: admin servers — Server ID is discoverable (#1504)', () => { + test.beforeEach(({}, testInfo) => { + test.skip( + testInfo.project.name !== 'chromium', + 'Browser-shape-agnostic server-rendered markup; skip the second project to avoid the truncate-vs-Apache race against sourcebans_e2e (see server-refresh-debounce.spec.ts).', + ); + }); + + test('the server card shows the numeric Server ID with a copy button', async ({ page }, testInfo) => { + await truncateE2eDb(); + + // Seed a server via the JSON API. RFC 5737 documentation IP so + // no real Source server can ever answer the A2S probe — this + // spec only cares about the server-rendered Server ID, not the + // live hydration cells. + await page.goto('/index.php?p=admin&c=servers§ion=add'); + const addEnvelope = await page.evaluate(async () => { + const w = window as unknown as { + sb: { api: { call: (a: string, p: Record) => Promise<{ ok: boolean }> } }; + Actions: Record; + }; + return await w.sb.api.call(w.Actions.ServersAdd, { + ip: '203.0.113.7', + port: '27015', + rcon: '', + rcon2: '', + mod: 1, + enabled: true, + group: '0', + }); + }); + expect(addEnvelope, 'servers.add envelope should round-trip ok').toMatchObject({ ok: true }); + + const p = new AdminServersPage(page); + await p.goto(); + await expect(p.pageMounted).toBeVisible(); + + // Resolve the seeded sid from the DOM without relying on the + // API envelope shape (that is not the contract under test): + // there is exactly one tile after the truncate + single seed. + const seededTile = page.locator('[data-testid="server-tile"][data-id]').first(); + await expect(seededTile).toBeVisible(); + const sidAttr = await seededTile.getAttribute('data-id'); + expect(sidAttr, 'seeded tile must carry a data-id sid').toBeTruthy(); + const sid = Number(sidAttr); + + // The labelled Server ID row is visible and prints the sid. + const idRow = p.serverIdRow(sid); + await expect(idRow).toBeVisible(); + await expect(idRow.locator('[data-testid="server-id-value"]')).toHaveText(String(sid)); + + // The copy button carries the sid as its clipboard payload so + // the shared [data-copy] delegate can write it verbatim. + const copyBtn = idRow.locator('[data-testid="server-id-copy"]'); + await expect(copyBtn).toBeVisible(); + await expect(copyBtn).toHaveAttribute('data-copy', String(sid)); + + // The literal label anchors discoverability — the whole point + // of the fix is that "Server ID" is written on the card. + await expect(p.tile(sid)).toContainText('Server ID'); + + // No critical a11y regressions on the populated card (the + // icon-only copy button carries aria-label + title; the smoke + // spec's axe pass only sees the empty state). + await expectNoCriticalA11y(page, testInfo); + }); +}); diff --git a/web/tests/integration/AdminServersListHydrationTest.php b/web/tests/integration/AdminServersListHydrationTest.php index 1296bc724..239df5592 100644 --- a/web/tests/integration/AdminServersListHydrationTest.php +++ b/web/tests/integration/AdminServersListHydrationTest.php @@ -191,6 +191,54 @@ public function testDisabledTileSkipsHydration(): void 'Disabled tile must NOT render a refresh button — there is nothing to re-query (#1313).'); } + /** + * Issue #1504 — the SourceMod plugin's `sourcebans.cfg` and the + * setup docs both tell operators to "check the admin panel -> + * servers" to find the numeric `ServerID` a game server needs, + * but the card grid never surfaced it. Every enabled AND disabled + * tile must render a labelled, copyable "Server ID" row showing + * the raw sid so operators can wire the plugin without guessing. + */ + public function testTileRendersCopyableServerId(): void + { + $_GET = ['p' => 'admin', 'c' => 'servers', 'section' => 'list']; + $html = $this->renderAdminServersPage(); + + foreach ([$this->enabledSid, $this->disabledSid] as $sid) { + $tileHtml = $this->extractTile($html, $sid); + + $this->assertStringContainsString('data-testid="server-id"', $tileHtml, + 'Every tile must expose data-testid="server-id" so operators can find the ServerID the plugin needs (#1504).'); + // The raw sid is rendered inside the value span. + $this->assertMatchesRegularExpression( + '/data-testid="server-id-value"[^>]*>' . $sid . 'assertMatchesRegularExpression( + '/]*\bdata-testid="server-id-copy")(?=[^>]*\bdata-copy="' . $sid . '")/', + $tileHtml, + "Tile for sid $sid must ship a copy button carrying data-copy=\"$sid\" (#1504)." + ); + // The human-readable label anchors discoverability — the + // whole point of the fix is that "Server ID" is written on + // the card, not just derivable from a URL. Assert the + // visible
, not a bare substring: the copy button's + // title="Copy Server ID" / aria-label="Copy Server ID" + // would otherwise satisfy a substring match even if the + // label row were deleted. + $this->assertMatchesRegularExpression( + '/]*>Server ID<\/dt>/', + $tileHtml, + "Tile for sid $sid must carry the visible 'Server ID'
label (#1504)." + ); + } + } + /** * The View DTO and handler must continue to feed the template the * fields the markup contract depends on. This is a belt-and-braces diff --git a/web/themes/default/page_admin_servers_list.tpl b/web/themes/default/page_admin_servers_list.tpl index af8e2070b..39f7d129e 100644 --- a/web/themes/default/page_admin_servers_list.tpl +++ b/web/themes/default/page_admin_servers_list.tpl @@ -146,11 +146,33 @@ Loading {else} - Disabled + Disabled {/if}
+ {* + Server ID (#1504): the numeric sid the SourceMod + plugin's sourcebans.cfg "ServerID" field needs. + The plugin config + docs both tell operators to + "check the admin panel -> servers" for it, but the + card grid never surfaced it — this labelled, + copyable row is the fix. The copy button rides the + document-level [data-copy] delegate in theme.js + (secure-context clipboard + execCommand fallback). + *} +
Server ID
+
+ {$server.sid} + +
Mod
{if isset($server.mod_name)}{$server.mod_name|escape}{else}unknown{/if}
Players