Skip to content

feat(notification): add unread count and per-notification read - #223

Merged
aquie00t merged 1 commit into
mainfrom
feat/notification-read-state
Aug 27, 2026
Merged

feat(notification): add unread count and per-notification read#223
aquie00t merged 1 commit into
mainfrom
feat/notification-read-state

Conversation

@aquie00t

Copy link
Copy Markdown
Collaborator

Closes findings 6-7 of the notification audit, following #221.

What was missing

6. No unread-count endpoint. getUnreadCount() was implemented on the repository and declared on the port, but nothing called it — dead code. The badge could only be built by paging through the whole list and counting client-side.

7. Read state was all-or-nothing. PATCH /notifications/read-all was the only way to mark anything read, so tapping a single notification had to clear every one of them.

New endpoints

GET /notifications/unread-count { data: { count }, meta: { timestamp } }, STANDARD rate limit
PATCH /notifications/:id/read 204, or 404 when the notification is not the caller's

The single update goes through updateMany with where: { id, recipientId } rather than update by id. That puts the recipient in the filter, so another user's notification simply matches nothing — it answers 404 exactly like an id that does not exist, instead of leaking that the id is real.

Three defects fixed on the way

PATCH /notifications/read-all marked every user's notifications as read. The controller called execute(userId) with a bare string while the use case takes { userId }. It destructured to undefined, and Prisma drops an undefined filter, so updateMany ran with no recipientId condition at all — across the whole table. The new e2e case asserts a second user's unread count survives someone else's read-all.

request.user was any project-wide. src/http/types/fastify-jwt.d.ts imported UserPayload from @core/interfaces/user-payload.interface, a path that stopped existing when the type moved to the token port in d4a508c. skipLibCheck: true means declaration files are not checked, so the dangling import never surfaced and FastifyJWT["user"] silently degraded to any — which is why the bug above type-checked for as long as it did. Repointing the import makes tsc catch it. payload is now UserPayload | RecoveryPayload, since the account-recovery token is signed with the same instance.

notificationController was any in the Cradle. src/http/types/fastify-awilix.d.ts imported it from @services/notification.controller, an alias that is not in tsconfig.json. Fixed for this controller; six others (auth, oauth, user, profile, follow-user, post) still use the same dead alias and are left for a follow-up, since fixing them all is a separate diff.

Verification

  • pnpm test:unit — 792 passed (7 new)
  • pnpm test:integration — 107 passed (3 new: single read, cross-user refusal, unknown id)
  • pnpm test:e2e — 325 passed (new tests/e2e/notification/read-state.test.ts: counts scoped per user, 204 + count drop, idempotent repeat, 404 for someone else's, 404 for unknown, 401s, and the read-all scoping regression)
  • tsc -p tsconfig.build.json --noEmit, pnpm lint, prettier — clean

Integration and e2e ran before the rebase onto the merged #221; unit tests and the typecheck were re-run after it.

Still open from the audit

8 (no notification cleanup on unlike/unfollow, duplicate notifications), 9 (realtime emit inside the transaction), 10 (realtime payload shape differs from REST), 11 (WebSocketManager keeps one socket per user, so a second tab silently steals delivery), 12 (default-avatar.png fallback points at a key that does not exist), 13-14 (dead NEW_POST type, unused getDescription()).

🤖 Generated with Claude Code

https://claude.ai/code/session_01PwzkQ5YGFXSB9jWCZKzX4H

The badge had no endpoint behind it: getUnreadCount() existed on the
repository and the port but nothing ever called it, so a client had to
page through the whole list and count. And read state was all or
nothing - tapping one notification could only clear every one of them,
because the notification id never left Prisma.

Adds GET /notifications/unread-count and PATCH /notifications/:id/read.
The repository scopes the single update by recipient with updateMany,
so another user's notification matches nothing and answers 404 rather
than confirming that the id is real.

Fixes three defects found on the way:

- PATCH /notifications/read-all marked EVERY user's notifications as
  read. The controller passed a bare string where the use case takes
  { userId }, so it destructured to undefined and Prisma dropped the
  recipientId filter from updateMany entirely.

- src/http/types/fastify-jwt.d.ts imported UserPayload from a path that
  has not existed since it moved to the token port. skipLibCheck hid
  the dangling import, request.user degraded to any project-wide, and
  that is why the call above type-checked. The sign payload is now a
  union with RecoveryPayload, which the recovery token needs.

- src/http/types/fastify-awilix.d.ts imported NotificationController
  from the non-existent @services alias, leaving it any in the Cradle.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PwzkQ5YGFXSB9jWCZKzX4H
@aquie00t
aquie00t merged commit fbd4b22 into main Aug 27, 2026
10 checks passed
github-actions Bot pushed a commit that referenced this pull request Aug 27, 2026
# [1.9.0](v1.8.0...v1.9.0) (2026-08-27)

### Features

* **notification:** add unread count and per-notification read ([#223](#223)) ([fbd4b22](fbd4b22))
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.9.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant