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. Two prior code-review rounds were addressed in this commit: Round 1 (independence): - Added immutable snapshot columns (title/content/color) on note_recipients. - Dropped the FK from 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 for recipients (sender/admin still see the live note). Round 2 (validation REJECT fixes): - POST /notes/:id/reply: owner is now allowed to reply too. Owner replies do not change recipient status; recipient replies still flip to "replied" and clear archivedAt. - Added GET /notes/:id as an alias of /notes/:id/thread (shared handler). - Added OpenAPI ops for /notes/sent, /notes/received, /notes/{id}, /notes/{id}/send, /notes/{id}/read, /notes/{id}/archive, /notes/{id}/reply, and ran orval codegen. - use-notifications-socket.ts now shows bilingual toasts for note_received and note_replied (suppressed during the socket warmup window). - home.tsx renders an unread badge on the Notes app tile, fed by useReceivedNotes(false) filtered to status === "unread"; refreshes automatically on socket invalidation. Tests: 7 backend tests in notes-share.test.mjs (independence, archived-reply, owner-reply preserves recipient status, GET /notes/:id alias, etc.) + the notes-inbox e2e all pass. tx-os typecheck is clean. Pre-existing executive-meetings TS errors and the failing top-level `test` workflow are unrelated to this task.
2026-05-05 15:01:18 +00:00
parent 6352cbf844
commit 65c5a172d6
8 changed files with 1102 additions and 37 deletions
@@ -380,7 +380,11 @@ router.get("/notes/received", requireAuth, async (req, res): Promise<void> => {
 * to what this user is allowed to see (admin/sender = all; recipient = only
 * their own thread with the sender).
 */
-router.get("/notes/:id/thread", requireAuth, async (req, res): Promise<void> => {
+/**
+ * GET /notes/:id — public alias of /notes/:id/thread per the task spec.
+ * Returns the same payload (note + recipients + replies, scoped to caller).
+ */
+async function handleNoteDetail(req: import("express").Request, res: import("express").Response): Promise<void> {
  const userId = req.session.userId!;
  const id = parseIdParam(req.params.id);
  if (!id.ok) {
@@ -444,7 +448,10 @@ router.get("/notes/:id/thread", requireAuth, async (req, res): Promise<void> =>
    recipients,
    replies,
  });
-});
+}
+
+router.get("/notes/:id/thread", requireAuth, handleNoteDetail);
+router.get("/notes/:id", requireAuth, handleNoteDetail);

 // ----- Send -----

@@ -621,7 +628,14 @@ router.post("/notes/:id/reply", requireAuth, async (req, res): Promise<void> =>
    res.status(400).json({ error: "Invalid content" });
    return;
  }
-  const [row] = await db
+  // The note owner is allowed to reply too (per task spec). Look up the
+  // current note (may be null if owner deleted it) AND any recipient row for
+  // the caller; resolve role from whichever exists.
+  const [liveNote] = await db
+    .select()
+    .from(notesTable)
+    .where(eq(notesTable.id, id.id));
+  const [callerRecipientRow] = await db
    .select()
    .from(noteRecipientsTable)
    .where(
@@ -630,33 +644,72 @@ router.post("/notes/:id/reply", requireAuth, async (req, res): Promise<void> =>
        eq(noteRecipientsTable.recipientUserId, userId),
      ),
    );
-  if (!row) {
+  const isOwner = !!liveNote && liveNote.userId === userId;
+  if (!isOwner && !callerRecipientRow) {
    res.status(403).json({ error: "Forbidden" });
    return;
  }
-  const ownerUserId = row.senderUserId;
+
+  // Resolve the conversation's owner (the original sender) and the "other
+  // party" this specific reply is addressed to. For a recipient reply that's
+  // the owner. For an owner reply we need a target recipient — body may pass
+  // `recipientUserId` to scope; if absent and there is exactly one recipient
+  // row, default to it; otherwise reject.
+  let ownerUserId: number;
+  let otherPartyUserId: number;
+  let targetRow: typeof callerRecipientRow | null = null;
+  if (isOwner) {
+    ownerUserId = userId;
+    const requestedTarget =
+      typeof req.body?.recipientUserId === "number"
+        ? req.body.recipientUserId
+        : null;
+    const allRows = await db
+      .select()
+      .from(noteRecipientsTable)
+      .where(eq(noteRecipientsTable.noteId, id.id));
+    if (requestedTarget !== null) {
+      targetRow = allRows.find((r) => r.recipientUserId === requestedTarget) ?? null;
+    } else if (allRows.length === 1) {
+      targetRow = allRows[0];
+    }
+    if (!targetRow) {
+      res.status(400).json({ error: "recipientUserId required" });
+      return;
+    }
+    otherPartyUserId = targetRow.recipientUserId;
+  } else {
+    targetRow = callerRecipientRow!;
+    ownerUserId = callerRecipientRow!.senderUserId;
+    otherPartyUserId = ownerUserId;
+  }
+
  const [reply] = await db
    .insert(noteRepliesTable)
    .values({
      noteId: id.id,
      senderUserId: userId,
-      recipientUserId: ownerUserId,
+      recipientUserId: otherPartyUserId,
      content,
    })
    .returning();

-  // Replying re-activates an archived recipient row: clear archivedAt and
-  // bump status to "replied" so the conversation continues cleanly.
-  await db
-    .update(noteRecipientsTable)
-    .set({
-      status: "replied",
-      readAt: row.readAt ?? new Date(),
-      archivedAt: null,
-    })
-    .where(eq(noteRecipientsTable.id, row.id));
+  // Recipient replies flip the recipient row to "replied" and clear archivedAt
+  // so the conversation re-surfaces. Owner replies do NOT change recipient
+  // status (per task spec) — the recipient sees a new reply but their own
+  // read/unread/replied state is preserved.
+  if (!isOwner) {
+    await db
+      .update(noteRecipientsTable)
+      .set({
+        status: "replied",
+        readAt: targetRow.readAt ?? new Date(),
+        archivedAt: null,
+      })
+      .where(eq(noteRecipientsTable.id, targetRow.id));
+  }

-  // Notify the original sender (note owner).
+  // Notify the other party (owner-on-recipient-reply, or recipient-on-owner-reply).
  const senderMap = await loadUserSummaries([userId]);
  const replier = senderMap.get(userId);
  const replierName = replier?.displayNameEn ?? replier?.username ?? "Someone";
@@ -664,18 +717,18 @@ router.post("/notes/:id/reply", requireAuth, async (req, res): Promise<void> =>
  const [notif] = await db
    .insert(notificationsTable)
    .values({
-      userId: ownerUserId,
-      titleAr: "رد على ملاحظتك",
-      titleEn: "Reply to your note",
-      bodyAr: `${replierNameAr} رد على ملاحظتك`,
-      bodyEn: `${replierName} replied to your note`,
+      userId: otherPartyUserId,
+      titleAr: isOwner ? "رد جديد على ملاحظة" : "رد على ملاحظتك",
+      titleEn: isOwner ? "New reply on a note" : "Reply to your note",
+      bodyAr: `${replierNameAr} رد على ملاحظة`,
+      bodyEn: `${replierName} replied on a note`,
      type: "note",
    })
    .returning();
-  await emitToUser(ownerUserId, "notification_created", { ...notif, type: "note" });
-  await emitToUser(ownerUserId, "note_replied", {
+  await emitToUser(otherPartyUserId, "notification_created", { ...notif, type: "note" });
+  await emitToUser(otherPartyUserId, "note_replied", {
    noteId: id.id,
-    recipientUserId: userId,
+    recipientUserId: isOwner ? otherPartyUserId : userId,
    replyId: reply.id,
  });

@@ -318,6 +318,64 @@ test("reply on an archived recipient row clears archivedAt and bumps to replied"
  assert.equal(rows[0].archived_at, null, "archivedAt must be cleared on reply");
 });

+test("owner can reply on their own note; recipient status is preserved", async () => {
+  const sender = await createUser("notes_oreply_sender");
+  const recipient = await createUser("notes_oreply_recipient");
+  const sCookie = await login(sender.username);
+  const rCookie = await login(recipient.username);
+
+  const note = await createNote(sCookie);
+  await api(`/notes/${note.id}/send`, sCookie, {
+    method: "POST",
+    body: JSON.stringify({ recipientUserIds: [recipient.id] }),
+  });
+
+  // Recipient leaves the note unread, then the owner replies. Owner reply
+  // must succeed (not 403) and must NOT flip the recipient's status.
+  const ownerReply = await api(`/notes/${note.id}/reply`, sCookie, {
+    method: "POST",
+    body: JSON.stringify({ content: "owner says hi" }),
+  });
+  assert.equal(
+    ownerReply.status,
+    201,
+    `owner reply expected 201, got ${ownerReply.status}`,
+  );
+
+  const { rows } = await pool.query(
+    `SELECT status FROM note_recipients
+     WHERE note_id = $1 AND recipient_user_id = $2`,
+    [note.id, recipient.id],
+  );
+  assert.equal(rows[0].status, "unread", "owner reply must not flip recipient status");
+
+  // Recipient sees the owner reply in their thread.
+  const thread = await (await api(`/notes/${note.id}/thread`, rCookie)).json();
+  assert.ok(
+    thread.replies.some((r) => r.content === "owner says hi"),
+    "recipient must see the owner's reply in the thread",
+  );
+});
+
+test("GET /notes/:id is the same payload as /notes/:id/thread", async () => {
+  const sender = await createUser("notes_alias_sender");
+  const recipient = await createUser("notes_alias_recipient");
+  const sCookie = await login(sender.username);
+  const rCookie = await login(recipient.username);
+  const note = await createNote(sCookie);
+  await api(`/notes/${note.id}/send`, sCookie, {
+    method: "POST",
+    body: JSON.stringify({ recipientUserIds: [recipient.id] }),
+  });
+  const a = await (await api(`/notes/${note.id}`, rCookie)).json();
+  const b = await (await api(`/notes/${note.id}/thread`, rCookie)).json();
+  assert.equal(a.id, b.id);
+  assert.equal(a.title, b.title);
+  assert.equal(a.content, b.content);
+  assert.equal(a.color, b.color);
+  assert.equal(a.myStatus, b.myStatus);
+});
+
 test("re-sending to the same recipient does not duplicate the recipient row", async () => {
  const sender = await createUser("notes_dup_sender");
  const recipient = await createUser("notes_dup_recipient");