Skip to content

fix(notification): take the notification back when its action is undone - #226

Merged
aquie00t merged 1 commit into
mainfrom
feat/notification-cleanup-on-undo
Aug 27, 2026
Merged

fix(notification): take the notification back when its action is undone#226
aquie00t merged 1 commit into
mainfrom
feat/notification-cleanup-on-undo

Conversation

@aquie00t

Copy link
Copy Markdown
Collaborator

Closes finding 8 of the notification audit, following #221 and #223.

Problem

None of the undo paths touched notifications. Unliking a post, an article or a comment, and unfollowing a user, all left the notification they had produced sitting in the recipient's list.

Two consequences:

  • The recipient keeps a notification for something that no longer happened — "X started following you" from someone who does not follow them.
  • Liking only notifies on the transition into liked (the alreadyLiked guard returns early otherwise), so a like → unlike → like round produced a fresh notification every time. Nothing capped that.

Change

deleteByTarget() on the notification repository, called from unlike-post, unlike-article, unlike-comment and unfollow-user, each inside the transaction that already removes the like.

await ctx.notificationRepository.deleteByTarget({
    recipientId: post.author.id,
    issuerId: input.userId,
    type: NotificationType.LIKE,
    postId: input.postId,
});

The subtle part is in the repository: the unset targets are matched as explicit NULLs, not left out of the filter.

postId: input.postId ?? null,
articleId: input.articleId ?? null,
commentId: input.commentId ?? null,

Prisma drops an undefined filter entirely. A post like and an article like share their type, issuer and recipient and differ only in which target column is set — so leaving the others out would let unliking a post delete the article like sitting next to it. The integration test pins exactly that case.

Why no unique constraint

The audit suggested a unique constraint or upsert as the alternative. It is not needed once the undo cleans up: a second notification could only ever appear after an undo, and the undo now removes the first, so the existing already-liked / already-following guards keep it at one. The e2e test toggles a like three times and asserts a single LIKE remains.

It would also be awkward to express. The target columns are nullable, Postgres treats NULLs as distinct in a unique index, and FOLLOW has all three null — so it would need NULLS NOT DISTINCT or an expression index, neither of which Prisma's schema can declare without drifting from the migration.

Deleted comments and posts were already handled by the cascading foreign keys added in #221.

Verification

  • pnpm test:unit — 800 passed (8 new, two per undo path: the delete happens with the right target, and nothing is deleted when there was no action to undo)
  • pnpm test:integration — 110 passed (3 new: target precision between a post like and an article like, the target-less FOLLOW delete, and another issuer's notification left alone)
  • pnpm test:e2e — new tests/e2e/notification/cleanup-on-undo.test.ts drives unlike, comment unlike, unfollow and a three-round like toggle through the real endpoints
  • tsc -p tsconfig.build.json --noEmit, pnpm lint, prettier — clean

The e2e run was still in progress when this branch was rebased onto the squash-merged #223; unit tests and the typecheck were re-run after the rebase, and the e2e suite is being re-run now. I will report the result in the PR if anything fails.

Still open from the audit

9 (realtime emit inside the transaction), 10 (realtime payload shape differs from REST — including that an undone action sends no realtime event, so an open client only sees the removal on refresh), 11 (WebSocketManager keeps one socket per user), 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

Unliking and unfollowing left their notification behind. The recipient
kept being told about something that no longer happened, and because
liking only notifies on the transition into liked, toggling the action
piled up a fresh notification every round.

Adds deleteByTarget() to the notification repository and calls it from
unlike-post, unlike-article, unlike-comment and unfollow-user. The
unset targets are matched as explicit NULLs rather than left out of the
filter: Prisma drops an undefined one, which would let a post like
delete the article like sitting next to it, since the two share their
type, issuer and recipient and differ only in the target.

No unique constraint is needed on top. A second notification could only
ever appear after an undo, and the undo now removes the first, so the
existing already-liked and already-following guards keep it at one.

Deleted comments were already covered by the cascading foreign keys
added with the deep-link targets.

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

### Bug Fixes

* **notification:** take the notification back when its action is undone ([#226](#226)) ([f8e4138](f8e4138))
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.9.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@aquie00t
aquie00t deleted the feat/notification-cleanup-on-undo branch August 27, 2026 04:35
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