rafraa/TX
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

notes(#454): per-recipient view/edit folder sharing

Browse Source 
- Schema: noteFolderShares.permission ('view'|'edit', default 'view').
- Server: resolveFolderEditAccess + emitFolderChanged helpers.
  - POST/PATCH/DELETE/checklist now allow folder editors.
  - Editors stamp notes with folder owner's userId; labels validated
    against owner; PATCH editors blocked from unfile (folderId=null)
    and cross-folder moves to non-matching owners.
  - emitFolderChanged fires for owner AND editor mutations across
    create / patch (old + new folder) / delete / checklist toggle.
  - PUT /shares accepts both legacy recipientUserIds and new
    recipients:[{userId,permission}]; diffs add/update/remove/perm-flip.
  - GET /shares, /shared-with-me, /shared-notes return permission /
    myPermission / readOnly.
- Client:
  - notes-api types: FolderSharePermission, myPermission on
    SharedFolder/SharedFolderView, permission on FolderShareRecipient,
    readOnly:boolean on SharedFolderNote.
  - useUpdateFolderShares takes recipients[].
  - useCreate/Update/DeleteNote also invalidate ['note-folders'] so
    actor's shared-folder rail badge stays fresh.
  - FolderShareDialog: per-row checkbox + segmented View/Edit toggle.
  - SharedFolderView: editor mode mounts Composer + NoteCard with
    full edit/delete/archive affordances; Send hidden in editor
    context (Composer + NoteCard hideSend prop) since /send is
    owner-only.
  - folders-rail: per-folder permission badge.
  - socket: note-folder-shared also invalidates ['notes'] +
    ['note-folders'] for fanout to owner / other editors.
  - Locales: shareDescriptionPerm, permissionView/Edit, canEdit
    (en + ar).
- Pre-existing executive-meetings.ts type errors are out of scope.
This commit is contained in:
riyadhafraa
2026-05-10 10:16:05 +00:00
parent e52c27f377
commit 1aad708cb7
8 changed files with 528 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files
artifacts/api-server/src/routes/notes.ts
+289 -53
View File
@@ -117,6 +117,58 @@ async function userOwnsFolder(userId: number, folderId: number): Promise<boolean
return !!row;
}
// Folder-level access helpers (Task #454).
//
// `userCanEditFolder` returns true if the caller is the owner OR has a share
// row with permission="edit" for the folder. Used by every write path that
// must accept folder editors as well as the folder owner (POST/PATCH/DELETE
// /notes inside the folder, checklist toggle, etc.). Returns the resolved
// owner id so callers can stamp newly-created notes with the folder owner's
// userId — an editor's note still belongs to the folder owner so the folder
// remains a single user_id namespace and shows up to every other recipient.
async function resolveFolderEditAccess(
userId: number,
folderId: number,
): Promise<{ ok: true; ownerId: number } | { ok: false }> {
const [folder] = await db
.select({ ownerId: noteFoldersTable.userId })
.from(noteFoldersTable)
.where(eq(noteFoldersTable.id, folderId));
if (!folder) return { ok: false };
if (folder.ownerId === userId) return { ok: true, ownerId: folder.ownerId };
const [share] = await db
.select({ permission: noteFolderSharesTable.permission })
.from(noteFolderSharesTable)
.where(
and(
eq(noteFolderSharesTable.folderId, folderId),
eq(noteFolderSharesTable.recipientUserId, userId),
),
);
if (share && share.permission === "edit") return { ok: true, ownerId: folder.ownerId };
return { ok: false };
}
// Notify the entire folder audience (owner + all current recipients except
// the actor) that a folder's note set changed. Drives live refresh of the
// shared-folder view AND the rail "Shared with me" note count.
async function emitFolderChanged(folderId: number, actorUserId: number) {
const [folder] = await db
.select({ ownerId: noteFoldersTable.userId })
.from(noteFoldersTable)
.where(eq(noteFoldersTable.id, folderId));
if (!folder) return;
const recipients = await db
.select({ uid: noteFolderSharesTable.recipientUserId })
.from(noteFolderSharesTable)
.where(eq(noteFolderSharesTable.folderId, folderId));
const audience = new Set<number>([folder.ownerId, ...recipients.map((r) => r.uid)]);
audience.delete(actorUserId);
for (const uid of audience) {
void emitToUser(uid, "note-folder-shared", { folderId });
}
}
function parseNoteInput(body: any, isCreate: boolean): { ok: true; data: NoteInput } | { ok: false; error: string } {
if (!body || typeof body !== "object") return { ok: false, error: "Invalid body" };
const data: NoteInput = {};
@@ -315,7 +367,10 @@ async function handleListMyNotes(
return;
}
const [share] = await db
.select({ folderId: noteFolderSharesTable.folderId })
.select({
folderId: noteFolderSharesTable.folderId,
permission: noteFolderSharesTable.permission,
})
.from(noteFolderSharesTable)
.where(
and(
@@ -335,10 +390,11 @@ async function handleListMyNotes(
res.status(404).json({ error: "Folder not found" });
return;
}
const readOnly = share.permission !== "edit";
const ownerNotes = await loadNotesWithLabels(folder.ownerId, false);
const filtered = ownerNotes
.filter((n) => n.folderId === parsed)
.map((n) => ({ ...n, readOnly: true as const }));
.map((n) => ({ ...n, readOnly }));
res.json(filtered);
return;
}
@@ -360,14 +416,24 @@ router.post("/notes", requireAuth, async (req, res): Promise<void> => {
return;
}
const { labelIds = [], folderId, ...data } = parsed.data;
if (folderId != null && !(await userOwnsFolder(userId, folderId))) {
res.status(400).json({ error: "Invalid folderId" });
return;
// Resolve who the new note's owner should be. The caller is allowed to
// create notes either in their own folders OR in a folder that's been
// shared with them with permission="edit"; in the editor case the note
// is stamped with the folder owner's userId so the folder remains a
// single user_id namespace and shows up to every other recipient.
let stampedUserId = userId;
if (folderId != null) {
const access = await resolveFolderEditAccess(userId, folderId);
if (!access.ok) {
res.status(400).json({ error: "Invalid folderId" });
return;
}
stampedUserId = access.ownerId;
}
const [note] = await db
.insert(notesTable)
.values({
userId,
userId: stampedUserId,
title: data.title ?? "",
content: data.content ?? "",
color: data.color ?? "default",
@@ -378,8 +444,20 @@ router.post("/notes", requireAuth, async (req, res): Promise<void> => {
folderId: folderId ?? null,
})
.returning();
if (labelIds.length > 0) await setNoteLabels(note.id, userId, labelIds);
const [withLabels] = await loadOne(note.id, userId);
// Labels are scoped to the note OWNER (stampedUserId), not necessarily
// the caller — when an editor creates a note in someone else's folder
// the note belongs to the folder owner, so labelIds are validated
// against that owner's label set.
if (labelIds.length > 0) await setNoteLabels(note.id, stampedUserId, labelIds);
const [withLabels] = await loadOne(note.id, stampedUserId);
// Notify every viewer of the shared folder (owner + recipients) so the
// shared-folder view and rail counts refresh live. Fires for both the
// editor branch and the owner-creating-in-their-own-shared-folder
// branch — `emitFolderChanged` is a no-op when the folder isn't
// actually shared with anyone.
if (folderId != null) {
void emitFolderChanged(folderId, userId);
}
res.status(201).json(withLabels);
});
@@ -397,8 +475,9 @@ router.patch("/notes/:id", requireAuth, async (req, res): Promise<void> => {
}
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.
// ambiguous 404). Recipients of a shared folder may now also mutate
// the owner's notes when their share grants permission="edit"
// (Task #454).
const [existing] = await db
.select()
.from(notesTable)
@@ -407,13 +486,38 @@ router.patch("/notes/:id", requireAuth, async (req, res): Promise<void> => {
res.status(404).json({ error: "Note not found" });
return;
}
let actingAsEditor = false;
if (existing.userId !== userId) {
res.status(403).json({ error: "Forbidden" });
return;
if (
existing.folderId == null ||
!(await resolveFolderEditAccess(userId, existing.folderId)).ok
) {
res.status(403).json({ error: "Forbidden" });
return;
}
actingAsEditor = true;
}
if (data.folderId != null && !(await userOwnsFolder(userId, data.folderId))) {
res.status(400).json({ error: "Invalid folderId" });
return;
// Folder-change validation. We must guard BOTH branches:
// - data.folderId === null → caller is unfiling the note. Editors
// don't own the note, so unfiling would yank it out of the shared
// folder and effectively orphan it from the owner's organization
// scheme; only the owner may unfile.
// - data.folderId !== null → caller is moving it into a folder.
// Owners can pick any of their own folders; editors are limited to
// a folder they have edit permission on AND that's owned by the
// same owner (i.e. effectively the same shared folder).
if (data.folderId !== undefined) {
if (actingAsEditor && data.folderId === null) {
res.status(403).json({ error: "Forbidden" });
return;
}
if (data.folderId !== null) {
const access = await resolveFolderEditAccess(userId, data.folderId);
if (!access.ok || access.ownerId !== existing.userId) {
res.status(400).json({ error: "Invalid folderId" });
return;
}
}
}
// If the caller patched `items` without specifying `kind`, normalize
// against the note's current kind so a text note can never end up
@@ -429,8 +533,18 @@ router.patch("/notes/:id", requireAuth, async (req, res): Promise<void> => {
if (Object.keys(data).length > 0) {
await db.update(notesTable).set(data).where(eq(notesTable.id, existing.id));
}
if (labelIds) await setNoteLabels(existing.id, userId, labelIds);
const [withLabels] = await loadOne(existing.id, userId);
if (labelIds) await setNoteLabels(existing.id, existing.userId, labelIds);
const [withLabels] = await loadOne(existing.id, existing.userId);
// Live-refresh shared-folder viewers regardless of whether the caller
// was the owner or an editor. If the move crossed folders, both the
// source AND destination need a ping so each side's note list updates.
const oldFolderId = existing.folderId;
const newFolderId =
data.folderId !== undefined ? data.folderId : existing.folderId;
if (oldFolderId != null) void emitFolderChanged(oldFolderId, userId);
if (newFolderId != null && newFolderId !== oldFolderId) {
void emitFolderChanged(newFolderId, userId);
}
res.json(withLabels);
});
@@ -441,21 +555,36 @@ router.delete("/notes/:id", requireAuth, async (req, res): Promise<void> => {
res.status(400).json({ error: id.error });
return;
}
// Two-step lookup so non-owners (e.g. shared-folder recipients) get
// a precise 403 instead of a 404 — see Task #445 acceptance criteria.
// Two-step lookup so non-owners (e.g. shared-folder view-only recipients)
// get a precise 403 instead of a 404 — see Task #445 acceptance criteria.
// Editors with permission="edit" on the note's folder are allowed
// (Task #454).
const [existing] = await db
.select({ userId: notesTable.userId })
.select({ userId: notesTable.userId, folderId: notesTable.folderId })
.from(notesTable)
.where(eq(notesTable.id, id.id));
if (!existing) {
res.status(404).json({ error: "Note not found" });
return;
}
let actingAsEditor = false;
if (existing.userId !== userId) {
res.status(403).json({ error: "Forbidden" });
return;
if (
existing.folderId == null ||
!(await resolveFolderEditAccess(userId, existing.folderId)).ok
) {
res.status(403).json({ error: "Forbidden" });
return;
}
actingAsEditor = true;
}
await db.delete(notesTable).where(eq(notesTable.id, id.id));
// Owner OR editor delete — every shared-folder viewer needs a refresh
// so the deleted note disappears from their cached note list and the
// rail count drops.
if (existing.folderId != null) {
void emitFolderChanged(existing.folderId, userId);
}
res.json({ success: true });
});
@@ -854,7 +983,13 @@ router.post(
// the same `items` array, modify their copy, and have the slower
// writer overwrite the faster one.
type ToggleOutcome =
| { kind: "ok"; updated: ChecklistItem[]; ownerUserId: number; recipientIds: number[] }
| {
kind: "ok";
updated: ChecklistItem[];
ownerUserId: number;
recipientIds: number[];
folderId: number | null;
}
| { kind: "noop"; items: ChecklistItem[] }
| { kind: "err"; status: number; error: string };
const outcome: ToggleOutcome = await db.transaction(async (tx) => {
@@ -863,6 +998,7 @@ router.post(
);
const lockedRows = (locked as unknown as { rows: Array<{
id: number; user_id: number; kind: string; items: unknown;
folder_id: number | null;
}> }).rows;
const note = lockedRows[0];
if (!note) return { kind: "err", status: 404, error: "Note not found" };
@@ -880,7 +1016,26 @@ router.post(
eq(noteRecipientsTable.recipientUserId, userId),
),
);
if (!recipientRow || recipientRow.status === "archived")
const sentToMe = !!recipientRow && recipientRow.status !== "archived";
// Task #454: also accept folder editors on a shared folder. We
// can't call resolveFolderEditAccess here (different db handle),
// so re-implement the lookup against `tx` for transactional read
// consistency.
let folderEditor = false;
const folderId = note.folder_id;
if (!sentToMe && folderId != null) {
const [share] = await tx
.select({ permission: noteFolderSharesTable.permission })
.from(noteFolderSharesTable)
.where(
and(
eq(noteFolderSharesTable.folderId, folderId),
eq(noteFolderSharesTable.recipientUserId, userId),
),
);
if (share && share.permission === "edit") folderEditor = true;
}
if (!sentToMe && !folderEditor)
return { kind: "err", status: 403, error: "Forbidden" };
}
@@ -911,6 +1066,7 @@ router.post(
updated,
ownerUserId: note.user_id,
recipientIds: recipientRows.map((r) => r.recipientUserId),
folderId: note.folder_id,
};
});
@@ -940,6 +1096,15 @@ router.post(
actorUserId: userId,
});
}
// Task #454: also notify shared-folder viewers (owner + every other
// editor / viewer of the folder) so the shared-folder note list
// refetches and shows the toggled item — the per-recipient
// `note_checklist_changed` audience above only covers users in the
// note's send-recipients table, which is disjoint from folder-share
// recipients.
if (outcome.folderId != null) {
void emitFolderChanged(outcome.folderId, userId);
}
res.json({ success: true, items: updated });
},
@@ -1348,6 +1513,7 @@ router.get(
name: noteFoldersTable.name,
ownerId: noteFoldersTable.userId,
sharedAt: noteFolderSharesTable.sharedAt,
myPermission: noteFolderSharesTable.permission,
ownerUsername: usersTable.username,
ownerDisplayNameAr: usersTable.displayNameAr,
ownerDisplayNameEn: usersTable.displayNameEn,
@@ -1394,6 +1560,7 @@ router.get(
displayNameAr: usersTable.displayNameAr,
displayNameEn: usersTable.displayNameEn,
sharedAt: noteFolderSharesTable.sharedAt,
permission: noteFolderSharesTable.permission,
})
.from(noteFolderSharesTable)
.innerJoin(usersTable, eq(noteFolderSharesTable.recipientUserId, usersTable.id))
@@ -1418,54 +1585,98 @@ router.put(
return;
}
const body = req.body;
if (!body || typeof body !== "object" || !Array.isArray(body.recipientUserIds)) {
res.status(400).json({ error: "Invalid recipientUserIds" });
// Task #454: accept BOTH the legacy `recipientUserIds: number[]`
// shape (treated as permission="view") AND the new
// `recipients: [{ userId, permission }]` shape so older clients keep
// working. Internally we collapse to a single Map<userId, permission>.
const desired = new Map<number, "view" | "edit">();
const ingest = (rawId: unknown, rawPerm: unknown): string | null => {
const n = Number(rawId);
if (!Number.isInteger(n) || n <= 0) return "Invalid recipient id";
if (n === userId) return "Cannot share folder with yourself";
let perm: "view" | "edit" = "view";
if (rawPerm !== undefined) {
if (rawPerm !== "view" && rawPerm !== "edit") return "Invalid permission";
perm = rawPerm;
}
desired.set(n, perm);
return null;
};
if (!body || typeof body !== "object") {
res.status(400).json({ error: "Invalid body" });
return;
}
const desired = new Set<number>();
for (const x of body.recipientUserIds) {
const n = Number(x);
if (!Number.isInteger(n) || n <= 0) {
res.status(400).json({ error: "Invalid recipientUserIds" });
return;
if (Array.isArray(body.recipients)) {
for (const r of body.recipients) {
if (!r || typeof r !== "object") {
res.status(400).json({ error: "Invalid recipients" });
return;
}
const err = ingest((r as any).userId, (r as any).permission);
if (err) {
res.status(400).json({ error: err });
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;
} else if (Array.isArray(body.recipientUserIds)) {
for (const x of body.recipientUserIds) {
const err = ingest(x, undefined);
if (err) {
res.status(400).json({ error: err });
return;
}
}
desired.add(n);
} else {
res.status(400).json({ error: "Invalid recipients" });
return;
}
// Validate that every desired recipient actually exists.
// Drop any desired ids that don't actually exist as users (silently
// skip — same behaviour as before for unknown ids in the legacy path).
if (desired.size > 0) {
const valid = await db
.select({ id: usersTable.id })
.from(usersTable)
.where(inArray(usersTable.id, Array.from(desired)));
.where(inArray(usersTable.id, Array.from(desired.keys())));
const validIds = new Set(valid.map((r) => r.id));
for (const id of Array.from(desired)) {
if (!validIds.has(id)) desired.delete(id);
for (const uid of Array.from(desired.keys())) {
if (!validIds.has(uid)) desired.delete(uid);
}
}
// Diff current vs desired and apply incrementally so repeated PUTs are
// idempotent and we only emit notifications for genuinely new recipients.
// idempotent and we only emit notifications for genuinely new
// recipients OR permission changes.
const current = await db
.select({ recipientUserId: noteFolderSharesTable.recipientUserId })
.select({
recipientUserId: noteFolderSharesTable.recipientUserId,
permission: noteFolderSharesTable.permission,
})
.from(noteFolderSharesTable)
.where(eq(noteFolderSharesTable.folderId, id.id));
const currentSet = new Set(current.map((r) => r.recipientUserId));
const currentMap = new Map(
current.map((r) => [r.recipientUserId, r.permission as "view" | "edit"]),
);
const toAdd: number[] = [];
const toAdd: Array<{ userId: number; permission: "view" | "edit" }> = [];
const toRemove: number[] = [];
for (const r of desired) if (!currentSet.has(r)) toAdd.push(r);
for (const r of currentSet) if (!desired.has(r)) toRemove.push(r);
const toUpdate: Array<{ userId: number; permission: "view" | "edit" }> = [];
for (const [uid, perm] of desired) {
const cur = currentMap.get(uid);
if (cur === undefined) toAdd.push({ userId: uid, permission: perm });
else if (cur !== perm) toUpdate.push({ userId: uid, permission: perm });
}
for (const uid of currentMap.keys()) if (!desired.has(uid)) toRemove.push(uid);
if (toAdd.length > 0) {
await db
.insert(noteFolderSharesTable)
.values(toAdd.map((rid) => ({ folderId: id.id, recipientUserId: rid })))
.values(
toAdd.map((a) => ({
folderId: id.id,
recipientUserId: a.userId,
permission: a.permission,
})),
)
.onConflictDoNothing();
}
if (toRemove.length > 0) {
@@ -1478,10 +1689,28 @@ router.put(
),
);
}
for (const u of toUpdate) {
await db
.update(noteFolderSharesTable)
.set({ permission: u.permission })
.where(
and(
eq(noteFolderSharesTable.folderId, id.id),
eq(noteFolderSharesTable.recipientUserId, u.userId),
),
);
}
// Emit live socket events so recipients' rails refresh without polling.
for (const rid of toAdd) {
void emitToUser(rid, "note-folder-shared", { folderId: id.id });
// Permission flips reuse the `note-folder-shared` event — recipients
// simply refetch shared-with-me / shared-notes and pick up the new
// myPermission, which is enough to flip their UI between view / edit
// affordances without a dedicated event type.
for (const a of toAdd) {
void emitToUser(a.userId, "note-folder-shared", { folderId: id.id });
}
for (const u of toUpdate) {
void emitToUser(u.userId, "note-folder-shared", { folderId: id.id });
}
for (const rid of toRemove) {
void emitToUser(rid, "note-folder-unshared", { folderId: id.id });
@@ -1494,6 +1723,7 @@ router.put(
displayNameAr: usersTable.displayNameAr,
displayNameEn: usersTable.displayNameEn,
sharedAt: noteFolderSharesTable.sharedAt,
permission: noteFolderSharesTable.permission,
})
.from(noteFolderSharesTable)
.innerJoin(usersTable, eq(noteFolderSharesTable.recipientUserId, usersTable.id))
@@ -1550,7 +1780,10 @@ router.get(
return;
}
const [share] = await db
.select({ folderId: noteFolderSharesTable.folderId })
.select({
folderId: noteFolderSharesTable.folderId,
permission: noteFolderSharesTable.permission,
})
.from(noteFolderSharesTable)
.where(
and(
@@ -1562,6 +1795,8 @@ router.get(
res.status(403).json({ error: "Forbidden" });
return;
}
const myPermission = share.permission === "edit" ? "edit" : "view";
const readOnly = myPermission !== "edit";
const [folder] = await db
.select()
.from(noteFoldersTable)
@@ -1611,7 +1846,8 @@ router.get(
res.json({
folder: { id: folder.id, name: folder.name, ownerId: folder.userId },
owner: owner ?? null,
notes: withLabels.map((n) => ({ ...n, readOnly: true as const })),
myPermission,
notes: withLabels.map((n) => ({ ...n, readOnly })),
});
},
);