Skip to content

fix(notifications): paginate the list and give it a way to be cleared - #386

Merged
singh-odyssey merged 1 commit into
TravellersMeet:mainfrom
MOHITKOURAV01:fix/381-notifications-pagination
Aug 11, 2026
Merged

fix(notifications): paginate the list and give it a way to be cleared#386
singh-odyssey merged 1 commit into
TravellersMeet:mainfrom
MOHITKOURAV01:fix/381-notifications-pagination

Conversation

@MOHITKOURAV01

Copy link
Copy Markdown
Contributor

Closes #381

The problem

GET /api/notifications was a single unbounded query:

prisma.notification.findMany({
  where: { userId: session.user.id },   // no take, no cursor, no expiry filter
  orderBy: { createdAt: "desc" },
})

NotificationBell fetches this on mount for every authenticated page, so an active user downloads every notification they have ever received in order to render a dropdown that is max-h-96 and shows about six of them.

It also returned rows whose expiresAt had already passed. The schema has the column and cleanupExpiredNotifications() sweeps it, but on a cron in batches of ≤500 — so between sweeps there are expired rows in the table and the read path was handing them to the client.

And there was no way to remove anything. PATCH /api/notifications/[id]/read existed; DELETE did not, and markAllNotificationsAsRead() was exported from src/lib/notifications.ts with no caller anywhere in the codebase. The helper was written, the route never was, so clearing a 500-item bell meant clicking 500 rows.

What this changes

Pagination

Uses the shared helpers from src/lib/pagination.ts — the same ones /api/messages, /api/routes, /api/tickets and /api/users already use. Default 20, capped at 100, pagination.nextCursor / hasMore in the response, 400 on a malformed limit or cursor. The notifications key is kept as an alias for items so nothing breaks.

Expiry

activeNotificationWhere() filters both the page and unreadCount, off a single clock reading so the badge and the list cannot disagree about a notification expiring mid-request. It uses gt: now against the sweeper's lte: now, so the two rules partition the table: a row is either sweepable or visible, never both and never neither.

The missing write paths

Route Does
PATCH /api/notifications Mark all read — wires up the orphaned markAllNotificationsAsRead()
DELETE /api/notifications/[id] Dismiss one
DELETE /api/notifications?all=true Clear read ones, or everything

The single-row delete goes through deleteMany with the userId in the same where, so a guessed id belonging to somebody else deletes nothing and returns 404 rather than throwing RecordNotFound.

UI

NotificationBell gets Mark all read, a per-row dismiss, and Load more. Both mutations are optimistic with a rollback on failure. The loading state renders skeletons instead of "No notifications yet" while the first page is still in flight — previously the dropdown claimed the list was empty during load.

Tests

45 new tests across three files:

  • src/lib/__tests__/notification-list.test.ts — the new helpers, including that the cursor filter and expiry filter both survive under AND rather than one replacing the other, and that deleteNotification reports 0 instead of throwing for a foreign id
  • src/app/api/notifications/__tests__/route.test.ts — GET/PATCH/DELETE, including the single-clock-reading assertion
  • src/app/api/notifications/[id]/__tests__/route.test.ts — the new DELETE

Verification

  • npx vitest run src/app/api/notifications src/lib/__tests__/notification-list.test.ts src/lib/__tests__/notifications.test.ts — 45 passed
  • Full suite unchanged from main's baseline: 9 failing files / 23 failing tests before and after
  • npx tsc --noEmit reports nothing new for these paths

Note on the base branch

main does not currently typecheck (#366, fixed by #379). Nothing here touches those files.

GET /api/notifications was a single unbounded findMany. Every notification a
user had ever received was serialised on every authenticated page load, to
render a dropdown that is max-h-96 and shows about six of them. It also
returned rows whose expiresAt had already passed, because the cron sweeper
deletes in batches of at most 500 and read paths were trusting it to have
caught up.

The endpoint now uses the same cursor helpers as /api/messages, /api/routes
and /api/tickets: limit defaults to 20, caps at 100, and the response carries
pagination.nextCursor / hasMore. The `notifications` key is kept as an alias
for `items` so nothing breaks. Expired rows are filtered from both the page
and unreadCount, off a single clock reading so the two cannot disagree.

Also adds the missing write paths:

- PATCH /api/notifications marks everything read. markAllNotificationsAsRead
  was already exported from src/lib/notifications.ts with no caller anywhere
  in the codebase — the helper existed, the route did not.
- DELETE /api/notifications/[id] dismisses one, scoped with deleteMany so a
  guessed id belonging to someone else deletes nothing and returns 404.
- DELETE /api/notifications clears the read ones, ?all=true clears the rest.

NotificationBell gets Mark all read, a per-row dismiss, and Load more, each
with an optimistic update that rolls back if the request fails. The list
loading state now renders skeletons instead of an empty dropdown.

Closes TravellersMeet#381
@github-actions

Copy link
Copy Markdown

Thanks for opening this PR, @MOHITKOURAV01! 👋

Our maintainers will review it shortly. Estimate Time is 5-8 hrs .Meanwhile please:

  • Ensure CI is green (lint, typecheck, build).
  • Hit the star ⭐ button to show your support!
  • Confirm the PR template checklist is complete.
  • Add screenshots for UI changes.

If anything changes, feel free to push updates—this thread will stay open.

@MOHITKOURAV01

Copy link
Copy Markdown
Contributor Author

On the red build-and-lint

The failure is inherited from main, not introduced here. The typecheck step reports 23 errors, all in files this PR does not touch:

Same 23 errors on main at 39e71eb, and the same failure on #376, #377 and #367. npx tsc --noEmit on this branch adds nothing beyond that set.

Test suite on this branch: no regressions against the main baseline of 9 failing files / 23 failing tests.

CI should go green here once #379 and #378 land — no rebase needed, this branch merges cleanly with both.

@singh-odyssey
singh-odyssey merged commit 56dfc89 into TravellersMeet:main Aug 11, 2026
1 of 2 checks passed
@singh-odyssey singh-odyssey added the ECSoC26 Elite coders summer of code label Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ECSoC26 Elite coders summer of code good-backend good-pr elite coders

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] GET /api/notifications returns every notification ever created — unpaginated, includes expired rows, and cannot be cleared

2 participants