-
Notifications
You must be signed in to change notification settings - Fork 13k
regression: fix notifications not being sent if user is idle or offline #35849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
regression: fix notifications not being sent if user is idle or offline #35849
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses a regression issue by ensuring that notifications are sent correctly when the user is offline. The key change conditionally includes the schedule property when it is truthy, preventing unintended behavior.
Comments suppressed due to low confidence (1)
apps/meteor/app/notification-queue/server/NotificationQueue.ts:172
- Review whether the conditional spread operator correctly handles all cases for schedule. Verify that this change addresses the offline notification issue without inadvertently omitting necessary scheduling data.
...(schedule && { schedule }),
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release-7.6.0 #35849 +/- ##
=================================================
- Coverage 61.16% 61.09% -0.07%
=================================================
Files 3003 3005 +2
Lines 71350 71432 +82
Branches 16333 16368 +35
=================================================
+ Hits 43638 43639 +1
- Misses 24746 24824 +78
- Partials 2966 2969 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
The way the notification queue works depends on the user's status, if the user was idle/offline there is no delay to send the notification, so
scheduleisundefinedand previously (before #31497) it was being ignored and not added to the record, so queries likeschedule: { $exists: false }would work.. but since then the code was savingschedule: undefinedwhich caused that query to not work as expected, causing the notification issue.Issue(s)
Introduced by #31497
Steps to test or reproduce
Further comments