feat(teams): phase 5 — Forum 5b, discussion + moderation + reports #155
Merged
whitlocktech
merged 5 commits from 2026-08-18 18:36:52 +00:00
feature/teams-phase5-discussion into edge
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| c970caee16 |
fix(teams): let a post-moderation mistake reach the model that explains it
Found on the live rig. `moderatePost` answers `pin` with «"pin" applies to a thread, not to a post» and an invented action with "Unknown moderation action" — the distinction exists because they are different mistakes and a caller who made the first one has a bug worth naming precisely. The route's validator listed only the four actions a post accepts, so `pin` never got there: it came back as a generic "Validation failed". The precise message was written, documented, unit-tested — and unreachable through the API, which is the worst of both, because the branch reads as live code and is only exercised by its own test. The validator now lists all eight and lets the model discriminate. Both answers are 400, neither is a security boundary, and widening the list is not removing it — an action outside the enum still stops at the validator, which the added route test asserts alongside the `pin` case. Nothing else the walk exercised needed changing. The whole phase 5 surface was driven against a real server, real MariaDB and real sessions across four identities — an ordinary member, a granted non-member guest, a Team leader and a staffer — plus a browser pass over the forum panel, the reports queue, the per-Team forum ledger and the settings screen. Notably confirmed live: a locked thread refuses replies from all four identities at 409; a hidden post renders for the leader and staff with Unhide and **no Edit control for anyone**; the report queue answers 200 to staff and 403 to the leader, the member and the guest alike; and turning the edit window down to 0 stops the author while leaving staff unbounded. Co-Authored-By: Claude <noreply@anthropic.com> |
|||
| 3f7e61af1c |
feat(teams): the phase 5 surface — discussion, replies, reports, and two admin screens
241 client tests pass (224 before).
**The forum panel becomes a forum.** It was "Announcements" with one composer;
it now has two, because phase 5 split one server capability into two: `canPost`
means "may open a discussion" and every participant may — a granted guest with no
game character included, which is path 3 doing its job — while `canAnnounce` is
the leader-only half `canPost` used to carry alone. Threads gain replies, an edit
control, per-post moderation and a report control, all still inside the one slot
the module declares, still navigating by `?thread=`.
**Almost nothing here is the client's decision, and the file says so.** `canPost`,
`canAnnounce`, `canReply` and each post's `canEdit`/`editableUntil` are read, not
computed. The one local judgement is a ticking clock that WITHDRAWS an edit offer
whose deadline passed while the page sat open — it can never grant one, because a
time-bounded permission must not take its clock from the party it bounds. That
asymmetry is the first thing client/test/teamForum.test.js asserts.
The panel's pure parts moved to `lib/teamForum.js` so they can be tested without a
browser, following teamActivity.js and teamAdmin.js. Two of them are subtler than
they look:
* `stripToText` decodes entities AFTER stripping tags, and `&` last of all.
Decoding first turns an author's literal "<script>" into a real tag the
strip pass then deletes — silently losing text that was never dangerous.
* `threadSummary` counts REPLIES, which is one fewer than `postCount`. Showing
the raw count tells a reader a brand-new thread already has one reply.
**Three admin surfaces.** The forum settings screen gains the edit-window field
(0 = posts permanent once written). The reports queue is a new screen beside
Appeals — under moderation rather than under Teams, because a staffer working a
queue should have one place to work and `target_type` is deliberately open-ended,
so the next reportable thing arrives as a row rather than as another nav entry.
Its copy tells a member where a report lands and that reporting changes nothing,
because a member who expects a post to vanish and watches it stay reports it
again. There is no leader-facing view and there is not meant to be.
And the per-Team forum moderation ledger finally renders: the route and
`api.admin.teamForumModeration()` have both existed since phase 4 with nothing
calling them, which made `actor_role` — the column that keeps a leader's ordinary
housekeeping distinguishable from a staff intervention — readable only from a DB
client.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|||
| 128de0ff2e |
test(teams): phase 5's server surface, and the negative property under it
1008 pass (972 before). The tests worth reading first are the ones that pin a
property no screen would look different without:
* **The edit window is decided on the server, twice.** One test proves the read
path stamps `canEdit` per post per viewer; another proves the WRITE path
re-derives it from `created_at` and refuses a stale edit even though the
client was told it could — because a time-bounded permission must not take its
clock from the party it bounds.
* **A locked thread refuses staff too**, asserted over member, leader and staff
in one loop, at 409 rather than 403: well-formed request, refusing state.
* **delete → restore is reversible for images.** Without the second half of the
pair a restored post returns its words and loses its pictures a retention
window later, silently — the test asserts both calls and that `hide` makes
neither.
* **Post moderation recomputes the thread's counters** rather than nudging them;
the test runs hide → unhide → hide, which is the cycle a delta gets wrong.
* **acceptance: nothing in the report model is reachable by a Team leader.** The
negative property is the whole point of §5.6 and negatives are what nobody
notices going, so it is asserted directly — the module's function surface is
pinned, and `queue`/`handle` are checked not to mention leadership at all. If
a leader-facing queue is ever wanted it is the org lead's decision, and this
test is what makes somebody ask.
* **A report never changes the content it is about**, proved by stubbing every
mutation the forum has to throw. If filing a report touched a status then
"report" would BE moderation, and the first person to work that out would have
found a way to hide anything on the site.
The test suite caught one real defect: `describeTarget` returned `undefined` for a
hard-deleted target, and `undefined` is dropped by JSON.stringify — so the
documented `target: null` would have reached clients as an absent key.
Two phase-4 tests were updated rather than added to, both because phase 5 changed
what they describe: `canPost` split into `canPost` (open a discussion, everyone)
and `canAnnounce` (leaders), and `discussion` is no longer a refused thread type.
Phase 5's four new player routes are added to acceptance criterion 2's list, so
"with the forum off every forum route 404s" keeps covering the whole surface.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|||
| fff14848f1 |
feat(moderation): member-raised abuse reports, to site staff only
TEAMS.md §5.6. **Core has had no user-facing report flow of any kind** — the `moderation`, `mod_notes` and `appeals` tables are all either staff-initiated or Discord-sanction-shaped, and nothing anywhere let a member say "this is a problem". That was survivable while every piece of content on the site came from staff; phase 5 lets players write to each other, so it stops being. The gap has a specific shape: leaders moderate their own Team's forum, and a Team's leaders are exactly the people who will not report their own Team. So the whole point of this queue is a path that routes AROUND a Team's own leadership. Org lead settled it on 2026-08-18: **reports are site administration only** — there is no leader-facing view of this queue, not even a read-only one scoped to their own Team. §5.6's "a leader may also see and act on reports for their own Team" is not implemented and is not deferred. `content_reports` is deliberately generic — `target_type` is a VARCHAR so a wiki page or a news comment becomes a value rather than a table — and the queue is mounted beside appeals under /admin/moderation rather than under Teams, because a staffer working a queue should have one place to work. **§5.6's literal unique key has a defect and this does not copy it.** Written as (target_type, target_id, reporter_user_id, status) it makes CLOSED rows collide with each other too: reporter reports a post, staff dismiss it, the behaviour recurs, they report again — and the second dismissal is an UPDATE into a tuple that already exists, so working the queue starts throwing duplicate-key errors on the first repeat reporter. The key is on a generated `open_marker` instead, the same trick `team_forum_grants.active_marker` uses: 1 while open, NULL once closed, and NULLs are distinct — which is what §5.6's prose asks for, "one open report per (target, reporter)". Two other departures from the doc, both small and both flagged in the docs PR: `handled_note`, because a queue whose resolution reason lives only in an activity_log line is one where the next staffer to see a repeat report cannot find out why the last was dismissed; and a CASCADE on `team_id`, so a deleted Team does not leave a queue full of reports about content that no longer exists. Also here: a report is filed against a target the model verifies really belongs to the Team the request came through, or the queue's per-Team filter would quietly be lying; the queue resolves every row's target in three batched reads rather than N+1, which is §5.6's fourth rule (uploader, size and sniffed type without hunting) actually paying for §5.5.4's attribution table; a target that has since been hard-deleted comes back null and the report still lists, because "somebody reported this and by the time we looked it was gone" is a fact a moderator needs; and every transition writes activity_log, `dismissed` included — a queue where acting is audited and declining to act is not is one where the cheapest way to make a report vanish leaves no trace. `teams_forum_edit_window_minutes` gains its range validation on the admin settings PUT and is seeded at 15, so the value on the settings screen is the value in force. Route manifest and OpenAPI regenerated: 6 operations added, 0 lost. Co-Authored-By: Claude <noreply@anthropic.com> |
|||
| ae0d27cf27 |
feat(teams): discussion threads, replies, the edit window and post moderation
Phase 5's server half — TEAMS.md §5.1's "5b". The schema for all of it landed in
phase 4, so this adds no ALTER: every column it needed (`type`, `locked`,
`edited_at`, `edited_by`, the post table's `status`, the ledger's
`target_type='post'`) was already there waiting.
* `teams_forum_edit_window_minutes` (0…1440, default 15) joins the forum's
settings. It fails closed to ZERO rather than to its default, which is the
opposite of what it looks like it should do: the risk an edit window bounds is
an author rewriting a post out from under a reader quoting it or a moderator
about to act on a report, so the safe answer during a DB fault is "nobody may
edit for the next minute". A stale uploads acknowledgement freezes this key
too — it is a forum setting.
* Thread creation splits its authority BY TYPE, which is what phase 4's comment
said would happen here rather than widening the leader gate. An announcement
stays leader-authored; a discussion is open to every participant, and
"participant" includes a granted non-member with no game identity — path 3
doing its job. `type` still defaults to `announcement`, so a phase-4 client
keeps meaning what it meant.
* Replies refuse three ways with deliberately different codes: 404 for absent or
hidden, 400 for an announcement (which takes no replies by TYPE, not by being
closed), and 409 for locked — well-formed request, refusing state. Locked
refuses staff too; they hold `unlock`, and unlock/post/relock reaches the same
place leaving three ledger rows that say so.
* The edit window is evaluated on the server twice, on purpose. The read path
stamps every post with `canEdit`/`editableUntil` so the client knows whether to
draw the control; the write re-derives it from `created_at` before allowing
anything. A time-bounded permission must not take its clock from the party it
bounds. Staff are not time-bounded, and a staff edit of someone else's words
writes `activity_log` while a member fixing their own typo does not (§5.3).
* Post moderation shares the thread ledger via `target_type='post'`, so
"everything moderated in this Team" stays one query. `pin`/`lock` are refused
by name rather than as unknown actions — they describe a thread's place in a
list and its openness to replies, neither of which a post has. Counters are
RECOMPUTED after each action rather than nudged, because hide → unhide → hide
is a cycle a delta gets wrong the first time a step is retried.
Two fixes to phase 4 code this work reached: `softDeleteUploadsForPost` bound its
two arguments in the wrong order (never fired — nothing called it until post
deletion did), and it had no inverse, so `delete` → `restore` would have returned
a post's words and silently lost its pictures a retention window later.
Co-Authored-By: Claude <noreply@anthropic.com>
|
