Skip to content

fix(banlist): surface demo download in the player drawer - #26

Merged
Rushaway merged 1 commit into
mainfrom
fix/issue-1554
Sep 6, 2026
Merged

Rushaway merged 1 commit into
mainfrom
fix/issue-1554

Conversation

@Rushaway

@Rushaway Rushaway commented Sep 3, 2026

Copy link
Copy Markdown
Member

Problem

Upstream issue sbpp#1554: the button to download a banned
player's demo is missing. In SourceBans 1.x the sliding ban panel showed
a "Review Demo" link; the 2.0 player drawer never rendered one.

Root cause

api_bans_detail returns demo_count, but renderOverviewPane() in
web/themes/default/js/theme.js never used it — the drawer Overview pane
had no demo affordance at all. getdemo.php?type=B&id=<bid> already
serves ban demos; nothing linked to it from the review surface.

Fix

Add a "Download demo" button to the drawer Overview pane (ban-focal only,
comm-blocks have no demo column) when demo_count > 0, linking to
getdemo.php?type=B&id=<bid>. New data-testid="drawer-demo-download".

Adds BanDrawerDemoDownloadTest pinning the wiring.

🤖 Generated with Claude Code

The SourceBans 1.x sliding ban panel rendered a "Review Demo" link when
a ban had an uploaded demo. The 2.0 player drawer dropped it: `bans.detail`
still returns `demo_count`, but `renderOverviewPane` in theme.js never
turned it into a download affordance, so demos attached to bans were
unreachable from the review surface.

Add a "Download demo" button to the Overview pane (ban-focal only) linking
to `getdemo.php?type=B&id=<bid>` when `demo_count > 0`.

Ports sbpp#1554.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Rushaway

Rushaway commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

Review (Sonnet — Opus quota exhausted, resets 19:00 Europe/Paris)

Verdict: LGTM, no code changes needed. CI green (PHPUnit / Playwright+axe / static / tsc all pass).

Verified:

  • bans.detail genuinely carries demo_count (web/api/handlers/bans.php:862 subquery → :1013 (int) cast) and top-level bid (web/scripts/api-contract.js typedef + __snapshots__/bans/detail_*.json). The > 0 guard coerces string/undefined safely.
  • Ban-focal only is correct — comms.detail has no demo column, and isComm is in scope in renderOverviewPane.
  • Markup escaping fine (escapeHtml(demoUrl), bid through encodeURIComponent); Lucide re-init runs via showDrawer()lucide.createIcons().
  • Button visibility matches existing fork behaviour: web/pages/page.banlist.php:1050-1052 already builds demo_link/demo_quick for any banlist viewer when demo_count > 0.

Note (pre-existing, not introduced here): web/getdemo.php performs no auth/authorization check — any caller who knows a bid can fetch the ban demo. This PR only makes the existing link more discoverable in the drawer. If ban demos should be admin-only, that's a separate hardening ticket on getdemo.php itself.

@maxijabase

Copy link
Copy Markdown
Collaborator

LGTM!

@Rushaway
Rushaway merged commit 47c6d5c into main Sep 6, 2026
4 checks passed
@Rushaway
Rushaway deleted the fix/issue-1554 branch September 6, 2026 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants