-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Do not send notifications in Celery tasks if notifications are disabled #11750
Milestone
Comments
Merged
12 tasks
giohappy
added a commit
that referenced
this issue
Dec 6, 2023
…ions are disabled (#11751) * Add test suite * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled --------- Co-authored-by: Giovanni Allegri <giohappy@gmail.com>
github-actions bot
pushed a commit
that referenced
this issue
Dec 6, 2023
…ions are disabled (#11751) * Add test suite * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled --------- Co-authored-by: Giovanni Allegri <giohappy@gmail.com> (cherry picked from commit ea71bcf)
giohappy
pushed a commit
that referenced
this issue
Dec 6, 2023
…ions are disabled (#11751) (#11759) * Add test suite * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled * [Fixes #11750] Do not send notifications in Celery tasks if notifications are disabled --------- Co-authored-by: Giovanni Allegri <giohappy@gmail.com> (cherry picked from commit ea71bcf) Co-authored-by: mattiagiupponi <51856725+mattiagiupponi@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There are two problems with the notifications in Celery tasks:
settings.NOTIFICATION_ENABLED
isTrue
. We already havegeonode.notification_helpers.has_notifications
for that, and when the module is imported it also automatically imports the module. Thegeonode.task.tasks
module should import the helper module and use thehas_notifications
inside thesend_queued_notifications
method.send_queued_notifications
itself shouldn't be called if notifications are disabled. We must do this check inside the geonode.notification_helpers.has_notifications.call_celery decorator and avoid triggering the Celery task in case notifications are disabled.The text was updated successfully, but these errors were encountered: