-
Notifications
You must be signed in to change notification settings - Fork 10
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
Email notification tasks and templates #472 #485
Conversation
Signed-off-by: nirmeen <nirmeen.sallam@uni.lu>
Signed-off-by: nirmeen <nirmeen.sallam@uni.lu>
…by execution date and missed notifications Signed-off-by: nirmeen <nirmeen.sallam@uni.lu>
Signed-off-by: nirmeen <nirmeen.sallam@uni.lu>
… to display object name Signed-off-by: nirmeen <nirmeen.sallam@uni.lu>
notification/tasks.py
Outdated
print( | ||
f"Failed: An error occurred while sending Email notification error report for data-stewards." | ||
f" Error: {e}" | ||
) |
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.
This could be removed no? Also, is it intentional for the error to not be raised again?
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.
yes it could be removed. The idea was that we log the error and then send datastewards an email in case of notification error and continue to the next user. you think we should raise the error anyways?
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.
It makes sense. I think the error email should be sent at the very end though, or we risk sending an email for every user on some days.
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.
i thought it is one email per user. I felt it would be easier for data stewards to track which user notifications errors they checked and which they didn't. Otherwise. if all users in one email it might be long and maybe they will not be able to keep track of where they stopped.
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.
It seems the print
is back :/
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.
sorry, removed it now.
Signed-off-by: nirmeen <nirmeen.sallam@uni.lu>
Signed-off-by: nirmeen <nirmeen.sallam@uni.lu>
…ation sending is disabled
…nd fixed settings of other tests
# Conflicts: # notification/models.py
…st for send notification and changed profile to notification settings
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.
Thanks for the changes!
notification/tasks.py
Outdated
print( | ||
f"Failed: An error occurred while sending Email notification error report for data-stewards." | ||
f" Error: {e}" | ||
) |
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.
It seems the print
is back :/
Implement email notification tasks:
Implement email templates to send a listing of notifications by entity name