Stop the docs editor eating a reader's typing, and three things it claimed but never did - #116
Conversation
Code was coloured while you wrote it and flat grey the moment anyone read it. renderMarkdown now runs lowlight over a fenced block and emits hljs token spans, which the sanitizer already allows, and the token colours move out of the editor only stylesheet into docProseClassName so the reader, the published page, the PDF print and the markdown preview all get them from one place.
Inserting a table was the only thing the editor could do with one. A
bubble menu keyed on editor.isActive('table') now carries insert row
above and below, delete row, insert column left and right, delete
column, toggle the header row and delete the table, all wired to the
commands TableKit already ships.
Duplication is how templates actually get used: people copy last week's notes rather than start from a static template. duplicateDoc copies the body, collection, project and parent of any doc the caller can read, names it after the source, and hands authorship to whoever asked, so someone with only read access can take their own editable copy. The copy never inherits the published link of the original, and a public source lands as a workspace doc rather than silently republishing itself. getDoc also starts reporting the effective per doc access level, decided in the same place assertDocWritable enforces it.
The doc surface decided between the editor and the reader from can(principal, 'doc:write'), a role capability every member holds. A member granted only read on a restricted doc got the full editing surface, typed into it, and watched autosave 403 behind a Save failed chip. The gate now follows the access level GET /api/docs/[id] reports, which is the same answer the write path enforces, and an archived doc is read only for the same reason: nothing it accepts can be saved. A reader sees why the page is read only, and can still take a copy.
Read time already existed and was tested, and rendered in exactly one place: the reader, which only members without write access ever see. Both numbers now sit in DocContextRow, so the editing majority sees them too, and the reader stops printing read time twice.
There was a problem hiding this comment.
imshashank has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 16 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (27)
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 |
There was a problem hiding this comment.
imshashank has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Five items off the docs survey, all Tier 1, chosen so the surface stops
claiming things it does not honour. Nothing from Tier 2 is here.
1. The read-only gate, which was a bug that ate typing
DocSurfacepicked between the editor and the reader fromcan(principal, 'doc:write'), a role capability every member holds. Theper-doc gate is a different check:
assertDocWritableallows the author,admins, non-restricted docs, and holders of a
writegrant. So a membergranted only read on someone's private doc got the full editing
surface, typed into it, and lost the lot to a 403 that surfaced as a
small "Save failed" chip.
getDocnow returns the effective per-docaccesslevel, decided bydocAccessLevel, whichassertDocWritableis rewritten in terms of, sothere is exactly one place that answers the question.
GET /api/docs/[id]carries it,
docDetailSchemaparses it (falling back toread, the safedirection, if an older server omits it), and the client gate is
canWriteDocs && access === 'write' && archivedAt === null.An archived doc is read-only for the same reason: everything it accepted
used to 409. A reader now sees a "Read only" or "Archived" pill instead
of wondering.
Not built: "ask to edit". A reader now has a real escape hatch below.
2. Syntax highlighting on the read path
Code was coloured while you wrote it and flat grey the instant anyone
read it:
renderMarkdownwas plainmarked, anddocProseClassNamedefined no
.hljs-*rules.renderMarkdownnow runs lowlight over afenced block and emits token spans, which the sanitizer already allows
(
span+class), and the token colours move out of the editor-onlystylesheet into a shared
code-theme.tsthatdocProseClassNamepullsin. That covers the reader, the guest view, the published page, the PDF
print path and the markdown preview from one definition.
The serializer only ever emits
<span class="...">and escapeseverything else, so a block containing
</code></pre><img onerror=...>comes out as text. There is a test for exactly that.
3. Table row and column controls
Inserting a table was the only thing the editor could do with one. A
bubble menu keyed on
editor.isActive('table')now carries insert rowabove/below, delete row, insert column left/right, delete column, toggle
header row, and delete table, wired to the commands
TableKitalreadyships. Merged cells and column widths stay out, deliberately: markdown
cannot represent either and they would vanish on save.
4. Duplicate a doc
duplicateDoccopies the body, collection, project and parent of any docthe caller can read, names it
<title> (copy)inside the 200 characterlimit, and hands authorship to whoever asked. Two things it refuses to
do: inherit the publish token of the original, and keep
publicorlinkvisibility (a copy of a published page lands as a workspace doc,it does not silently republish itself).
The button sits in the doc header for anyone with
doc:write, includingsomeone who only has read access to this doc, which is the point: it is
the reader's way out of a page they cannot edit.
Not built: recursive duplication of children. One page at a time.
5. Word count and read time where people write
Read time already existed and was tested, and rendered in exactly one
place:
DocReader, which only people without write access ever see.Both numbers now sit in
DocContextRow, so the editing majority seesthem, and the reader stops printing read time twice.
Deliberately not in this PR
Docs in the command palette, doc presence avatars,
?parent=on/docs/new, tip and danger slash commands, the/docshome, doc icons,issue mention chips, and
doc.sortOrder. Each is real work with its owntest surface and none of them shares a seam with the five above.
Tests
Every test here was watched go red against a deliberate mutation of the
code it covers.
doc-service.test.ts"agrees with updateDoc for every reader"can(principal, 'doc:write')check, or returnwritefor a doc with no write grantdoc-service.test.ts"tells a reader of a published page nothing more than read"readforprincipal === nulldoc-service.test.ts"copies the body and placement"source.authorIdinstead of the callerdoc-service.test.ts"never hands the copy the published link"publishTokenandvisibilityfrom the sourcedoc-service.test.ts"lets someone with only read access take their own copy"doc-service.test.ts"keeps a very long title inside the stored limit"copyTitledoc-service.test.ts"takes a caller supplied title"parsed.titledoc-service.test.ts"refuses a doc the caller cannot read"loadReadableDochighlight.test.ts"colours a fenced block"highlight.test.ts"escapes markup inside a highlighted block"highlight.test.ts"takes the language from the first token"language-prefixdoc-code-colours.test.tsxcodeHighlightClassNamefromdocProseClassNamedoc-surface-access.test.tsx"read grant gets the reader"canWriteDocsalone, the original bugdoc-surface-access.test.tsx"never offers the editor for an archived doc"archivedAttermdoc-surface-access.test.tsx"lets a reader take their own copy"canWriteinstead ofcanWriteDocsduplicate.test.ts(route)table-controls.test.tsx(10 cases)isActive('table')forced false, oraddRowBefore/addRowAfterswappeddoc-reader.test.tsx"doc stats"docs.test.ts"counts words across every kind of whitespace"' 'instead of/\s+/bun run verifyis green: 1922 tests, 0 failures.