[www] show lifted bans

apps/api/migrations/0029_report_ban_lifted.sql

@@ -0,0 +1,41 @@
+-- Who LIFTED a ban, and when. Owned by the `api` worker; generated from src/reports-db.ts
+-- (SCHEMA_DDL) — keep in sync.
+--
+-- A lift clears `banned`, `ban_expires`, `banned_by_player_id` and `banned_at` (see
+-- 0020_report_ban_audit.sql), which leaves the row looking exactly like a report nobody
+-- ever acted on. On the staff panel that read as "Ban…" next to a report whose ban a
+-- colleague had deliberately lifted an hour earlier, and a second moderator re-banned the
+-- player without knowing. The `audit_log` still had the lift, but nothing reads it per row.
+--
+-- So the lift now signs the row: `unbanned_by_player_id` is the moderator who lifted the
+-- ban and `unbanned_at` when they did. Both are NULL until a ban is lifted, and are CLEARED
+-- again by a re-ban (the row describes the ban's current state, and a standing ban has not
+-- been lifted). A non-null `unbanned_at` on a `banned = 0` row therefore means "somebody
+-- decided this player should be back in the game", which is what the panel shows.
+--
+-- Rows lifted before this migration are backfilled from `audit_log` (0024), where every
+-- lift has been recorded as an `unban` row naming the report in `data.reportId` — the
+-- latest such row per report, since a report can be banned and lifted more than once.
+-- Only rows not banned NOW are touched: a re-ban after a lift has cleared the lift.
+--
+-- Neither column is indexed: read per row on the panel's tables, never filtered on.
+
+ALTER TABLE report ADD COLUMN unbanned_by_player_id INTEGER;
+ALTER TABLE report ADD COLUMN unbanned_at TEXT;
+
+UPDATE report
+SET unbanned_by_player_id = (
+      SELECT player_id FROM audit_log
+      WHERE action = 'unban' AND json_extract(data, '$.reportId') = report.id
+      ORDER BY audit_log_id DESC LIMIT 1
+    ),
+    unbanned_at = (
+      SELECT date FROM audit_log
+      WHERE action = 'unban' AND json_extract(data, '$.reportId') = report.id
+      ORDER BY audit_log_id DESC LIMIT 1
+    )
+WHERE banned = 0
+  AND EXISTS (
+    SELECT 1 FROM audit_log
+    WHERE action = 'unban' AND json_extract(data, '$.reportId') = report.id
+  );

apps/api/src/reports-db.ts

@@ -42,7 +42,7 @@
 /**
  * Schema DDL (mirror of migrations/0004_report.sql + 0009_report_ban.sql +
  * 0011_report_event.sql + 0016_report_invention.sql + 0017_report_custom_avatar_item.sql +
- * 0020_report_ban_audit.sql + 0025_report_chat_message.sql).
+ * 0020_report_ban_audit.sql + 0025_report_chat_message.sql + 0029_report_ban_lifted.sql).
  *
  * None of `event_id`, `invention_id`, `custom_avatar_item_id` or `chat_message_id` is
  * indexed: each is written on every report of its kind and read by nothing — no query here filters on any of them,
@@ -69,7 +69,9 @@ export const SCHEMA_DDL: string[] = [
 		custom_avatar_item_id TEXT,
 		banned_by_player_id INTEGER,
 		banned_at TEXT,
-		chat_message_id INTEGER
+		chat_message_id INTEGER,
+		unbanned_by_player_id INTEGER,
+		unbanned_at TEXT
 	)`,
 	`CREATE INDEX IF NOT EXISTS idx_report_reported ON report (reported_player_id)`,
 	`CREATE INDEX IF NOT EXISTS idx_report_reporter ON report (reporter_player_id)`,
@@ -137,6 +139,16 @@ export interface ReportRow {
 	 * reference.
 	 */
 	chat_message_id: number | null
+	/**
+	 * The moderator who LIFTED the ban that once stood on this row, or NULL when none has
+	 * been lifted — including while one stands: a re-ban clears it, with `unbanned_at`. A
+	 * lift wipes the four ban columns above, so without this pair a lifted ban is
+	 * indistinguishable from a report nobody acted on, and the staff panel offered to ban
+	 * again. Read per row by the panel; nothing filters on it.
+	 */
+	unbanned_by_player_id: number | null
+	/** ISO-8601 UTC instant the ban was lifted; NULL with `unbanned_by_player_id`. */
+	unbanned_at: string | null
 }
 
 /**
@@ -264,6 +276,11 @@ export async function isPlayerBanned(
  * not keep an audit trail saying a ban runs from somewhere, and `banned = 0` with a
  * `banned_at` still set would read as a ban to anything checking the timestamp.
  *
+ * A lift instead SIGNS `unbanned_by_player_id`/`unbanned_at` with the same `bannedBy` and
+ * the same instant, and a re-ban clears those two — a standing ban has not been lifted.
+ * That is what lets the staff panel tell "a colleague let this player back in" from
+ * "nobody has acted on this report", so a second moderator doesn't undo the first.
+ *
  * Returns the updated row, or null when there is no report with that id — so the caller
  * can tell "banned" from "banned nobody" (wrangler's `d1 execute --json` reports no
  * changes count, hence RETURNING).
@@ -274,10 +291,12 @@ export async function banFromReport(
 	options: { banned?: boolean; banExpires?: string | null; bannedBy?: number | null } = {}
 ): Promise<ReportRow | null> {
 	const banned = options.banned ?? true
+	const now = new Date().toISOString()
 	return db
 		.prepare(
 			`UPDATE report
-			 SET banned = ?2, ban_expires = ?3, banned_by_player_id = ?4, banned_at = ?5
+			 SET banned = ?2, ban_expires = ?3, banned_by_player_id = ?4, banned_at = ?5,
+			     unbanned_by_player_id = ?6, unbanned_at = ?7
 			 WHERE id = ?1
 			 RETURNING *`
 		)
@@ -286,7 +305,9 @@ export async function banFromReport(
 			banned ? 1 : 0,
 			banned ? (options.banExpires ?? null) : null,
 			banned ? (options.bannedBy ?? null) : null,
-			banned ? new Date().toISOString() : null
+			banned ? now : null,
+			banned ? null : (options.bannedBy ?? null),
+			banned ? null : now
 		)
 		.first<ReportRow>()
 }

apps/www/src/client/Moderation.tsx

@@ -94,6 +94,14 @@ interface ReportRow {
 	banned_by_player_id: number | null
 	banned_at: string | null
 	chat_message_id: number | null
+	/**
+	 * Who lifted the ban that once stood on this row, and when — null until one is lifted,
+	 * and cleared again by a re-ban. A lift wipes the four ban columns, so this pair is the
+	 * only thing on the row that says a colleague decided this player belongs back in the
+	 * game; without it the row reads as untouched and the next moderator bans them again.
+	 */
+	unbanned_by_player_id: number | null
+	unbanned_at: string | null
 }
 
 /** A moderator-issued warning, as the `warning` table stores it. */
@@ -171,18 +179,17 @@ const categoryLabel = (id: number): string => CATEGORY_LABEL[id] ?? `Category ${
 /** How many reports one page of search results holds. */
 const PAGE_SIZE = 25
 
-/** A date as a moderator reads it — local time, to the minute, no seconds. */
+/**
+ * A date as a moderator reads it — ISO 8601 in UTC, to the minute (`2026-09-25T13:04Z`).
+ * Not local time: the moderators are in different countries, and a ban that expires
+ * "01:04 PM" means a different instant to each of them and a different date on either
+ * side of midnight. One unambiguous spelling, the same on every screen.
+ */
 function when(iso: string | null): string {
 	if (iso === null) return '—'
 	const parsed = Date.parse(iso)
 	if (Number.isNaN(parsed)) return iso
-	return new Date(parsed).toLocaleString(undefined, {
-		year: 'numeric',
-		month: 'short',
-		day: 'numeric',
-		hour: '2-digit',
-		minute: '2-digit',
-	})
+	return new Date(parsed).toISOString().slice(0, 16) + 'Z'
 }
 
 /**
@@ -193,6 +200,35 @@ function when(iso: string | null): string {
 const expiryLabel = (report: ReportRow): string =>
 	report.banned !== 1 ? '—' : report.ban_expires === null ? 'Permanent' : when(report.ban_expires)
 
+/**
+ * A report whose ban a moderator LIFTED — as opposed to one nobody acted on, which the row
+ * otherwise looks identical to. A standing ban is never "lifted": a re-ban clears the pair.
+ */
+const banLifted = (report: ReportRow): boolean => report.banned !== 1 && report.unbanned_at !== null
+
+/**
+ * "(lifted) by X" — shown wherever a lifted ban would otherwise pass for a clean report.
+ * It is what stops a second moderator undoing the first one's decision without knowing
+ * there was one: the row says who let the player back in, so the question goes to them
+ * rather than straight back to the ban button. Just the who — when it was lifted is on
+ * the row (`unbanned_at`) but not worth the width in a table cell.
+ */
+function LiftedBan({ report, names }: { report: ReportRow; names: Map<number, PublicAccount> }) {
+	return (
+		<>
+			<span className="badge mod-lifted">lifted</span>{' '}
+			<span className="muted">
+				by{' '}
+				{report.unbanned_by_player_id === null ? (
+					'staff'
+				) : (
+					<PlayerName id={report.unbanned_by_player_id} names={names} />
+				)}
+			</span>
+		</>
+	)
+}
+
 /**
  * What kind of thing a report is against. The four id columns are mutually exclusive and
  * a row with none of them is an ordinary player report (see reports-db) — which is the
@@ -206,13 +242,16 @@ function reportKind(report: ReportRow): string {
 	return 'Player'
 }
 
-/** A player as a name plus id — the id always shown, since ids are what the rows carry. */
+/**
+ * A player as `@name`. The id is in the title, not on screen: it costs width in every
+ * cell of every table and staff can look it up when they need it. It IS shown while the
+ * name is still loading or for an account with none, so a row is never nameless.
+ */
 function PlayerName({ id, names }: { id: number; names: Map<number, PublicAccount> }) {
 	const account = names.get(id)
 	return (
-		<span className="mod-player">
-			{account?.username ? `@${account.username}` : 'unknown'}
-			<span className="muted"> #{id}</span>
+		<span className="mod-player" title={`#${id}`}>
+			{account?.username ? `@${account.username}` : <span className="muted">#{id}</span>}
 		</span>
 	)
 }
@@ -673,7 +712,13 @@ function ReportSearch({
 		[revision]
 	)
 	const reports = data?.reports ?? []
-	const names = useNames(reports.flatMap((r) => [r.reported_player_id, r.reporter_player_id]))
+	const names = useNames(
+		reports.flatMap((r) => [
+			r.reported_player_id,
+			r.reporter_player_id,
+			r.unbanned_by_player_id ?? 0,
+		])
+	)
 
 	const submit = (e: React.FormEvent) => {
 		e.preventDefault()
@@ -888,6 +933,8 @@ function ReportTable({
 							<td>
 								{report.banned === 1 ? (
 									<span className="badge mod-banned">{expiryLabel(report)}</span>
+								) : banLifted(report) ? (
+									<LiftedBan report={report} names={names} />
 								) : (
 									<span className="muted">—</span>
 								)}
@@ -897,7 +944,7 @@ function ReportTable({
 									<LiftBanButton report={report} onDone={onChanged} />
 								) : (
 									<button className="linkish" onClick={() => onBan(report)}>
-										Ban…
+										{banLifted(report) ? 'Ban again…' : 'Ban…'}
 									</button>
 								)}
 							</td>
@@ -1056,8 +1103,14 @@ function PlayerHistory({
 	const names = useNames([
 		playerId,
 		...reports.map((r) => r.reporter_player_id),
+		...reports.map((r) => r.unbanned_by_player_id ?? 0),
 		...warnings.map((w) => w.moderator_player_id),
 	])
+	// The most recent lift, if any — it is the thing a moderator about to ban needs to know
+	// first, since the report rows below say "Ban again…" and the reason is up here.
+	const lastLifted = reports
+		.filter(banLifted)
+		.sort((a, b) => (a.unbanned_at! < b.unbanned_at! ? 1 : -1))[0]
 
 	return (
 		<>
@@ -1070,10 +1123,18 @@ function PlayerHistory({
 				{data === null ? (
 					!error && <p className="muted">Loading…</p>
 				) : data.activeBan === null ? (
-					<p className="muted">
-						Not banned. {reports.length} report{reports.length === 1 ? '' : 's'} on file,{' '}
-						{warnings.length} warning{warnings.length === 1 ? '' : 's'} handed down.
-					</p>
+					<>
+						<p className="muted">
+							Not banned. {reports.length} report{reports.length === 1 ? '' : 's'} on file,{' '}
+							{warnings.length} warning{warnings.length === 1 ? '' : 's'} handed down.
+						</p>
+						{lastLifted !== undefined && (
+							<p className="mod-lifted-note">
+								<LiftedBan report={lastLifted} names={names} /> — on report #{lastLifted.id} for{' '}
+								{categoryLabel(lastLifted.report_category)}. Check with them before banning again.
+							</p>
+						)}
+					</>
 				) : (
 					<p className="ok">
 						Banned on report #{data.activeBan.id} for{' '}
@@ -1329,7 +1390,9 @@ function BanDialog({ reportId, navigate }: { reportId: number; navigate: (to: st
 	const { data: report, error: loadError } = useStaffData<ReportRow>(
 		`/api/staff/reports/${reportId}`
 	)
-	const names = useNames(report ? [report.reported_player_id] : [])
+	const names = useNames(
+		report ? [report.reported_player_id, report.unbanned_by_player_id ?? 0] : []
+	)
 
 	if (loadError !== '') {
 		return (
@@ -1378,6 +1441,15 @@ function BanDialog({ reportId, navigate }: { reportId: number; navigate: (to: st
 				On report #{report.id} — {categoryLabel(report.report_category)}
 				{report.details ? `: “${report.details}”` : ''}
 			</p>
+			{/* The one thing this form must not do quietly: a ban on this very report was
+			    lifted on purpose, and applying another undoes a colleague's decision. The
+			    form still works — sometimes a re-ban IS the decision — but not by accident. */}
+			{banLifted(report) && (
+				<p className="mod-lifted-note">
+					<LiftedBan report={report} names={names} />. Banning again overrides that — check with
+					them first unless something new has happened.
+				</p>
+			)}
 
 			<LinkedAccountsPanel playerId={report.reported_player_id} />
 
@@ -1422,7 +1494,7 @@ function BanDialog({ reportId, navigate }: { reportId: number; navigate: (to: st
 				{error && <p className="error">{error}</p>}
 				<div className="mod-filter-actions">
 					<button type="submit" disabled={pending}>
-						{pending ? 'Banning…' : 'Apply ban'}
+						{pending ? 'Banning…' : banLifted(report) ? 'Ban again' : 'Apply ban'}
 					</button>
 					<button
 						type="button"

apps/www/src/client/styles.css

@@ -1694,6 +1694,20 @@ button[type='submit']:disabled {
 	margin-left: 8px;
 }
 
+/* A lifted ban: the OTHER state that has to be noticed without reading, since the row is
+   otherwise indistinguishable from one nobody acted on — and acting on it again undoes a
+   colleague's decision. Accent rather than red: it is a caution, not an alarm. */
+.badge.mod-lifted {
+	color: var(--accent);
+	border-color: color-mix(in srgb, var(--accent) 45%, transparent);
+}
+
+.mod-lifted-note {
+	border-left: 3px solid var(--accent);
+	padding-left: 12px;
+	margin: 12px 0;
+}
+
 .mod-pager {
 	display: flex;
 	align-items: center;

apps/www/src/staff.ts

@@ -416,7 +416,11 @@ export async function createReportHandler(c: Context<App>) {
  *
  * `banned: false` lifts it, clearing the expiry and the audit columns and leaving the
  * report itself intact — the panel needs to be able to undo a ban, and a lifted ban has to
- * be distinguishable from an expired one (`banned = 0` versus a past `ban_expires`).
+ * be distinguishable from an expired one (`banned = 0` versus a past `ban_expires`). The
+ * lift is signed onto the row (`unbanned_by_player_id`/`unbanned_at`, by `banFromReport`)
+ * so the tables can say a colleague let the player back in, rather than offering "Ban…"
+ * on a row that looks untouched — which is how a lifted ban got re-applied by the next
+ * moderator to see it.
  *
  * Applying one also EJECTS the player from any instance they are in (see
  * {@link kickBannedPlayer}), best-effort and after the row is committed. The response

apps/www/src/test/integration/api.test.ts

@@ -994,7 +994,7 @@ it('lifts a ban, clearing the expiry and the audit columns but keeping the repor
 		200
 	)
 
-	const lifted = await staffPost(`/api/staff/reports/${report.id}/ban`, 8110, { banned: false })
+	const lifted = await staffPost(`/api/staff/reports/${report.id}/ban`, 8111, { banned: false })
 	expect(lifted.status).toBe(200)
 	const row = (await lifted.json()) as Record<string, unknown>
 	expect(row.banned).toBe(0)
@@ -1008,6 +1008,62 @@ it('lifts a ban, clearing the expiry and the audit columns but keeping the repor
 	expect(bans.map((b) => b.id)).not.toContain(report.id)
 })
 
+// A lift wipes the ban columns, so without its own signature the row reads exactly like a
+// report nobody acted on — and the next moderator to search the player saw "Ban…" and
+// re-banned someone a colleague had deliberately let back in. The lift signs the row with
+// who and when; the search serves it; a re-ban clears it, since a standing ban is not lifted.
+it('signs a lifted ban with who lifted it, and clears that on a re-ban', async () => {
+	const player = 8240
+	const report = await createReport(env.DB, { reporterPlayerId: 8241, reportedPlayerId: player })
+	expect((await staffPost(`/api/staff/reports/${report.id}/ban`, 8110, { days: 1 })).status).toBe(
+		200
+	)
+	// Untouched until a lift — a standing ban has not been lifted.
+	const banned = (await (await staffGet(`/api/staff/reports/${report.id}`, 8110)).json()) as Record<
+		string,
+		unknown
+	>
+	expect(banned).toMatchObject({ banned: 1, unbanned_by_player_id: null, unbanned_at: null })
+
+	const lifted = (await (
+		await staffPost(`/api/staff/reports/${report.id}/ban`, 8111, { banned: false })
+	).json()) as Record<string, unknown>
+	// Signed by the LIFTING moderator, not the banning one.
+	expect(lifted).toMatchObject({ banned: 0, unbanned_by_player_id: 8111 })
+	expect(lifted.unbanned_at).toEqual(expect.any(String))
+
+	// The search — `/moderation/search?reportedPlayerId=…` — is where the next moderator
+	// meets the row, so it has to carry the signature too.
+	const search = (await (
+		await staffGet(`/api/staff/reports?reportedPlayerId=${player}`, 8110)
+	).json()) as { reports: Array<Record<string, unknown>> }
+	expect(search.reports).toHaveLength(1)
+	expect(search.reports[0]).toMatchObject({
+		id: report.id,
+		banned: 0,
+		unbanned_by_player_id: 8111,
+		unbanned_at: lifted.unbanned_at,
+	})
+	// And the player's own page, which the search links to.
+	const history = (await (await staffGet(`/api/staff/players/${player}`, 8110)).json()) as {
+		activeBan: unknown
+		reports: Array<Record<string, unknown>>
+	}
+	expect(history.activeBan).toBeNull()
+	expect(history.reports[0]).toMatchObject({ unbanned_by_player_id: 8111 })
+
+	// Banning again — knowingly — is a standing ban once more, with no lift on it.
+	const rebanned = (await (
+		await staffPost(`/api/staff/reports/${report.id}/ban`, 8112, { days: 7 })
+	).json()) as Record<string, unknown>
+	expect(rebanned).toMatchObject({
+		banned: 1,
+		banned_by_player_id: 8112,
+		unbanned_by_player_id: null,
+		unbanned_at: null,
+	})
+})
+
 // The report row keeps only the ban's CURRENT state — a lift wipes who banned and when —
 // so the audit log is the one place a ban and its lift are both still on record.
 it('records a ban and its lift on the audit log, against the moderator', async () => {