-
Notifications
You must be signed in to change notification settings - Fork 0
Add document bookmarks #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
2b00801
Add document bookmarks
tnsardesai 71d2638
Honor view tokens in bookmarks, add unbookmark, hide revoked titles
tnsardesai a5cb13f
Document /bookmarks in llms.txt and the skill
tnsardesai 20852dd
add way to test easily locally
tnsardesai 40a9a3a
Use a star icon for the doc-page bookmark toggle
tnsardesai dfcf42d
Show the bookmarked title (not the doc id) for revoked rows
tnsardesai 11eceb4
Use a bookmark icon instead of a star for the doc-page toggle
tnsardesai 6ded664
Toggle the doc-page bookmark via fetch instead of a form submit
tnsardesai 775dd8d
Only persist a bookmark's view token when it matches the doc
tnsardesai File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,201 @@ | ||
| import { getSession } from "@/lib/auth/session"; | ||
| import type { Session } from "@/lib/auth/session"; | ||
| import { sanitizeNext } from "@/lib/auth/url"; | ||
| import { canViewSession } from "@/lib/docs/access"; | ||
| import { safeEqualStr } from "@/lib/auth/tokens"; | ||
| import { accessRoleLabel, resolveAccess } from "@/lib/docs/grants"; | ||
| import { htmlResponse, manPage, esc, redirect } from "@/lib/page"; | ||
| import { query } from "@/lib/db"; | ||
| import { | ||
| listBookmarks, | ||
| saveBookmark, | ||
| removeBookmark, | ||
| type BookmarkDocRow, | ||
| findBySlug, | ||
| } from "@/lib/docs/store"; | ||
| import { bookmarkRow } from "@/lib/docs/bookmarks-view"; | ||
|
|
||
| export const dynamic = "force-dynamic"; | ||
|
|
||
| const LIST_LIMIT = 500; | ||
|
|
||
| // Rows lay out as a flex line: the man-page `<pre>` (title + dimmed tail) and a | ||
| // quiet inline `remove` form sitting on the same baseline. | ||
| const ROW_STYLE = ` | ||
| <style> | ||
| .row { display: flex; align-items: baseline; gap: 0.75rem; padding: 0.1rem 0; } | ||
| .row pre { min-width: 0; } | ||
| .row a.title { font-weight: 700; } | ||
| .row .tail { color: #888; } | ||
| .row.revoked .title { color: #999; } | ||
| .row.revoked .tail { color: #b00020; } | ||
| .row .rm { margin: 0; } | ||
| .row .remove { background: none; border: none; padding: 0; color: #888; text-decoration: underline; cursor: pointer; } | ||
| .row .remove:hover { background: none; color: #b00020; } | ||
| </style>`; | ||
|
|
||
| async function accountOwnerId(email: string): Promise<number | null> { | ||
| const { rows } = await query<{ id: number }>(`SELECT id FROM users WHERE email = $1`, [email]); | ||
| return rows[0]?.id ?? null; | ||
| } | ||
|
|
||
| /** | ||
| * Re-resolve a shared bookmark's access using the token it was saved through | ||
| * (owner/grant/public need none). Revoked → not linkable. A private doc still | ||
| * reachable only via that token is labeled "link" and keeps the token so its | ||
| * row links back with it appended. | ||
| */ | ||
| async function bookmarkAccess( | ||
| doc: BookmarkDocRow, | ||
| session: Session, | ||
| ownerId: number | null | ||
| ): Promise<{ access: string; linkable: boolean; token: string | null }> { | ||
| if (doc.deleted_at) return { access: "revoked", linkable: false, token: null }; | ||
|
|
||
| if (!(await canViewSession(doc, session, doc.bookmark_token))) { | ||
| return { access: "revoked", linkable: false, token: null }; | ||
| } | ||
|
|
||
| const resolved = await resolveAccess(doc, session.email, ownerId ?? -1); | ||
| if (resolved.kind === "owner") return { access: "owner", linkable: true, token: null }; | ||
| if (resolved.kind === "none") { | ||
| // No grant: reachable via public, or only through the stored view token. | ||
| return doc.is_public | ||
| ? { access: "public", linkable: true, token: null } | ||
| : { access: "link", linkable: true, token: doc.bookmark_token }; | ||
| } | ||
| return { access: accessRoleLabel(resolved), linkable: true, token: null }; | ||
| } | ||
|
|
||
| function emptySection(heading: string, copy: string): string { | ||
| return `<h2>${heading}</h2> | ||
| <div class="body"><pre>${copy}</pre></div>`; | ||
| } | ||
|
|
||
| function section(heading: string, rows: string[]): string { | ||
| return `<h2>${heading}</h2> | ||
| <div class="body"> | ||
| ${rows.join("\n")} | ||
| </div>`; | ||
| } | ||
|
|
||
| export async function GET(req: Request): Promise<Response> { | ||
| const session = await getSession(req); | ||
| if (!session) return redirect("/login?next=%2Fbookmarks"); | ||
|
|
||
| const email = session.email; | ||
| const ownerId = session.user_id ?? (await accountOwnerId(email)); | ||
| const bookmarks = await listBookmarks(email, LIST_LIMIT); | ||
|
|
||
| const owned: BookmarkDocRow[] = []; | ||
| const shared: BookmarkDocRow[] = []; | ||
| for (const bookmark of bookmarks) { | ||
| if (ownerId != null && bookmark.owner_id === ownerId) owned.push(bookmark); | ||
| else shared.push(bookmark); | ||
| } | ||
|
|
||
| const intro = `<div class="body"><pre>Signed in as <code>${esc(email)}</code>.\n\nBookmarks track current access. If a doc's access is later revoked it stays\nlisted with a revoked label and no link — use remove to drop it.</pre></div>`; | ||
|
|
||
| // Revoked rows show the title snapshotted at bookmark time (bookmark_title), | ||
| // never the doc's current title, which the viewer can no longer see. | ||
| const ownedRows = owned.map((doc) => { | ||
| const revoked = doc.deleted_at != null; | ||
| return bookmarkRow({ | ||
| docId: doc.id, | ||
| slug: doc.slug, | ||
| title: revoked ? doc.bookmark_title : doc.title, | ||
| access: revoked ? "revoked" : "owner", | ||
| isPublic: doc.is_public, | ||
| bookmarkedAt: doc.bookmarked_at, | ||
| linkable: !revoked, | ||
| token: null, | ||
| }); | ||
| }); | ||
|
|
||
| const sharedRows: string[] = []; | ||
| for (const doc of shared) { | ||
| const a = await bookmarkAccess(doc, session, ownerId); | ||
| sharedRows.push( | ||
| bookmarkRow({ | ||
| docId: doc.id, | ||
| slug: doc.slug, | ||
| title: a.linkable ? doc.title : doc.bookmark_title, | ||
| access: a.access, | ||
| isPublic: doc.is_public, | ||
| bookmarkedAt: doc.bookmarked_at, | ||
| linkable: a.linkable, | ||
| token: a.token, | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
| const body = [ | ||
| ROW_STYLE, | ||
| intro, | ||
| owned.length | ||
| ? section("YOUR DOCUMENTS", ownedRows) | ||
| : emptySection( | ||
| "YOUR DOCUMENTS", | ||
| "You haven't bookmarked any of your own documents yet. Use the bookmark\nbutton on a doc to save it here." | ||
| ), | ||
| shared.length | ||
| ? section("SHARED WITH YOU", sharedRows) | ||
| : emptySection( | ||
| "SHARED WITH YOU", | ||
| "Nothing shared with you is bookmarked yet. Use the bookmark button on a\ndoc to save it here." | ||
| ), | ||
| ].join("\n"); | ||
|
|
||
| return htmlResponse( | ||
| manPage({ | ||
| title: "justhtml.sh — bookmarks", | ||
| bodyHtml: body, | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
| // Content-negotiated: the doc viewer's bookmark button posts via fetch with | ||
| // `Accept: application/json` and gets a bare status back (it updates the icon | ||
| // optimistically, no navigation). The zero-JS forms on the /bookmarks list | ||
| // (per-row remove) omit that header and get the usual 303 back to `next`. | ||
| export async function POST(req: Request): Promise<Response> { | ||
| const wantsJson = (req.headers.get("accept") ?? "").includes("application/json"); | ||
| const status = (code: number) => new Response(null, { status: code }); | ||
|
|
||
| const session = await getSession(req); | ||
| if (!session) return wantsJson ? status(401) : redirect("/login?next=%2Fbookmarks"); | ||
|
|
||
| const form = await req.formData(); | ||
| const action = String(form.get("action") ?? "add").trim(); | ||
| const slug = String(form.get("slug") ?? "").trim(); | ||
| const next = sanitizeNext( | ||
| String(form.get("next") ?? (slug ? `/d/${encodeURIComponent(slug)}` : "/bookmarks")) | ||
| ); | ||
| const done = () => (wantsJson ? status(204) : redirect(next)); | ||
|
|
||
| // Remove is keyed by doc id so a revoked/deleted doc (whose slug no longer | ||
| // resolves) can still be dropped. No access check: you can only ever remove | ||
| // your own bookmark. | ||
| if (action === "remove") { | ||
| const docId = Number(form.get("doc_id")); | ||
| if (Number.isInteger(docId)) await removeBookmark(session.email, docId); | ||
| return done(); | ||
| } | ||
|
|
||
| const viewtoken = String(form.get("viewtoken") ?? "").trim() || null; | ||
| if (!slug) return wantsJson ? status(400) : redirect("/bookmarks"); | ||
|
|
||
| const doc = await findBySlug(slug); | ||
| if (!doc || !(await canViewSession(doc, session, viewtoken))) { | ||
| return wantsJson ? status(403) : redirect(next); | ||
| } | ||
|
|
||
| // Persist the token only when it actually matches this doc. Access may have | ||
| // come from ownership, a grant, or the doc being public, in which case the | ||
| // submitted token is irrelevant — storing it would gate the later re-check on | ||
| // a token that was never the basis for access (and could overwrite a good | ||
| // one on upsert). | ||
| const tokenToStore = viewtoken && safeEqualStr(viewtoken, doc.view_token) ? viewtoken : null; | ||
| await saveBookmark(session.email, doc.id, tokenToStore, doc.title); | ||
| return done(); | ||
| } |
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bookmark icon stale after navigation
Medium Severity
The doc viewer keeps bookmark UI in
useState(props.bookmarked)without resetting when the server sends a newbookmarkedvalue for another document. Client navigations that reuseCommentsShellcan show the previous doc’s filled or empty bookmark icon and wrongaria-pressedlabels on the current page.Reviewed by Cursor Bugbot for commit 6ded664. Configure here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Valid as a general React pattern, but not a reachable bug in this app, so leaving as-is.
So the trigger the check describes ("client navigations that reuse CommentsShell") does not exist. If soft routing between docs is ever introduced, the correct fix is
key={slug}on<CommentsShell>so all prop-seeded state resets together — a bookmark-only useEffect would be the wrong shape.