MM-69273 - Page comments: threaded footer/inline comments, resolve, editing, move re-home, retention, audit - #40
Draft
catalintomai wants to merge 3 commits into
Draft
MM-69273 - Page comments: threaded footer/inline comments, resolve, editing, move re-home, retention, audit#40catalintomai wants to merge 3 commits into
catalintomai wants to merge 3 commits into
Conversation
…e, editing, move re-home, retention, and audit
A page comment is a core Posts row (Type custom_page_comment) on the
space's backing channel, carrying a page_id prop as its only provenance.
- Seven routes under /spaces/{space_id}/pages/{page_id}/comments:
footer and inline creates with anchor validation, keyset-paged roots
listing with resolved/comment_type filters, offset-paged replies,
resolve/unresolve with last-state-change attribution both directions,
author-only message editing (edit_at on the payload), and a delete
guarded 409 by live-reply count with member force-through.
- WithPageCommentLock (page FOR SHARE + per-thread advisory lock)
serializes comment writes against page moves; commit-ambiguity probes
decide error paths by re-read, and committed writes always publish
their ids-only page_comment_* events.
- Cross-space page moves re-home comment threads through the core
MovePostsToChannel primitive, chunked, with straggler detection keyed
on the surviving page_id and a non-convergence guard. Edit-history
rows are excluded from the misplaced-roots detection: they ride the
primitive's OriginalId leg and are rejected as direct input.
- A RetentionPolicyId setting enrols space backing channels into a
granular retention policy at creation, with a reconcile sweep that
re-homes channels off a previous policy by remove-then-add.
- Every mutating route emits an audit record (fail-by-default, success
after the write, Auditable() payloads that exclude user content),
with a source-scan ratchet pinning the mutating-route count.
- API-driven Playwright specs (comment lifecycle, cursor walk, move
re-home) with a comment-RPC preflight; CI resolves the paired core
image from the e2e-core-commit PR marker.
- server/public pinned to the paired core branch head.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds page comments to the Docs plugin: threaded footer and inline comments with resolve/unresolve, author-only message editing, a has-replies delete guard, keyset-paged listings with filters, ids-only WebSocket events, cross-space move re-home, granular-retention enrolment for space channels, and audit records on every mutating route. The server surface is this PR entire; the RHS Comments panel that consumes the API is client work in a later PR.
A comment is a core
Postsrow (Type: custom_page_comment) on the space's backing channel, carrying apage_idprop as its only provenance — reusing the platform's post lifecycle (authorship, soft-delete cascade, reply threading) instead of a parallel comment store, while the paired core PR keeps those rows out of every chat surface.API
Seven routes under
/spaces/{space_id}/pages/{page_id}/comments:POST /commentsanchor_id(≤128 runes); both inline↔anchor half-states are 400sGET /commentsafter),resolvedandcomment_typefilters composed in SQLPOST /comments/{id}/repliesGET /comments/{id}/repliesGET /comments/{id}PATCH /comments/{id}resolved(roots only, any member, attribution both directions viaresolved_by/resolved_at) andmessage(author-only, roots and replies, refused 403 whole for a non-author;edit_aton the payload)DELETE /comments/{id}reply_countin the body; any other member force-deletes through and the platform cascade takes the repliesEvery mutating route emits an audit record through
LogAuditRec— fail-by-default, flipped to success after the write, withAuditable()result/prior state (comment text and page bodies never reach the log). A source-scan ratchet pins the 21-route count so a future mutating route cannot ship unaudited by omission.Storage and concurrency
Postsrows, read through the plugin store with literal-key JSON predicates (Props->>'page_id') that match the paired core PR's partial index.WithPageCommentLock— aFOR SHAREon the page row plus a per-thread advisory lock in one transaction — serializes comment writes againstMovePageToSpace, so a comment lands in the space its caller was authorized against, or not at all. Commit-ambiguity probes decide error paths by re-reading committed state, and a committed-but-errored write still publishes its WebSocket event.MovePostsToChannelprimitive (source-channel-narrowed on the routine path, space-wide on the re-issue repair path, chunked, with a non-convergence guard). Stragglers from a failed re-home are detectable by their survivingpage_idand repaired by re-issuing the move. Detection names live and soft-deleted roots only: a root's edit-history rows carryRootId=''withOriginalIdset, so they are excluded as inputs and travel through the primitive'sOriginalIdleg instead — the primitive rejects a batch that names one directly.Retention
A
RetentionPolicyIdplugin setting enrols every space backing channel into a granular data-retention policy — at space creation, and via a reconcile sweep on configuration change that re-homes channels off a previous policy (core's assignment is keyed on the channel alone and its add silently no-ops on conflict, so the sweep must remove-then-add). Without the setting, nothing is enrolled and comments follow the server's ordinary clocks.Core/plugin boundary
Core owns the post rows and every invariant around them: the write paths and hook clamps, counters and thread metadata, caches, the post-id authorization gates, retention, and the move primitive. The plugin owns the comment product policy: which posts are comments, the API and its locking, projections (
ChannelIdis never serialized), WS events, re-home and enrolment timing. The plugin writes no core table — every core-owned mutation goes through the plugin API.Pre-RBAC authorization posture: space membership is the gate on every route, matching the rest of the plugin; the per-permission split (
comment_page,delete_page_comment) wires up when the MM-69269 RBAC branch merges.Verified live against the paired stack
Docs home on the paired stack.
A space and its page tree, served by the branch build.
A published page — the object every comment hangs off. Its thread lives in the API until the RHS panel lands; the comments seeded on this page are the ones the transcripts below exercise.
Testing
UpdatePostorphans the comment), resolve attribution both directions, cursor semantics under mid-walk deletes andCreateAtties, the delete guard and cascade, edit-history-row invisibility (the app harness emulates core's re-keyed soft-deleted history rows and conditionalEditAt), move re-home with straggler repair, chunking and non-convergence, retention enrolment and policy re-home, audit record shapes, and a wire-level reply+edit round-trip. Full suite green against the pinnedserver/public;golangci-lintclean../scripts/run-tests.sh e2e-ui-local)../scripts/run-smoke.shdrives flag-off (501 gates over every route group, comments included), space/page CRUD, and a live comment suite against a real server built from the paired core branch — including the core-REST 403s on a comment's post id and a DB-asserted re-home.Dependency
Requires mattermost/mattermost#38207 (
MovePostsToChannel, retention assignment plugin APIs, backing-channel post writes and suppression).server/publicis pinned to that PR's head commit; CI E2E selects the paired image through the marker below.Ticket Link
Fixes: https://mattermost.atlassian.net/browse/MM-69273
Release Note