notes: re-send fires popup + notification again
Previously POST /api/notes/:id/send used onConflictDoNothing on (noteId, recipientUserId) and only iterated `inserted` rows for side-effects, so a re-send to the same recipient was silent — no note_received socket event, no notification — even though the API returned 200. Reported by the user: "I sent again to rh, why didn't it reach them?" Fix: after the idempotent insert, load the existing recipient rows for any recipients that were skipped, and run the notification + socket emit loop over BOTH inserted and existing rows. The DB row is still not duplicated (snapshot semantics preserved), but the recipient now sees the popup and gets a fresh notification on every re-send. Verified: - @workspace/api-server tests/notes-share.test.mjs — 12/12 pass (including "re-sending to the same recipient does not duplicate the recipient row"). - tx-os e2e notes-popup-on-receive — 3/3 pass.
This commit is contained in:
@@ -606,8 +606,10 @@ router.get("/notes/:id", requireAuth, handleNoteDetail);
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* POST /notes/:id/send — owner sends note to recipients.
|
* POST /notes/:id/send — owner sends note to recipients.
|
||||||
* Self-send and dupes are filtered out. Existing recipients are silently
|
* Self-send and dupes are filtered out. The (noteId, recipientUserId)
|
||||||
* skipped (idempotent).
|
* unique constraint keeps a single recipient row per (note, user), but
|
||||||
|
* a re-send still re-emits the note_received socket event and creates
|
||||||
|
* a fresh notification, so the recipient sees the popup again.
|
||||||
*/
|
*/
|
||||||
router.post("/notes/:id/send", requireAuth, async (req, res): Promise<void> => {
|
router.post("/notes/:id/send", requireAuth, async (req, res): Promise<void> => {
|
||||||
const userId = req.session.userId!;
|
const userId = req.session.userId!;
|
||||||
@@ -652,7 +654,13 @@ router.post("/notes/:id/send", requireAuth, async (req, res): Promise<void> => {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Snapshot the note as of right now into each recipient row so that any
|
// Snapshot the note as of right now into each recipient row so that any
|
||||||
// future sender edit/delete leaves delivered copies untouched.
|
// future sender edit/delete leaves delivered copies untouched. The
|
||||||
|
// unique (noteId, recipientUserId) means a re-send to the same person
|
||||||
|
// does NOT duplicate the row — but we still want the popup +
|
||||||
|
// notification to fire again so the recipient sees the re-send. So:
|
||||||
|
// 1) insert with onConflictDoNothing (gives us new rows only)
|
||||||
|
// 2) load the existing rows for any recipients that were skipped
|
||||||
|
// 3) emit note_received + create a fresh notification for ALL of them
|
||||||
const inserted = await db
|
const inserted = await db
|
||||||
.insert(noteRecipientsTable)
|
.insert(noteRecipientsTable)
|
||||||
.values(
|
.values(
|
||||||
@@ -672,11 +680,27 @@ router.post("/notes/:id/send", requireAuth, async (req, res): Promise<void> => {
|
|||||||
})
|
})
|
||||||
.returning();
|
.returning();
|
||||||
|
|
||||||
|
const insertedRecipientIds = new Set(inserted.map((r) => r.recipientUserId));
|
||||||
|
const resendRecipientIds = toSend.filter((rid) => !insertedRecipientIds.has(rid));
|
||||||
|
const existingRows =
|
||||||
|
resendRecipientIds.length > 0
|
||||||
|
? await db
|
||||||
|
.select()
|
||||||
|
.from(noteRecipientsTable)
|
||||||
|
.where(
|
||||||
|
and(
|
||||||
|
eq(noteRecipientsTable.noteId, note.id),
|
||||||
|
inArray(noteRecipientsTable.recipientUserId, resendRecipientIds),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
: [];
|
||||||
|
const allRows = [...inserted, ...existingRows];
|
||||||
|
|
||||||
const senderMap = await loadUserSummaries([userId]);
|
const senderMap = await loadUserSummaries([userId]);
|
||||||
const sender = senderMap.get(userId);
|
const sender = senderMap.get(userId);
|
||||||
const senderName = sender?.displayNameEn ?? sender?.username ?? "Someone";
|
const senderName = sender?.displayNameEn ?? sender?.username ?? "Someone";
|
||||||
const senderNameAr = sender?.displayNameAr ?? senderName;
|
const senderNameAr = sender?.displayNameAr ?? senderName;
|
||||||
for (const r of inserted) {
|
for (const r of allRows) {
|
||||||
const [notif] = await db
|
const [notif] = await db
|
||||||
.insert(notificationsTable)
|
.insert(notificationsTable)
|
||||||
.values({
|
.values({
|
||||||
@@ -703,7 +727,7 @@ router.post("/notes/:id/send", requireAuth, async (req, res): Promise<void> => {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
res.json({ success: true, sent: inserted.length });
|
res.json({ success: true, sent: allRows.length });
|
||||||
});
|
});
|
||||||
|
|
||||||
// ----- Read / Archive (recipient-side) -----
|
// ----- Read / Archive (recipient-side) -----
|
||||||
|
|||||||
Reference in New Issue
Block a user