fix(banlist/commslist): restore per-punishment comment buttons (#1544)

[Rushaway]

What

Restores the per-punishment comment call-to-action buttons on the public ban list and comms list that were dropped in the 2.0.0 theme rebuild, and surfaces the same add/edit/delete affordances in the player drawer.

Ports upstream issue sbpp#1544 ("bug: Missing comment buttons on banlist/commslist"). No upstream fix exists yet; this is an original fix in fork style.

Why

page.banlist.php / page.commslist.php already build addcomment, editcomlink and delcomlink for every row (and editcomlink / delcomlink are permission-gated in the handler). The 2.0 templates render the inline comment thread but never printed these controls, so there was no way to add / edit / delete a comment from the list — only view.

Changes

Inline disclosure (banlist / commslist)

• page_bans.tpl / page_comms.tpl
  • Render $com.editcomlink + $com.delcomlink beneath each inline comment. Delete reuses the existing data-action="comment-delete" dispatcher (web/scripts/comment-actions.js, Audit follow-up: hard-broken admin surfaces from removed sourcebans.js helpers (#1397 cluster) sbpp/sourcebans-pp#1402) — confirm dialog + JSON API round-trip, no new JS.
  • Add an "Add comment" footer link inside the comments disclosure.
  • For admins the disclosure now also renders on rows with zero comments (summary count 0) so a thread can be started there; public viewers are unchanged (disclosure only on rows that have comments, no CTA).
  • Multi-comment blocks already iterate a <ul> foreach, so the "render with multiple comments" concern from the issue is covered.
• BanListView / CommsListView: new can_comment flag, splatted as $userbank->is_admin() (matches the login-only bans.add_comment API gate). Defaulted false so third-party callers / existing tests keep working.
• theme.css: styling for the action row + add-comment footer (theme-token based, light/dark safe).
• BanlistCommentsVisibilityTest: replaced the now-obsolete "disclosure never renders on an uncommented row" assertion with coverage for the admin Add-comment CTA and the public no-CTA path.

Review follow-ups

• Lucide icons (6d0be6ef): editcomlink / delcomlink / addcomment were still emitting Font Awesome markup (<i class="fas fa-edit fa-lg"> etc.). The 2.0 theme dropped Font Awesome, so the icon-only edit/delete controls rendered as empty <a> elements. Swapped all three to <i data-lucide="…"> (pencil / trash-2 / message-square-plus), matching the icon vocabulary the rest of the banlist chrome uses (and the Lucide-not-glyph rule PublicBanListRegressionTest pins for the row-action buttons). Icon-only anchors gained aria-label.

• Player-drawer comment affordances (a77efe66): the drawer's Comments section was read-only, so once a punishment had comments the only add path was to expand the inline disclosure and scroll past the thread. 1.x kept an always-present "Add comment" admin action on the expanded row.

  • api_bans_detail / api_comms_detail: new can_comment (login-only, mirrors the page handlers' splat) plus per-comment can_edit (own comment or Owner) / can_delete (Owner) — same gating as page.banlist.php's $commentres loop. C.aid added to the comment SELECT to compute ownership.
  • theme.js renderOverviewPane: renders the per-comment edit/delete row and an "Add comment" CTA. Edit/add link to the same server-rendered ?p=…&comment=… surface the inline disclosure uses; delete reuses the global data-action="comment-delete" dispatcher with data-page="-1".
  • api-contract.js regenerated (also absorbs the stale ApiSystemCheckVersion doc re-wrap left by Retarget GHCR, releases, and docs to this repo #18's hand-edit).
  • partials/player-drawer.tpl: reference shape + variable contract updated.
  • bans.detail / comms.detail snapshots + BansTest / CommsTest assertions for the new flags; banlist-comments-visibility.spec.ts asserts the admin Add-comment CTA in the drawer.

Testing

No local SourcePawn/PHP toolchain for the full suite; relying on CI. php -l clean on all changed PHP files, node -c clean on theme.js, composer api-contract regenerated deterministically.

🤖 Generated with Claude Code

[Rushaway]

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

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

Verified:

• Edit/delete gating is correct. $cdata['editcomlink'] is emitted only for own comments ($crow['aid'] == $userbank->GetAid()) or WebPermission::Owner; $cdata['delcomlink'] only for Owner; both "" otherwise (page.banlist.php:1086-1109, page.commslist.php mirror). Template renders them behind {if $com.editcomlink != '' ...}, so no unauthorized controls.
• No Smarty undefined-key warnings on any path (anon/flag-off, anon/flag-on, admin/0 comments, admin/N) — the 9b1569e5 isset() guard covers the empty-row case; editcomlink/delcomlink are always set whenever commentdata is iterated. PHPUnit (fails on warnings) is green.
• Delete wiring reuses the existing data-action="comment-delete" dispatcher in web/scripts/comment-actions.js (confirm dialog + JSON round-trip) — no new JS, CSRF handled there.
• can_comment defaults to false on both View constructors → existing/third-party callers unaffected.
• CSS uses theme tokens + html.dark overrides, consistent with the surrounding .ban-comments-inline__* rules.
• New/updated tests assert real behaviour (admin sees disclosure on every desktop row + ban-comment-add; anon sees it only on commented rows, never the CTA) — not tautologies.

Minor, non-blocking: can_comment: $userbank->is_admin() is stricter than the bans.add_comment API's login-only gate — a logged-in non-admin who is allowed to comment won't see the "Add comment" CTA. Conservative (missing affordance, not a hole); fine to keep, or widen to match the API in a follow-up.

[maxijabase]

LGTM on the restore itself. Delete correctly reuses comment-actions.js, and can_comment being is_admin() is conservative rather than a hole.

Two follow-ups, neither blocking:

1. Edit is Font Awesome in a Lucide theme. editcomlink is still CreateLinkR('<i class="fas fa-edit fa-lg"></i>', …) with no label text. Default theme does not load FA, so the edit control is an empty <a>. Delete already ships as Lucide in the template; edit should match (or at least keep visible text).
2. Add Comment only renders when the thread is empty. Once a ban/comm has comments, there is no CTA to add another from the list (only the raw ?comment= URL). 1.x kept add below the thread.

[Rushaway]

@maxijabase both follow-ups addressed — re-requested your review.

1. FA icons in a Lucide theme (6d0be6ef)

You were right, and it was worse than flagged: delcomlink is also Font Awesome (fas fa-trash fa-lg) — the trash trigger, not just the confirm dialog — so both edit and delete rendered as empty <a> in the default theme. addcomment was FA too but survived on its "Add Comment" text.

All three now emit <i data-lucide="…"> (pencil / trash-2 / message-square-plus), matching the row-action vocabulary PublicBanListRegressionTest::testBanlistRowActionsUseLucideIconsNotEntityGlyphs pins. Icon-only edit/delete anchors got aria-label.

2. Add-comment CTA when the thread is non-empty (a77efe66)

Small correction on the premise: ban-comments-inline__add is gated only on $can_comment, not on comment count — it does render on commented rows, at the foot of the expanded disclosure. It's just not discoverable (collapsed summary shows only a count).

Rather than move the inline link, I surfaced add/edit/delete in the player drawer (the 2.0 equivalent of 1.x's always-present admin action on the expanded row):

• api_bans_detail / api_comms_detail: can_comment + per-comment can_edit / can_delete, gated exactly like page.banlist.php's $commentres loop.
• theme.js renderOverviewPane: per-comment edit/delete row + "Add comment" CTA. Edit/add hit the same ?p=…&comment=… surface the inline disclosure uses; delete reuses the comment-actions.js dispatcher.
• api-contract.js regenerated — note it also picks up a stale ApiSystemCheckVersion doc re-wrap from Retarget GHCR, releases, and docs to this repo #18's hand-edit (unavoidable once handlers change; run composer api-contract to verify).
• Snapshots + BansTest/CommsTest assertions + an e2e assertion for the drawer CTA.

Also merged current main — the theme.js conflict was with #26's drawer demo-download button; both sections now coexist in renderOverviewPane.

No local PHP/Playwright toolchain here — php -l / node -c / composer api-contract clean, the rest is on CI. The hand-edited bans.detail snapshots are the thing most worth a close look.

[maxijabase]

LGTM!