Skip to content
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

Merge notification systems #251

Open
nlsvgtr opened this issue Sep 26, 2022 · 1 comment
Open

Merge notification systems #251

nlsvgtr opened this issue Sep 26, 2022 · 1 comment

Comments

@nlsvgtr
Copy link
Collaborator

nlsvgtr commented Sep 26, 2022

The code contains two notification services.

The first can be found in ./src/services/notificationService.js which is used by ./src/services/eventService.js. This uses rulesets, and seems to be an attempt towards a generic and flexible notification system. The problem is that it is only used by Submissions.

The second can be found in ./src/notifications.js. It uses queues which are processed by crons. This one is used by the rest of the API.

These services should be merged.

@nlsvgtr
Copy link
Collaborator Author

nlsvgtr commented Sep 29, 2022

On crete idea a mail is sent to the user. This is done in the route, using mail.sendThankYouMail. Newletter signups are followed by a sendNewsletterSignupConfirmationMail. These should be integrated in the notification system as well.

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

No branches or pull requests

1 participant