fix(admin): let moderators preview a post before approving it - #1340
Merged
Conversation
The "in review" queue showed a title, author and excerpt but gave no way
to read the post, so there was nothing to base an Approve/Decline on.
Each queued item now has a Preview link that opens where the post
actually renders: /d/{slug} for discussions and questions, the
destination itself for shared links, and /{username}/{slug} for
everything the site renders. For that last case the reader resolvers now
grant admins the bypass authors already had for their own in_review and
rejected posts, so the preview is the article exactly as readers would
eventually see it, "Awaiting review" banner and all.
That visibility rule was about to be a third copy of the same
published-or-owner check, so it moves to server/lib/postVisibility.ts
with unit tests covering the anonymous, author and admin branches — the
author predicate is the only thing stopping one member reading another's
drafts, so it is worth pinning down.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Uh oh! @vercel[bot], the image you shared is missing helpful alt text. Check #1340 (comment). Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs. |
Contributor
WalkthroughThis change centralizes post visibility rules, passes administrator context to post pages, and adds preview links for posts in the moderation queue. ChangesModeration preview visibility
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AdminModerationQueue
participant PreviewLink
participant PostPage
participant postVisibilityFilter
AdminModerationQueue->>PreviewLink: provide post type and destination fields
PreviewLink->>PostPage: navigate to selected preview route
PostPage->>postVisibilityFilter: provide viewer ID and administrator role
postVisibilityFilter-->>PostPage: return visibility predicate
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
NiallJoeMaher
had a problem deploying
to
production
August 12, 2026 06:50 — with 
GitHub Actions
Failure
GitHub Actions
Failure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/`(admin)/admin/moderation/_client.tsx:
- Around line 55-70: Update postPreviewHref and the RSS import path to validate
external links with safeExternalHref before navigation or persistence. For link
posts, pass post.externalUrl through safeExternalHref and return null for
rejected schemes; ensure RSS imports also reject invalid item.link schemes
rather than storing them in posts.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e6e3da8-4c83-4d41-bf3b-421f01f4b4b2
📒 Files selected for processing (7)
app/(admin)/admin/moderation/_client.tsxapp/(app)/[username]/[slug]/page.tsxapp/(app)/d/[slug]/page.tsxcomponents/ContentDetail/PostReader.tsxserver/api/router/admin.tsserver/lib/postVisibility.test.tsserver/lib/postVisibility.ts
Comment on lines
+55
to
+70
| function postPreviewHref(post: PreviewablePost): string | null { | ||
| if (post.type === "link") return post.externalUrl; | ||
| if (!post.slug) return null; | ||
| if (post.type === "discussion" || post.type === "question") { | ||
| return `/d/${post.slug}`; | ||
| } | ||
| if (!post.authorUsername) return null; | ||
| return `/${post.authorUsername}/${post.slug}`; | ||
| } | ||
|
|
||
| const PreviewLink = ({ post }: { post: PreviewablePost }) => { | ||
| const href = postPreviewHref(post); | ||
| if (!href) return null; | ||
|
|
||
| return ( | ||
| <Link href={href} target="_blank" className="secondary-button"> |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the schema and all TypeScript write/read paths for external URLs.
fd -t f -e ts -e tsx | while IFS= read -r file; do
rg -n -C 3 'externalUrl|external_url' "$file" || true
doneRepository: codu-code/codu
Length of output: 35426
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- httpUrl and safeExternalHref definitions ---'
rg -n -C 8 'function httpUrl|const httpUrl|export .*httpUrl|safeExternalHref' .
printf '%s\n' '--- post write handlers and schemas ---'
rg -l 'externalUrl' --glob '*.ts' --glob '*.tsx' | while IFS= read -r file; do
if rg -q 'insert\(posts\)|update\(posts\)|INSERT INTO posts|externalUrl: input\.externalUrl|externalUrl: item\.link|externalUrl = ' "$file"; then
printf '\n### %s\n' "$file"
rg -n -C 12 'insert\(posts\)|update\(posts\)|INSERT INTO posts|externalUrl: input\.externalUrl|externalUrl: item\.link|externalUrl = ' "$file"
fi
done
printf '%s\n' '--- URL normalization and direct navigation helpers ---'
rg -n -C 8 'normalizeUrl|ensureHttps|new URL\(|startsWith\("http"\)|startsWith\('\''http' "$(
rg -l 'externalUrl' --glob '*.ts' --glob '*.tsx' .
)" || trueRepository: codu-code/codu
Length of output: 16142
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- URL schema and render guard ---'
cat -n schema/shared.ts | sed -n '1,35p'
cat -n utils/url.ts | sed -n '1,70p'
printf '%s\n' '--- direct post write sites ---'
rg -l --glob '*.ts' --glob '*.tsx' \
'externalUrl: input\.externalUrl|externalUrl: item\.link|INSERT INTO posts|createLinkPost' . |
while IFS= read -r file; do
printf '\n### %s\n' "$file"
rg -n -C 18 \
'externalUrl: input\.externalUrl|externalUrl: item\.link|INSERT INTO posts|createLinkPost' \
"$file"
done
printf '%s\n' '--- all externalUrl validation declarations ---'
rg -n -C 6 \
'externalUrl: (httpUrl\(\)|z\.string\(\)\.url|z\.string\(\)\.trim|item\.link|input\.externalUrl)' \
--glob '*.ts' --glob '*.tsx' .Repository: codu-code/codu
Length of output: 17334
Guard externalUrl before navigation.
RSS importers persist item.link directly to posts, so httpUrl() does not cover every write path. Use safeExternalHref before passing externalUrl to Link, and reject invalid schemes during RSS imports.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/`(admin)/admin/moderation/_client.tsx around lines 55 - 70, Update
postPreviewHref and the RSS import path to validate external links with
safeExternalHref before navigation or persistence. For link posts, pass
post.externalUrl through safeExternalHref and return null for rejected schemes;
ensure RSS imports also reject invalid item.link schemes rather than storing
them in posts.
|
Uh oh! @vercel[bot], the image you shared is missing helpful alt text. Check #1340 (comment). Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs. |
This was referenced Aug 12, 2026
NiallJoeMaher
added a commit
that referenced
this pull request
Aug 20, 2026
…gin (#1344) * fix: moderation preview scope, comment vote races, and email link origin Three fixes, consolidated into one PR by request. 1. Move the moderation preview off the public reader routes (#1340 follow-up) #1340 let admins resolve in_review/rejected posts at their public URLs. That put the public reader — vote, bookmark and comment controls — on a post that may be about to be rejected, and `post.vote` has no status guard, so a misclick writes a vote and author points onto content the moderator is declining. It also stopped admins seeing the site the way readers do, and made the rejected-post banner's "not visible to anyone else" untrue. Preview now lives at /admin/moderation/preview/{id}: read-only, inside the admin gate. The public routes and their visibility filter revert. It also fixes the link path. #1340 sent Preview straight off-site, so the member's own title/excerpt/body — where a spammer would put the payload — was never shown; the preview renders both halves. And that off-site href skipped `safeExternalHref` and rel, so an externalUrl that never passed `httpUrl()` validation would run as a `javascript:` URL inside the authenticated admin session, and the page under review received the admin surface as its referrer. 2. Drop the frozen sort snapshot, serialise votes per comment (#1341 follow-up) Freezing sort scores was more than the fix needed and wrong on its own terms: counts still updated on refetch while the order did not, so a thread could show a 42-point comment below a 3-point one; the documented "re-pick the sort" escape hatch never fired, because selecting the already-selected option is a no-op; and the added tiebreak made Top identical to New on the common all-zero thread. Not refetching after a successful vote is the whole fix. Ordering is derived from the data on screen again, so it cannot contradict the counts beside it. Votes are serialised per comment (newest click wins), since #1341 dropped the in-flight guard without replacing it and overlapping writes could land in either order. The resync remount is per comment too. 3. Email links pointed at the deployment, not the site `getAppOrigin()` fell back to VERCEL_URL, which is the unique per-deployment hostname — and Vercel sets it in production too. With DOMAIN_NAME unset, every link it built (the admin's "post awaiting review" deep link, report emails, the verification link) went out as *.vercel.app. Production now resolves to the project's production domain, falling back to the canonical origin; preview deploys still get their own URL. The duplicate copy of this logic in utils/emailToken.ts is gone. * fix: address review of the consolidated fixes Review of #1344 found nine issues, three of them mine from the previous round. Draft exposure (the serious one): the new preview route loaded any post by id with no status predicate, so an admin with a post id could read a member's private, never-submitted draft. It is now restricted to work that actually entered the pipeline — published, in_review, rejected — and `postVisibility.ts` is back, minus the admin bypass, carrying tests for both rules including "never exposes a draft, whoever is looking". Deleting that module was never required to drop the bypass, and doing so had also left the same rule inlined twice in two different shapes. Failed votes stranded earlier ones: I removed the pre-remount refetch on the reasoning that the cache never saw the failed vote. That is true of the failed vote but not of earlier successful ones, which never refetch by design — so remounting reseeded the control from a cache that predates them. The refetch is back, before the key bump. Comment ordering on ties was arbitrary: the "Top" comparator leaned on sort stability to keep "the server's order", but the server orders by ltree path, built from a random uuid. On a thread where most scores are 0, that is no order at all. Ties now sort oldest-first, so Top degrades to chronological rather than to a copy of New. Reordering under the reader: with the score snapshot gone, a window-focus refetch could re-rank the thread mid-read. That query no longer refetches on focus; the explicit refetches after create/edit/delete stay, since those follow something the reader did. Also: raw tRPC error text could reach the vote toast (only the rate-limit message, which is written for readers, is surfaced now); a malformed post id 500'd on the uuid cast instead of 404ing; the preview's two queries ran serially when both key off the route param; and a fork deploying to production without VERCEL_PROJECT_PRODUCTION_URL got codu.co hardcoded over its own configured NEXTAUTH_URL. * ci(e2e): serve a production build instead of the dev server The suite has been failing on CI while staying green locally: 19 failures spread across admin navigation, the editor publish flow, bookmarking, the feed sidebar and moderation. Identical failures on develop and on every branch off it, so nothing in the feature work caused them. They all share a cause. Playwright's webServer ran `next dev`, so the first request to each route blocked on an on-demand Turbopack compile. Locally that is under a second; on a cold runner with three workers compiling at once it outlasts the 10s expect timeout — hence assertions like `toHaveURL(/admin/users)` polling 13 times and giving up while the navigation was still compiling. CI now builds once and serves it. Measured on the same machine, a cold route costs ~0.7-1.2s under `next dev` and ~0.02-0.03s prebuilt. Local runs keep `next dev` for the fast feedback loop. EMAIL_AUTH_ENABLED is set for the job because a production build runs with NODE_ENV=production, which would otherwise disable the passwordless provider that dev turns on implicitly. * fix: review round three, and build the app inside the e2e webServer The e2e harness fix in the previous commit did not take. The job is triggered by `pull_request_target`, which takes the WORKFLOW from the base branch and the CODE from the PR head — so the build step added to the workflow never ran, while playwright.config (from the head) had already switched to serving a prebuilt app. Result: "Could not find a production build". The build now happens inside the webServer command, where head and workflow cannot disagree, and the env it needs moved into the npm scripts for the same reason. Review fixes: - moderationPreviewFilter missed `scheduled` and `unlisted`, so preview 404'd on a post the admin had just approved-with-schedule. The status list is now derived from the enum instead of hand-listed. - The "top" tie-break went back to newest-first. Oldest-first read better as conversation order but buried a comment the moment you posted it, which is worse than Top resembling New on an unvoted thread. - create/edit awaited react-query's void `mutate`, so their try/catch was dead and the editor cleared before the request finished: a failed post discarded what you typed, silently. Both use mutateAsync now. - The preview fetched the cover image and never rendered it. Clean body copy under an abusive image would have sailed through; it is shown now, behind the same scheme guard as the external URL. - PostBody rendered the site-wide 404 component for an empty tiptap body, which put a "page not found" panel inside the admin shell. The empty state is the caller's to choose now. - getAppOrigin fell back to the hardcoded codu.co ahead of the deployment's own URL, so an unconfigured fork mailed its users to this site. - A failed vote now clears its queued follow-up explicitly. Accepted, not fixed: the thread no longer refetches on window focus, so an open tab does not pick up other people's comments until you post, navigate or reload. That is the cost of not moving comments under someone mid-read. * ci(e2e): trust the request host when serving the production build The prebuilt server ran, but every authenticated test failed: NextAuth rejects the request host under NODE_ENV=production unless AUTH_TRUST_HOST is set, so /api/auth/session returned UntrustedHost, session.user was undefined, and pages blew up on `session.user.username`. 159 failures. Dev never hits this — it trusts the host implicitly — which is why the suite passed locally. My earlier local check passed for the wrong reason: I had happened to pass AUTH_URL on the command line, which also satisfies the trust check, so the gap only showed up on CI where it is not set. Reproduced locally against a production build: without AUTH_TRUST_HOST, /admin redirects and the log carries UntrustedHost; with it, /admin and /admin/moderation both return 200 and the log is clean. * test(e2e): let the suite point at a throwaway database `setup.ts` and `teardown.ts` hardcoded localhost:5432/postgres, which is also the dev database. Running the suite locally therefore writes fixtures into whatever you have been working on, so the remaining pre-existing failures cannot be debugged without risking your own data. Both now honour DATABASE_URL, falling back to the same string, so CI is unaffected and a local run can be aimed at a scratch database instead. * test(e2e): enable moderation, fix the sidebar viewport, centralise the DB url Three causes behind the long-standing failures, found by running the suite locally against a throwaway database. MODERATION_ENABLED was never set for the e2e app. The whole pipeline — publish gating, the review queue, link dedupe — is behind that flag, so e2e/moderation.spec.ts was asserting behaviour the server had switched off. All four of its tests pass with the flag on. The feed sidebar test asserted the right rail is visible on any non-mobile viewport, but .app-main folds the rail away under 1300px and Playwright's desktop viewport is 1280 — so it was asserting against a width where the rail is correctly hidden. The test now widens past the breakpoint. Four helpers in e2e/utils/utils.ts still hardcoded the connection string that the file had already centralised, so a run aimed at a scratch DATABASE_URL wrote its fixtures into the default database instead and then failed on foreign keys. Local suite on Desktop Chrome: 129 passed, 1 failed (a flaky multi-user notification test), down from 6 failures before these changes. * test(e2e): give the bookmark specs their own fixture per browser project Bookmarks are per-user and all four browser projects run as the same e2e user, so a shared article made the bookmark specs race each other: one project saved it while another was asserting it was still unsaved. That is why they failed on Firefox and mobile but never on Desktop Chrome, which happened to get there first. saved.spec.ts already ran serially, but serial mode only orders tests within a project, not across them. Each project now creates and cleans up its own article. Reproduced the failure locally across all four projects, and all 52 tests in those two files pass afterwards. The /saved assertion no longer needs its "or the empty state" escape hatch either, since nothing else can unbookmark it. Also gives the discussion editor's submit button a data-testid. "Reply" is the label of both the editor's submit button and every comment's expand-reply button, so the notification spec was picking it out with .last() — ambiguous as soon as a comment has nested children. That spec still fails intermittently for a separate reason (its reply lands as a top-level comment, so the server correctly raises "commented on your post" rather than "replied to your comment"); it retries green and is left for a follow-up rather than papered over here. * test(e2e): check the card's content link, not its first anchor The routeless-/[id] guard took the FIRST anchor inside a content card, but that is the author handle ("/{username}") — a real page that nonetheless reads as a bare single-segment path to the shape check, so the test failed with 'href "/e2e-test-user-one-111" looks like a routeless /[id] page'. It only surfaced on some browsers because which card ranks first varies, and a source card's handle link ("/s/{slug}") has two segments and slips through. The card's content link now carries data-testid="content-card-link" and the test targets that, so it checks the link the guard is actually about. All 44 tests in the file pass across all four browser projects. * test(e2e): bypass rate limits, and wait for comments to actually post Two causes behind the long tail of failures, found once the suite could be run locally against a throwaway database on the Node version .nvmrc pins. `discussion-create` allows 10 comments per 10 minutes per user, and the suite drives all four browser projects through the same two seeded users — so later projects were silently throttled, their comments never reached the database, and the tests that depend on them failed. Rate limiting now has an explicit `RATE_LIMIT_DISABLED` bypass that the e2e scripts set. It is guarded on that one env var and documented as test-only. The notification specs asserted a comment had posted with `getByText(commentText)`, which also matches the editor's own textarea — so it resolved the instant `fill()` ran, before the request had been sent. The test then switched users mid-flight, and the comment landed under whoever was authenticated by the time it went out: the "user one comments, user two replies" scenario ended up with both comments written by user two, replying to itself, so no reply notification was ever generated. They now wait for the create response and assert on a rendered `section.group/comment`. Local suite across all four browser projects, fresh database: 518 passed, 2 failed — both Mobile Safari, a publish redirect and a dedupe toast — down from 19 failures.
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
1 participant
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.
What
The in review queue at
/admin/moderationlisted a title, author and (now) an excerpt, but there was no way to actually read the post — so Approve/Decline was a judgement call with nothing to judge.Each queued item now has a Preview link that opens where the post really renders:
discussion,question/d/{slug}link/{username}/{slug}For the last case the reader resolvers now grant admins the bypass authors already had for their own
in_review/rejectedposts, so a moderator sees the article exactly as readers eventually will — "Awaiting review" banner included — rather than a bespoke admin rendering that could drift from the real thing.Why the shared filter
That published-or-owner rule was about to exist in a third place, in a third shape. It moves to
server/lib/postVisibility.tsand both reader resolvers call it.The author predicate in there is the only thing stopping one member reading another's drafts, so it gets unit tests: anonymous sees live posts only, a signed-in member only bypasses for their own posts, an admin bypasses for any author, and no viewer ever sees
draft.Verified locally
in_reviewpost; signed-in non-author → 404; author → 200; admin → 200.linkpost moved into review (which would otherwise 404, since the link resolver is published-only).npm run lint,npm run prettier,npm run test:unit(123 passing),npm run build.