From d382dd4671074be2a2a8ac2a2746c78b6bdcbc4f Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Wed, 26 Jun 2024 12:28:53 +0200 Subject: [PATCH] [#2581] Refactor notifications for expiring actions/plans, messages, cases - replace cron jobs with celery tasks - explicitly mark internal utility functions for case notifications --- src/open_inwoner/accounts/forms.py | 1 + .../management/commands/actions_expire.py | 48 ---- .../commands/notify_about_messages.py | 59 ----- src/open_inwoner/accounts/managers.py | 10 + .../accounts/notifications/__init__.py | 9 + .../notifications/notify_expiring_actions.py | 39 ++++ .../notifications/notify_expiring_plans.py | 42 ++++ .../accounts/notifications/notify_messages.py | 47 ++++ src/open_inwoner/accounts/tasks.py | 116 ++++++++++ .../accounts/registration_necessary.html | 1 + .../accounts/tests/test_actions_expire.py | 52 ----- .../tests/test_notify_about_messages.py | 95 -------- .../tests/test_notify_expiring_actions.py | 65 ++++++ .../tests/test_notify_expiring_plans.py} | 88 +++++--- .../accounts/tests/test_notify_messages.py | 77 +++++++ src/open_inwoner/conf/base.py | 12 + src/open_inwoner/configurations/models.py | 23 ++ src/open_inwoner/openzaak/admin.py | 9 +- src/open_inwoner/openzaak/notifications.py | 213 +++++++++--------- .../openzaak/tests/test_notification_utils.py | 10 +- .../test_notification_zaak_infoobject.py | 22 +- .../tests/test_notification_zaak_status.py | 20 +- src/open_inwoner/plans/management/__init__.py | 0 .../plans/management/commands/__init__.py | 0 .../plans/management/commands/plans_expire.py | 62 ----- src/open_inwoner/plans/managers.py | 16 ++ .../templates/pages/profile/me.html | 1 + src/open_inwoner/utils/constants.py | 6 + 28 files changed, 665 insertions(+), 478 deletions(-) delete mode 100644 src/open_inwoner/accounts/management/commands/actions_expire.py delete mode 100644 src/open_inwoner/accounts/management/commands/notify_about_messages.py create mode 100644 src/open_inwoner/accounts/notifications/__init__.py create mode 100644 src/open_inwoner/accounts/notifications/notify_expiring_actions.py create mode 100644 src/open_inwoner/accounts/notifications/notify_expiring_plans.py create mode 100644 src/open_inwoner/accounts/notifications/notify_messages.py create mode 100644 src/open_inwoner/accounts/tasks.py delete mode 100644 src/open_inwoner/accounts/tests/test_actions_expire.py delete mode 100644 src/open_inwoner/accounts/tests/test_notify_about_messages.py create mode 100644 src/open_inwoner/accounts/tests/test_notify_expiring_actions.py rename src/open_inwoner/{plans/tests/test_plans_expire.py => accounts/tests/test_notify_expiring_plans.py} (56%) create mode 100644 src/open_inwoner/accounts/tests/test_notify_messages.py delete mode 100644 src/open_inwoner/plans/management/__init__.py delete mode 100644 src/open_inwoner/plans/management/commands/__init__.py delete mode 100644 src/open_inwoner/plans/management/commands/plans_expire.py create mode 100644 src/open_inwoner/utils/constants.py diff --git a/src/open_inwoner/accounts/forms.py b/src/open_inwoner/accounts/forms.py index e66f493d17..8f9131616d 100644 --- a/src/open_inwoner/accounts/forms.py +++ b/src/open_inwoner/accounts/forms.py @@ -202,6 +202,7 @@ def __init__(self, user, *args, **kwargs): self.fields["last_name"].required = True # notifications + # TODO: add condition to check if notifications should be displayed if not (user.login_type == LoginTypeChoices.digid and case_page_is_published()): del self.fields["cases_notifications"] if not inbox_page_is_published(): diff --git a/src/open_inwoner/accounts/management/commands/actions_expire.py b/src/open_inwoner/accounts/management/commands/actions_expire.py deleted file mode 100644 index aebe951a6c..0000000000 --- a/src/open_inwoner/accounts/management/commands/actions_expire.py +++ /dev/null @@ -1,48 +0,0 @@ -import logging -from datetime import date - -from django.core.management.base import BaseCommand -from django.db.models.query import QuerySet -from django.urls import reverse - -from mail_editor.helpers import find_template - -from open_inwoner.accounts.models import Action, User -from open_inwoner.utils.url import build_absolute_url - -logger = logging.getLogger(__name__) - - -class Command(BaseCommand): - help = "Send emails about expiring actions to the users" - - def handle(self, *args, **options): - today = date.today() - user_ids = Action.objects.filter(end_date=today).values_list( - "is_for_id", flat=True - ) - receivers = User.objects.filter( - is_active=True, pk__in=user_ids, plans_notifications=True - ).distinct() - - for receiver in receivers: - """send email to each user""" - actions = Action.objects.filter(is_for=receiver, end_date=today) - self.send_email( - receiver=receiver, - actions=actions, - ) - - logger.info( - f"The email was sent to the user {receiver} about {actions.count()} expiring actions" - ) - - def send_email(self, receiver: User, actions: QuerySet[Action]): - actions_link = build_absolute_url(reverse("profile:action_list")) - template = find_template("expiring_action") - context = { - "actions": actions, - "actions_link": actions_link, - } - - return template.send_email([receiver.email], context) diff --git a/src/open_inwoner/accounts/management/commands/notify_about_messages.py b/src/open_inwoner/accounts/management/commands/notify_about_messages.py deleted file mode 100644 index 216c0b74f3..0000000000 --- a/src/open_inwoner/accounts/management/commands/notify_about_messages.py +++ /dev/null @@ -1,59 +0,0 @@ -import logging - -from django.core.management.base import BaseCommand -from django.db.models import Count -from django.urls import reverse - -from mail_editor.helpers import find_template - -from open_inwoner.accounts.models import Message, User -from open_inwoner.utils.url import build_absolute_url - -logger = logging.getLogger(__name__) - - -class Command(BaseCommand): - help = "Send emails about new messages to the users" - - def handle(self, *args, **options): - receivers = User.objects.filter( - received_messages__seen=False, - received_messages__sent=False, - is_active=True, - messages_notifications=True, - ).distinct() - - for receiver in receivers: - """send email to each user""" - messages_to_update = Message.objects.filter( - receiver=receiver, - seen=False, - sent=False, - ) - total_senders = messages_to_update.aggregate( - total_senders=Count("sender", distinct=True) - )["total_senders"] - total_messages = messages_to_update.count() - self.send_email( - receiver=receiver, - total_senders=total_senders, - total_messages=total_messages, - ) - - """update messages""" - messages_to_update.update(sent=True) - - logger.info( - f"The email was sent to the user {receiver} about {total_messages} new messages" - ) - - def send_email(self, receiver: User, total_senders: int, total_messages: int): - inbox_url = build_absolute_url(reverse("inbox:index")) - template = find_template("new_messages") - context = { - "total_messages": total_messages, - "total_senders": total_senders, - "inbox_link": inbox_url, - } - - return template.send_email([receiver.email], context) diff --git a/src/open_inwoner/accounts/managers.py b/src/open_inwoner/accounts/managers.py index bf4d3db227..65e84a6591 100644 --- a/src/open_inwoner/accounts/managers.py +++ b/src/open_inwoner/accounts/managers.py @@ -1,3 +1,5 @@ +from datetime import date + from django.contrib.auth.models import BaseUserManager from django.db.models import Q, QuerySet @@ -76,3 +78,11 @@ def visible(self): def connected(self, user): return self.filter(Q(created_by=user) | Q(is_for=user)).distinct() + + def expiring_actions_user_ids(self, end_date: date): + """ + Returns: + id's of users with expiring actions + """ + user_ids = self.filter(end_date=end_date).values_list("is_for_id", flat=True) + return user_ids diff --git a/src/open_inwoner/accounts/notifications/__init__.py b/src/open_inwoner/accounts/notifications/__init__.py new file mode 100644 index 0000000000..f5e020e492 --- /dev/null +++ b/src/open_inwoner/accounts/notifications/__init__.py @@ -0,0 +1,9 @@ +from .notify_expiring_actions import notify_about_expiring_actions +from .notify_expiring_plans import notify_about_expiring_plans +from .notify_messages import notify_about_messages + +__all__ = [ + "notify_about_expiring_actions", + "notify_about_expiring_plans", + "notify_about_messages", +] diff --git a/src/open_inwoner/accounts/notifications/notify_expiring_actions.py b/src/open_inwoner/accounts/notifications/notify_expiring_actions.py new file mode 100644 index 0000000000..d28d2f3d76 --- /dev/null +++ b/src/open_inwoner/accounts/notifications/notify_expiring_actions.py @@ -0,0 +1,39 @@ +import logging + +from django.db.models.query import QuerySet +from django.urls import reverse + +from mail_editor.helpers import find_template + +from open_inwoner.accounts.models import Action, User +from open_inwoner.utils.url import build_absolute_url + +logger = logging.getLogger(__name__) + + +def notify_about_expiring_actions( + receiver: User, objects: QuerySet[Action], channel: str +): + send = _channels[channel] + + send(receiver=receiver, actions=objects) + + +def _send_email(receiver: User, actions: QuerySet[Action]): + actions_link = build_absolute_url(reverse("profile:action_list")) + template = find_template("expiring_action") + context = { + "actions": actions, + "actions_link": actions_link, + } + + template.send_email([receiver.email], context) + + logger.info( + f"The email was sent to the user {receiver} about {actions.count()} expiring actions" + ) + + +_channels = { + "email": _send_email, +} diff --git a/src/open_inwoner/accounts/notifications/notify_expiring_plans.py b/src/open_inwoner/accounts/notifications/notify_expiring_plans.py new file mode 100644 index 0000000000..4d964815d2 --- /dev/null +++ b/src/open_inwoner/accounts/notifications/notify_expiring_plans.py @@ -0,0 +1,42 @@ +import logging + +from django.db.models.query import QuerySet +from django.urls import reverse + +from mail_editor.helpers import find_template + +from open_inwoner.accounts.models import User +from open_inwoner.plans.models import Plan +from open_inwoner.userfeed import hooks as userfeed_hooks +from open_inwoner.utils.url import build_absolute_url + +logger = logging.getLogger(__name__) + + +def notify_about_expiring_plans(receiver: User, objects: QuerySet[Plan], channel: str): + for plan in objects: + userfeed_hooks.plan_expiring(receiver, plan) + + send = _channels[channel] + + send(receiver=receiver, plans=objects) + + +def _send_email(receiver: User, plans: QuerySet[Plan]): + plan_list_link = build_absolute_url(reverse("collaborate:plan_list")) + template = find_template("expiring_plan") + context = { + "plans": plans, + "plan_list_link": plan_list_link, + } + + template.send_email([receiver.email], context) + + logger.info( + f"The email was sent to the user {receiver} about {plans.count()} expiring plans" + ) + + +_channels = { + "email": _send_email, +} diff --git a/src/open_inwoner/accounts/notifications/notify_messages.py b/src/open_inwoner/accounts/notifications/notify_messages.py new file mode 100644 index 0000000000..5914923944 --- /dev/null +++ b/src/open_inwoner/accounts/notifications/notify_messages.py @@ -0,0 +1,47 @@ +import logging + +from django.db.models import Count +from django.db.models.query import QuerySet +from django.urls import reverse + +from mail_editor.helpers import find_template + +from open_inwoner.accounts.models import Message, User +from open_inwoner.utils.url import build_absolute_url + +logger = logging.getLogger(__name__) + + +def notify_about_messages(receiver: User, objects: QuerySet[Message], channel: str): + send = _channels[channel] + + send(receiver=receiver, messages=objects) + + +def _send_email(receiver: User, messages: QuerySet[Message]): + inbox_url = build_absolute_url(reverse("inbox:index")) + template = find_template("new_messages") + + total_messages = messages.count() + total_senders = messages.aggregate(total_senders=Count("sender", distinct=True))[ + "total_senders" + ] + + context = { + "total_messages": total_messages, + "total_senders": total_senders, + "inbox_link": inbox_url, + } + + template.send_email([receiver.email], context) + + messages.update(sent=True) + + logger.info( + f"The email was sent to the user {receiver} about {total_messages} new messages" + ) + + +_channels = { + "email": _send_email, +} diff --git a/src/open_inwoner/accounts/tasks.py b/src/open_inwoner/accounts/tasks.py new file mode 100644 index 0000000000..1e37a79e34 --- /dev/null +++ b/src/open_inwoner/accounts/tasks.py @@ -0,0 +1,116 @@ +from datetime import date +from typing import Callable + +from django.db.models import Q +from django.db.models.query import QuerySet +from django.utils.translation import gettext as _ + +from celery import group +from celery.result import allow_join_result + +from open_inwoner.accounts.models import Action, Message, User +from open_inwoner.celery import app +from open_inwoner.plans.models import Plan +from open_inwoner.utils.constants import NotificationChannel + +from .notifications import ( + notify_about_expiring_actions, + notify_about_expiring_plans, + notify_about_messages, +) + + +@app.task +def send_email(receiver: User, objects: QuerySet, notify: Callable): + """ + Send email to `receiver` about `objects` using `notify` + """ + notify( + receiver=receiver, + objects=objects, + channel=NotificationChannel.email, + ) + + +@app.task(name=_("Send emails about expiring actions")) +def send_emails_about_expiring_actions(): + today = date.today() + user_ids = Action.objects.expiring_actions_user_ids(end_date=today) + receivers = User.objects.filter( + is_active=True, plans_notifications=True, pk__in=user_ids + ).distinct() + + task_group = group( + send_email.s( + receiver=receiver, + objects=Action.objects.filter(is_for=receiver, end_date=today), + notify=notify_about_expiring_actions, + ) + for receiver in receivers + ) + + promise = task_group.apply_async() + with allow_join_result(): + return promise.get() + + +@app.task(name=_("Send emails about expiring plans")) +def send_emails_about_expiring_plans(): + today = date.today() + user_ids = Plan.objects.expiring_plans_user_ids(end_date=today) + receivers = User.objects.filter( + is_active=True, plans_notifications=True, pk__in=user_ids + ).distinct() + + task_group = group( + send_email.s( + receiver=receiver, + objects=Plan.objects.filter(end_date=today).filter( + Q(created_by=receiver) | Q(plan_contacts=receiver) + ), + notify=notify_about_expiring_plans, + ) + for receiver in receivers + ) + + promise = task_group.apply_async() + with allow_join_result(): + return promise.get() + + +@app.task(name=_("Send emails about messages")) +def send_emails_about_messages(): + receivers = User.objects.filter( + received_messages__seen=False, + received_messages__sent=False, + is_active=True, + messages_notifications=True, + ).distinct() + + email_data = [] + + for receiver in receivers: + messages_to_update = Message.objects.filter( + receiver=receiver, + seen=False, + sent=False, + ) + email_data.append( + { + "receiver": receiver, + "messages_to_update": messages_to_update, + } + ) + + task_group = group( + send_email.s( + receiver=data["receiver"], + objects=data["messages_to_update"], + notify=notify_about_messages, + ) + for data in email_data + ) + + promise = task_group.apply_async() + with allow_join_result(): + return promise.get() diff --git a/src/open_inwoner/accounts/templates/accounts/registration_necessary.html b/src/open_inwoner/accounts/templates/accounts/registration_necessary.html index 95d0a02fbd..bec0a12c26 100644 --- a/src/open_inwoner/accounts/templates/accounts/registration_necessary.html +++ b/src/open_inwoner/accounts/templates/accounts/registration_necessary.html @@ -34,6 +34,7 @@

{% trans "Registratie voltooien" %}


Notificatie voorkeuren

{# Start of multiple checkbox fields #} + {# TODO: only show notifications if enabled in siteconfig #}