RunicGateway/website
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Activity

feat(teams): discussion threads, replies, the edit window and post moderation

Browse Source 
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>
This commit is contained in:
claude
2026-08-18 10:43:09 -05:00
parent 763de66ebb
commit ae0d27cf27
5 changed files with 542 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
server/src/model/teams/teamForum.db.js
View File

@@ -104,6 +104,45 @@ async function setPostStatus(id, status) {
return res.affectedRows > 0
}
/**
* Rewrite a post's body, stamping who edited it and when.
*
* `edited_at` is set unconditionally, including when a staffer edits — the column
* answers "has this been changed since it was written", which a reader needs to
* know regardless of whose hand did it. `edited_by` is the second half of that
* answer and is why the two are separate columns rather than a boolean.
*/
async function updatePostBody(id, bodyHtml, editedBy) {
const res = await query(
'UPDATE team_forum_posts SET body_html = ?, edited_at = NOW(), edited_by = ? WHERE id = ?',
[bodyHtml, editedBy, id],
)
return res.affectedRows > 0
}
/**
* Recompute a thread's denormalised counters from the posts that are actually
* visible.
*
* Called after every post moderation rather than incrementing and decrementing,
* because hide → unhide → delete → restore is a sequence in which a counter kept
* by deltas drifts the first time any step is retried or raced. The read is one
* indexed aggregate over one thread; correctness is worth more than the write it
* saves. `last_post_at` falls back to NULL for an emptied thread, which is what
* `threadsByTeam`'s COALESCE onto `created_at` already expects.
*/
async function recountThread(threadId) {
await query(
`UPDATE team_forum_threads t
SET t.post_count = (SELECT COUNT(*) FROM team_forum_posts p
WHERE p.thread_id = t.id AND p.status = 'visible'),
t.last_post_at = (SELECT MAX(p.created_at) FROM team_forum_posts p
WHERE p.thread_id = t.id AND p.status = 'visible')
WHERE t.id = ?`,
[threadId],
)
}
// ── the moderation ledger (append-only) ────────────────────────────────────
async function insertModeration({ teamId, targetType, targetId, action, actorUserId, actorUsername, actorRole, reason }) {
@@ -222,6 +261,8 @@ module.exports = {
postById,
insertPost,
setPostStatus,
updatePostBody,
recountThread,
insertModeration,
moderationForTeam,
insertUpload,
@@ -230,6 +271,7 @@ module.exports = {
listUploads,
softDeleteUpload,
softDeleteUploadsForPost,
restoreUploadsForPost,
sweepableUploads,
orphanedUploads,
deleteUploadRows,

282
server/src/model/teams/teamForum.model.js
View File

@@ -1,11 +1,11 @@
// ── The forum, phase 4 ("5a": access + announcements) ──────────────────────
// ── The forum: access + announcements (5a), discussion + moderation (5b) ───
//
// TEAMS.md §5.1's split is BY LAYER, not by feature: 5a ships the whole access
// model and a single announcements stream per Team; 5b opens discussion threads,
// replies and editing. The schema for all of it landed together, so 5b enables
// paths here rather than migrating data — which is why `type` is a parameter
// below and not a constant, and why `locked` is honoured on a thread nothing can
// reply to yet.
// TEAMS.md §5.1's split is BY LAYER, not by feature: 5a shipped the whole access
// model and a single announcements stream per Team; 5b (phase 5) opens discussion
// threads, replies, editing and post-level moderation. The schema for all of it
// landed together, so this phase added 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.
//
// **Every function here takes an already-resolved access decision.** Nothing in
// this file reads `team_members` or `team_forum_grants`; the caller asks
@@ -22,11 +22,26 @@ const forumDb = require('./teamForum.db')
const forumSettings = require('./teamForumSettings.model')
const { cleanForumBody, renderForumBody } = require('../../utils/forumHtml')
// Announcements are leader-authored and replies are disabled; 5b's discussion
// threads are member-authored and take replies. Both types exist in the enum from
// day one — this is the list of what 5a will CREATE.
// Announcements are leader-authored and take no replies; discussion threads are
// member-authored and do. Both have been in the enum since 5a — what phase 5
// changed is that both are now CREATABLE, and by different people.
//
// **The authority split lives in the controller, not here.** This list says what
// kinds of thread exist; who may make one is a question about the caller, which
// this file deliberately never asks (see the header on access decisions).
const CREATABLE_TYPES = ['announcement', 'discussion']
// Kept as an export because it names a real fact — the one type 5a could create —
// and because removing a name from a module's surface to save a line is how a
// consumer outside this repo breaks. It is not used to decide anything.
const CREATABLE_TYPES_5A = ['announcement']
// Which thread types accept replies. An announcement's `locked` stays false even
// though nothing may reply to it: replies are refused because the TYPE takes none,
// not because the thread was closed, and conflating the two would make "unlock"
// look like it would open replies on an announcement.
const REPLYABLE_TYPES = ['discussion']
const DELETED_AUTHOR = '[deleted account]'
/**
@@ -48,6 +63,17 @@ const THREAD_ACTIONS = {
restore: { status: 'visible' },
}
// Post-level moderation. A strict subset of THREAD_ACTIONS: `pin` and `lock`
// describe a thread's place in a list and its openness to replies, neither of
// which a post has. Naming them here as "not applicable" rather than as "unknown"
// is what lets `moderatePost` tell a caller which mistake they made.
const POST_ACTIONS = {
hide: { status: 'hidden' },
unhide: { status: 'visible' },
delete: { status: 'deleted' },
restore: { status: 'visible' },
}
function publicThread(row) {
return {
id: row.id,
@@ -65,14 +91,44 @@ function publicThread(row) {
}
/**
* One post, rendered for one image policy.
* May this viewer edit this post, and until when?
*
* **Computed on the server and handed to the client, never the other way round** —
* the same rule §5.5.3 applies to the image policy, for the same reason. A client
* that decided this would be deciding it against its own clock, and a clock is the
* one input a time-bounded permission must not take from the party it bounds.
*
* Staff get `editableUntil: null`, which reads as "no deadline" rather than as "no
* permission" — `canEdit` is the permission and this is only its expiry. An author
* past their window keeps a past `editableUntil`, so the UI can say *why* the
* control is gone instead of silently dropping it.
*/
function editability(row, { userId = null, isStaff = false, windowMinutes = 0, now = Date.now() } = {}) {
// A hidden or deleted post is not editable by anybody, staff included. Restoring
// it is a moderation action with a ledger row; quietly rewriting it while it is
// out of sight is the same act with no record.
if (row.status !== 'visible') return { canEdit: false, editableUntil: null }
if (isStaff) return { canEdit: true, editableUntil: null }
if (!userId || row.author_user_id == null || row.author_user_id !== userId) {
return { canEdit: false, editableUntil: null }
}
const until = new Date(row.created_at).getTime() + windowMinutes * 60_000
return { canEdit: until > now, editableUntil: new Date(until).toISOString() }
}
/**
* One post, rendered for one image policy and one viewer.
*
* `body` is what the reader gets and `mode` decides whether it carries images.
* The STORED html is never modified — flipping the policy changes this function's
* output and nothing on disk, which is the property §5.5.3 exists to give and the
* one acceptance criterion 3 measures.
*
* `viewer` is optional so that every 5a caller keeps working unchanged; omitting
* it yields `canEdit: false`, which is the right answer for a caller that has not
* said who is reading.
*/
function renderPost(row, mode) {
function renderPost(row, mode, viewer) {
return {
id: row.id,
author: row.author_username || DELETED_AUTHOR,
@@ -81,6 +137,8 @@ function renderPost(row, mode) {
createdAt: row.created_at,
editedAt: row.edited_at,
status: row.status,
mine: Boolean(viewer?.userId) && row.author_user_id === viewer.userId,
...editability(row, viewer),
}
}
@@ -97,8 +155,16 @@ async function listThreads(teamId, { canModerate = false, limit = 50, offset = 0
return rows.map(publicThread)
}
/** One thread with its posts, rendered under the current image policy. */
async function getThread(teamId, threadId, { canModerate = false } = {}) {
/**
* One thread with its posts, rendered under the current image policy and for one
* viewer.
*
* `viewer` carries who is reading and what the edit window is, so every post comes
* back already knowing whether this caller may edit it. The alternative — shipping
* the window to the client and letting it compare timestamps — is the thing
* `editability` exists not to do.
*/
async function getThread(teamId, threadId, { canModerate = false, viewer } = {}) {
const thread = await forumDb.threadById(threadId)
// The team check is here rather than in the SQL so a thread id from another
// Team reads as "not found" and not as "found, but not yours" — a forum is a
@@ -109,25 +175,34 @@ async function getThread(teamId, threadId, { canModerate = false } = {}) {
const mode = await forumSettings.imageMode()
const posts = await forumDb.postsByThread(threadId, { includeHidden: canModerate })
return { ...publicThread(thread), posts: posts.map((p) => renderPost(p, mode)) }
return {
...publicThread(thread),
// A reply control is offered when the TYPE takes replies and the thread is
// open. Both halves are reported separately (`type`, `locked`) so the UI can
// say which one is why, but the decision itself is made here — a client that
// recomputed it would be a second place for the rule to live.
canReply: REPLYABLE_TYPES.includes(thread.type) && !thread.locked && thread.status === 'visible',
posts: posts.map((p) => renderPost(p, mode, viewer)),
}
}
/**
* Post an announcement: a thread and its first post, in one call.
* Open a thread: the thread and its first post, in one call.
*
* An announcement is a degenerate thread rather than its own thing (§5.1) — which
* is why this writes the ordinary tables and 5b adds no migration. `locked` is
* left false: replies are refused because the TYPE takes none, not because the
* thread was closed, and conflating the two would make "unlock" look like it
* would open replies on an announcement.
* An announcement is a degenerate thread rather than its own thing (§5.1), which
* is why phase 5 added no migration — a discussion thread is the same two writes
* with a different `type`. The FIRST post is an ordinary post and is moderated,
* edited and reported like any other; nothing here marks it as special, because a
* thread whose opening post could not be moderated would be a hole shaped exactly
* like the one moderation exists to close.
*/
async function createThread({ team, actor, type, title, body }) {
if (!CREATABLE_TYPES_5A.includes(type)) {
return { ok: false, status: 400, error: 'Only announcements can be posted yet' }
if (!CREATABLE_TYPES.includes(type)) {
return { ok: false, status: 400, error: 'Unknown thread type' }
}
const cleaned = cleanForumBody(body)
if (!cleaned || !cleaned.replace(/<[^>]*>/g, '').trim()) {
return { ok: false, status: 400, error: 'An announcement needs a body' }
return { ok: false, status: 400, error: 'A post needs a body' }
}
const threadId = await forumDb.insertThread({
teamId: team.id,
@@ -136,13 +211,102 @@ async function createThread({ team, actor, type, title, body }) {
createdBy: actor.id,
createdUsername: actor.username,
})
await forumDb.insertPost({
const postId = await forumDb.insertPost({
threadId,
authorUserId: actor.id,
authorUsername: actor.username,
bodyHtml: cleaned,
})
return { ok: true, threadId }
return { ok: true, threadId, postId }
}
/**
* Reply to a discussion thread.
*
* Three refusals, and the status codes are chosen to be distinguishable rather
* than uniform. A thread that is not there, or is hidden from this caller, is 404
* for the §5.5.1 reason. An announcement is 400 — the request is malformed for
* this thread, and no amount of retrying fixes it. A locked thread is **409**: the
* request is fine and the resource's state is what refuses, which is exactly the
* distinction a client needs to tell "you cannot" from "not right now".
*
* **Locked refuses staff too.** They hold `unlock`, so nothing is lost — and what
* is gained is that `locked` means the same thing to every reader. A moderator's
* reply appearing in a thread nobody else may answer is the last word by fiat;
* unlock, post, relock is the same outcome with three ledger rows saying so.
*/
async function createPost({ team, threadId, actor, body }) {
const thread = await forumDb.threadById(threadId)
if (!thread || thread.team_id !== team.id || thread.status !== 'visible') {
return { ok: false, status: 404, error: 'Thread not found' }
}
if (!REPLYABLE_TYPES.includes(thread.type)) {
return { ok: false, status: 400, error: 'Announcements do not take replies' }
}
if (thread.locked) {
return { ok: false, status: 409, error: 'This thread is locked' }
}
const cleaned = cleanForumBody(body)
if (!cleaned || !cleaned.replace(/<[^>]*>/g, '').trim()) {
return { ok: false, status: 400, error: 'A reply needs a body' }
}
const postId = await forumDb.insertPost({
threadId,
authorUserId: actor.id,
authorUsername: actor.username,
bodyHtml: cleaned,
})
return { ok: true, threadId, postId }
}
/**
* Edit a post: the author inside the window, staff at any time (§5.4).
*
* The window is re-derived HERE from `created_at` and never trusted from the
* request, which is also why `editability` runs on the read path — the read tells
* the client whether to draw the control, and this decides whether the edit
* happens. Two evaluations of one rule, deliberately: the read one is advice and
* this one is enforcement.
*
* A staffer editing someone else's post is reported back as `staffEdit` so the
* controller can write the §5.3 accountability row. A staffer editing their OWN
* post is an ordinary edit and is not: the trail records interventions, and
* everything a staffer ever typed is not an intervention.
*/
async function editPost({ team, postId, actor, isStaff = false, windowMinutes = 0, body }) {
const post = await forumDb.postById(postId)
if (!post) return { ok: false, status: 404, error: 'Post not found' }
const thread = await forumDb.threadById(post.thread_id)
if (!thread || thread.team_id !== team.id) return { ok: false, status: 404, error: 'Post not found' }
if (post.status !== 'visible' || thread.status !== 'visible') {
return { ok: false, status: 404, error: 'Post not found' }
}
const isAuthor = post.author_user_id != null && post.author_user_id === actor.id
if (!isAuthor && !isStaff) {
return { ok: false, status: 403, error: 'You may only edit your own posts' }
}
if (!isStaff) {
if (thread.locked) return { ok: false, status: 409, error: 'This thread is locked' }
const { canEdit } = editability(post, { userId: actor.id, windowMinutes })
if (!canEdit) {
return {
ok: false,
status: 403,
error: windowMinutes > 0
? `The ${windowMinutes}-minute edit window for this post has closed`
: 'Posts cannot be edited on this site',
}
}
}
const cleaned = cleanForumBody(body)
if (!cleaned || !cleaned.replace(/<[^>]*>/g, '').trim()) {
return { ok: false, status: 400, error: 'A post needs a body' }
}
await forumDb.updatePostBody(postId, cleaned, actor.id)
return { ok: true, postId, threadId: post.thread_id, staffEdit: isStaff && !isAuthor }
}
/**
@@ -175,19 +339,85 @@ async function moderateThread({ team, threadId, action, actor, actorRole, reason
return { ok: true, action, threadId }
}
/**
* Apply a moderation action to a POST, and record which authority did it.
*
* The same ledger as `moderateThread`, with `target_type='post'` — one table, two
* target kinds, because "show me everything that was moderated in this Team" is
* the question the admin view asks and two tables would make it a union.
*
* `pin` and `unpin`, `lock` and `unlock` are refused with a message that names the
* mistake rather than a bare "unknown action": they are real actions applied to
* the wrong kind of object, and a caller who sent one has a bug worth telling
* them about precisely.
*
* **The opening post of a thread is moderatable like any other.** Hiding it leaves
* a thread with a title and its replies and no body, which looks odd and is
* correct — an abusive opener does not have to take a good discussion with it, and
* a moderator who wants the whole thing gone has `hide` on the thread.
*/
async function moderatePost({ team, postId, action, actor, actorRole, reason }) {
const effect = POST_ACTIONS[action]
if (!effect) {
return {
ok: false,
status: 400,
error: THREAD_ACTIONS[action]
? `"${action}" applies to a thread, not to a post`
: 'Unknown moderation action',
}
}
const post = await forumDb.postById(postId)
if (!post) return { ok: false, status: 404, error: 'Post not found' }
const thread = await forumDb.threadById(post.thread_id)
if (!thread || thread.team_id !== team.id) return { ok: false, status: 404, error: 'Post not found' }
await forumDb.setPostStatus(postId, effect.status)
// The counters are recomputed rather than nudged, because these four actions
// form cycles (hide → unhide → hide) that a delta gets wrong the first time one
// is retried.
await forumDb.recountThread(post.thread_id)
// Images follow their post. Soft on the way out and reversible on the way back
// in, so `delete` → `restore` inside the retention window returns the post
// whole; past it, the sweep has taken the bytes and nothing can.
if (action === 'delete') await forumDb.softDeleteUploadsForPost(postId, actor.id)
if (action === 'restore') await forumDb.restoreUploadsForPost(postId)
await forumDb.insertModeration({
teamId: team.id,
targetType: 'post',
targetId: postId,
action,
actorUserId: actor.id,
actorUsername: actor.username,
actorRole,
reason,
})
return { ok: true, action, postId, threadId: post.thread_id }
}
/** The ledger for the admin Team page. Staff-only by its route, not by this function. */
async function moderationLedger(teamId, opts) {
return forumDb.moderationForTeam(teamId, opts)
}
module.exports = {
CREATABLE_TYPES,
CREATABLE_TYPES_5A,
REPLYABLE_TYPES,
THREAD_ACTIONS,
POST_ACTIONS,
listThreads,
getThread,
createThread,
createPost,
editPost,
moderateThread,
moderatePost,
moderationLedger,
publicThread,
renderPost,
editability,
}

61
server/src/model/teams/teamForumSettings.model.js
View File

@@ -1,29 +1,35 @@
// ── The operator's two forum controls, and the acknowledgement gate ────────
// ── The operator's forum controls, and the acknowledgement gate ────────────
//
// TEAMS.md §5.5. Three `settings` keys, and the reason they live in their own
// file rather than in settings.model.js is that only one of them is an ordinary
// key: `teams_forum_images` has a server-side precondition, and a precondition
// buried in the generic setMany() loop is one nobody reading that loop would
// know about.
// TEAMS.md §5.5, plus phase 5's edit window. Four `settings` keys, and the reason
// they live in their own file rather than in settings.model.js is that only two
// of them are ordinary keys: `teams_forum_images` has a server-side precondition,
// and a precondition buried in the generic setMany() loop is one nobody reading
// that loop would know about.
//
// teams_forums_enabled '0' | '1' default '0' — off
// teams_forum_images 'disabled' | 'remote' | 'uploads' default 'disabled'
// teams_forum_uploads_ack the acknowledged TEXT VERSION absent until given
// teams_forum_edit_window_minutes 0 … 1440 default 15 (phase 5)
//
// **Both reads fail closed.** A DB fault reports the forum off and images
// disabled, because the alternative is a transient error opening a feature the
// operator turned off, or rendering third-party images on a site whose operator
// chose not to. The cost of failing closed here is a forum that 404s for a minute;
// the cost of failing open is a policy that is not a policy.
// **Every read fails closed.** A DB fault reports the forum off, images disabled
// and the edit window shut, because the alternative is a transient error opening a
// feature the operator turned off, or rendering third-party images on a site whose
// operator chose not to. The cost of failing closed here is a forum that 404s for a
// minute; the cost of failing open is a policy that is not a policy.
const settingsDb = require('../settings/settings.db')
const ENABLED_KEY = 'teams_forums_enabled'
const IMAGES_KEY = 'teams_forum_images'
const ACK_KEY = 'teams_forum_uploads_ack'
const EDIT_WINDOW_KEY = 'teams_forum_edit_window_minutes'
const IMAGE_MODES = ['disabled', 'remote', 'uploads']
// How long an author may edit their own post. Staff are not bound by it (§5.4).
const EDIT_WINDOW_DEFAULT = 15
const EDIT_WINDOW_MAX = 1440 // a day; beyond that "window" stops meaning anything
// The version of the §5.5.5 warning text currently in force. Bumping this is what
// makes every stored acknowledgement stale — see `ackState` below for what that
// then does, which is deliberately NOT "turn uploads off".
@@ -52,6 +58,33 @@ async function imageMode() {
}
}
/**
* How many minutes an author has to edit their own post.
*
* Fails closed to ZERO rather than to the default, and that 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 who is quoting it or a moderator who
* is about to act on a report — so the safe answer during a DB fault is "nobody
* may edit for the next minute", not "everyone may edit for fifteen". Staff are
* unaffected either way, because their authority is not time-bounded.
*
* `0` is also a legitimate STORED value, meaning an operator who wants posts
* immutable once written. There is deliberately no distinction between "off" and
* "unreadable" here: both deny, and inventing a third state would only give the
* caller a decision to get wrong.
*/
async function editWindowMinutes() {
try {
const raw = await settingsDb.get(EDIT_WINDOW_KEY)
if (raw == null || raw === '') return EDIT_WINDOW_DEFAULT
const n = Number(raw)
if (!Number.isFinite(n) || n < 0 || n > EDIT_WINDOW_MAX) return EDIT_WINDOW_DEFAULT
return Math.floor(n)
} catch {
return 0
}
}
/** Are uploads accepted? The one mode where files come to rest on the operator's disk. */
async function uploadsEnabled() {
return (await imageMode()) === 'uploads'
@@ -127,7 +160,7 @@ async function assertAcknowledged(nextMode, acknowledge) {
* key.
*/
async function assertSettingsWritable(keys, acknowledge) {
const touchesForum = keys.some((k) => k === ENABLED_KEY || k === IMAGES_KEY)
const touchesForum = keys.some((k) => k === ENABLED_KEY || k === IMAGES_KEY || k === EDIT_WINDOW_KEY)
if (!touchesForum) return { ok: true }
const state = await ackState()
if (!state.stale) return { ok: true }
@@ -148,10 +181,14 @@ module.exports = {
ENABLED_KEY,
IMAGES_KEY,
ACK_KEY,
EDIT_WINDOW_KEY,
IMAGE_MODES,
ACK_VERSION,
EDIT_WINDOW_DEFAULT,
EDIT_WINDOW_MAX,
forumsEnabled,
imageMode,
editWindowMinutes,
uploadsEnabled,
ackState,
assertAcknowledged,