From f998219e3f1fec7dfaf6ea4b67074d4dae455537 Mon Sep 17 00:00:00 2001 From: olathedev Date: Sun, 23 Aug 2026 05:55:28 +0100 Subject: [PATCH 1/7] fix(notifications): stop dual-writing new notifications onto the user document Notification creation wrote the same payload to both the Notification collection and the embedded User.notifications array. The second write was non-atomic (a failed User.save left an orphaned collection document) and it grew the user document without bound. The POST handler now writes only to the Notification collection and keeps a cheap User.exists check so an unknown recipient still returns 404. Refs #200 --- app/api/notifications/route.ts | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/app/api/notifications/route.ts b/app/api/notifications/route.ts index eb23d503..ce1fa7be 100644 --- a/app/api/notifications/route.ts +++ b/app/api/notifications/route.ts @@ -53,8 +53,11 @@ export async function POST(request: Request) { await dbConnect() - const targetUser = await User.findById(body.data.userId).select("notifications") - if (!targetUser) { + // Existence check only: the Notification collection is the single source of + // truth for notification content and read state, so nothing is written back + // onto the user document here. + const recipientExists = await User.exists({ _id: body.data.userId }) + if (!recipientExists) { return NextResponse.json({ error: "User not found" }, { status: 404 }) } @@ -69,17 +72,6 @@ export async function POST(request: Request) { link: body.data.actionUrl, }) - targetUser.notifications = Array.isArray(targetUser.notifications) ? targetUser.notifications : [] - targetUser.notifications.push({ - id: notification._id.toString(), - title: body.data.title, - message: body.data.message, - read: false, - timestamp: new Date(), - link: body.data.actionUrl, - }) - await targetUser.save() - await logAuditEvent({ actor: authContext.user, action: "notification.create", From db57dc01b5ae7bb874858d59c1c7dbb65882f39a Mon Sep 17 00:00:00 2001 From: olathedev Date: Sun, 23 Aug 2026 05:56:00 +0100 Subject: [PATCH 2/7] fix(notifications): read unread state from the Notification collection only Auth responses shipped the embedded User.notifications array and the driver KYC status page derived its unread badge from it, so a mark-read against the Notification collection left users staring at a stale count until the embedded copy happened to change. - /api/auth/me no longer selects or returns the embedded array, and AuthUser drops the field. - The KYC status header stops passing a fallback count; ActivityUnreadBell already fetches the live count from /api/activity and updates on the activity-count-changed event. - markNotificationsAsRead now updates the Notification collection, scoped by userId, and revalidates the activity route the driver notifications page redirects to. Refs #200 --- actions/notification.ts | 25 ++++++++++-------------- app/api/auth/me/route.ts | 5 ++--- app/dashboard/driver/kyc/status/page.tsx | 1 - hooks/use-auth.ts | 8 -------- 4 files changed, 12 insertions(+), 27 deletions(-) diff --git a/actions/notification.ts b/actions/notification.ts index e7d54eb1..f5c59ea6 100644 --- a/actions/notification.ts +++ b/actions/notification.ts @@ -1,32 +1,27 @@ "use server" -import User from "@/models/User" +import Notification from "@/models/Notification" import dbConnect from "@/lib/dbConnect" import { revalidatePath } from "next/cache" +const OBJECT_ID_PATTERN = /^[a-f\d]{24}$/i + export async function markNotificationsAsRead(userId: string, notificationIds: string[]) { try { await dbConnect() - const user = await User.findById(userId) - - if (!user) { - return { success: false, message: "User not found." } + // The Notification collection is the single source of truth for read state, + // so this scopes the update by userId instead of loading the user document. + const ids = notificationIds.filter((id) => OBJECT_ID_PATTERN.test(id)) + if (ids.length === 0) { + return { success: false, message: "No valid notification ids supplied." } } - // Mark specified notifications as read - user.notifications = user.notifications.map((notif: any) => { - if (notificationIds.includes(notif.id)) { - return { ...notif, read: true } - } - return notif - }) - - await user.save() + await Notification.updateMany({ _id: { $in: ids }, userId }, { $set: { read: true } }) // Revalidate paths to reflect changes in UI revalidatePath(`/dashboard/driver`) - revalidatePath(`/dashboard/driver/notifications`) + revalidatePath(`/dashboard/driver/activity`) revalidatePath(`/dashboard/driver/kyc/status`) // In case notification count is shown here return { success: true, message: "Notifications marked as read." } diff --git a/app/api/auth/me/route.ts b/app/api/auth/me/route.ts index 42890ebe..a2fe7273 100644 --- a/app/api/auth/me/route.ts +++ b/app/api/auth/me/route.ts @@ -15,7 +15,6 @@ function toAuthResponse(user: any) { physicalMeetingStatus: user.physicalMeetingStatus || "none", isKycVerified: user.isKycVerified === true, kycVerified: user.kycVerified === true, - notifications: Array.isArray(user.notifications) ? user.notifications : [], } } @@ -26,7 +25,7 @@ export async function GET(request: Request) { const session = await getSessionFromCookies() if (session?.userId) { const user = await User.findById(session.userId).select( - `${USER_PROFILE_SELECT} kycStatus kycDocuments kycRejectionReason physicalMeetingDate physicalMeetingStatus isKycVerified kycVerified notifications`, + `${USER_PROFILE_SELECT} kycStatus kycDocuments kycRejectionReason physicalMeetingDate physicalMeetingStatus isKycVerified kycVerified`, ) if (!user) { @@ -48,7 +47,7 @@ export async function GET(request: Request) { const user = await User.findOne({ $or: [{ privyUserId: profile.privyUserId }, ...(profile.email ? [{ email: profile.email.toLowerCase() }] : [])], }).select( - `${USER_PROFILE_SELECT} kycStatus kycDocuments kycRejectionReason physicalMeetingDate physicalMeetingStatus isKycVerified kycVerified notifications`, + `${USER_PROFILE_SELECT} kycStatus kycDocuments kycRejectionReason physicalMeetingDate physicalMeetingStatus isKycVerified kycVerified`, ) if (!user) { diff --git a/app/dashboard/driver/kyc/status/page.tsx b/app/dashboard/driver/kyc/status/page.tsx index 72633894..e28a2b98 100644 --- a/app/dashboard/driver/kyc/status/page.tsx +++ b/app/dashboard/driver/kyc/status/page.tsx @@ -229,7 +229,6 @@ export default function KycStatusPage() {
!n.read).length || 0} // Display unread notifications className="md:pl-6 lg:pl-8" />
diff --git a/hooks/use-auth.ts b/hooks/use-auth.ts index de7c88e0..d4ebea6f 100644 --- a/hooks/use-auth.ts +++ b/hooks/use-auth.ts @@ -3,14 +3,6 @@ import { useState, useEffect, useCallback } from "react" export interface AuthUser { - notifications?: Array<{ - id: string - title: string - message: string - read: boolean - timestamp: string - link?: string - }> id: string name?: string fullName?: string From 3e6ae2796e43e6031b2454a91da683b419ec6f0d Mon Sep 17 00:00:00 2001 From: olathedev Date: Sun, 23 Aug 2026 06:09:55 +0100 Subject: [PATCH 3/7] feat(notifications): retire the embedded User.notifications array The embedded array is now deprecated: it has no default and is never selected, so no new user document grows one and no code path can read a stale copy by accident. Legacy documents that still carry it are retired by an explicit backfill. - lib/notifications/embedded-migration.ts copies each embedded entry into the Notification collection and then unsets the array. Target ids are derived deterministically (the ObjectId the dual-write path stored, otherwise a hash of owner/title/message/timestamp), so a re-run after a partial failure resumes without duplicating feed entries. A user whose entries did not all migrate keeps its array for the next run. - Read state is merged, not overwritten: an embedded read:true promotes the collection document to read, because the old server action recorded reads only on the embedded copy. The reverse is never applied. - scripts/migrate-embedded-notifications.ts runs it, with --dry-run, wired up as `notifications:migrate-embedded`. - Account anonymization now deletes the user's Notification documents; clearing the embedded array no longer erases anything. Refs #200 --- lib/notifications/embedded-migration.ts | Bin 0 -> 5614 bytes models/User.ts | 8 ++++- package.json | 1 + scripts/migrate-embedded-notifications.ts | 35 ++++++++++++++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 lib/notifications/embedded-migration.ts create mode 100644 scripts/migrate-embedded-notifications.ts diff --git a/lib/notifications/embedded-migration.ts b/lib/notifications/embedded-migration.ts new file mode 100644 index 0000000000000000000000000000000000000000..fe37ceee5837dd8558a4bcf076b60ef3e93b7325 GIT binary patch literal 5614 zcmb7I?QR>#742_6#Wf)Wq%AECBk2#Js_Q5Usv?mBN%^I!=4f{)jby>i{x&Pk?W`Lex9iSaZ5Ae1wQ;svy26QNnDytn4ZjBMpDyGvh@QQC zIii>Jy~^(wrYr;1OHGBYyvF&uAicWPMS(AK-jLaz=rAYcz1onmwc~w!P+vaft}I~= z)ZC}_(&S4rQ9v;EuGAD<<8w`JLA7t{B`ut%z63rnced8HCS?nP9}5aK6x(NcmKqy$ z9gpl3&0V$0u#-1m!I+h*Du`pzg|4+<8EZmqa!40rSLV(~S7Jp_3r(=b>jU3d5o+~E z?fI~xsY>>`4z$qh8e4m#!(ozjKqqkkrn;bMZi+bzRhG_gKfn#A1ugX_=e;KvU5YK;`pD0s{}awbdRTH+2ceuXG6NPK$49gI~2; zRr~BsML{E61@sOGit=|UIPo;myl{E5g6G-@U34L3U#&FuAkMHGh=skD9K4@gb=Wxy zP{c4P!4@3k?{&dK`rru>F3y%4xKFkYFcMX^J%ls7Aj3C$&WBaVRL9YVGJTJ?CPbX5 zA`_T-h6%zhaDfl2O4~sH{m<`k6ra|~o<}hYRR%q$Tv;d$ARh?0)xZRi+;H+DCWNHP z1sH`1D-!}7RWH43?v^xDm6@?OKAKvgy$FrgJF8R2GbV|rQDRCUvYVF;N<=R+I#22VM?*I6$ zNIxJ-QMm=hSr9&w9$*A<%j`Gb(2FyPeP+U$I*ZFPq(7fKnNFw3#k%p9S z5r4B;OQqheLl^6gM>_`#EvO1Wu8YfRUgjX6q4tjZo}|4 z#wNp2tI3oOX*}*_R3|gq*Jp~;V#365irkE6?MqGC9PZ>5V63NAj4OO~1b)357{dj3 zlbM2}%K&=DZ5i0Qo?tOZDcvKaZzb zQx(9)q-Vr5E6g36KSq;*6#>19p1j_tH{S|G62+^qu{m|Ii_MSTMfYI>9bDwY-GS>0 zLd%**^zgVpLOg##=`=zAW!Jggu2@F?D*3F|%GA`gy8S?TsW$&dgH)2BKo`huS$ZAg zCAF$6N@;LVbGnkWYqDREcFlL?C{4D-==z4mBT0o<<^zFqZhGcHbC-Nbu_chc1kT`+ zkEZWd0|@_a5LxgXf{3u9591=AfGlPK!qJ7lBJVQ5qL@4kS_xFe2inu0(E8#l_c;5s zXF>GWH@K8MM#9XZCX?~2ftG?sqKpe!j0`vGlbA;=o@qNzs4ld!2W!q%t(%_(tL^Wf4 z@rQZ#MKljp`9-T&$}?V(Vm)wh%7qkjUWK@$s5R(cqeA+Ico(vw_!`=7Xp5hAZK^~F#wrd&HZ7ti5N+kP7@>B}y(_wB5 zQ1gvlxbf3Dz6RepkBUAH8&K2g*$Pxvhu)Z1ly3cOn!8c((9R0ab5m{t8(U~_nNB5n zYX(R7sY+d8m193!b%)yq?j8JiphhAX$+8~@X(FO7463*cG37Yu;@CB|?nCQq!SzUZ zztNI#Y~Jt)r0dUoHwIzJ6<;7OFCnq>RJ6U7@?AxhyaG=FO!&HJvzfJy@WlI0x?cSO zhysDTK)Np!3b26-_y#Q;+V>cXMfCw*xeCQaoSR&E`tfLUn%A>yw@PMs-ol$#=iXhU zvn|lQl%?p^UF)BdR0=^qmA-OgOKV`+8twl?ihn%2RyAQcne%Ge zdx$ywc!;ku-M@I9@Ld)O^%3;nzH7;n@#DO8FJEy|M 0) { + process.stderr.write(`${result.errors.length} user(s) failed to migrate; re-run to retry.\n`) + process.exit(1) + } +} + +main() + .then(() => process.exit(0)) + .catch((error) => { + process.stderr.write(`${error instanceof Error ? error.message : "Embedded notification migration failed"}\n`) + process.exit(1) + }) From b34bed8327bb8e2b3752eedb9461ee21d3ea1b31 Mon Sep 17 00:00:00 2001 From: olathedev Date: Sun, 23 Aug 2026 06:15:46 +0100 Subject: [PATCH 4/7] test(notifications): cover the single-store read state and legacy migration - POST /api/notifications writes only the collection, still 404s an unknown recipient, and leaves the user document untouched across 200 creations. - /api/activity mark-all-read and set-read agree with the next GET's unread count, and set-read stays scoped to the caller. - /api/auth/me neither projects nor returns the embedded array, including for a not-yet-migrated user whose document still carries one. - The backfill maps ids deterministically, merges legacy read state in one direction only, skips unusable rows, keeps the array after a partial write failure and finishes on re-run without duplicating, and handles a 500-entry legacy user. Refs #200 --- __tests__/api/activity/route.test.ts | 135 ++++++++++++ __tests__/api/auth/me.test.ts | 84 ++++++++ __tests__/api/notifications/route.test.ts | 110 ++++++++++ .../notifications/embedded-migration.test.ts | 198 ++++++++++++++++++ 4 files changed, 527 insertions(+) create mode 100644 __tests__/api/activity/route.test.ts create mode 100644 __tests__/api/auth/me.test.ts create mode 100644 __tests__/api/notifications/route.test.ts create mode 100644 __tests__/lib/notifications/embedded-migration.test.ts diff --git a/__tests__/api/activity/route.test.ts b/__tests__/api/activity/route.test.ts new file mode 100644 index 00000000..c8727408 --- /dev/null +++ b/__tests__/api/activity/route.test.ts @@ -0,0 +1,135 @@ +import { beforeEach, describe, expect, it, vi } from "vitest" + +const { requireAuthenticatedUser, finalizeAuthenticatedResponse, find, countDocuments, updateMany, updateOne } = vi.hoisted( + () => ({ + requireAuthenticatedUser: vi.fn(), + finalizeAuthenticatedResponse: vi.fn(async (response: unknown) => response), + find: vi.fn(), + countDocuments: vi.fn(), + updateMany: vi.fn(), + updateOne: vi.fn(), + }), +) + +vi.mock("@/lib/api/route-guard", () => ({ requireAuthenticatedUser, finalizeAuthenticatedResponse })) +vi.mock("@/lib/dbConnect", () => ({ default: vi.fn() })) +vi.mock("@/models/Notification", () => ({ default: { find, countDocuments, updateMany, updateOne } })) + +import { GET, PATCH } from "@/app/api/activity/route" + +/** Route handlers are typed as possibly returning nothing; fail loudly instead. */ +function expectResponse(response: Response | undefined): Response { + if (!response) throw new Error("route handler returned no response") + return response +} + +const USER_ID = "507f1f77bcf86cd799439011" +const ACTIVITY_ID = "507f1f77bcf86cd799439021" + +/** + * A tiny stand-in for the Notification collection so read state has one place + * to live in these tests, exactly as it does in production. + */ +function createStore(seed: Array<{ _id: string; read: boolean }>) { + const rows = seed.map((row) => ({ + ...row, + userId: USER_ID, + title: "Notice", + message: "Body", + type: "info", + category: "system" as const, + priority: "low" as const, + timestamp: new Date("2026-01-01T00:00:00.000Z"), + })) + + find.mockImplementation(() => ({ + sort: () => ({ limit: () => ({ lean: async () => rows }) }), + })) + countDocuments.mockImplementation(async () => rows.filter((row) => !row.read).length) + updateMany.mockImplementation(async (_filter: unknown, update: { $set: { read: boolean } }) => { + for (const row of rows) row.read = update.$set.read + return { modifiedCount: rows.length } + }) + updateOne.mockImplementation(async (filter: { _id: string }, update: { $set: { read: boolean } }) => { + const row = rows.find((candidate) => candidate._id === filter._id) + if (!row) return { matchedCount: 0, modifiedCount: 0 } + row.read = update.$set.read + return { matchedCount: 1, modifiedCount: 1 } + }) + + return rows +} + +function patchRequest(body: Record) { + return new Request("http://localhost/api/activity", { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }) +} + +describe("/api/activity read state", () => { + beforeEach(() => { + vi.clearAllMocks() + requireAuthenticatedUser.mockResolvedValue({ + user: { _id: { toString: () => USER_ID }, role: "driver", name: "Driver" }, + }) + }) + + it("reports the unread count from the notification collection", async () => { + createStore([ + { _id: ACTIVITY_ID, read: false }, + { _id: "507f1f77bcf86cd799439022", read: true }, + { _id: "507f1f77bcf86cd799439023", read: false }, + ]) + + const response = expectResponse(await GET(new Request("http://localhost/api/activity"))) + + expect(response.status).toBe(200) + await expect(response.json()).resolves.toMatchObject({ unreadCount: 2 }) + }) + + it("returns a zero unread count immediately after mark-all-read", async () => { + createStore([ + { _id: ACTIVITY_ID, read: false }, + { _id: "507f1f77bcf86cd799439022", read: false }, + ]) + + const patched = expectResponse(await PATCH(patchRequest({ action: "mark-all-read" }))) + await expect(patched.json()).resolves.toMatchObject({ success: true, unreadCount: 0 }) + + // The next read agrees with the mutation response — there is no second + // store left holding a stale count. + const refetched = expectResponse(await GET(new Request("http://localhost/api/activity"))) + await expect(refetched.json()).resolves.toMatchObject({ unreadCount: 0 }) + }) + + it("returns the decremented count after a single set-read", async () => { + createStore([ + { _id: ACTIVITY_ID, read: false }, + { _id: "507f1f77bcf86cd799439022", read: false }, + ]) + + const patched = expectResponse( + await PATCH(patchRequest({ action: "set-read", activityId: ACTIVITY_ID, read: true })), + ) + await expect(patched.json()).resolves.toMatchObject({ success: true, unreadCount: 1 }) + + const refetched = expectResponse(await GET(new Request("http://localhost/api/activity"))) + await expect(refetched.json()).resolves.toMatchObject({ unreadCount: 1 }) + }) + + it("scopes set-read to the caller and 404s on someone else's activity", async () => { + createStore([{ _id: ACTIVITY_ID, read: false }]) + + const response = expectResponse( + await PATCH(patchRequest({ action: "set-read", activityId: "507f1f77bcf86cd799439099", read: true })), + ) + + expect(response.status).toBe(404) + expect(updateOne).toHaveBeenCalledWith( + expect.objectContaining({ userId: USER_ID }), + expect.objectContaining({ $set: { read: true } }), + ) + }) +}) diff --git a/__tests__/api/auth/me.test.ts b/__tests__/api/auth/me.test.ts new file mode 100644 index 00000000..a2a3ad3c --- /dev/null +++ b/__tests__/api/auth/me.test.ts @@ -0,0 +1,84 @@ +import { beforeEach, describe, expect, it, vi } from "vitest" + +const { findById, getSessionFromCookies, extractPrivyTokenFromRequest } = vi.hoisted(() => ({ + findById: vi.fn(), + getSessionFromCookies: vi.fn(), + extractPrivyTokenFromRequest: vi.fn(() => null), +})) + +vi.mock("@/lib/dbConnect", () => ({ default: vi.fn() })) +vi.mock("@/models/User", () => ({ default: { findById, findOne: vi.fn() } })) +vi.mock("@/lib/auth/session", () => ({ + getSessionFromCookies, + setSessionCookie: vi.fn(), + signSessionToken: vi.fn(async () => "token"), +})) +vi.mock("@/lib/auth/privy", () => ({ + extractPrivyTokenFromRequest, + getPrivyProfileFromPayload: vi.fn(), + verifyPrivyToken: vi.fn(), +})) + +import { GET } from "@/app/api/auth/me/route" + +const USER_ID = "507f1f77bcf86cd799439011" + +let selectArgument = "" + +function seedUser(overrides: Record = {}) { + const user = { + _id: { toString: () => USER_ID }, + name: "Driver", + fullName: "Driver One", + email: "driver@example.com", + role: "driver", + kycStatus: "approved", + isKycVerified: true, + ...overrides, + } + + findById.mockImplementation(() => ({ + select: (fields: string) => { + selectArgument = fields + return Promise.resolve(user) + }, + })) + + return user +} + +describe("GET /api/auth/me", () => { + beforeEach(() => { + vi.clearAllMocks() + selectArgument = "" + getSessionFromCookies.mockResolvedValue({ userId: USER_ID }) + }) + + it("does not project the deprecated embedded notifications array", async () => { + seedUser() + + await GET(new Request("http://localhost/api/auth/me")) + + expect(selectArgument).not.toContain("notifications") + }) + + it("omits notifications from the auth payload entirely", async () => { + seedUser() + + const response = await GET(new Request("http://localhost/api/auth/me")) + const payload = await response.json() + + expect(response.status).toBe(200) + expect(payload).toMatchObject({ id: USER_ID, role: "driver" }) + expect(payload).not.toHaveProperty("notifications") + }) + + it("never leaks a stale embedded array even when the document still carries one", async () => { + // A user who has not yet been migrated still has the legacy field on disk. + seedUser({ notifications: [{ id: "legacy", title: "Old", message: "Old", read: false }] }) + + const payload = await (await GET(new Request("http://localhost/api/auth/me"))).json() + + expect(payload).not.toHaveProperty("notifications") + }) +}) diff --git a/__tests__/api/notifications/route.test.ts b/__tests__/api/notifications/route.test.ts new file mode 100644 index 00000000..cd9e7522 --- /dev/null +++ b/__tests__/api/notifications/route.test.ts @@ -0,0 +1,110 @@ +import { NextResponse } from "next/server" +import { beforeEach, describe, expect, it, vi } from "vitest" + +const { requireAuthenticatedUser, finalizeAuthenticatedResponse, logAuditEvent, notificationCreate, userExists, userFindById } = + vi.hoisted(() => ({ + requireAuthenticatedUser: vi.fn(), + finalizeAuthenticatedResponse: vi.fn(async (response: unknown) => response), + logAuditEvent: vi.fn(async () => undefined), + notificationCreate: vi.fn(), + userExists: vi.fn(), + userFindById: vi.fn(), + })) + +vi.mock("@/lib/api/route-guard", () => ({ requireAuthenticatedUser, finalizeAuthenticatedResponse })) +vi.mock("@/lib/dbConnect", () => ({ default: vi.fn() })) +vi.mock("@/lib/security/audit-log", () => ({ logAuditEvent })) +vi.mock("@/lib/security/rate-limit", () => ({ + buildRateLimitKey: (...segments: unknown[]) => segments.join(":"), + consumeRateLimit: () => ({ allowed: true, remaining: 100, resetAt: Date.now() + 1000 }), + getClientIpAddress: () => "127.0.0.1", + rateLimitExceededResponse: () => new Response(JSON.stringify({ error: "Too many requests" }), { status: 429 }), +})) +vi.mock("@/models/Notification", () => ({ default: { create: notificationCreate, find: vi.fn() } })) + +// Only `exists` is provided: any attempt to load or mutate the user document +// during notification creation fails loudly instead of silently dual-writing. +vi.mock("@/models/User", () => ({ default: { exists: userExists, findById: userFindById } })) + +import { POST } from "@/app/api/notifications/route" + +/** Route handlers are typed as possibly returning nothing; fail loudly instead. */ +function expectResponse(response: Response | undefined): Response { + if (!response) throw new Error("route handler returned no response") + return response +} + +const RECIPIENT_ID = "507f1f77bcf86cd799439011" +const ADMIN_ID = "507f1f77bcf86cd799439012" + +function buildRequest(body: Record = {}) { + return new Request("http://localhost/api/notifications", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + userId: RECIPIENT_ID, + title: "Repayment received", + message: "Your repayment has been confirmed.", + ...body, + }), + }) +} + +describe("POST /api/notifications", () => { + beforeEach(() => { + vi.clearAllMocks() + requireAuthenticatedUser.mockResolvedValue({ + user: { _id: { toString: () => ADMIN_ID }, role: "admin", name: "Admin" }, + }) + userExists.mockResolvedValue({ _id: RECIPIENT_ID }) + notificationCreate.mockImplementation(async (doc: Record) => ({ + ...doc, + _id: { toString: () => "507f1f77bcf86cd799439013" }, + })) + }) + + it("writes the notification only to the Notification collection", async () => { + const response = expectResponse(await POST(buildRequest())) + + expect(response.status).toBe(200) + expect(notificationCreate).toHaveBeenCalledTimes(1) + expect(notificationCreate.mock.calls[0][0]).toMatchObject({ userId: RECIPIENT_ID, title: "Repayment received" }) + + // The old dual-write loaded the user document to append an embedded copy. + expect(userFindById).not.toHaveBeenCalled() + }) + + it("still rejects an unknown recipient", async () => { + userExists.mockResolvedValue(null) + + const response = expectResponse(await POST(buildRequest())) + + expect(response.status).toBe(404) + expect(notificationCreate).not.toHaveBeenCalled() + }) + + it("keeps the user document untouched across a high volume of notifications", async () => { + for (let index = 0; index < 200; index++) { + const response = expectResponse(await POST(buildRequest({ title: `Notice ${index}` }))) + expect(response.status).toBe(200) + } + + expect(notificationCreate).toHaveBeenCalledTimes(200) + // Nothing accumulated on the user: the document was never loaded or saved, + // so it cannot grow without bound. + expect(userFindById).not.toHaveBeenCalled() + expect(userExists).toHaveBeenCalledTimes(200) + }) + + it("refuses non-admin callers before touching either store", async () => { + requireAuthenticatedUser.mockResolvedValue({ + response: NextResponse.json({ message: "Admin access required" }, { status: 403 }), + }) + + const response = expectResponse(await POST(buildRequest())) + + expect(response.status).toBe(403) + expect(notificationCreate).not.toHaveBeenCalled() + expect(userExists).not.toHaveBeenCalled() + }) +}) diff --git a/__tests__/lib/notifications/embedded-migration.test.ts b/__tests__/lib/notifications/embedded-migration.test.ts new file mode 100644 index 00000000..781b6a52 --- /dev/null +++ b/__tests__/lib/notifications/embedded-migration.test.ts @@ -0,0 +1,198 @@ +import { beforeEach, describe, expect, it, vi } from "vitest" + +const { userFind, userUpdateOne, notificationUpdateOne } = vi.hoisted(() => ({ + userFind: vi.fn(), + userUpdateOne: vi.fn(), + notificationUpdateOne: vi.fn(), +})) + +vi.mock("@/lib/dbConnect", () => ({ default: vi.fn() })) +vi.mock("@/models/User", () => ({ default: { find: userFind, updateOne: userUpdateOne } })) +vi.mock("@/models/Notification", () => ({ default: { updateOne: notificationUpdateOne } })) + +import { deriveNotificationId, migrateEmbeddedNotifications } from "@/lib/notifications/embedded-migration" + +const USER_ID = "507f1f77bcf86cd799439011" +const EMBEDDED_ID = "507f1f77bcf86cd799439021" + +interface FakeUser { + _id: { toString: () => string } + notifications?: Array> +} + +/** + * Minimal in-memory stand-ins for the two collections, so the migration's + * upsert/merge/unset behaviour is exercised end to end without a live Mongo. + */ +function createWorld(users: Array<{ id: string; notifications: Array> }>) { + const store = new Map>() + const failingIds = new Set() + const documents: FakeUser[] = users.map((user) => ({ + _id: { toString: () => user.id }, + notifications: user.notifications, + })) + + userFind.mockImplementation(() => ({ + select: () => ({ + lean: async () => documents.filter((doc) => Array.isArray(doc.notifications) && doc.notifications.length > 0), + }), + })) + + userUpdateOne.mockImplementation(async (filter: { _id: { toString: () => string } }, update: any) => { + const doc = documents.find((candidate) => candidate._id.toString() === filter._id.toString()) + if (doc && update?.$unset && "notifications" in update.$unset) delete doc.notifications + return { modifiedCount: 1 } + }) + + notificationUpdateOne.mockImplementation(async (filter: any, update: any, options?: { upsert?: boolean }) => { + const id = String(filter._id) + if (failingIds.has(id)) throw new Error("simulated write failure") + + const existing = store.get(id) + if (!existing) { + if (!options?.upsert) return { upsertedCount: 0, modifiedCount: 0, matchedCount: 0 } + store.set(id, { _id: id, ...update.$setOnInsert }) + return { upsertedCount: 1, modifiedCount: 0, matchedCount: 0 } + } + + if (filter.userId !== undefined && existing.userId !== filter.userId) { + return { upsertedCount: 0, modifiedCount: 0, matchedCount: 0 } + } + if (filter.read !== undefined && existing.read !== filter.read) { + return { upsertedCount: 0, modifiedCount: 0, matchedCount: 0 } + } + if (!update.$set) return { upsertedCount: 0, modifiedCount: 0, matchedCount: 1 } + + Object.assign(existing, update.$set) + return { upsertedCount: 0, modifiedCount: 1, matchedCount: 1 } + }) + + return { store, failingIds, documents } +} + +function legacyEntry(overrides: Record = {}) { + return { + id: EMBEDDED_ID, + title: "KYC approved", + message: "Your KYC has been approved.", + read: false, + timestamp: new Date("2026-01-01T00:00:00.000Z"), + link: "/dashboard/driver/activity", + ...overrides, + } +} + +describe("migrateEmbeddedNotifications", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it("derives a stable id from an embedded ObjectId", () => { + expect(deriveNotificationId(USER_ID, legacyEntry())).toBe(EMBEDDED_ID) + }) + + it("derives a stable id for legacy entries with no usable id", () => { + const entry = legacyEntry({ id: "not-an-object-id" }) + const first = deriveNotificationId(USER_ID, entry) + + expect(first).toMatch(/^[a-f\d]{24}$/) + expect(deriveNotificationId(USER_ID, entry)).toBe(first) + expect(deriveNotificationId("507f1f77bcf86cd799439099", entry)).not.toBe(first) + }) + + it("backfills embedded notifications and unsets the array", async () => { + const world = createWorld([{ id: USER_ID, notifications: [legacyEntry()] }]) + + const result = await migrateEmbeddedNotifications() + + expect(result).toMatchObject({ usersScanned: 1, usersCleared: 1, notificationsCreated: 1, errors: [] }) + expect(world.store.get(EMBEDDED_ID)).toMatchObject({ + userId: USER_ID, + title: "KYC approved", + read: false, + link: "/dashboard/driver/activity", + }) + expect(world.documents[0].notifications).toBeUndefined() + }) + + it("promotes read state recorded only on the embedded copy", async () => { + const world = createWorld([{ id: USER_ID, notifications: [legacyEntry({ read: true })] }]) + world.store.set(EMBEDDED_ID, { _id: EMBEDDED_ID, userId: USER_ID, read: false }) + + const result = await migrateEmbeddedNotifications() + + expect(result).toMatchObject({ notificationsCreated: 0, notificationsReconciled: 1 }) + expect(world.store.get(EMBEDDED_ID)).toMatchObject({ read: true }) + }) + + it("never un-reads a notification already dismissed in the collection", async () => { + const world = createWorld([{ id: USER_ID, notifications: [legacyEntry({ read: false })] }]) + world.store.set(EMBEDDED_ID, { _id: EMBEDDED_ID, userId: USER_ID, read: true }) + + const result = await migrateEmbeddedNotifications() + + expect(result).toMatchObject({ notificationsCreated: 0, notificationsReconciled: 0, notificationsSkipped: 1 }) + expect(world.store.get(EMBEDDED_ID)).toMatchObject({ read: true }) + }) + + it("skips entries the Notification schema could not accept", async () => { + const world = createWorld([ + { id: USER_ID, notifications: [legacyEntry({ title: " " }), legacyEntry({ id: "x", message: "" })] }, + ]) + + const result = await migrateEmbeddedNotifications() + + expect(result).toMatchObject({ notificationsCreated: 0, notificationsSkipped: 2, usersCleared: 1 }) + expect(world.store.size).toBe(0) + }) + + it("keeps the embedded array after a partial write failure and finishes on re-run without duplicating", async () => { + const secondId = "507f1f77bcf86cd799439022" + const world = createWorld([ + { + id: USER_ID, + notifications: [legacyEntry(), legacyEntry({ id: secondId, title: "Payout sent" })], + }, + ]) + world.failingIds.add(secondId) + + const first = await migrateEmbeddedNotifications() + + expect(first.errors).toHaveLength(1) + expect(first).toMatchObject({ notificationsCreated: 1, usersCleared: 0 }) + // The array survives so nothing is lost. + expect(world.documents[0].notifications).toHaveLength(2) + + world.failingIds.clear() + const second = await migrateEmbeddedNotifications() + + expect(second).toMatchObject({ notificationsCreated: 1, usersCleared: 1, errors: [] }) + // The already-migrated entry was not written a second time. + expect(world.store.size).toBe(2) + expect(world.documents[0].notifications).toBeUndefined() + }) + + it("writes nothing in dry-run mode", async () => { + const world = createWorld([{ id: USER_ID, notifications: [legacyEntry()] }]) + + const result = await migrateEmbeddedNotifications({ dryRun: true }) + + expect(result).toMatchObject({ usersScanned: 1, usersCleared: 0, notificationsCreated: 1 }) + expect(notificationUpdateOne).not.toHaveBeenCalled() + expect(userUpdateOne).not.toHaveBeenCalled() + expect(world.documents[0].notifications).toHaveLength(1) + }) + + it("handles a high-volume legacy user in a single pass", async () => { + const notifications = Array.from({ length: 500 }, (_, index) => + legacyEntry({ id: `legacy-${index}`, title: `Notice ${index}` }), + ) + const world = createWorld([{ id: USER_ID, notifications }]) + + const result = await migrateEmbeddedNotifications() + + expect(result).toMatchObject({ notificationsCreated: 500, usersCleared: 1, errors: [] }) + expect(world.store.size).toBe(500) + expect(world.documents[0].notifications).toBeUndefined() + }) +}) From dbdf299096d59e8fc7df7dcb903d3c3eef6bc614 Mon Sep 17 00:00:00 2001 From: olathedev Date: Sun, 23 Aug 2026 06:16:08 +0100 Subject: [PATCH 5/7] docs(notifications): describe the single read-state store and the backfill Refs #200 --- docs/notifications.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/notifications.md b/docs/notifications.md index 51afb53f..4a691602 100644 --- a/docs/notifications.md +++ b/docs/notifications.md @@ -27,3 +27,20 @@ Templates are keyed by event type and integer version. Rendering is deterministi `publishNotificationEvent` creates one `NotificationDelivery` per channel. The unique `{eventId}:{userId}:{channel}` key makes duplicate events harmless. In-app delivery is independent of email. Email progresses `scheduled → processing → delivered`; failures retry with exponential backoff and enter `dead_letter` after five attempts. Attempts retain timestamps, provider IDs, and redacted errors. A scheduler calls `POST /api/notifications/process` with `Authorization: Bearer $NOTIFICATION_WORKER_SECRET`. User notification reads are user-scoped; admins may inspect a specified user. Delivery history and dead letters are operational data and must only be exposed to authorized admins. + +## Read-state store + +The `Notification` collection is the single source of truth for notification content and read state. `GET /api/activity` derives every unread count from it, and `PATCH /api/activity` (`set-read` and `mark-all-read`) is the only way that state changes, so a mutation response and the next read always agree. + +The embedded `User.notifications` array is deprecated. Nothing writes to it, it has no schema default, and it is `select: false`, so no query returns it by accident and no user document grows one. `/api/auth/me` no longer projects or returns it, and unread badges come from `ActivityUnreadBell`, which fetches the live count and listens for `chainmove:activity-count-changed`. + +### Retiring legacy embedded records + +Documents written before the split was closed still carry the array. Retire them with: + +```bash +bun run notifications:migrate-embedded -- --dry-run # inventory only +bun run notifications:migrate-embedded # backfill and unset +``` + +Each embedded entry is copied into the `Notification` collection and the array is then unset. Target IDs are derived deterministically — the ObjectId the old dual-write stored, otherwise a hash of owner, title, message and timestamp — so re-running never duplicates feed entries. Read state is merged in one direction: an embedded `read: true` promotes the collection document to read, because the retired server action recorded reads only on the embedded copy; an embedded `read: false` never un-reads a notification the user has since dismissed. A user whose entries did not all migrate keeps its array so the next run can finish the job. From 74d12c44be41431f3137d3bf4afd213410346957 Mon Sep 17 00:00:00 2001 From: olathedev Date: Sun, 23 Aug 2026 06:32:44 +0100 Subject: [PATCH 6/7] fix(notifications): escape the NUL fingerprint separator in the backfill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The separator was written as a literal NUL byte, which made git classify lib/notifications/embedded-migration.ts as binary and hid it from diffs and review. Escaping it keeps the same delimiter — and the same derived ids — in a plain-text source file. Refs #200 --- lib/notifications/embedded-migration.ts | Bin 5614 -> 5762 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/lib/notifications/embedded-migration.ts b/lib/notifications/embedded-migration.ts index fe37ceee5837dd8558a4bcf076b60ef3e93b7325..11f7784c750be60f9caf7d4edfb223319ecf96fc 100644 GIT binary patch delta 165 zcmWlRJqp4=5QRG-5%2JNe`4GrR0;6f40ih~Ne6h1|f6oUO;k1aDTdAKm+n6lz`b9 v?;IzV3~Tacv@e9uoXjgNr Date: Tue, 25 Aug 2026 04:53:40 +0100 Subject: [PATCH 7/7] fix(privacy): unset the deprecated embedded array when anonymizing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The privacy lifecycle landed on main while this branch was open. Its User entry lists `notifications` as a personal field, so the anonymizer would pseudonymize the embedded array into a string; the `update.notifications = []` corrective existed only to undo that. Now that the array is deprecated with no schema default, writing `[]` would recreate the field on every anonymized user and contradict the invariant that nothing writes there any more. A not-yet-migrated document can still hold real notification text, so the field is `$unset` instead — erasing the legacy content rather than leaving an empty array behind. The notifications themselves are already erased by the data map's own "Notifications" entry, which hard-deletes the collection by userId. This also replaces the equivalent fix from src/server/PrivacyController.ts, which main deleted when the logic moved into lib/privacy/. Refs #200 --- lib/privacy/privacy-deletion.service.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/privacy/privacy-deletion.service.ts b/lib/privacy/privacy-deletion.service.ts index 94332f4c..1a178629 100644 --- a/lib/privacy/privacy-deletion.service.ts +++ b/lib/privacy/privacy-deletion.service.ts @@ -153,6 +153,7 @@ async function applyEntry( if (entry.deletionStrategy === "anonymize" || entry.deletionStrategy === "pseudonymize") { const update: Record = {} + const unset: Record = {} for (const field of entry.personalFields) { if (entry.model === "User" && (field === "name" || field === "fullName")) { update[field] = fullAnonymizeFor(userId) @@ -163,12 +164,19 @@ async function applyEntry( // useful tombstone value for a password hash. continue } + if (entry.model === "User" && field === "notifications") { + // Deprecated embedded array. Notifications live in their own + // collection, which the "Notifications" entry hard-deletes; a + // not-yet-migrated document may still hold notification text here, so + // drop the field outright instead of leaving an empty array behind. + unset[field] = "" + continue + } update[field] = pseudonymFor(userId, field) } if (entry.model === "User") { update.kycDocuments = [] update.kycRejectionReason = null - update.notifications = [] update.password = null update.privyUserId = null update.anonymizedAt = new Date() @@ -180,7 +188,10 @@ async function applyEntry( anonymizedAt: new Date().toISOString(), } } - const result = await model.updateMany(filter, { $set: update }) + const result = await model.updateMany( + filter, + Object.keys(unset).length > 0 ? { $set: update, $unset: unset } : { $set: update }, + ) return { affectedCount: result.modifiedCount || 0 } }