From e6db86b572f00950b023750f191fe9ccfe69adbc Mon Sep 17 00:00:00 2001 From: Rushaway Date: Thu, 3 Sep 2026 10:59:43 +0200 Subject: [PATCH 1/4] fix(banlist/commslist): restore per-punishment comment CTAs (#1544) The 2.0.0 theme rebuild rendered the inline comment thread on the public ban list and comms list but dropped the add / edit / delete call-to-action buttons. page.banlist.php / page.commslist.php still build addcomment, editcomlink and delcomlink for every row (already permission-gated), the templates just never printed them. - page_bans.tpl / page_comms.tpl: render $com.editcomlink + $com.delcomlink under each inline comment (delete reuses the existing data-action="comment-delete" dispatcher in comment-actions.js), and an "Add comment" footer link in the disclosure. For admins the disclosure now also renders on rows with zero comments so a thread can be started there. - BanListView / CommsListView: new can_comment flag ($userbank->is_admin(), matching the login-only bans.add_comment API gate), defaulted false for third-party callers. - theme.css: styles for the new action row + add-comment footer. - BanlistCommentsVisibilityTest: cover the admin add-comment CTA and the public no-CTA path (replaces the now-obsolete "disclosure never renders on an uncommented row" assertion). Upstream issue: sbpp/sourcebans-pp#1544 Co-Authored-By: Claude Sonnet 5 --- web/includes/View/BanListView.php | 8 ++++ web/includes/View/CommsListView.php | 6 +++ web/pages/page.banlist.php | 3 ++ web/pages/page.commslist.php | 3 ++ .../BanlistCommentsVisibilityTest.php | 48 +++++++++++++++---- web/themes/default/css/theme.css | 21 ++++++++ web/themes/default/page_bans.tpl | 30 ++++++++++-- web/themes/default/page_comms.tpl | 23 +++++++-- 8 files changed, 125 insertions(+), 17 deletions(-) diff --git a/web/includes/View/BanListView.php b/web/includes/View/BanListView.php index f4cbb77ed..715e6a5e2 100644 --- a/web/includes/View/BanListView.php +++ b/web/includes/View/BanListView.php @@ -116,6 +116,14 @@ public function __construct( // gets `aria-pressed="true"` on first paint without a JS // round-trip. public readonly string $chip_base_link, + // #1544: gates the per-row "Add comment" CTA in the inline + // comments disclosure (and lets the disclosure render on rows + // with zero comments so an admin can start a thread). Splatted + // as `$userbank->is_admin()` from `web/pages/page.banlist.php` — + // matches the `bans.add_comment` API's login-only gate. The + // per-comment edit / delete links stay permission-gated inside + // the page handler ($com.editcomlink / $com.delcomlink). + public readonly bool $can_comment = false, ) { } } diff --git a/web/includes/View/CommsListView.php b/web/includes/View/CommsListView.php index 6d29f37d8..ea31dc621 100644 --- a/web/includes/View/CommsListView.php +++ b/web/includes/View/CommsListView.php @@ -127,6 +127,12 @@ public function __construct( // Mirrors the post-submit auto-open contract #1303 introduced // for admin-admins. public readonly bool $is_advanced_search_open, + // #1544: gates the per-row "Add comment" CTA in the inline + // comments disclosure (and lets the disclosure render on rows + // with zero comments). Splatted as `$userbank->is_admin()` from + // `web/pages/page.commslist.php`. Per-comment edit / delete + // links stay permission-gated in the page handler. + public readonly bool $can_comment = false, ) { } } diff --git a/web/pages/page.banlist.php b/web/pages/page.banlist.php index 6dfac6356..f6a9581cc 100644 --- a/web/pages/page.banlist.php +++ b/web/pages/page.banlist.php @@ -1504,4 +1504,7 @@ function setPostKey() is_advanced_search_open: $banlistAdvancedOpen, active_state: $stateFilter, chip_base_link: $banlistChipBaseLink, + // #1544: restores the per-punishment "Add comment" CTA (and the + // per-comment edit/delete controls) dropped in the 2.0.0 migration. + can_comment: $userbank->is_admin(), )); diff --git a/web/pages/page.commslist.php b/web/pages/page.commslist.php index 2c07cbb64..bf97276e0 100644 --- a/web/pages/page.commslist.php +++ b/web/pages/page.commslist.php @@ -1275,4 +1275,7 @@ function setPostKey() view_comments: (bool) $view_comments, view_bans: $viewBans, is_advanced_search_open: $commsAdvancedOpen, + // #1544: restores the per-punishment "Add comment" CTA (and the + // per-comment edit/delete controls) dropped in the 2.0.0 migration. + can_comment: $userbank->is_admin(), )); diff --git a/web/tests/integration/BanlistCommentsVisibilityTest.php b/web/tests/integration/BanlistCommentsVisibilityTest.php index 89718f914..a8c802dff 100644 --- a/web/tests/integration/BanlistCommentsVisibilityTest.php +++ b/web/tests/integration/BanlistCommentsVisibilityTest.php @@ -182,26 +182,56 @@ public function testBanlistMobileCardEmitsCountIndicator(): void #[RunInSeparateProcess] #[PreserveGlobalState(false)] - public function testBanlistDisclosureDoesNotRenderOnUncommentedRow(): void + public function testBanlistAdminSeesAddCommentCtaOnUncommentedRow(): void { + // #1544: the per-punishment "Add comment" CTA was dropped in the + // 2.0.0 migration. An admin now sees the inline disclosure on + // EVERY row (desktop) so a comment thread can be started even on + // a ban that has none yet — the summary count reads 0 and the + // only body content is the `ban-comment-add` CTA. $this->loginAsAdmin(); $this->setPublicCommentsFlag(false); $_GET = ['p' => 'banlist']; $html = $this->renderBanlistPage(); - // The disclosure renders TWICE per commented ban (desktop - // table + mobile-card branch... wait, we don't render - // a `
` on mobile — only the count indicator). - // The seeded fixture has exactly one commented ban - // (banWithCommentsBid) and one uncommented ban - // (banWithoutCommentsBid), so the disclosure count must be - // exactly 1 (desktop only). + // One commented ban + one uncommented ban seeded → disclosure + // renders twice for an admin (desktop table only). + $disclosureMatchCount = substr_count($html, 'data-testid="ban-comments-inline"'); + $this->assertSame( + 2, + $disclosureMatchCount, + 'admin sees the inline disclosure on every desktop row so the Add-comment CTA is reachable on uncommented bans too (#1544)', + ); + $this->assertStringContainsString( + 'data-testid="ban-comment-add"', + $html, + 'admin must see the per-row "Add comment" CTA (#1544)', + ); + } + + #[RunInSeparateProcess] + #[PreserveGlobalState(false)] + public function testBanlistAnonymousSeesNoDisclosureOnUncommentedRow(): void + { + // Public callers with the flag ON still only get the disclosure + // on rows that actually have comments — no CTA, no empty + // disclosure. + $this->setPublicCommentsFlag(true); + $_GET = ['p' => 'banlist']; + + $html = $this->renderBanlistPage(); + $disclosureMatchCount = substr_count($html, 'data-testid="ban-comments-inline"'); $this->assertSame( 1, $disclosureMatchCount, - 'disclosure must render exactly once (one commented ban, desktop only) — uncommented rows do NOT emit the surface', + 'public caller sees the disclosure only on commented rows — uncommented rows stay bare', + ); + $this->assertStringNotContainsString( + 'data-testid="ban-comment-add"', + $html, + 'public callers never see the Add-comment CTA', ); } diff --git a/web/themes/default/css/theme.css b/web/themes/default/css/theme.css index 603b83a08..2ce278722 100644 --- a/web/themes/default/css/theme.css +++ b/web/themes/default/css/theme.css @@ -1424,6 +1424,27 @@ html.dark .pill--online { color: #6ee7b7; } html.dark .ban-comments-inline__text a { color: var(--brand-400); } .ban-comments-inline__edit { margin-top: 0.25rem; } +/* #1544: per-comment edit/delete CTAs + the "Add comment" footer link. + Buttons come pre-gated from the PHP handlers ($com.editcomlink / + $com.delcomlink are empty strings when the viewer lacks rights); the + footer link is gated on $can_comment in the template. */ +.ban-comments-inline__actions { + display: flex; + align-items: center; + gap: 0.5rem; + margin-top: 0.375rem; +} +.ban-comments-inline__actions a { color: var(--text-muted); line-height: 1; } +.ban-comments-inline__actions a:hover { color: var(--text); } +.ban-comments-inline__add { + margin-top: 0.5rem; + padding-top: 0.5rem; + border-top: 1px solid var(--border); + font-size: var(--fs-xs); +} +.ban-comments-inline__add a { color: var(--brand-600); font-weight: 500; } +html.dark .ban-comments-inline__add a { color: var(--brand-400); } + /* ---- Filter chip ---- */ .chip { display: inline-flex; align-items: center; gap: 0.375rem; padding: 0 0.625rem; diff --git a/web/themes/default/page_bans.tpl b/web/themes/default/page_bans.tpl index 1d9d949b2..a9572f480 100644 --- a/web/themes/default/page_bans.tpl +++ b/web/themes/default/page_bans.tpl @@ -345,18 +345,21 @@ is icon+count only (GitHub/Linear shape); body lists comments when opened. Drawer Overview still mirrors the same data via api_bans_detail. *} - {if $view_comments && $ban.commentdata != "None" && $ban.commentdata|@count > 0} + {assign var=_ban_ccount value=0} + {if $ban.commentdata != "None" && $ban.commentdata|@count > 0}{assign var=_ban_ccount value=$ban.commentdata|@count}{/if} + {if $view_comments && ($_ban_ccount > 0 || $can_comment)}
+ title="{$_ban_ccount} comment{if $_ban_ccount != 1}s{/if}" + aria-label="{$_ban_ccount} comment{if $_ban_ccount != 1}s{/if}"> - {$ban.commentdata|@count} + {$_ban_ccount}
    + {if $_ban_ccount > 0} {foreach from=$ban.commentdata item=com}
  • @@ -376,9 +379,28 @@ {if !empty($com.edittime)}
    last edit {$com.edittime} by {if $hideadminname}Hidden{elseif !empty($com.editname)}{$com.editname|escape}{else}deleted admin{/if}
    {/if} + {* #1544: per-comment edit / delete CTAs. Both strings are + built (and permission-gated) in page.banlist.php — empty + when the viewer can't act. The trash trigger carries the + `data-action="comment-delete"` hooks consumed by + web/scripts/comment-actions.js. *} + {if $com.editcomlink != '' || $com.delcomlink != ''} +
    + {if $com.editcomlink != ''}{$com.editcomlink nofilter}{/if} + {if $com.delcomlink != ''}{$com.delcomlink nofilter}{/if} +
    + {/if}
    • {/foreach} + {/if}
+ {* #1544: "Add comment" CTA — restores the per-punishment + comment affordance dropped in the 2.0.0 migration. Gated on + $can_comment ($userbank->is_admin()); the link lands on the + ?comment=N comment-edit branch of this template. *} + {if $can_comment} +
{$ban.addcomment nofilter}
+ {/if}
{/if} diff --git a/web/themes/default/page_comms.tpl b/web/themes/default/page_comms.tpl index 48803245e..d3e762e2b 100644 --- a/web/themes/default/page_comms.tpl +++ b/web/themes/default/page_comms.tpl @@ -248,18 +248,21 @@ href="?p=commslist&id={$comm.cid}" data-drawer-cid="{$comm.cid}" data-testid="drawer-trigger">{if $comm.name}{$comm.name|escape}{else}no nickname{/if} - {if $view_comments && $comm.commentdata != "None" && isset($comm.commentdata) && $comm.commentdata|@count > 0} + {assign var=_comm_ccount value=0} + {if isset($comm.commentdata) && $comm.commentdata != "None" && $comm.commentdata|@count > 0}{assign var=_comm_ccount value=$comm.commentdata|@count}{/if} + {if $view_comments && ($_comm_ccount > 0 || $can_comment)}
+ title="{$_comm_ccount} comment{if $_comm_ccount != 1}s{/if}" + aria-label="{$_comm_ccount} comment{if $_comm_ccount != 1}s{/if}"> - {$comm.commentdata|@count} + {$_comm_ccount}
    + {if $_comm_ccount > 0} {foreach from=$comm.commentdata item=com}
  • @@ -273,9 +276,21 @@ {if !empty($com.edittime)}
    last edit {$com.edittime} by {if $hideadminname}Hidden{elseif !empty($com.editname)}{$com.editname|escape}{else}deleted admin{/if}
    {/if} + {* #1544: per-comment edit / delete CTAs — both strings are built and permission-gated in page.commslist.php (empty when the viewer can't act). The trash trigger carries the `data-action="comment-delete"` hooks consumed by web/scripts/comment-actions.js. *} + {if $com.editcomlink != '' || $com.delcomlink != ''} +
    + {if $com.editcomlink != ''}{$com.editcomlink nofilter}{/if} + {if $com.delcomlink != ''}{$com.delcomlink nofilter}{/if} +
    + {/if}
    • {/foreach} + {/if}
+ {* #1544: "Add comment" CTA — restores the per-punishment comment affordance dropped in the 2.0.0 migration. Gated on $can_comment ($userbank->is_admin()); the link lands on the ?comment=N comment-edit branch of this template. *} + {if $can_comment} +
{$comm.addcomment nofilter}
+ {/if}
{/if} From 9b1569e5836fbc24a134761fe17abf7a766d28d5 Mon Sep 17 00:00:00 2001 From: Rushaway Date: Thu, 3 Sep 2026 11:07:27 +0200 Subject: [PATCH 2/4] fix: guard commentdata access when public comments are disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit page_bans.tpl accessed $ban.commentdata before the $view_comments gate, tripping an 'Undefined array key' warning (PHPUnit treats it as a failure) for anonymous callers when config.enablepubliccomments=0 — the handler doesn't build commentdata in that branch. Co-Authored-By: Claude Sonnet 5 --- web/themes/default/page_bans.tpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/themes/default/page_bans.tpl b/web/themes/default/page_bans.tpl index a9572f480..ea660e0d0 100644 --- a/web/themes/default/page_bans.tpl +++ b/web/themes/default/page_bans.tpl @@ -346,7 +346,7 @@ comments when opened. Drawer Overview still mirrors the same data via api_bans_detail. *} {assign var=_ban_ccount value=0} - {if $ban.commentdata != "None" && $ban.commentdata|@count > 0}{assign var=_ban_ccount value=$ban.commentdata|@count}{/if} + {if $view_comments && isset($ban.commentdata) && $ban.commentdata != "None" && $ban.commentdata|@count > 0}{assign var=_ban_ccount value=$ban.commentdata|@count}{/if} {if $view_comments && ($_ban_ccount > 0 || $can_comment)}
Date: Sun, 6 Sep 2026 14:35:41 +0200 Subject: [PATCH 3/4] fix(comments): render banlist/commslist comment CTAs as Lucide icons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The restored per-comment edit/delete controls and the "Add comment" link were still emitting Font Awesome markup (`` / `fa-trash` / `fa-comment-dots`). The 2.0 theme dropped Font Awesome, so `editcomlink` and `delcomlink` — which are icon-only — rendered as empty `` elements; `addcomment` lost its glyph but kept the visible "Add Comment" text. Swap all three to `` (pencil / trash-2 / message-square-plus), matching the icon vocabulary the rest of the banlist/commslist chrome uses (and the Lucide-not-glyph rule PublicBanListRegressionTest pins for the row-action buttons). Add `aria-label` to the icon-only edit/delete anchors so they carry an accessible name the way the row-action buttons do. Follow-up to #28 review. Co-Authored-By: Claude Sonnet 5 --- web/pages/page.banlist.php | 16 ++++++++++++---- web/pages/page.commslist.php | 16 ++++++++++++---- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/web/pages/page.banlist.php b/web/pages/page.banlist.php index f6a9581cc..0ab67df7f 100644 --- a/web/pages/page.banlist.php +++ b/web/pages/page.banlist.php @@ -1085,7 +1085,13 @@ function setPostKey() $cdata = []; $cdata['morecom'] = ($morecom == 1 ? true : false); if ($crow['aid'] == $userbank->GetAid() || $userbank->HasAccess(WebPermission::Owner)) { - $cdata['editcomlink'] = CreateLinkR('', 'index.php?p=banlist&comment=' . $data['ban_id'] . '&ctype=B&cid=' . $crow['cid'] . $pagelink, 'Edit Comment'); + // #1544: icon-only edit link. `data-lucide` (not Font Awesome, which + // the 2.0 theme no longer loads — the old `` rendered + // an empty ``); `aria-label` gives the icon-only control an + // accessible name the way the row-action buttons do. + $cdata['editcomlink'] = ''; if ($userbank->HasAccess(WebPermission::Owner)) { // #1402: `onclick="RemoveComment(...)"` was the v1.x bridge into // the deleted sourcebans.js helper — every click threw @@ -1096,12 +1102,12 @@ function setPostKey() // four comment-thread surfaces (banlist / commslist / protests // / submissions). data-page lets the handler land the operator // back on the same paginated banlist view post-delete. - $cdata['delcomlink'] = ''; + . '>'; } } else { $cdata['editcomlink'] = ""; @@ -1135,7 +1141,9 @@ function setPostKey() } - $data['addcomment'] = CreateLinkR(' Add Comment', 'index.php?p=banlist&comment=' . $data['ban_id'] . '&ctype=B' . $pagelink); + // #1544: Lucide icon (the 2.0 theme dropped Font Awesome); keeps the + // visible "Add Comment" label so it degrades gracefully anyway. + $data['addcomment'] = CreateLinkR(' Add Comment', 'index.php?p=banlist&comment=' . $data['ban_id'] . '&ctype=B' . $pagelink); //----------------------------------- $data['ub_reason'] = (isset($data['ub_reason']) ? $data['ub_reason'] : ""); diff --git a/web/pages/page.commslist.php b/web/pages/page.commslist.php index bf97276e0..09c88fe12 100644 --- a/web/pages/page.commslist.php +++ b/web/pages/page.commslist.php @@ -873,15 +873,21 @@ function setPostKey() $cdata = []; $cdata['morecom'] = ($morecom == 1 ? true : false); if ($crow['aid'] == $userbank->GetAid() || $userbank->HasAccess(WebPermission::Owner)) { - $cdata['editcomlink'] = CreateLinkR('', 'index.php?p=commslist&comment=' . $data['ban_id'] . '&ctype=C&cid=' . $crow['cid'] . $pagelink, 'Edit Comment'); + // #1544: icon-only edit link. `data-lucide` (not Font Awesome, which + // the 2.0 theme no longer loads — the old `` rendered + // an empty ``); `aria-label` gives the icon-only control an + // accessible name the way the row-action buttons do. + $cdata['editcomlink'] = ''; if ($userbank->HasAccess(WebPermission::Owner)) { // #1402: see web/scripts/comment-actions.js for the dispatcher. - $cdata['delcomlink'] = ''; + . '>'; } } else { $cdata['editcomlink'] = ""; @@ -914,7 +920,9 @@ function setPostKey() $data['commentdata'] = $comment; } - $data['addcomment'] = CreateLinkR(' Add Comment', 'index.php?p=commslist&comment=' . $data['ban_id'] . '&ctype=C' . $pagelink); + // #1544: Lucide icon (the 2.0 theme dropped Font Awesome); keeps the + // visible "Add Comment" label so it degrades gracefully anyway. + $data['addcomment'] = CreateLinkR(' Add Comment', 'index.php?p=commslist&comment=' . $data['ban_id'] . '&ctype=C' . $pagelink); //----------------------------------- $data['counts'] = $delimiter . $mutes . $gags; From a77efe665fe55fd71171f013a4eb7ea10d869df1 Mon Sep 17 00:00:00 2001 From: Rushaway Date: Sun, 6 Sep 2026 14:35:53 +0200 Subject: [PATCH 4/4] feat(drawer): add/edit/delete comment affordances in the player drawer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 2.0 player drawer's Comments section was read-only, so once a ban/comm had comments the only way to add another from the list view was to expand the inline disclosure and scroll past the thread (the review's "only the raw ?comment= URL" point). 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' `can_comment` 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: render 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 (comment-actions.js) with `data-page="-1"`. - api-contract.js: regenerated (also picks up the stale ApiSystemCheckVersion re-wrap left by #18's hand-edit). - player-drawer.tpl: reference shape + variable contract updated. - Tests: bans/comms detail snapshots + assertions for the new flags; e2e drawer-mirror asserts the admin Add-comment CTA. Follow-up to #28 review. Co-Authored-By: Claude Sonnet 5 --- web/api/handlers/bans.php | 16 ++++++- web/api/handlers/comms.php | 18 ++++++-- web/scripts/api-contract.js | 14 +++--- web/tests/api/BansTest.php | 4 ++ web/tests/api/CommsTest.php | 6 +++ .../__snapshots__/bans/detail_admin_view.json | 5 ++- .../bans/detail_public_hidden.json | 1 + .../flows/banlist-comments-visibility.spec.ts | 4 ++ web/themes/default/js/theme.js | 44 +++++++++++++++++++ web/themes/default/partials/player-drawer.tpl | 25 ++++++++++- 10 files changed, 123 insertions(+), 14 deletions(-) diff --git a/web/api/handlers/bans.php b/web/api/handlers/bans.php index 24a1b01b8..d4a63eb48 100644 --- a/web/api/handlers/bans.php +++ b/web/api/handlers/bans.php @@ -826,8 +826,9 @@ function api_bans_view_community(array $params): array * demo_count: int, * history_count: int, * comments_visible: bool, + * can_comment: bool, * notes_visible: bool, - * comments: list + * comments: list * } */ function api_bans_detail(array $params): array @@ -947,8 +948,14 @@ function api_bans_detail(array $params): array $comments = []; $commentsVisible = Config::getBool('config.enablepubliccomments') || $isAdmin; if ($commentsVisible) { + // #1544: per-comment edit/delete gating for the drawer, mirroring + // page.banlist.php's `$commentres` loop — edit is own-comment OR + // Owner, delete is Owner only. Never true for a non-admin caller + // (only admins author comments, and Owner is an admin flag). + $viewerAid = $userbank->GetAid(); + $isOwner = $isAdmin && $userbank->HasAccess(WebPermission::Owner); $commentRows = $GLOBALS['PDO']->query( - "SELECT C.cid, C.commenttxt, C.added, C.edittime, + "SELECT C.cid, C.aid, C.commenttxt, C.added, C.edittime, (SELECT user FROM `:prefix_admins` WHERE aid = C.aid) AS author, (SELECT user FROM `:prefix_admins` WHERE aid = C.editaid) AS editor FROM `:prefix_comments` AS C @@ -973,6 +980,8 @@ function api_bans_detail(array $params): array 'text' => (string)$crow['commenttxt'], 'edited_at' => $editTime, 'edited_by' => (!$hideAdmin && $crow['editor'] !== null) ? (string)$crow['editor'] : null, + 'can_edit' => $isOwner || ($isAdmin && (int)$crow['aid'] === $viewerAid), + 'can_delete' => $isOwner, ]; } } @@ -1013,6 +1022,9 @@ function api_bans_detail(array $params): array 'demo_count' => (int)$row['demo_count'], 'history_count' => (int)$row['history_count'], 'comments_visible' => $commentsVisible, + // #1544: gates the drawer's "Add comment" CTA — same login-only + // gate page.banlist.php splats as `can_comment` ($userbank->is_admin()). + 'can_comment' => $isAdmin, // notes_visible is the drawer's signal for whether to render the // Notes tab at all (#1165). It mirrors the dispatcher gate on // `notes.list` (requireAdmin=true) so a public visitor sees three diff --git a/web/api/handlers/comms.php b/web/api/handlers/comms.php index 548710ac0..2dda81bbe 100644 --- a/web/api/handlers/comms.php +++ b/web/api/handlers/comms.php @@ -429,10 +429,12 @@ function api_comms_prepare_block_from_ban(array $params): array * admin: array{name: string|null}, * server: array{sid: int, name: string|null, mod_icon: string|null}, * comments_visible: bool, + * can_comment: bool, * notes_visible: bool, * comments: list, + * author: string|null, author_hidden: bool, text: string, + * edited_at: int|null, edited_by: string|null, + * can_edit: bool, can_delete: bool}>, * } */ function api_comms_detail(array $params): array @@ -529,12 +531,17 @@ function api_comms_detail(array $params): array $comments = []; $commentsVisible = Config::getBool('config.enablepubliccomments') || $isAdmin; if ($commentsVisible) { + // #1544: per-comment edit/delete gating for the drawer — mirrors + // page.commslist.php ($commentres loop): edit is own-comment OR + // Owner, delete is Owner only. + $viewerAid = $userbank->GetAid(); + $isOwner = $isAdmin && $userbank->HasAccess(WebPermission::Owner); // Comm comments live on `:prefix_comments` with `type = 'C'`, // keyed by the comm row's `bid` column (despite our public // surface naming it `cid` — the column is shared between the // bans/comms/protests trio via the `type` letter). $commentRows = $GLOBALS['PDO']->query( - "SELECT C.cid, C.commenttxt, C.added, C.edittime, + "SELECT C.cid, C.aid, C.commenttxt, C.added, C.edittime, (SELECT user FROM `:prefix_admins` WHERE aid = C.aid) AS author, (SELECT user FROM `:prefix_admins` WHERE aid = C.editaid) AS editor FROM `:prefix_comments` AS C @@ -559,6 +566,8 @@ function api_comms_detail(array $params): array 'text' => (string)$crow['commenttxt'], 'edited_at' => $editTime, 'edited_by' => (!$hideAdmin && $crow['editor'] !== null) ? (string)$crow['editor'] : null, + 'can_edit' => $isOwner || ($isAdmin && (int)$crow['aid'] === $viewerAid), + 'can_delete' => $isOwner, ]; } } @@ -602,6 +611,9 @@ function api_comms_detail(array $params): array 'mod_icon' => !empty($row['mod_icon']) ? (string)$row['mod_icon'] : null, ], 'comments_visible' => $commentsVisible, + // #1544: gates the drawer's "Add comment" CTA — same login-only + // gate page.commslist.php splats as `can_comment`. + 'can_comment' => $isAdmin, // Mirrors `api_bans_detail`: the drawer's Notes tab is // admin-only, gated on this flag. The dispatcher gate on // `notes.list` is the load-bearing one; this signal lets the diff --git a/web/scripts/api-contract.js b/web/scripts/api-contract.js index b749a2f73..43584d491 100644 --- a/web/scripts/api-contract.js +++ b/web/scripts/api-contract.js @@ -158,7 +158,7 @@ * page.banlist.php's `$view_comments` switch. * * @typedef {Object} ApiBansDetailRequest - * @typedef {{ bid: number, player: {name: string, type: number, steam_id: string, steam_id_3: string, community_id: string, ip: string|null, country: string|null}, ban: {reason: string, banned_at: number, banned_at_human: string, length_seconds: number, length_human: string, expires_at: number|null, expires_at_human: string|null, state: string, unban_reason: string, removed_at: number|null, removed_at_human: string|null, removed_by: string|null}, admin: {name: string|null}, server: {sid: number, name: string|null, mod_icon: string|null}, demo_count: number, history_count: number, comments_visible: boolean, notes_visible: boolean, comments: Array<{cid: number, added: number, added_human: string, author: string|null, text: string, edited_at: number|null, edited_by: string|null}> }} ApiBansDetailResponse + * @typedef {{ bid: number, player: {name: string, type: number, steam_id: string, steam_id_3: string, community_id: string, ip: string|null, country: string|null}, ban: {reason: string, banned_at: number, banned_at_human: string, length_seconds: number, length_human: string, expires_at: number|null, expires_at_human: string|null, state: string, unban_reason: string, removed_at: number|null, removed_at_human: string|null, removed_by: string|null}, admin: {name: string|null}, server: {sid: number, name: string|null, mod_icon: string|null}, demo_count: number, history_count: number, comments_visible: boolean, can_comment: boolean, notes_visible: boolean, comments: Array<{cid: number, added: number, added_human: string, author: string|null, author_hidden: boolean, text: string, edited_at: number|null, edited_by: string|null, can_edit: boolean, can_delete: boolean}> }} ApiBansDetailResponse */ /** * @typedef {Object} ApiBansEditCommentRequest @@ -337,7 +337,7 @@ * template emits). * * @typedef {Object} ApiCommsDetailRequest - * @typedef {{ cid: number, player: { name: string, steam_id: string, steam_id_3: string, community_id: string, ip: null, country: string|null }, block: { type: number, type_label: string, reason: string, started_at: number, started_at_human: string, length_seconds: number, length_human: string, expires_at: number|null, expires_at_human: string|null, state: string, unblock_reason: string, removed_at: number|null, removed_at_human: string|null, removed_by: string|null }, admin: {name: string|null}, server: {sid: number, name: string|null, mod_icon: string|null}, comments_visible: boolean, notes_visible: boolean, comments: Array<{cid: number, added: number, added_human: string, author: string|null, text: string, edited_at: number|null, edited_by: string|null}> }} ApiCommsDetailResponse + * @typedef {{ cid: number, player: { name: string, steam_id: string, steam_id_3: string, community_id: string, ip: null, country: string|null }, block: { type: number, type_label: string, reason: string, started_at: number, started_at_human: string, length_seconds: number, length_human: string, expires_at: number|null, expires_at_human: string|null, state: string, unblock_reason: string, removed_at: number|null, removed_at_human: string|null, removed_by: string|null }, admin: {name: string|null}, server: {sid: number, name: string|null, mod_icon: string|null}, comments_visible: boolean, can_comment: boolean, notes_visible: boolean, comments: Array<{cid: number, added: number, added_human: string, author: string|null, author_hidden: boolean, text: string, edited_at: number|null, edited_by: string|null, can_edit: boolean, can_delete: boolean}> }} ApiCommsDetailResponse */ /** * @typedef {Object} ApiCommsPasteRequest @@ -567,11 +567,11 @@ */ /** * Public action: report whether a newer SourceBans++ release is available. - * Sources from `api.github.com/repos/srcdslab/sourcebans-pp/releases/latest` with - * a 1-day on-disk cache + stale-while-error fallback (the cached payload is - * served regardless of TTL when the upstream call fails) so a busy panel can't - * blow through GitHub's 60 req/hr unauthenticated limit and a transient GitHub - * blip doesn't paint the panel red. + * Sources from `api.github.com/repos/srcdslab/sourcebans-pp/releases/latest` + * with a 1-day on-disk cache + stale-while-error fallback (the cached payload + * is served regardless of TTL when the upstream call fails) so a busy panel + * can't blow through GitHub's 60 req/hr unauthenticated limit and a transient + * GitHub blip doesn't paint the panel red. * * @typedef {Object} ApiSystemCheckVersionRequest * @typedef {{release_latest: string, release_url: string, release_msg: string, release_update: boolean}} ApiSystemCheckVersionResponse diff --git a/web/tests/api/BansTest.php b/web/tests/api/BansTest.php index f8714e7be..ac6562771 100644 --- a/web/tests/api/BansTest.php +++ b/web/tests/api/BansTest.php @@ -463,6 +463,7 @@ public function testDetailPublicViewHidesAdminFields(): void $this->assertNull($env['data']['admin']['name'], 'admin should be hidden for public + hideadminname'); $this->assertFalse($env['data']['comments_visible'], 'comments should be hidden when public + flag off'); $this->assertSame([], $env['data']['comments']); + $this->assertFalse($env['data']['can_comment'], 'public callers cannot add comments from the drawer (#1544)'); $this->assertFalse($env['data']['notes_visible'], 'notes_visible should be false for public callers (#1165)'); $this->assertSnapshot('bans/detail_public_hidden', $env, ['data.bid', 'data.ban.banned_at', 'data.ban.banned_at_human', 'data.ban.expires_at', 'data.ban.expires_at_human']); } @@ -498,6 +499,9 @@ public function testDetailAdminViewExposesEverything(): void $this->assertTrue($env['data']['notes_visible'], 'notes_visible should be true for admin callers (#1165)'); $this->assertCount(1, $env['data']['comments']); $this->assertSame('note for the drawer', $env['data']['comments'][0]['text']); + $this->assertTrue($env['data']['can_comment'], 'admin can add comments from the drawer (#1544)'); + $this->assertTrue($env['data']['comments'][0]['can_edit'], 'admin (Owner / own comment) can edit (#1544)'); + $this->assertTrue($env['data']['comments'][0]['can_delete'], 'admin (Owner) can delete (#1544)'); $this->assertSnapshot('bans/detail_admin_view', $env, [ 'data.bid', 'data.ban.banned_at', diff --git a/web/tests/api/CommsTest.php b/web/tests/api/CommsTest.php index 3bec6fb75..1beb5ba4a 100644 --- a/web/tests/api/CommsTest.php +++ b/web/tests/api/CommsTest.php @@ -616,8 +616,11 @@ public function testDetailHidesCommentAuthorForPublicWhenHideAdminName(): void $env = $this->api('comms.detail', ['cid' => $cid]); $this->assertTrue($env['ok'], json_encode($env)); $this->assertTrue($env['data']['comments_visible'], 'public comments are enabled'); + $this->assertFalse($env['data']['can_comment'], 'public callers cannot add comments from the drawer (#1544)'); $this->assertCount(1, $env['data']['comments']); $this->assertSame('leaky comment', $env['data']['comments'][0]['text']); + $this->assertFalse($env['data']['comments'][0]['can_edit'], 'public callers cannot edit comments (#1544)'); + $this->assertFalse($env['data']['comments'][0]['can_delete'], 'public callers cannot delete comments (#1544)'); $this->assertNull($env['data']['comments'][0]['author'], 'comment author must be hidden for public + hideadminname (#1500)'); $this->assertNull($env['data']['comments'][0]['edited_by'], @@ -636,6 +639,9 @@ public function testDetailHidesCommentAuthorForPublicWhenHideAdminName(): void 'admins still see the comment editor'); $this->assertFalse($adminEnv['data']['comments'][0]['author_hidden'], 'author_hidden must be false for admin viewers (#1500 m1)'); + $this->assertTrue($adminEnv['data']['can_comment'], 'admin can add comments from the drawer (#1544)'); + $this->assertTrue($adminEnv['data']['comments'][0]['can_edit'], 'admin (Owner / own comment) can edit (#1544)'); + $this->assertTrue($adminEnv['data']['comments'][0]['can_delete'], 'admin (Owner) can delete (#1544)'); } public function testDetailReportsUnmutedForLiftedBlock(): void diff --git a/web/tests/api/__snapshots__/bans/detail_admin_view.json b/web/tests/api/__snapshots__/bans/detail_admin_view.json index 683aa69c7..ff085859d 100644 --- a/web/tests/api/__snapshots__/bans/detail_admin_view.json +++ b/web/tests/api/__snapshots__/bans/detail_admin_view.json @@ -36,6 +36,7 @@ "demo_count": 0, "history_count": 1, "comments_visible": true, + "can_comment": true, "notes_visible": true, "comments": [ { @@ -46,7 +47,9 @@ "author_hidden": false, "text": "note for the drawer", "edited_at": null, - "edited_by": null + "edited_by": null, + "can_edit": true, + "can_delete": true } ] } diff --git a/web/tests/api/__snapshots__/bans/detail_public_hidden.json b/web/tests/api/__snapshots__/bans/detail_public_hidden.json index fdf71d059..bf1e16b5c 100644 --- a/web/tests/api/__snapshots__/bans/detail_public_hidden.json +++ b/web/tests/api/__snapshots__/bans/detail_public_hidden.json @@ -36,6 +36,7 @@ "demo_count": 0, "history_count": 1, "comments_visible": false, + "can_comment": false, "notes_visible": false, "comments": [] } diff --git a/web/tests/e2e/specs/flows/banlist-comments-visibility.spec.ts b/web/tests/e2e/specs/flows/banlist-comments-visibility.spec.ts index 9a0854de0..266105df6 100644 --- a/web/tests/e2e/specs/flows/banlist-comments-visibility.spec.ts +++ b/web/tests/e2e/specs/flows/banlist-comments-visibility.spec.ts @@ -207,6 +207,10 @@ test.describe('#BANLIST-COMMENTS: per-row comments visibility', () => { const commentsBlock = overview.locator('[data-testid="drawer-comments"]'); await expect(commentsBlock).toBeVisible(); await expect(commentsBlock).toContainText(FIRST_COMMENT); + + // #1544: the drawer carries the "Add comment" CTA for admins so a + // thread can be extended without hunting for the inline disclosure. + await expect(commentsBlock.locator('[data-testid="drawer-comment-add"]')).toBeVisible(); }); test('mobile banlist: non-interactive count indicator renders inside the card', async ({ page }, testInfo) => { diff --git a/web/themes/default/js/theme.js b/web/themes/default/js/theme.js index 4324beb85..ba11119ea 100644 --- a/web/themes/default/js/theme.js +++ b/web/themes/default/js/theme.js @@ -724,10 +724,19 @@ const server = (data && data.server) || {}; const comments = Array.isArray(data && data.comments) ? data.comments : []; const commentsVisible = !!(data && data.comments_visible); + const canComment = !!(data && data.can_comment); const isComm = drawerKind === 'comm'; const focal = isComm ? ((data && data.block) || {}) : ((data && data.ban) || {}); + // #1544: comment CTAs route to the same server-rendered + // `?p=…&comment=…` edit surface the inline banlist/commslist + // disclosure uses; delete reuses the global `data-action= + // "comment-delete"` dispatcher in web/scripts/comment-actions.js. + const commentPage = isComm ? 'commslist' : 'banlist'; + const commentCtype = isComm ? 'C' : 'B'; + const focalId = isComm ? (data && data.cid) : (data && data.bid); + const escFocalId = escapeHtml(String(focalId)); /** @type {Array<[string, string]>} */ const idRows = []; @@ -778,8 +787,41 @@ ).join('') + ''; + /** + * Per-comment edit / delete row. Both gates come pre-computed from + * the handler (`c.can_edit` / `c.can_delete`, mirroring + * page.banlist.php); `data-page="-1"` tells the delete dispatcher to + * redirect back to the un-paginated list after the API round-trip. + * @param {any} c + * @returns {string} + */ + const commentActions = (c) => { + if (!c || (!c.can_edit && !c.can_delete)) return ''; + const cid = escapeHtml(String(c.cid)); + return '
' + + (c.can_edit + ? '' + + '' + : '') + + (c.can_delete + ? '' + + '' + : '') + + '
'; + }; + let commentsHtml = ''; if (commentsVisible) { + const addCta = canComment + ? '' + + ' Add comment' + + '' + : ''; commentsHtml = '
' + '

Comments

' + (comments.length === 0 @@ -792,9 +834,11 @@ + '' + escapeHtml(c.added_human || '') + '' + '' + '
' + escapeHtml(c.text || '') + '
' + + commentActions(c) + '' ).join('') + '') + + addCta + '
'; } diff --git a/web/themes/default/partials/player-drawer.tpl b/web/themes/default/partials/player-drawer.tpl index 5427b06a1..a514f2063 100644 --- a/web/themes/default/partials/player-drawer.tpl +++ b/web/themes/default/partials/player-drawer.tpl @@ -33,10 +33,14 @@ - $detail.admin.name string|null - $detail.server.name string|null - $detail.comments_visible bool + - $detail.can_comment bool (#1544: gates the "Add comment" CTA) + - $detail.comments[].cid int - $detail.comments[].author string|null - $detail.comments[].author_hidden bool (#1500: name suppressed by banlist.hideadminname) - $detail.comments[].added_human string - $detail.comments[].text string + - $detail.comments[].can_edit bool (#1544: own comment or Owner) + - $detail.comments[].can_delete bool (#1544: Owner only) Smarty's auto-escape is on globally (init.php), so {$value} renders safely without per-line nofilter. @@ -117,7 +121,7 @@ {if $detail.comments_visible} -
+

Comments

{if $detail.comments}
    @@ -128,11 +132,30 @@ {$c.added_human}
    {$c.text}
    + {* #1544: per-comment edit / delete — mirrors renderOverviewPane()'s + commentActions(). Delete reuses the global data-action="comment-delete" + dispatcher (web/scripts/comment-actions.js). *} + {if $c.can_edit || $c.can_delete} +
    + {if $c.can_edit} + + {/if} + {if $c.can_delete} + + {/if} +
    + {/if} {/foreach}
{else}

No comments.

{/if} + {* #1544: "Add comment" CTA — gated on $detail.can_comment ($userbank->is_admin()). *} + {if $detail.can_comment} + + Add comment + + {/if}
{/if}