-
Notifications
You must be signed in to change notification settings - Fork 35
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
Filter unique pending notifications to avoid duplicate emails #2459
Filter unique pending notifications to avoid duplicate emails #2459
Conversation
…borative#2372) * Remove duplicated query and limit users to 9999 * Trigger deploy * Fix map of editors * Fix admin test to use mock * Add missing users to. mock * Fix users list * Fix user selection on userfields dropdown
✅ Deploy Preview for staging-aiid ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Can you add a test that has duplicated subscriptions/notifications and makes sure the emails are not sent multiple times?
@cesarvarela done! please review |
This PR solved the problem of sending duplicated emails to the same user. Issue: #2404
Testing notes
Preview link https://deploy-preview-32--pablocosta-staging.netlify.app/
Note that in order to trigger the
process notification
function it should be triggered manually from Pablo's Atlas account.Note
We still have an issue in the origin of the actual bug that inserts multiple pending notifications and subscriptions for the same user. But this can be treated in another PR.
Duplicated pending notifications in Production:
Duplicated subscriptions in Production: