fix(notifications): enforce category preferences on every generation path (closes #1605) - #1799
Open
rudrasatani13 wants to merge 2 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1605.
What was wrong
The preference check existed -
shouldSendNotificationreads the legacy per-type flags and the per-category row - but it was only wired into three of the generation paths:handleEventCreated,handleUserJoinedEventandhandlePredictionSubmitted. The other seven queued unconditionally:handleMatchAdded,handleMatchResultSubmitted,handleWinnersVerified,handleEventCancellednotifyDisputeSlaApproaching,notifyDisputeSlaBreached,notifyOracleDivergenceSo a user who turned a category off still received those notifications.
What changed
isAllowedByPreferences(prefs, categoryPreference, type)- the decision itself, extracted so the single-recipient path and the batched path cannot drift apart. Legacy per-type flags still reject when explicitly false, and a missing category row still counts as opted in (the safe fallback).filterAllowedRecipients(addresses, type)- a batched filter: one user query plus one category-preference query for the whole recipient list, rather than two queries per recipient. An address with no user record is kept, which matches what the single-recipient path already did.Tests
Three new cases in
notification-generator.service.spec.ts: a disabled category produces no notification, an enabled category does, and a missing category row is treated as opted in. The category-preference repository mock gained thefindthe batched filter uses.Verification:
npx jest src/notifications-> 6 suites, 133 tests passing (130 before, plus the three new ones).npx tsc --noEmit-> 66 pre-existing errors repo-wide, unchanged from before this branch, and none in the two files touched here.Assignment and reward are not confirmed on my side, so please treat this as a voluntary contribution. I applied for the issue on GrantFox (#1605).