diff --git a/artifacts/api-server/src/routes/notes.ts b/artifacts/api-server/src/routes/notes.ts index 2396faf7..2d9f99e4 100644 --- a/artifacts/api-server/src/routes/notes.ts +++ b/artifacts/api-server/src/routes/notes.ts @@ -303,6 +303,45 @@ async function handleListMyNotes( res: import("express").Response, ): Promise { const userId = req.session.userId!; + // ?sharedFolderId=:id — read-only access to the live notes inside a + // folder shared WITH the caller. Spec alias for the same data the + // dedicated `/note-folders/:id/shared-notes` endpoint returns; kept + // here so clients that prefer the unified `/notes?…` shape work too. + const sharedFolderIdRaw = req.query.sharedFolderId; + if (typeof sharedFolderIdRaw === "string" && sharedFolderIdRaw.length > 0) { + const parsed = Number(sharedFolderIdRaw); + if (!Number.isInteger(parsed) || parsed <= 0) { + res.status(400).json({ error: "Invalid sharedFolderId" }); + return; + } + const [share] = await db + .select({ folderId: noteFolderSharesTable.folderId }) + .from(noteFolderSharesTable) + .where( + and( + eq(noteFolderSharesTable.folderId, parsed), + eq(noteFolderSharesTable.recipientUserId, userId), + ), + ); + if (!share) { + res.status(403).json({ error: "Folder is not shared with you" }); + return; + } + const [folder] = await db + .select({ ownerId: noteFoldersTable.userId }) + .from(noteFoldersTable) + .where(eq(noteFoldersTable.id, parsed)); + if (!folder) { + res.status(404).json({ error: "Folder not found" }); + return; + } + const ownerNotes = await loadNotesWithLabels(folder.ownerId, false); + const filtered = ownerNotes + .filter((n) => n.folderId === parsed) + .map((n) => ({ ...n, readOnly: true as const })); + res.json(filtered); + return; + } const archived = req.query.archived === "true"; const notes = await loadNotesWithLabels(userId, archived); res.json(notes); @@ -357,14 +396,21 @@ router.patch("/notes/:id", requireAuth, async (req, res): Promise => { return; } const { labelIds, ...data } = parsed.data; + // Two-step lookup so non-owners get a precise 403 (instead of an + // ambiguous 404). Recipients of a shared folder must never be able to + // mutate the owner's notes — see Task #445 acceptance criteria. const [existing] = await db .select() .from(notesTable) - .where(and(eq(notesTable.id, id.id), eq(notesTable.userId, userId))); + .where(eq(notesTable.id, id.id)); if (!existing) { res.status(404).json({ error: "Note not found" }); return; } + if (existing.userId !== userId) { + res.status(403).json({ error: "Forbidden" }); + return; + } if (data.folderId != null && !(await userOwnsFolder(userId, data.folderId))) { res.status(400).json({ error: "Invalid folderId" }); return; @@ -395,14 +441,21 @@ router.delete("/notes/:id", requireAuth, async (req, res): Promise => { res.status(400).json({ error: id.error }); return; } - const result = await db - .delete(notesTable) - .where(and(eq(notesTable.id, id.id), eq(notesTable.userId, userId))) - .returning(); - if (result.length === 0) { + // Two-step lookup so non-owners (e.g. shared-folder recipients) get + // a precise 403 instead of a 404 — see Task #445 acceptance criteria. + const [existing] = await db + .select({ userId: notesTable.userId }) + .from(notesTable) + .where(eq(notesTable.id, id.id)); + if (!existing) { res.status(404).json({ error: "Note not found" }); return; } + if (existing.userId !== userId) { + res.status(403).json({ error: "Forbidden" }); + return; + } + await db.delete(notesTable).where(eq(notesTable.id, id.id)); res.json({ success: true }); }); @@ -1278,9 +1331,11 @@ router.delete("/note-folders/:id", requireAuth, async (req, res): Promise // // A folder owner can share a folder read-only with other users. Recipients see // the owner's CURRENT notes inside the folder (live, not snapshot). They cannot -// edit, add, move, or delete anything; the existing per-note routes already -// filter by `notesTable.userId = req.session.userId`, so any write attempt by a -// recipient just 404s — no extra gating required there. +// edit, add, move, or delete anything; the per-note write routes +// (`PATCH/DELETE /notes/:id`, archive, checklist toggle for non-recipients) +// look up the row by id and explicitly return 403 when the caller is not +// the owner, so a shared-folder recipient that probes a write endpoint +// gets a clear authorization error instead of an ambiguous 404. router.get( "/note-folders/shared-with-me", @@ -1296,6 +1351,15 @@ router.get( ownerUsername: usersTable.username, ownerDisplayNameAr: usersTable.displayNameAr, ownerDisplayNameEn: usersTable.displayNameEn, + // Live count of the owner's non-archived notes in this folder so + // the recipient's rail can show "Folder name (12)" without a + // second roundtrip per row. + noteCount: sql`( + SELECT COUNT(*)::int FROM ${notesTable} + WHERE ${notesTable.folderId} = ${noteFoldersTable.id} + AND ${notesTable.userId} = ${noteFoldersTable.userId} + AND ${notesTable.isArchived} = false + )`.as("note_count"), }) .from(noteFolderSharesTable) .innerJoin( @@ -1361,7 +1425,17 @@ router.put( const desired = new Set(); for (const x of body.recipientUserIds) { const n = Number(x); - if (Number.isInteger(n) && n > 0 && n !== userId) desired.add(n); + if (!Number.isInteger(n) || n <= 0) { + res.status(400).json({ error: "Invalid recipientUserIds" }); + return; + } + // Self-share is meaningless and the spec calls for an explicit + // 400 rather than silently dropping the id. + if (n === userId) { + res.status(400).json({ error: "Cannot share folder with yourself" }); + return; + } + desired.add(n); } // Validate that every desired recipient actually exists. if (desired.size > 0) { diff --git a/artifacts/tx-os/src/components/notes/folders-rail.tsx b/artifacts/tx-os/src/components/notes/folders-rail.tsx index aeb71a02..7c4f9762 100644 --- a/artifacts/tx-os/src/components/notes/folders-rail.tsx +++ b/artifacts/tx-os/src/components/notes/folders-rail.tsx @@ -583,6 +583,14 @@ export function FoldersRail({ })} + + {sf.noteCount} + ); diff --git a/artifacts/tx-os/src/lib/notes-api.ts b/artifacts/tx-os/src/lib/notes-api.ts index 2c49e9c9..a7527997 100644 --- a/artifacts/tx-os/src/lib/notes-api.ts +++ b/artifacts/tx-os/src/lib/notes-api.ts @@ -58,6 +58,9 @@ export interface SharedFolder { ownerUsername: string; ownerDisplayNameAr: string | null; ownerDisplayNameEn: string | null; + // Live count of the owner's non-archived notes inside this folder. Lets + // the rail render "Folder name (12)" without a per-row roundtrip. + noteCount: number; } // One row in a folder's share list (people the owner has shared TO). diff --git a/artifacts/tx-os/src/pages/notes.tsx b/artifacts/tx-os/src/pages/notes.tsx index e5235318..22162ba4 100644 --- a/artifacts/tx-os/src/pages/notes.tsx +++ b/artifacts/tx-os/src/pages/notes.tsx @@ -478,6 +478,7 @@ export default function NotesPage() { setFolderSelection({ kind: "all" })} /> )} @@ -2964,16 +2965,35 @@ function LabelsDialog({ function SharedFolderView({ folderId, labels: _labels, + search, onBack, }: { folderId: number; labels: NoteLabel[]; + search: string; onBack: () => void; }) { const { t, i18n } = useTranslation(); const lang = i18n.language; const { data, isLoading, error } = useSharedFolderNotes(folderId); + // Recipients use the same top-bar search input as their personal notes; + // filter the read-only list client-side over title + content + checklist + // item text so the experience matches the owner's view. + const visibleNotes = useMemo(() => { + if (!data) return []; + const q = search.trim().toLowerCase(); + if (!q) return data.notes; + return data.notes.filter((n) => { + if (n.title?.toLowerCase().includes(q)) return true; + if (n.content?.toLowerCase().includes(q)) return true; + if (n.kind === "checklist" && Array.isArray(n.items)) { + return n.items.some((it) => it.text?.toLowerCase().includes(q)); + } + return false; + }); + }, [data, search]); + if (isLoading) return ; // 403/404 — the owner unshared the folder while we were viewing it. Show a @@ -3030,10 +3050,14 @@ function SharedFolderView({ {t("notes.folders.readOnly", "Read-only")} - {data.notes.length === 0 ? ( + {visibleNotes.length === 0 ? ( } - text={t("notes.folders.sharedEmpty", "This folder has no notes yet")} + text={ + search.trim() + ? t("notes.noMatches", "No notes match your search") + : t("notes.folders.sharedEmpty", "This folder has no notes yet") + } /> ) : (
- {data.notes.map((n) => ( + {visibleNotes.map((n) => (