feat(teams): phase 5 — Forum 5b, discussion + moderation + reports #155
Reference in New Issue
Block a user
No description provided.
Delete Branch "feature/teams-phase5-discussion"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Teams Phase 5 (
TEAMS.md§5.1's "5b"), intoedge. Docs half: RunicGateway/docs#158.One repo, where phase 4 needed two. Phase 4 spanned
module-uobecause the forum had no surface after phase 3 and a slot had to be declared. Phase 5 grows the component that fillsuo.guild.forum, so the module is untouched.No migration. Every column this needed —
type,locked,edited_at,edited_by, the post table'sstatus, the ledger'starget_type='post'— landed in phase 4. That was the point of §5.1's split-by-layer. The only new table iscontent_reports.What it ships
teams_forum_edit_window_minutes, 0–1440, default 15; staff unboundedtarget_type='post'content_reports, the report control, and the staff queueThree properties worth reviewing directly
1. Reports reach site staff and nobody else. §5.6 originally added "a leader may also see and act on reports for their own Team". Org lead settled 2026-08-18 that this is decided against, not deferred — the gap the feature closes is that leaders moderate their own forum and a Team's leaders are exactly the people who will not report their own Team. It is a negative property, and negatives are what nobody notices going, so it is asserted directly:
contentReports.test.jspins the model's whole function surface and checksqueue/handlenever mention leadership.2. The edit window is resolved on the server twice. The read path stamps each post with
canEdit/editableUntilso a client knows whether to draw the control; the write re-derives it fromcreated_atbefore allowing anything. The read is advice, the write is enforcement — a time-bounded permission must not take its clock from the party it bounds. The client may only ever withdraw an offer whose deadline passed while a page sat open.3. A locked thread refuses staff too. They hold
unlock, so nothing is lost — andlockedthen means the same thing to every reader. Unlock/post/relock reaches the same place leaving three ledger rows that say what happened; a moderator's reply in a thread nobody else may answer is the last word by fiat.§5.6's DDL does not work as written
With
statusin the unique key, closed rows collide with each other: dismiss a report, let the behaviour recur, dismiss the second — and theUPDATElands on a(…, 'dismissed')tuple that already exists, so the queue throws duplicate-key errors on the first repeat reporter. The table keys on a generatedopen_markerinstead (1 while open, NULL once closed), the same encodingteam_forum_grants.active_markeruses. Three smaller departures are recorded in the docs PR:handled_note, the two §2.10 username snapshots, and a real CASCADE onteam_id.Tests — 1009 server (was 972), 241 client (was 224)
Four acceptance criteria, plus the negative property above. The suite caught one real defect while being written:
describeTargetreturnedundefinedfor a hard-deleted target, andundefinedis dropped byJSON.stringify, so the documentedtarget: nullwould have reached clients as an absent key.Walked on the live rig before opening this
Per the org lead's call. Real server, real MariaDB, real sessions, four identities — ordinary member, granted non-member guest, Team leader, staffer — plus a browser pass over the forum panel, the reports queue, the per-Team ledger and the settings screen. Confirmed live: a locked thread refuses all four at 409; a hidden post renders for 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; the edit window at 0 stops the author while leaving staff unbounded.
It found one defect (last commit): the post-moderation validator listed only the four actions a post accepts, so
pinreturned a generic "Validation failed" instead of "pin applies to a thread, not to a post" — leaving that branch reachable only from its own unit test.Two phase-4 fixes this work reached
softDeleteUploadsForPostbound its arguments in the wrong order (never fired — nothing called it until post deletion did), and had no inverse, sodelete→restorewould have returned a post's words and silently lost its pictures a retention window later.Route manifest and OpenAPI regenerated: 6 operations added, 0 lost (checked explicitly — that is the silent-deletion failure mode
slotSpecs.jswarns about).🤖 Generated with Claude Code
https://claude.ai/code/session_01WnDSWzpUjw8t8C2hghysNz
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>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>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>