Skip to content

fix(notifications): exclude soft-deleted rows from counts and mutations (closes #1606) - #1798

Open
rudrasatani13 wants to merge 1 commit into
Arena1X:mainfrom
rudrasatani13:fix/1606-soft-deleted-notifications
Open

fix(notifications): exclude soft-deleted rows from counts and mutations (closes #1606)#1798
rudrasatani13 wants to merge 1 commit into
Arena1X:mainfrom
rudrasatani13:fix/1606-soft-deleted-notifications

Conversation

@rudrasatani13

Copy link
Copy Markdown

Closes #1606.

What was wrong

TypeORM applies the @DeleteDateColumn filter only to select queries - it lives in QueryBuilder.createWhereExpression() behind queryType === "select" (checked in the installed typeorm 0.3.28). So find* and count() already skip deleted rows, but every update(criteria, patch) in notifications.service.ts did not:

  • markAsRead(id) returned success for a soft-deleted notification and mutated it.
  • markAllAsUnread / markMultipleAsUnread flipped soft-deleted rows back to read = false - invisible rows that keep that state, and get picked up by any consumer that does not use a TypeORM select (raw SQL, aggregations, withDeleted reads).
  • markAllAsRead / markMultipleAsRead kept touching rows the user had deleted.

What changed

  • Added activeNotificationWhere(userAddress, extra) - one private helper that adds deleted_at: IsNull() to any criteria.
  • Every read and write in the service now goes through it: findAllForUser (page query and its unread count), getUnreadCount, markAsRead, markAllAsRead, markMultipleAsRead, markAllAsUnread, markMultipleAsUnread.
  • Count and list behaviour is unchanged, but no longer depends on ORM defaults: the scope is explicit and future queries inherit it from one place.

Tests

notifications.service.spec.ts gains five cases - the list query is scoped, the unread count is scoped, the unread count returned with a page is scoped, deleted rows are not flipped back to unread, and marking a deleted notification read is treated as not found. The existing update-criteria expectations were updated to include the scope.

Verification:

  • npx jest src/notifications -> 6 suites, 130 tests, all passing.
  • npx tsc --noEmit -> 66 pre-existing errors in this repo, none in the two files touched here (they are in unrelated spec and e2e fixtures).

Assignment and reward are not confirmed on my side, so please treat this as a voluntary contribution. I applied for the issue on GrantFox (#1606).

@vercel

vercel Bot commented Sep 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
insight-arena-4rll Ready Ready Preview Sep 13, 2026 2:41pm UTC

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Backend] — Soft-Deleted Notifications Excluded from Counts

1 participant