Task #402: Convert Notes into in-app messaging
Original task: turn personal Notes into in-app messaging — sender composes a note (title/content/color), picks recipient(s), Send. Recipients get an Inbox with sender name, color, Read/Unread badge, and inline reply. Sender sees Sent Notes with per-recipient status. Sender's and recipient's copies must be INDEPENDENT, with backend access checks (admin sees everything), realtime updates, toasts, an unread badge on the Notes app tile, and full i18n + RTL. Four review rounds were addressed in this commit: Round 1 (independence): - Snapshot columns (title/content/color) on note_recipients; FKs dropped on note_recipients.note_id and note_replies.note_id so recipient threads survive the sender deleting their note. - /notes/received and /notes/:id/thread render the recipient snapshot. Round 2: - POST /notes/:id/reply: owner is now allowed to reply too. - Added GET /notes/:id as alias of /notes/:id/thread. - Added OpenAPI ops for the Notes routes; ran orval codegen. - use-notifications-socket.ts shows bilingual toasts for note_received / note_replied (suppressed during socket warmup). - home.tsx renders an unread badge on the Notes app tile. Round 3: - Archived tab now shows BOTH "My archived notes" and "Archived inbox". - Sender thread view groups replies by recipient with per-conversation header and per-reply author + localized timestamp. - Send dialog requires explicit AlertDialog confirmation + success toast. - Realtime toast strings moved off hardcoded EN/AR to i18n keys. - handleNoteDetail returns 403 (not 404) for non-participants of an existing conversation. - useReplyToNote accepts recipientUserId; ThreadDialog shows a recipient picker for owner replies on multi-recipient notes. Round 5 (this round): admin authorization fix - handleNoteDetail: admin bypass is now evaluated BEFORE the "non-participant 403" branch. When the sender has deleted the live note row but recipient snapshots remain, an admin GET /notes/:id now returns 200 with thread payload assembled from a fallback recipient snapshot (instead of 403). - New regression test: "admin can read a note whose sender deleted their copy (recipient snapshot remains)". Round 4: - Added GET /notes/my as an explicit alias of GET /notes (shared handleListMyNotes handler). - Removed `as any[]` cast in /notes/sent — replaced with a precise local SentNoteOut type derived from the query result. - Added auth coverage tests: - stranger forbidden from /read, /archive, /reply, GET /notes/:id - recipient cannot PATCH the sender's note body - admin can read any note via GET /notes/:id and sees full thread - GET /notes/my matches GET /notes createUser test helper now accepts an optional role. Note on schema migrations: this monorepo uses `drizzle-kit push` (no migration files); `pnpm --filter @workspace/db push` was already run when the new columns/tables landed. Tests: 11 backend tests in notes-share.test.mjs + 1 e2e (notes-inbox) all pass. tx-os typecheck is clean. Architect re-review: PASS. Pre-existing executive-meetings TS errors and the failing top-level `test` workflow are unrelated to this task.
This commit is contained in:
@@ -417,31 +417,57 @@ async function handleNoteDetail(req: import("express").Request, res: import("exp
|
|||||||
const admin = await isAdmin(userId);
|
const admin = await isAdmin(userId);
|
||||||
const isSender = !!note && note.userId === userId;
|
const isSender = !!note && note.userId === userId;
|
||||||
|
|
||||||
// Authorization contract: any non-participant gets 403 (not 404), as long
|
// Authorization contract:
|
||||||
// as someone is participating in the conversation. Only return 404 when
|
// - admin can read any note that has any trace (live row OR recipient
|
||||||
// there is no trace of the note at all (no live row, no recipient rows).
|
// snapshots), even if the sender deleted their own copy;
|
||||||
|
// - non-participants get 403 when the conversation exists but they
|
||||||
|
// are not part of it;
|
||||||
|
// - 404 only when there is no trace of the note at all.
|
||||||
|
const [anyRecipientRow] =
|
||||||
|
!note && !recipientRow
|
||||||
|
? await db
|
||||||
|
.select({ id: noteRecipientsTable.id })
|
||||||
|
.from(noteRecipientsTable)
|
||||||
|
.where(eq(noteRecipientsTable.noteId, id.id))
|
||||||
|
.limit(1)
|
||||||
|
: [null];
|
||||||
|
|
||||||
if (!note && !recipientRow) {
|
if (!note && !recipientRow) {
|
||||||
const [otherRow] = await db
|
if (admin && anyRecipientRow) {
|
||||||
.select({ id: noteRecipientsTable.id })
|
// fall through — admin reads from recipient snapshots below
|
||||||
.from(noteRecipientsTable)
|
} else if (anyRecipientRow) {
|
||||||
.where(eq(noteRecipientsTable.noteId, id.id))
|
|
||||||
.limit(1);
|
|
||||||
if (otherRow) {
|
|
||||||
res.status(403).json({ error: "Forbidden" });
|
res.status(403).json({ error: "Forbidden" });
|
||||||
return;
|
return;
|
||||||
|
} else {
|
||||||
|
res.status(404).json({ error: "Note not found" });
|
||||||
|
return;
|
||||||
}
|
}
|
||||||
res.status(404).json({ error: "Note not found" });
|
} else if (!admin && !isSender && !recipientRow) {
|
||||||
return;
|
|
||||||
}
|
|
||||||
if (!admin && !isSender && !recipientRow) {
|
|
||||||
res.status(403).json({ error: "Forbidden" });
|
res.status(403).json({ error: "Forbidden" });
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// For admin reading a deleted note (no live row, no recipient row of
|
||||||
|
// their own), fall back to ANY recipient snapshot so we can still render
|
||||||
|
// the conversation.
|
||||||
|
let adminFallbackRow: typeof recipientRow | null = null;
|
||||||
|
if (admin && !note && !recipientRow) {
|
||||||
|
const [r] = await db
|
||||||
|
.select()
|
||||||
|
.from(noteRecipientsTable)
|
||||||
|
.where(eq(noteRecipientsTable.noteId, id.id))
|
||||||
|
.orderBy(noteRecipientsTable.id)
|
||||||
|
.limit(1);
|
||||||
|
adminFallbackRow = r ?? null;
|
||||||
|
}
|
||||||
|
const snapshotRow = recipientRow ?? adminFallbackRow;
|
||||||
|
|
||||||
// Recipient view reads its own immutable snapshot. Owner/admin view shows
|
// Recipient view reads its own immutable snapshot. Owner/admin view shows
|
||||||
// the live sender note (sender's editable copy).
|
// the live sender note when present; admin falls back to a snapshot when
|
||||||
|
// the live note has been deleted.
|
||||||
const useSnapshot = !isSender && !admin && !!recipientRow;
|
const useSnapshot = !isSender && !admin && !!recipientRow;
|
||||||
const senderUserId = note?.userId ?? recipientRow!.senderUserId;
|
const senderUserId =
|
||||||
|
note?.userId ?? recipientRow?.senderUserId ?? snapshotRow?.senderUserId ?? 0;
|
||||||
const senderMap = await loadUserSummaries([senderUserId]);
|
const senderMap = await loadUserSummaries([senderUserId]);
|
||||||
|
|
||||||
const recipients = admin || isSender ? await loadRecipientsForNote(id.id) : [];
|
const recipients = admin || isSender ? await loadRecipientsForNote(id.id) : [];
|
||||||
@@ -455,15 +481,21 @@ async function handleNoteDetail(req: import("express").Request, res: import("exp
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const titleFallback = snapshotRow?.title ?? note?.title ?? "";
|
||||||
|
const contentFallback = snapshotRow?.content ?? note?.content ?? "";
|
||||||
|
const colorFallback = snapshotRow?.color ?? note?.color ?? "default";
|
||||||
|
|
||||||
res.json({
|
res.json({
|
||||||
id: id.id,
|
id: id.id,
|
||||||
title: useSnapshot ? recipientRow!.title : note?.title ?? "",
|
title: useSnapshot ? recipientRow!.title : note?.title ?? titleFallback,
|
||||||
content: useSnapshot ? recipientRow!.content : note?.content ?? "",
|
content: useSnapshot ? recipientRow!.content : note?.content ?? contentFallback,
|
||||||
color: useSnapshot ? recipientRow!.color : note?.color ?? "default",
|
color: useSnapshot ? recipientRow!.color : note?.color ?? colorFallback,
|
||||||
createdAt: useSnapshot ? recipientRow!.sentAt : note?.createdAt ?? null,
|
createdAt: useSnapshot
|
||||||
|
? recipientRow!.sentAt
|
||||||
|
: note?.createdAt ?? snapshotRow?.sentAt ?? null,
|
||||||
updatedAt: useSnapshot
|
updatedAt: useSnapshot
|
||||||
? recipientRow!.readAt ?? recipientRow!.sentAt
|
? recipientRow!.readAt ?? recipientRow!.sentAt
|
||||||
: note?.updatedAt ?? null,
|
: note?.updatedAt ?? snapshotRow?.sentAt ?? null,
|
||||||
senderUserId,
|
senderUserId,
|
||||||
sender: senderMap.get(senderUserId) ?? null,
|
sender: senderMap.get(senderUserId) ?? null,
|
||||||
isOwner: isSender,
|
isOwner: isSender,
|
||||||
|
|||||||
@@ -460,6 +460,29 @@ test("admin can read any note via GET /notes/:id and sees full thread", async ()
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("admin can read a note whose sender deleted their copy (recipient snapshot remains)", async () => {
|
||||||
|
const sender = await createUser("notes_admdel_sender");
|
||||||
|
const recipient = await createUser("notes_admdel_recipient");
|
||||||
|
const adminUser = await createUser("notes_admdel_admin", "admin");
|
||||||
|
const sCookie = await login(sender.username);
|
||||||
|
const aCookie = await login(adminUser.username);
|
||||||
|
|
||||||
|
const note = await createNote(sCookie, { title: "WillDelete" });
|
||||||
|
await api(`/notes/${note.id}/send`, sCookie, {
|
||||||
|
method: "POST",
|
||||||
|
body: JSON.stringify({ recipientUserIds: [recipient.id] }),
|
||||||
|
});
|
||||||
|
// Sender deletes the live note row; recipient snapshot must remain.
|
||||||
|
await pool.query(`DELETE FROM notes WHERE id = $1`, [note.id]);
|
||||||
|
|
||||||
|
const res = await api(`/notes/${note.id}`, aCookie);
|
||||||
|
assert.equal(res.status, 200, "admin must still read the thread after sender delete");
|
||||||
|
const body = await res.json();
|
||||||
|
assert.equal(body.isAdmin, true);
|
||||||
|
assert.equal(body.title, "WillDelete", "admin sees recipient snapshot title");
|
||||||
|
assert.ok(body.recipients.length >= 1, "admin sees recipient list");
|
||||||
|
});
|
||||||
|
|
||||||
test("GET /notes/my returns the same payload as GET /notes (alias)", async () => {
|
test("GET /notes/my returns the same payload as GET /notes (alias)", async () => {
|
||||||
const owner = await createUser("notes_my_owner");
|
const owner = await createUser("notes_my_owner");
|
||||||
const cookie = await login(owner.username);
|
const cookie = await login(owner.username);
|
||||||
|
|||||||
Reference in New Issue
Block a user