From 56e454cc8b9d2f75dcf83cc6aec7c94ed0709bb7 Mon Sep 17 00:00:00 2001 From: Elia Bieri Date: Fri, 25 Aug 2023 09:39:14 +0200 Subject: [PATCH 01/11] Squash merge of code from Elia --- README.md | 6 +- compose/django/Dockerfile | 19 +- compose/django/docker_start.sh | 5 + .../django/send_attendance_course_reminders | 1 + server/config/settings/base.py | 2 +- server/vbv_lernwelt/assignment/services.py | 4 +- .../course/creators/test_course.py | 8 +- server/vbv_lernwelt/feedback/models.py | 3 +- server/vbv_lernwelt/importer/services.py | 10 +- .../importer/tests/test_import_students.py | 1 + .../send_attendance_course_reminders.py | 65 +++++++ server/vbv_lernwelt/notify/service.py | 175 ++++++++++++++---- .../test_send_attendance_course_reminders.py | 125 +++++++++++++ .../vbv_lernwelt/notify/tests/test_service.py | 104 +++++++++-- trufflehog-allow.json | 9 +- 15 files changed, 469 insertions(+), 68 deletions(-) create mode 100644 compose/django/send_attendance_course_reminders create mode 100644 server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py create mode 100644 server/vbv_lernwelt/notify/tests/test_send_attendance_course_reminders.py diff --git a/README.md b/README.md index 38346e28..68c667d1 100644 --- a/README.md +++ b/README.md @@ -121,12 +121,12 @@ There are multiple ways on how to add new translations to Locize: ### Process one: Let Locize add missing keys automatically When running the app, it will automatically add the missing translation -keys to Locize. +keys to Locize. There you can translate them, and also add the German translation. ### Process two: Add keys manually -You can add the new keys manually to the German locale file in +You can add the new keys manually to the German locale file in ./client/locales/de/translation.json Then you can run the following command to add the keys to Locize: @@ -149,7 +149,6 @@ The command will add the keys and the German translation to Locize. Bonus: Use the "i18n ally" plugin in VSCode or IntelliJ to get extract untranslated texts directly from the code to the translation.json file. - ### "_many" plural form in French and Italian See https://github.com/i18next/i18next/issues/1691#issuecomment-968063348 @@ -296,7 +295,6 @@ npm run dev If you run `npm run dev`, the codegen command will be run automatically in watch mode. - For the `ObjectTypes` on the server, please use the postfix `ObjectType` for types, like `LearningContentAttendanceCourseObjectType`. diff --git a/compose/django/Dockerfile b/compose/django/Dockerfile index aa489641..85e4ba33 100644 --- a/compose/django/Dockerfile +++ b/compose/django/Dockerfile @@ -18,13 +18,28 @@ COPY ./server ${APP_HOME} # define an alias for the specfic python version used in this file. FROM python:${PYTHON_VERSION} as python +# Setup Supersonic (Cron scheduler for containers) +# https://github.com/aptible/supercronic +ENV SUPERCRONIC_URL=https://github.com/aptible/supercronic/releases/download/v0.2.26/supercronic-linux-amd64 \ + SUPERCRONIC=supercronic-linux-amd64 \ + SUPERCRONIC_SHA1SUM=7a79496cf8ad899b99a719355d4db27422396735 + +RUN apt-get update && apt-get install --no-install-recommends -y curl +RUN curl -fsSLO "$SUPERCRONIC_URL" \ + && echo "${SUPERCRONIC_SHA1SUM} ${SUPERCRONIC}" | sha1sum -c - \ + && chmod +x "$SUPERCRONIC" \ + && mv "$SUPERCRONIC" "/usr/local/bin/${SUPERCRONIC}" \ + && ln -s "/usr/local/bin/${SUPERCRONIC}" /usr/local/bin/supercronic + +COPY ./compose/django/send_attendance_course_reminders /app/send_attendance_course_reminders + # Python build stage FROM python as python-build-stage ARG BUILD_ENVIRONMENT=production # Install apt packages -RUN apt-get update && apt-get install --no-install-recommends -y \ +RUN apt-get install --no-install-recommends -y \ # dependencies for building Python packages build-essential \ # psycopg2 dependencies @@ -56,7 +71,7 @@ RUN addgroup --system django \ # Install required system dependencies -RUN apt-get update && apt-get install --no-install-recommends -y \ +RUN apt-get install --no-install-recommends -y \ # psycopg2 dependencies libpq-dev \ # Translations dependencies diff --git a/compose/django/docker_start.sh b/compose/django/docker_start.sh index d78b21d7..2f359bd2 100644 --- a/compose/django/docker_start.sh +++ b/compose/django/docker_start.sh @@ -15,4 +15,9 @@ else python /app/manage.py migrate fi +if [[ $IT_APP_ENVIRONMENT != dev* ]]; then + # Start periodic tasks + /usr/local/bin/supercronic -sentry-dsn "$IT_SENTRY_DSN" -split-logs /app/send_attendance_course_reminders & +fi + newrelic-admin run-program gunicorn config.asgi --bind 0.0.0.0:7555 --chdir=/app -k uvicorn.workers.UvicornWorker diff --git a/compose/django/send_attendance_course_reminders b/compose/django/send_attendance_course_reminders new file mode 100644 index 00000000..4d3af4d1 --- /dev/null +++ b/compose/django/send_attendance_course_reminders @@ -0,0 +1 @@ +@daily /usr/local/bin/python /app/manage.py send_attendance_course_reminders diff --git a/server/config/settings/base.py b/server/config/settings/base.py index 87ffd78f..1465ca93 100644 --- a/server/config/settings/base.py +++ b/server/config/settings/base.py @@ -636,7 +636,7 @@ EDONIQ_CERTIFICATE = env("IT_EDONIQ_CERTIFICATE", default="") # Notifications # django-notifications -DJANGO_NOTIFICATIONS_CONFIG = {"SOFT_DELETE": True} +DJANGO_NOTIFICATIONS_CONFIG = {"SOFT_DELETE": True, "USE_JSONFIELD": True} NOTIFICATIONS_NOTIFICATION_MODEL = "notify.Notification" # sendgrid (email notifications) SENDGRID_API_KEY = env("IT_SENDGRID_API_KEY", default="") diff --git a/server/vbv_lernwelt/assignment/services.py b/server/vbv_lernwelt/assignment/services.py index e31913d0..66487d6d 100644 --- a/server/vbv_lernwelt/assignment/services.py +++ b/server/vbv_lernwelt/assignment/services.py @@ -16,7 +16,7 @@ from vbv_lernwelt.core.models import User from vbv_lernwelt.core.utils import find_first from vbv_lernwelt.course.models import CourseCompletionStatus, CourseSession from vbv_lernwelt.course.services import mark_course_completion -from vbv_lernwelt.notify.service import NotificationService +from vbv_lernwelt.notify.service import EmailTemplate, NotificationService def update_assignment_completion( @@ -144,6 +144,7 @@ def update_assignment_completion( sender=ac.assignment_user, course=course_session.course.title, target_url=ac.get_assignment_evaluation_frontend_url(), + email_template=EmailTemplate.CASEWORK_SUBMITTED, ) elif completion_status == AssignmentCompletionStatus.EVALUATION_SUBMITTED: ac.evaluation_submitted_at = timezone.now() @@ -162,6 +163,7 @@ def update_assignment_completion( sender=evaluation_user, course=course_session.course.title, target_url=assignment_frontend_url, + email_template=EmailTemplate.CASEWORK_EVALUATED, ) ac.completion_status = completion_status.value diff --git a/server/vbv_lernwelt/course/creators/test_course.py b/server/vbv_lernwelt/course/creators/test_course.py index f79428da..264965bf 100644 --- a/server/vbv_lernwelt/course/creators/test_course.py +++ b/server/vbv_lernwelt/course/creators/test_course.py @@ -105,14 +105,14 @@ def create_test_course(include_uk=True, include_vv=True, with_sessions=False): location="Handelsschule KV Bern, Zimmer 123, Eigerstrasse 16, 3012 Bern", trainer="Roland Grossenbacher, roland.grossenbacher@helvetia.ch", ) - tuesday_in_two_weeks = ( - datetime.now() + relativedelta(weekday=TU(2)) + relativedelta(weeks=2) + tuesday_in_one_week = ( + datetime.now() + relativedelta(weekday=TU) + relativedelta(weeks=1) ) csac.due_date.start = timezone.make_aware( - tuesday_in_two_weeks.replace(hour=8, minute=30, second=0, microsecond=0) + tuesday_in_one_week.replace(hour=8, minute=30, second=0, microsecond=0) ) csac.due_date.end = timezone.make_aware( - tuesday_in_two_weeks.replace(hour=17, minute=0, second=0, microsecond=0) + tuesday_in_one_week.replace(hour=17, minute=0, second=0, microsecond=0) ) csac.due_date.save() diff --git a/server/vbv_lernwelt/feedback/models.py b/server/vbv_lernwelt/feedback/models.py index f9c0aaf1..b12cb286 100644 --- a/server/vbv_lernwelt/feedback/models.py +++ b/server/vbv_lernwelt/feedback/models.py @@ -6,7 +6,7 @@ from django.utils.translation import gettext_lazy as _ from vbv_lernwelt.core.models import User from vbv_lernwelt.course.models import CourseSessionUser -from vbv_lernwelt.notify.service import NotificationService +from vbv_lernwelt.notify.service import EmailTemplate, NotificationService class FeedbackIntegerField(models.IntegerField): @@ -55,6 +55,7 @@ class FeedbackResponse(models.Model): recipient=csu.user, verb=f"{_('New feedback for circle')} {self.circle.title}", target_url=f"/course/{self.course_session.course.slug}/cockpit/feedback/{self.circle_id}/", + email_template=EmailTemplate.NEW_FEEDBACK, ) super(FeedbackResponse, self).save(*args, **kwargs) diff --git a/server/vbv_lernwelt/importer/services.py b/server/vbv_lernwelt/importer/services.py index 3e9802c1..912b6145 100644 --- a/server/vbv_lernwelt/importer/services.py +++ b/server/vbv_lernwelt/importer/services.py @@ -25,6 +25,7 @@ from vbv_lernwelt.learnpath.models import ( LearningContentAttendanceCourse, LearningContentEdoniqTest, ) +from vbv_lernwelt.notify.models import NotificationType logger = structlog.get_logger(__name__) @@ -248,7 +249,11 @@ def create_or_update_user( if not user: # create user - user = User(sso_id=sso_id, email=email, username=email) + user = User( + sso_id=sso_id, + email=email, + username=email, + ) user.email = email user.sso_id = user.sso_id or sso_id @@ -699,8 +704,9 @@ def sync_students_from_t2l(data): def update_user_json_data(user: User, data: Dict[str, Any]): + # Set E-Mail notification settings for new users user.additional_json_data = user.additional_json_data | sanitize_json_data_input( - data + {**data, "email_notification_types": [str(NotificationType.INFORMATION)]} ) diff --git a/server/vbv_lernwelt/importer/tests/test_import_students.py b/server/vbv_lernwelt/importer/tests/test_import_students.py index d771d89d..46880b60 100644 --- a/server/vbv_lernwelt/importer/tests/test_import_students.py +++ b/server/vbv_lernwelt/importer/tests/test_import_students.py @@ -52,6 +52,7 @@ class CreateOrUpdateStudentTestCase(TestCase): "Lehrvertragsnummer": "1234", "Tel. Privat": "079 593 83 43", "Geburtsdatum": "01.01.2000", + "email_notification_types": ["INFORMATION"], } def test_create_student(self): diff --git a/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py b/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py new file mode 100644 index 00000000..7690d744 --- /dev/null +++ b/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py @@ -0,0 +1,65 @@ +from datetime import timedelta + +import djclick as click +import structlog +from django.utils import timezone +from django.utils.translation import gettext_lazy as _ + +from vbv_lernwelt.core.models import User +from vbv_lernwelt.course.models import CourseSessionUser +from vbv_lernwelt.course_session.models import CourseSessionAttendanceCourse +from vbv_lernwelt.notify.service import EmailTemplate, NotificationService + +logger = structlog.get_logger(__name__) + +PRESENCE_COURSE_REMINDER_LEAD_TIME = timedelta(weeks=2) +DATETIME_FORMAT_STR = "%H:%M %d.%m.%Y" + + +def format_datetime(dt: timezone.datetime) -> str: + return dt.astimezone(timezone.get_current_timezone()).strftime(DATETIME_FORMAT_STR) + + +def send_notification( + recipient: User, attendance_course: CourseSessionAttendanceCourse +): + NotificationService.send_information_notification( + recipient=recipient, + verb=_("Erinnerung: Bald findet ein Präsenzkurs statt"), + target_url=attendance_course.learning_content.get_frontend_url(), + email_template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, + template_data={ + "attendance_course": attendance_course.learning_content.title, + "location": attendance_course.location, + "trainer": attendance_course.trainer, + "start": format_datetime(attendance_course.due_date.start), + "end": format_datetime(attendance_course.due_date.end), + }, + ) + + +def check_attendance_course(): + """Checks if an attendance course is coming up and sends a reminder to the participants""" + start = timezone.now() + end = timezone.now() + PRESENCE_COURSE_REMINDER_LEAD_TIME + logger.info( + "Querying for attendance courses in specified time range", + start_time=start.strftime(DATETIME_FORMAT_STR), + end_time=end.strftime(DATETIME_FORMAT_STR), + ) + attendance_courses = CourseSessionAttendanceCourse.objects.filter( + due_date__start__lte=end, + due_date__start__gte=start, + ) + for attendance_course in attendance_courses: + cs_id = attendance_course.course_session.id + csu = CourseSessionUser.objects.filter(course_session_id=cs_id) + for user in csu: + send_notification(user.user, attendance_course) + if not attendance_courses: + logger.info("No attendance courses found") + + +@click.command() +def command(): + check_attendance_course() diff --git a/server/vbv_lernwelt/notify/service.py b/server/vbv_lernwelt/notify/service.py index b0c07b79..2f0ea02e 100644 --- a/server/vbv_lernwelt/notify/service.py +++ b/server/vbv_lernwelt/notify/service.py @@ -1,3 +1,4 @@ +from enum import Enum from typing import Optional import structlog @@ -11,35 +12,60 @@ from vbv_lernwelt.notify.models import Notification, NotificationType logger = structlog.get_logger(__name__) +class EmailTemplate(Enum): + """Enum for the different Sendgrid email templates.""" + + # VBV - Erinnerung Präsenzkurse + ATTENDANCE_COURSE_REMINDER = { + "de": "d-9af079f98f524d85ac6e4166de3480da", + "it": "d-ab78ddca8a7a46b8afe50aaba3efee81", + "fr": "d-f88d9912e5484e55a879571463e4a166", + } + + # VBV - Geleitete Fallarbeit abgegeben + CASEWORK_SUBMITTED = {"de": "d-599f0b35ddcd4fac99314cdf8f5446a2"} + + # VBV - Geleitete Fallarbeit bewertet + CASEWORK_EVALUATED = {"de": "d-8c57fa13116b47be8eec95dfaf2aa030"} + + # VBV - Neues Feedback für Circle + NEW_FEEDBACK = {"de": "d-40fb94d5149949e7b8e7ddfcf0fcfdde"} + + class EmailService: + """Email service class implemented using the Sendgrid API""" + _sendgrid_client = SendGridAPIClient(setting("SENDGRID_API_KEY")) @classmethod - def send_email(cls, recipient: User, verb: str, target_url) -> bool: + def send_email( + cls, + recipient: User, + template: EmailTemplate, + template_data: dict, + ) -> None: message = Mail( - from_email="info@iterativ.ch", + from_email="noreply@my.vbv-afa.ch", to_emails=recipient.email, - subject=f"myVBV - {verb}", - ## TODO: Add HTML content. - html_content=f"{verb}: Link", ) - try: - cls._sendgrid_client.send(message) - logger.info(f"Successfully sent email to {recipient}", label="email") - return True - except Exception as e: - logger.error( - f"Failed to send email to {recipient}: {e}", - exc_info=True, - label="email", - ) - return False + message.template_id = template.value.get( + recipient.language, template.value["de"] + ) + message.dynamic_template_data = template_data + cls._sendgrid_client.send(message) class NotificationService: @classmethod def send_user_interaction_notification( - cls, recipient: User, verb: str, sender: User, course: str, target_url: str + cls, + recipient: User, + verb: str, + sender: User, + course: str, + target_url: str, + email_template: EmailTemplate, + template_data: dict = {}, ) -> None: cls._send_notification( recipient=recipient, @@ -48,11 +74,19 @@ class NotificationService: course=course, target_url=target_url, notification_type=NotificationType.USER_INTERACTION, + email_template=email_template, + template_data=template_data, ) @classmethod def send_progress_notification( - cls, recipient: User, verb: str, course: str, target_url: str + cls, + recipient: User, + verb: str, + course: str, + target_url: str, + email_template: EmailTemplate, + template_data: dict = {}, ) -> None: cls._send_notification( recipient=recipient, @@ -60,17 +94,26 @@ class NotificationService: course=course, target_url=target_url, notification_type=NotificationType.PROGRESS, + email_template=email_template, + template_data=template_data, ) @classmethod def send_information_notification( - cls, recipient: User, verb: str, target_url: str + cls, + recipient: User, + verb: str, + target_url: str, + email_template: EmailTemplate, + template_data: dict = {}, ) -> None: cls._send_notification( recipient=recipient, verb=verb, target_url=target_url, notification_type=NotificationType.INFORMATION, + email_template=email_template, + template_data=template_data, ) @classmethod @@ -79,42 +122,66 @@ class NotificationService: recipient: User, verb: str, notification_type: NotificationType, - sender: Optional[User] = None, - course: Optional[str] = None, - target_url: Optional[str] = None, + email_template: EmailTemplate, + template_data: dict, + sender: User | None = None, + course: str | None = None, + target_url: str | None = None, ) -> None: actor_avatar_url: Optional[str] = None if not sender: sender = User.objects.get(email="admin") else: actor_avatar_url = sender.avatar_url - emailed = False - if cls._should_send_email(notification_type, recipient): - emailed = cls._send_email(recipient, verb, target_url) log = logger.bind( + recipient=recipient.get_full_name(), + sender=sender.get_full_name(), verb=verb, notification_type=notification_type, - sender=sender.get_full_name(), - recipient=recipient.get_full_name(), course=course, target_url=target_url, + template_data=template_data, ) + if NotificationService._is_duplicate_notification( + recipient=recipient, + verb=verb, + notification_type=notification_type, + target_url=target_url, + template_name=email_template.name, + template_data=template_data, + ): + log.warn("A duplicate notification was omitted from being sent") + return + + emailed = False + if cls._should_send_email(notification_type, recipient): + emailed = cls._send_email( + recipient=recipient, + template=email_template, + template_data={ + "target_url": f"https://my.vbv-afa.ch{target_url}", + **template_data, + }, + ) + try: response = notify.send( sender=sender, recipient=recipient, verb=verb, + emailed=emailed, + # The metadata is saved in the 'data' member of the AbstractNotification model + email_template=email_template.name, + template_data=template_data, ) - sent_notification: Notification = response[0][1][0] + sent_notification: Notification = response[0][1][0] # 🫨 sent_notification.target_url = target_url sent_notification.notification_type = notification_type sent_notification.course = course sent_notification.actor_avatar_url = actor_avatar_url - sent_notification.emailed = emailed sent_notification.save() except Exception as e: - log.bind(exception=e) - log.error("Failed to send notification") + log.error("Failed to send notification", exception=str(e)) else: log.info("Notification sent successfully") @@ -127,13 +194,49 @@ class NotificationService: ) @staticmethod - def _send_email(recipient: User, verb: str, target_url: Optional[str]) -> bool: + def _send_email( + recipient: User, + template: EmailTemplate, + template_data: dict | None, + ) -> bool: + log = logger.bind( + recipient=recipient.username, + template=template.name, + template_data=template_data, + ) try: - return EmailService.send_email( + EmailService.send_email( recipient=recipient, - verb=verb, - target_url=target_url, + template=template, + template_data=template_data, ) + log.info("Email sent successfully") + return True except Exception as e: - logger.error(f"Failed to send email to {recipient}: {e}") + log.error( + "Failed to send Email", exception=str(e), exc_info=True, stack_info=True + ) return False + + @staticmethod + def _is_duplicate_notification( + recipient: User, + verb: str, + notification_type: NotificationType, + template_name: str, + template_data: dict, + target_url: str | None, + ) -> bool: + """Check if a notification with the same parameters has already been sent to the recipient. + This is to prevent duplicate notifications from being sent and to protect against potential programming errors. + """ + return Notification.objects.filter( + recipient=recipient, + verb=verb, + notification_type=notification_type, + target_url=target_url, + data={ + "email_template": template_name, + "template_data": template_data, + }, + ).exists() diff --git a/server/vbv_lernwelt/notify/tests/test_send_attendance_course_reminders.py b/server/vbv_lernwelt/notify/tests/test_send_attendance_course_reminders.py new file mode 100644 index 00000000..e618d6fb --- /dev/null +++ b/server/vbv_lernwelt/notify/tests/test_send_attendance_course_reminders.py @@ -0,0 +1,125 @@ +from dataclasses import dataclass +from datetime import datetime, timedelta +from unittest.mock import patch + +from django.contrib.auth.models import User +from django.test import TestCase +from django.utils import timezone +from django.utils.translation import gettext_lazy as _ + +from vbv_lernwelt.core.constants import TEST_COURSE_SESSION_BERN_ID +from vbv_lernwelt.core.create_default_users import create_default_users +from vbv_lernwelt.course.creators.test_course import create_test_course +from vbv_lernwelt.course.models import CourseSession, CourseSessionUser +from vbv_lernwelt.course_session.models import CourseSessionAttendanceCourse +from vbv_lernwelt.learnpath.models import LearningContentAttendanceCourse +from vbv_lernwelt.notify.management.commands.send_attendance_course_reminders import ( + check_attendance_course, +) +from vbv_lernwelt.notify.service import EmailTemplate + + +@dataclass +class SentNotification: + recipient: User + verb: str + target_url: str + email_template: EmailTemplate + template_data: dict + + +sent_notifications: list[SentNotification] = [] + + +def on_send_notification(**kwargs) -> None: + sent_notifications.append(SentNotification(**kwargs)) + + +@patch( + "vbv_lernwelt.notify.service.NotificationService.send_information_notification", + on_send_notification, +) +class TestAttendanceCourseReminders(TestCase): + def setUp(self): + create_default_users() + create_test_course(with_sessions=True) + CourseSessionAttendanceCourse.objects.all().delete() + + cs_bern = CourseSession.objects.get( + id=TEST_COURSE_SESSION_BERN_ID, + ) + self.csac = CourseSessionAttendanceCourse.objects.create( + course_session=cs_bern, + learning_content=LearningContentAttendanceCourse.objects.get( + slug="test-lehrgang-lp-circle-fahrzeug-lc-präsenzkurs-fahrzeug" + ), + location="Handelsschule KV Bern, Zimmer 123, Eigerstrasse 16, 3012 Bern", + trainer="Roland Grossenbacher", + ) + in_one_week = datetime.now() + timedelta(weeks=1) + self.csac.due_date.start = timezone.make_aware( + in_one_week.replace(hour=7, minute=30, second=0, microsecond=0) + ) + + self.csac.due_date.end = timezone.make_aware( + in_one_week.replace(hour=16, minute=15, second=0, microsecond=0) + ) + self.csac.due_date.save() + + # Attendance course more than two weeks in the future + self.csac_future = CourseSessionAttendanceCourse.objects.create( + course_session=cs_bern, + learning_content=LearningContentAttendanceCourse.objects.get( + slug="test-lehrgang-lp-circle-fahrzeug-lc-präsenzkurs-fahrzeug" + ), + location="Handelsschule BV Bern, Zimmer 122", + trainer="Thomas Berger", + ) + in_two_weeks = datetime.now() + timedelta(weeks=2, days=1) + self.csac_future.due_date.start = timezone.make_aware( + in_two_weeks.replace(hour=5, minute=20, second=0, microsecond=0) + ) + self.csac_future.due_date.end = timezone.make_aware( + in_two_weeks.replace(hour=15, minute=20, second=0, microsecond=0) + ) + self.csac_future.due_date.save() + + def test_happy_day(self): + check_attendance_course() + self.assertEquals(3, len(sent_notifications)) + recipients = CourseSessionUser.objects.filter( + course_session_id=self.csac.course_session.id + ) + self.assertEquals( + set(map(lambda n: n.recipient, sent_notifications)), + set(map(lambda csu: csu.user, recipients)), + ) + for notification in sent_notifications: + self.assertEquals( + _("Erinnerung: Bald findet ein Präsenzkurs statt"), + notification.verb, + ) + self.assertEquals( + "/course/test-lehrgang/learn/fahrzeug/präsenzkurs-fahrzeug", + notification.target_url, + ) + self.assertEquals( + self.csac.learning_content.title, + notification.template_data["attendance_course"], + ) + self.assertEquals( + self.csac.location, + notification.template_data["location"], + ) + self.assertEquals( + self.csac.trainer, + notification.template_data["trainer"], + ) + self.assertEquals( + self.csac.due_date.start.strftime("%H:%M %d.%m.%Y"), + notification.template_data["start"], + ) + self.assertEquals( + self.csac.due_date.end.strftime("%H:%M %d.%m.%Y"), + notification.template_data["end"], + ) diff --git a/server/vbv_lernwelt/notify/tests/test_service.py b/server/vbv_lernwelt/notify/tests/test_service.py index b98f51b0..423245a1 100644 --- a/server/vbv_lernwelt/notify/tests/test_service.py +++ b/server/vbv_lernwelt/notify/tests/test_service.py @@ -1,18 +1,25 @@ import json -from typing import List from django.test import TestCase from vbv_lernwelt.core.models import User from vbv_lernwelt.core.tests.factories import UserFactory from vbv_lernwelt.notify.models import Notification, NotificationType -from vbv_lernwelt.notify.service import NotificationService +from vbv_lernwelt.notify.service import EmailTemplate, NotificationService class TestNotificationService(TestCase): + def _on_send_email(self, *_, **__) -> bool: + self._emails_sent += 1 + return True + + def _has_sent_emails(self) -> bool: + return self._emails_sent != 0 + def setUp(self) -> None: + self._emails_sent = 0 self.notification_service = NotificationService - self.notification_service._send_email = lambda a, b, c: True + self.notification_service._send_email = self._on_send_email self.admin = UserFactory(username="admin", email="admin") self.sender_username = "Bob" @@ -36,11 +43,10 @@ class TestNotificationService(TestCase): recipient=self.recipient, verb=verb, target_url=target_url, + email_template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, ) - - notifications: List[Notification] = Notification.objects.all() - self.assertEqual(1, len(notifications)) - notification = notifications[0] + self.assertEqual(1, Notification.objects.count()) + notification: Notification = Notification.objects.first() self.assertEqual(self.admin, notification.actor) self.assertEqual(verb, notification.verb) self.assertEqual(target_url, notification.target_url) @@ -54,12 +60,14 @@ class TestNotificationService(TestCase): target_url = "https://www.vbv.ch" course = "Versicherungsvermittler/in" self.notification_service.send_progress_notification( - recipient=self.recipient, verb=verb, target_url=target_url, course=course + recipient=self.recipient, + verb=verb, + target_url=target_url, + course=course, + email_template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, ) - - notifications: List[Notification] = Notification.objects.all() - self.assertEqual(1, len(notifications)) - notification = notifications[0] + self.assertEqual(1, Notification.objects.count()) + notification: Notification = Notification.objects.first() self.assertEqual(self.admin, notification.actor) self.assertEqual(verb, notification.verb) self.assertEqual(target_url, notification.target_url) @@ -77,11 +85,10 @@ class TestNotificationService(TestCase): verb=verb, target_url=target_url, course=course, + email_template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, ) - - notifications: List[Notification] = Notification.objects.all() - self.assertEqual(1, len(notifications)) - notification = notifications[0] + self.assertEqual(1, Notification.objects.count()) + notification: Notification = Notification.objects.first() self.assertEqual(self.sender, notification.actor) self.assertEqual(verb, notification.verb) self.assertEqual(target_url, notification.target_url) @@ -90,3 +97,68 @@ class TestNotificationService(TestCase): str(NotificationType.USER_INTERACTION), notification.notification_type ) self.assertTrue(notification.emailed) + + def test_does_not_send_duplicate_notification(self): + verb = "Anne hat deinen Auftrag bewertet" + target_url = "https://www.vbv.ch" + course = "Versicherungsvermittler/in" + + for i in range(2): + self.notification_service.send_user_interaction_notification( + sender=self.sender, + recipient=self.recipient, + verb=verb, + target_url=target_url, + course=course, + email_template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, + template_data={ + "blah": 123, + "foo": "ich habe hunger", + }, + ) + + self.assertEqual(1, Notification.objects.count()) + notification: Notification = Notification.objects.first() + self.assertEqual(self.sender, notification.actor) + self.assertEqual(verb, notification.verb) + self.assertEqual(target_url, notification.target_url) + self.assertEqual(course, notification.course) + self.assertEqual( + str(NotificationType.USER_INTERACTION), notification.notification_type + ) + self.assertTrue(notification.emailed) + + def test_only_sends_email_if_enabled(self): + # Assert no mail is sent if corresponding email notification type is not enabled + self.recipient.additional_json_data["email_notification_types"] = json.dumps( + ["INFORMATION"] + ) + self.recipient.save() + self.notification_service.send_user_interaction_notification( + sender=self.sender, + recipient=self.recipient, + verb="should not be sent", + target_url="", + course="", + email_template=EmailTemplate.CASEWORK_EVALUATED, + template_data={}, + ) + self.assertEqual(1, Notification.objects.count()) + self.assertFalse(self._has_sent_emails()) + + # Assert mail is sent if corresponding email notification type is enabled + self.recipient.additional_json_data["email_notification_types"] = json.dumps( + ["USER_INTERACTION"] + ) + self.recipient.save() + self.notification_service.send_user_interaction_notification( + sender=self.sender, + recipient=self.recipient, + verb="should be sent", + target_url="", + course="", + email_template=EmailTemplate.CASEWORK_EVALUATED, + template_data={}, + ) + self.assertEqual(2, Notification.objects.count()) + self.assertTrue(self._has_sent_emails()) diff --git a/trufflehog-allow.json b/trufflehog-allow.json index ec85115f..649d7e2f 100644 --- a/trufflehog-allow.json +++ b/trufflehog-allow.json @@ -11,5 +11,12 @@ "img base64 content": "regex:data:image/png;base64,.*", "sentry url": "https://2df6096a4fd94bd6b4802124d10e4b8d@o8544.ingest.sentry.io/4504157846372352", "git commit": "bdadf52b849bb5fa47854a3094f4da6fe9d54d02", - "customDomainVerificationId": "A2AB57353045150ADA4488FAA8AA9DFBBEDDD311934653F55243B336C2F3358E" + "customDomainVerificationId": "A2AB57353045150ADA4488FAA8AA9DFBBEDDD311934653F55243B336C2F3358E", + "SENDGRID_TEMPLATE_ATTENDANCE_COURSE_REMINDER_DE": "d-9af079f98f524d85ac6e4166de3480da", + "SENDGRID_TEMPLATE_ATTENDANCE_COURSE_REMINDER_FR": "d-f88d9912e5484e55a879571463e4a166", + "SENDGRID_TEMPLATE_ATTENDANCE_COURSE_REMINDER_IT": "d-ab78ddca8a7a46b8afe50aaba3efee81", + "SENDGRID_TEMPLATE_CASEWORK_SUBMITTED_DE": "d-599f0b35ddcd4fac99314cdf8f5446a2", + "SENDGRID_TEMPLATE_CASEWORK_EVALUATED_DE": "d-8c57fa13116b47be8eec95dfaf2aa030", + "SENDGRID_TEMPLATE_NEW_FEEDBACK_DE": "d-40fb94d5149949e7b8e7ddfcf0fcfdde", + "SUPERCRONIC_SHA1SUM": "7a79496cf8ad899b99a719355d4db27422396735" } From 31af4e933f2a2d6ab6f1035fa81315b7294759d6 Mon Sep 17 00:00:00 2001 From: Daniel Egger Date: Fri, 25 Aug 2023 09:52:37 +0200 Subject: [PATCH 02/11] Refactor default handling of dict parameter --- server/vbv_lernwelt/notify/service.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/server/vbv_lernwelt/notify/service.py b/server/vbv_lernwelt/notify/service.py index 2f0ea02e..5176928a 100644 --- a/server/vbv_lernwelt/notify/service.py +++ b/server/vbv_lernwelt/notify/service.py @@ -65,7 +65,7 @@ class NotificationService: course: str, target_url: str, email_template: EmailTemplate, - template_data: dict = {}, + template_data: dict = None, ) -> None: cls._send_notification( recipient=recipient, @@ -86,7 +86,7 @@ class NotificationService: course: str, target_url: str, email_template: EmailTemplate, - template_data: dict = {}, + template_data: dict = None, ) -> None: cls._send_notification( recipient=recipient, @@ -105,7 +105,7 @@ class NotificationService: verb: str, target_url: str, email_template: EmailTemplate, - template_data: dict = {}, + template_data: dict = None, ) -> None: cls._send_notification( recipient=recipient, @@ -123,11 +123,14 @@ class NotificationService: verb: str, notification_type: NotificationType, email_template: EmailTemplate, - template_data: dict, + template_data: dict | None = None, sender: User | None = None, course: str | None = None, target_url: str | None = None, ) -> None: + if template_data is None: + template_data = {} + actor_avatar_url: Optional[str] = None if not sender: sender = User.objects.get(email="admin") From d83f6609189cf02ae27ac3216069fb6ad8ccf33e Mon Sep 17 00:00:00 2001 From: Daniel Egger Date: Fri, 25 Aug 2023 11:48:50 +0200 Subject: [PATCH 03/11] Change email function to use email address directly --- env_secrets/local_daniel.env | Bin 446 -> 539 bytes scripts/send_sendgrid_email.py | 40 ++++++++ server/requirements/requirements-dev.in | 1 + server/requirements/requirements-dev.txt | 3 + server/vbv_lernwelt/assignment/services.py | 3 +- server/vbv_lernwelt/feedback/models.py | 3 +- .../notify/email/email_services.py | 97 ++++++++++++++++++ .../send_attendance_course_reminders.py | 33 +++--- .../notify/{service.py => services.py} | 71 ++----------- .../test_send_attendance_course_reminders.py | 18 ++-- .../vbv_lernwelt/notify/tests/test_service.py | 3 +- 11 files changed, 179 insertions(+), 93 deletions(-) create mode 100644 scripts/send_sendgrid_email.py create mode 100644 server/vbv_lernwelt/notify/email/email_services.py rename server/vbv_lernwelt/notify/{service.py => services.py} (74%) diff --git a/env_secrets/local_daniel.env b/env_secrets/local_daniel.env index 24e09d029825db39befd1a52594fade63ebc599c..2a0d57f30ff79c98c92fdc9e2bb20822e31df6f4 100644 GIT binary patch literal 539 zcmV+$0_6PwM@dveQdv+`0EotwUzQ0oC%UNcs%E$0d@d=7l%g|POhLb93rRHqg9PI-0 z1dBXDjv^e?2RT#kw6QN#5-bx@$mU&w?=Tt_%#0GC+I(s!mu>(NDL}MweEI=)x9n+2h^4pFUp2 z#(ta46n!d|-pIJG+~O^*4>Y8|4)v1_0v2Z)U>+Un=xyuRkb!Qp@ zc&wh=Ebl>ZvbSmDPx96krot0hq&|X77o=paVI^?TN4e=BO9B&dpb{njS)XX%S!@^A zn)L)Xj?b@A<$sv-c_?a1$Y8g(hfQLR;+?;K<(xn#_>7G`N4Uh@FL2x^Af5}_a9;;n d!XV|ESBRJ9<^l0|8hLlJT1gG+eV=PM7{B*y2?GEC literal 446 zcmV;v0YUx%M@dveQdv+`0M#OZ*THv%mFl8~OK177?bi``dTdcG9gEBWkiuw-c#yHU zyk(L{yGhqsK$>?l0#xL^+_6x+fQ5d(Hw+Tw4Fex(_){|>VZQ8buK{MjC5TpMm45Y+ zoEAEldde9}7r&j$S-J4U5&Wx*9YEdt{pA*ziiQN%Ec&`Lp4VTv%pHN9X2a(NKR(~` z#Mi|qyl*F0z0(sX!0OQMrfUQTVLhTa3EZzIU3 z8)fp@vc_S=CJK@|NM}zR8?Scia)vP0OKAFiMW)^gONEnSe9C1XDjyX2YYI9Tc*3pM zJ(ZNgPIEm|dd-68YNQ88lKVm|j%QT~eF3!q$f?7kA&T80=DZf*?;t92swbS7UUE-= zf0`q?qJ=`LPt07%y>=)Rn_1!(8vkUeWcCOhkg2{-CHNEj(wyeOQ_rDIz4&yd7T@HM oF7F0%cH7L?Pp&Q05w2VA;J-+bZR63z71m!sXCv#=9qm$RDn8fPd;kCd diff --git a/scripts/send_sendgrid_email.py b/scripts/send_sendgrid_email.py new file mode 100644 index 00000000..7a157eab --- /dev/null +++ b/scripts/send_sendgrid_email.py @@ -0,0 +1,40 @@ +# -*- coding: utf-8 -*- +import os +import sys + +import django + +sys.path.append("../server") + +os.environ.setdefault("IT_APP_ENVIRONMENT", "local") +os.environ.setdefault("DJANGO_SETTINGS_MODULE", "config.settings.base") +django.setup() + +from vbv_lernwelt.notify.email.email_services import ( + EmailTemplate, + send_email, + create_template_data_from_course_session_attendance_course, +) +from vbv_lernwelt.course_session.models import CourseSessionAttendanceCourse + + +def main(): + print("start") + + +if __name__ == "__main__": + main() + + csac = CourseSessionAttendanceCourse.objects.get(pk=1) + print(csac) + print(csac.trainer) + print(csac.due_date) + + result = send_email( + to_emails="daniel.egger+sendgrid@gmail.com", + template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, + template_data=create_template_data_from_course_session_attendance_course(csac), + template_language="de", + fail_silently=False, + ) + print(result) diff --git a/server/requirements/requirements-dev.in b/server/requirements/requirements-dev.in index e48515ea..bba2128e 100644 --- a/server/requirements/requirements-dev.in +++ b/server/requirements/requirements-dev.in @@ -30,6 +30,7 @@ django-debug-toolbar # https://github.com/jazzband/django-debug-toolbar django-extensions # https://github.com/django-extensions/django-extensions django-coverage-plugin # https://github.com/nedbat/django_coverage_plugin pytest-django # https://github.com/pytest-dev/pytest-django +freezegun # https://github.com/spulec/freezegun # django-watchfiles custom PR https://github.com/q0w/django-watchfiles/archive/issue-1.zip diff --git a/server/requirements/requirements-dev.txt b/server/requirements/requirements-dev.txt index 04cf3105..71898040 100644 --- a/server/requirements/requirements-dev.txt +++ b/server/requirements/requirements-dev.txt @@ -218,6 +218,8 @@ flake8==6.1.0 # flake8-isort flake8-isort==6.0.0 # via -r requirements-dev.in +freezegun==1.2.2 + # via -r requirements-dev.in gitdb==4.0.10 # via gitdb2 gitdb2==4.0.2 @@ -409,6 +411,7 @@ python-dateutil==2.8.2 # -r requirements.in # botocore # faker + # freezegun python-dotenv==1.0.0 # via # environs diff --git a/server/vbv_lernwelt/assignment/services.py b/server/vbv_lernwelt/assignment/services.py index 66487d6d..95e84229 100644 --- a/server/vbv_lernwelt/assignment/services.py +++ b/server/vbv_lernwelt/assignment/services.py @@ -16,7 +16,8 @@ from vbv_lernwelt.core.models import User from vbv_lernwelt.core.utils import find_first from vbv_lernwelt.course.models import CourseCompletionStatus, CourseSession from vbv_lernwelt.course.services import mark_course_completion -from vbv_lernwelt.notify.service import EmailTemplate, NotificationService +from vbv_lernwelt.notify.email.email_services import EmailTemplate +from vbv_lernwelt.notify.services import NotificationService def update_assignment_completion( diff --git a/server/vbv_lernwelt/feedback/models.py b/server/vbv_lernwelt/feedback/models.py index b12cb286..59b7dc9b 100644 --- a/server/vbv_lernwelt/feedback/models.py +++ b/server/vbv_lernwelt/feedback/models.py @@ -6,7 +6,8 @@ from django.utils.translation import gettext_lazy as _ from vbv_lernwelt.core.models import User from vbv_lernwelt.course.models import CourseSessionUser -from vbv_lernwelt.notify.service import EmailTemplate, NotificationService +from vbv_lernwelt.notify.email.email_services import EmailTemplate +from vbv_lernwelt.notify.services import NotificationService class FeedbackIntegerField(models.IntegerField): diff --git a/server/vbv_lernwelt/notify/email/email_services.py b/server/vbv_lernwelt/notify/email/email_services.py new file mode 100644 index 00000000..91be1747 --- /dev/null +++ b/server/vbv_lernwelt/notify/email/email_services.py @@ -0,0 +1,97 @@ +from enum import Enum + +import structlog +from django.conf import settings +from django.utils import timezone + +from sendgrid import Mail, SendGridAPIClient + +from vbv_lernwelt.course_session.models import CourseSessionAttendanceCourse + +logger = structlog.get_logger(__name__) + +DATETIME_FORMAT_SWISS_STR = "%d.%m.%Y %H:%M" + + +def format_swiss_datetime(dt: timezone.datetime) -> str: + return dt.astimezone(timezone.get_current_timezone()).strftime( + DATETIME_FORMAT_SWISS_STR + ) + + +class EmailTemplate(Enum): + """Enum for the different Sendgrid email templates.""" + + # VBV - Erinnerung Präsenzkurse + ATTENDANCE_COURSE_REMINDER = { + "de": "d-9af079f98f524d85ac6e4166de3480da", + "it": "d-ab78ddca8a7a46b8afe50aaba3efee81", + "fr": "d-f88d9912e5484e55a879571463e4a166", + } + + # VBV - Geleitete Fallarbeit abgegeben + CASEWORK_SUBMITTED = {"de": "d-599f0b35ddcd4fac99314cdf8f5446a2"} + + # VBV - Geleitete Fallarbeit bewertet + CASEWORK_EVALUATED = {"de": "d-8c57fa13116b47be8eec95dfaf2aa030"} + + # VBV - Neues Feedback für Circle + NEW_FEEDBACK = {"de": "d-40fb94d5149949e7b8e7ddfcf0fcfdde"} + + +def send_email( + to_emails: str | list[str], + template: EmailTemplate, + template_data: dict, + template_language: str = "de", + fail_silently: bool = True, +) -> bool: + log = logger.bind( + recipient_emails=to_emails, + template=template.name, + template_data=template_data, + template_language=template_language, + ) + try: + send_sendgrid_email( + to_emails=to_emails, + template=template, + template_data=template_data, + template_language=template_language, + ) + log.info("Email sent successfully") + return True + except Exception as e: + log.error( + "Failed to send Email", exception=str(e), exc_info=True, stack_info=True + ) + if not fail_silently: + raise e + return False + + +def send_sendgrid_email( + to_emails: str | list[str], + template: EmailTemplate, + template_data: dict, + template_language: str = "de", +) -> None: + message = Mail( + from_email="noreply@my.vbv-afa.ch", + to_emails=to_emails, + ) + message.template_id = template.value.get(template_language, template.value["de"]) + message.dynamic_template_data = template_data + SendGridAPIClient(settings.SENDGRID_API_KEY).send(message) + + +def create_template_data_from_course_session_attendance_course( + attendance_course: CourseSessionAttendanceCourse, +): + return { + "attendance_course": attendance_course.learning_content.title, + "location": attendance_course.location, + "trainer": attendance_course.trainer, + "start": format_swiss_datetime(attendance_course.due_date.start), + "end": format_swiss_datetime(attendance_course.due_date.end), + } diff --git a/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py b/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py index 7690d744..2cd8ae45 100644 --- a/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py +++ b/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py @@ -8,19 +8,18 @@ from django.utils.translation import gettext_lazy as _ from vbv_lernwelt.core.models import User from vbv_lernwelt.course.models import CourseSessionUser from vbv_lernwelt.course_session.models import CourseSessionAttendanceCourse -from vbv_lernwelt.notify.service import EmailTemplate, NotificationService +from vbv_lernwelt.notify.email.email_services import ( + create_template_data_from_course_session_attendance_course, + EmailTemplate, +) +from vbv_lernwelt.notify.services import NotificationService logger = structlog.get_logger(__name__) PRESENCE_COURSE_REMINDER_LEAD_TIME = timedelta(weeks=2) -DATETIME_FORMAT_STR = "%H:%M %d.%m.%Y" -def format_datetime(dt: timezone.datetime) -> str: - return dt.astimezone(timezone.get_current_timezone()).strftime(DATETIME_FORMAT_STR) - - -def send_notification( +def send_attendance_course_reminder_notification( recipient: User, attendance_course: CourseSessionAttendanceCourse ): NotificationService.send_information_notification( @@ -28,24 +27,20 @@ def send_notification( verb=_("Erinnerung: Bald findet ein Präsenzkurs statt"), target_url=attendance_course.learning_content.get_frontend_url(), email_template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, - template_data={ - "attendance_course": attendance_course.learning_content.title, - "location": attendance_course.location, - "trainer": attendance_course.trainer, - "start": format_datetime(attendance_course.due_date.start), - "end": format_datetime(attendance_course.due_date.end), - }, + template_data=create_template_data_from_course_session_attendance_course( + attendance_course=attendance_course + ), ) -def check_attendance_course(): +def attendance_course_reminder_notification_job(): """Checks if an attendance course is coming up and sends a reminder to the participants""" start = timezone.now() end = timezone.now() + PRESENCE_COURSE_REMINDER_LEAD_TIME logger.info( "Querying for attendance courses in specified time range", - start_time=start.strftime(DATETIME_FORMAT_STR), - end_time=end.strftime(DATETIME_FORMAT_STR), + start_time=start, + end_time=end, ) attendance_courses = CourseSessionAttendanceCourse.objects.filter( due_date__start__lte=end, @@ -55,11 +50,11 @@ def check_attendance_course(): cs_id = attendance_course.course_session.id csu = CourseSessionUser.objects.filter(course_session_id=cs_id) for user in csu: - send_notification(user.user, attendance_course) + send_attendance_course_reminder_notification(user.user, attendance_course) if not attendance_courses: logger.info("No attendance courses found") @click.command() def command(): - check_attendance_course() + attendance_course_reminder_notification_job() diff --git a/server/vbv_lernwelt/notify/service.py b/server/vbv_lernwelt/notify/services.py similarity index 74% rename from server/vbv_lernwelt/notify/service.py rename to server/vbv_lernwelt/notify/services.py index 5176928a..4a493dff 100644 --- a/server/vbv_lernwelt/notify/service.py +++ b/server/vbv_lernwelt/notify/services.py @@ -1,60 +1,15 @@ -from enum import Enum from typing import Optional import structlog from notifications.signals import notify -from sendgrid import Mail, SendGridAPIClient -from storages.utils import setting from vbv_lernwelt.core.models import User +from vbv_lernwelt.notify.email.email_services import EmailTemplate, send_email from vbv_lernwelt.notify.models import Notification, NotificationType logger = structlog.get_logger(__name__) -class EmailTemplate(Enum): - """Enum for the different Sendgrid email templates.""" - - # VBV - Erinnerung Präsenzkurse - ATTENDANCE_COURSE_REMINDER = { - "de": "d-9af079f98f524d85ac6e4166de3480da", - "it": "d-ab78ddca8a7a46b8afe50aaba3efee81", - "fr": "d-f88d9912e5484e55a879571463e4a166", - } - - # VBV - Geleitete Fallarbeit abgegeben - CASEWORK_SUBMITTED = {"de": "d-599f0b35ddcd4fac99314cdf8f5446a2"} - - # VBV - Geleitete Fallarbeit bewertet - CASEWORK_EVALUATED = {"de": "d-8c57fa13116b47be8eec95dfaf2aa030"} - - # VBV - Neues Feedback für Circle - NEW_FEEDBACK = {"de": "d-40fb94d5149949e7b8e7ddfcf0fcfdde"} - - -class EmailService: - """Email service class implemented using the Sendgrid API""" - - _sendgrid_client = SendGridAPIClient(setting("SENDGRID_API_KEY")) - - @classmethod - def send_email( - cls, - recipient: User, - template: EmailTemplate, - template_data: dict, - ) -> None: - message = Mail( - from_email="noreply@my.vbv-afa.ch", - to_emails=recipient.email, - ) - message.template_id = template.value.get( - recipient.language, template.value["de"] - ) - message.dynamic_template_data = template_data - cls._sendgrid_client.send(message) - - class NotificationService: @classmethod def send_user_interaction_notification( @@ -153,7 +108,7 @@ class NotificationService: template_name=email_template.name, template_data=template_data, ): - log.warn("A duplicate notification was omitted from being sent") + log.info("A duplicate notification was omitted from being sent") return emailed = False @@ -200,26 +155,14 @@ class NotificationService: def _send_email( recipient: User, template: EmailTemplate, - template_data: dict | None, + template_data: dict, ) -> bool: - log = logger.bind( - recipient=recipient.username, - template=template.name, + return send_email( + to_emails=recipient.email, + template=template, template_data=template_data, + template_language=recipient.language, ) - try: - EmailService.send_email( - recipient=recipient, - template=template, - template_data=template_data, - ) - log.info("Email sent successfully") - return True - except Exception as e: - log.error( - "Failed to send Email", exception=str(e), exc_info=True, stack_info=True - ) - return False @staticmethod def _is_duplicate_notification( diff --git a/server/vbv_lernwelt/notify/tests/test_send_attendance_course_reminders.py b/server/vbv_lernwelt/notify/tests/test_send_attendance_course_reminders.py index e618d6fb..04bc4cc2 100644 --- a/server/vbv_lernwelt/notify/tests/test_send_attendance_course_reminders.py +++ b/server/vbv_lernwelt/notify/tests/test_send_attendance_course_reminders.py @@ -6,6 +6,7 @@ from django.contrib.auth.models import User from django.test import TestCase from django.utils import timezone from django.utils.translation import gettext_lazy as _ +from freezegun import freeze_time from vbv_lernwelt.core.constants import TEST_COURSE_SESSION_BERN_ID from vbv_lernwelt.core.create_default_users import create_default_users @@ -13,10 +14,10 @@ from vbv_lernwelt.course.creators.test_course import create_test_course from vbv_lernwelt.course.models import CourseSession, CourseSessionUser from vbv_lernwelt.course_session.models import CourseSessionAttendanceCourse from vbv_lernwelt.learnpath.models import LearningContentAttendanceCourse +from vbv_lernwelt.notify.email.email_services import EmailTemplate from vbv_lernwelt.notify.management.commands.send_attendance_course_reminders import ( - check_attendance_course, + attendance_course_reminder_notification_job, ) -from vbv_lernwelt.notify.service import EmailTemplate @dataclass @@ -36,7 +37,7 @@ def on_send_notification(**kwargs) -> None: @patch( - "vbv_lernwelt.notify.service.NotificationService.send_information_notification", + "vbv_lernwelt.notify.services.NotificationService.send_information_notification", on_send_notification, ) class TestAttendanceCourseReminders(TestCase): @@ -75,6 +76,9 @@ class TestAttendanceCourseReminders(TestCase): location="Handelsschule BV Bern, Zimmer 122", trainer="Thomas Berger", ) + + @freeze_time("2023-08-25 13:02:01") + def test_happy_day(self): in_two_weeks = datetime.now() + timedelta(weeks=2, days=1) self.csac_future.due_date.start = timezone.make_aware( in_two_weeks.replace(hour=5, minute=20, second=0, microsecond=0) @@ -84,8 +88,7 @@ class TestAttendanceCourseReminders(TestCase): ) self.csac_future.due_date.save() - def test_happy_day(self): - check_attendance_course() + attendance_course_reminder_notification_job() self.assertEquals(3, len(sent_notifications)) recipients = CourseSessionUser.objects.filter( course_session_id=self.csac.course_session.id @@ -94,6 +97,7 @@ class TestAttendanceCourseReminders(TestCase): set(map(lambda n: n.recipient, sent_notifications)), set(map(lambda csu: csu.user, recipients)), ) + for notification in sent_notifications: self.assertEquals( _("Erinnerung: Bald findet ein Präsenzkurs statt"), @@ -116,10 +120,10 @@ class TestAttendanceCourseReminders(TestCase): notification.template_data["trainer"], ) self.assertEquals( - self.csac.due_date.start.strftime("%H:%M %d.%m.%Y"), + self.csac.due_date.start.strftime("%d.%m.%Y %H:%M"), notification.template_data["start"], ) self.assertEquals( - self.csac.due_date.end.strftime("%H:%M %d.%m.%Y"), + self.csac.due_date.end.strftime("%d.%m.%Y %H:%M"), notification.template_data["end"], ) diff --git a/server/vbv_lernwelt/notify/tests/test_service.py b/server/vbv_lernwelt/notify/tests/test_service.py index 423245a1..15dbac49 100644 --- a/server/vbv_lernwelt/notify/tests/test_service.py +++ b/server/vbv_lernwelt/notify/tests/test_service.py @@ -4,8 +4,9 @@ from django.test import TestCase from vbv_lernwelt.core.models import User from vbv_lernwelt.core.tests.factories import UserFactory +from vbv_lernwelt.notify.email.email_services import EmailTemplate from vbv_lernwelt.notify.models import Notification, NotificationType -from vbv_lernwelt.notify.service import EmailTemplate, NotificationService +from vbv_lernwelt.notify.services import NotificationService class TestNotificationService(TestCase): From 6badbc480c30494f219a69c7e904f0d75d856b46 Mon Sep 17 00:00:00 2001 From: Daniel Egger Date: Fri, 25 Aug 2023 16:39:14 +0200 Subject: [PATCH 04/11] Use django constance to add EMAIL_RECIPIENT_WHITELIST --- scripts/send_sendgrid_email.py | 2 +- server/config/settings/base.py | 10 ++++++ server/requirements/requirements-dev.txt | 5 +++ server/requirements/requirements.in | 1 + server/requirements/requirements.txt | 5 +++ .../notify/email/email_services.py | 36 +++++++++++-------- server/vbv_lernwelt/notify/services.py | 11 +++--- 7 files changed, 51 insertions(+), 19 deletions(-) diff --git a/scripts/send_sendgrid_email.py b/scripts/send_sendgrid_email.py index 7a157eab..c3dcb501 100644 --- a/scripts/send_sendgrid_email.py +++ b/scripts/send_sendgrid_email.py @@ -31,7 +31,7 @@ if __name__ == "__main__": print(csac.due_date) result = send_email( - to_emails="daniel.egger+sendgrid@gmail.com", + recipient_email="daniel.egger+sendgrid@gmail.com", template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, template_data=create_template_data_from_course_session_attendance_course(csac), template_language="de", diff --git a/server/config/settings/base.py b/server/config/settings/base.py index 1465ca93..3872dbdc 100644 --- a/server/config/settings/base.py +++ b/server/config/settings/base.py @@ -111,6 +111,7 @@ THIRD_PARTY_APPS = [ "graphene_django", "notifications", "django_jsonform", + "constance", ] LOCAL_APPS = [ @@ -687,6 +688,15 @@ WHITENOISE_SKIP_COMPRESS_EXTENSIONS = ( ) STATICFILES_STORAGE = "whitenoise.storage.CompressedManifestStaticFilesStorage" +CONSTANCE_BACKEND = "constance.backends.database.DatabaseBackend" +CONSTANCE_CONFIG = { + "EMAIL_RECIPIENT_WHITELIST": ( + "daniel.egger+sendgrid@gmail.com, elia.bieri@iterativ.ch", + "Which emails will recieve emails. Use single `*` for all OR comma separated list of emails. " + "Default value is empty and will not send any emails. (No regex support!)", + ), +} + if APP_ENVIRONMENT == "local": # http://whitenoise.evans.io/en/latest/django.html#using-whitenoise-in-development INSTALLED_APPS = ["whitenoise.runserver_nostatic"] + INSTALLED_APPS # noqa F405 diff --git a/server/requirements/requirements-dev.txt b/server/requirements/requirements-dev.txt index 71898040..a66e01e8 100644 --- a/server/requirements/requirements-dev.txt +++ b/server/requirements/requirements-dev.txt @@ -122,6 +122,7 @@ django==3.2.20 # django-modelcluster # django-notifications-hq # django-permissionedforms + # django-picklefield # django-redis # django-storages # django-stubs @@ -137,6 +138,8 @@ django==3.2.20 # wagtail-localize django-click==2.3.0 # via -r requirements.in +django-constance==3.1.0 + # via -r requirements.in django-cors-headers==4.2.0 # via -r requirements.in django-coverage-plugin==3.1.0 @@ -163,6 +166,8 @@ django-notifications-hq==1.8.2 # via -r requirements.in django-permissionedforms==0.1 # via wagtail +django-picklefield==3.1 + # via django-constance django-ratelimit==4.1.0 # via -r requirements.in django-redis==5.3.0 diff --git a/server/requirements/requirements.in b/server/requirements/requirements.in index a47be82c..ae9264d7 100644 --- a/server/requirements/requirements.in +++ b/server/requirements/requirements.in @@ -28,6 +28,7 @@ django-storages django-storages[azure] django-notifications-hq django-jsonform +django-constance psycopg2-binary gunicorn diff --git a/server/requirements/requirements.txt b/server/requirements/requirements.txt index 30ae967d..1261d7d5 100644 --- a/server/requirements/requirements.txt +++ b/server/requirements/requirements.txt @@ -84,6 +84,7 @@ django==3.2.20 # django-modelcluster # django-notifications-hq # django-permissionedforms + # django-picklefield # django-redis # django-storages # django-taggit @@ -96,6 +97,8 @@ django==3.2.20 # wagtail-localize django-click==2.3.0 # via -r requirements.in +django-constance==3.1.0 + # via -r requirements.in django-cors-headers==4.2.0 # via -r requirements.in django-csp==3.7 @@ -116,6 +119,8 @@ django-notifications-hq==1.8.2 # via -r requirements.in django-permissionedforms==0.1 # via wagtail +django-picklefield==3.1 + # via django-constance django-ratelimit==4.1.0 # via -r requirements.in django-redis==5.3.0 diff --git a/server/vbv_lernwelt/notify/email/email_services.py b/server/vbv_lernwelt/notify/email/email_services.py index 91be1747..d62d3421 100644 --- a/server/vbv_lernwelt/notify/email/email_services.py +++ b/server/vbv_lernwelt/notify/email/email_services.py @@ -1,9 +1,9 @@ from enum import Enum import structlog +from constance import config from django.conf import settings from django.utils import timezone - from sendgrid import Mail, SendGridAPIClient from vbv_lernwelt.course_session.models import CourseSessionAttendanceCourse @@ -40,27 +40,35 @@ class EmailTemplate(Enum): def send_email( - to_emails: str | list[str], + recipient_email: str, template: EmailTemplate, template_data: dict, template_language: str = "de", fail_silently: bool = True, ) -> bool: log = logger.bind( - recipient_emails=to_emails, + recipient_email=recipient_email, template=template.name, template_data=template_data, template_language=template_language, ) try: - send_sendgrid_email( - to_emails=to_emails, - template=template, - template_data=template_data, - template_language=template_language, - ) - log.info("Email sent successfully") - return True + whitelist_emails = [ + email.strip() + for email in config.EMAIL_RECIPIENT_WHITELIST.strip().split(",") + ] + if "*" in whitelist_emails or recipient_email in whitelist_emails: + _send_sendgrid_email( + recipient_email=recipient_email, + template=template, + template_data=template_data, + template_language=template_language, + ) + log.info("Email sent successfully") + return True + else: + log.info("Email not sent because recipient is not whitelisted") + return False except Exception as e: log.error( "Failed to send Email", exception=str(e), exc_info=True, stack_info=True @@ -70,15 +78,15 @@ def send_email( return False -def send_sendgrid_email( - to_emails: str | list[str], +def _send_sendgrid_email( + recipient_email: str, template: EmailTemplate, template_data: dict, template_language: str = "de", ) -> None: message = Mail( from_email="noreply@my.vbv-afa.ch", - to_emails=to_emails, + to_emails=recipient_email, ) message.template_id = template.value.get(template_language, template.value["de"]) message.dynamic_template_data = template_data diff --git a/server/vbv_lernwelt/notify/services.py b/server/vbv_lernwelt/notify/services.py index 4a493dff..af329da7 100644 --- a/server/vbv_lernwelt/notify/services.py +++ b/server/vbv_lernwelt/notify/services.py @@ -92,8 +92,8 @@ class NotificationService: else: actor_avatar_url = sender.avatar_url log = logger.bind( - recipient=recipient.get_full_name(), - sender=sender.get_full_name(), + recipient=recipient.email, + sender=sender.email, verb=verb, notification_type=notification_type, course=course, @@ -113,6 +113,7 @@ class NotificationService: emailed = False if cls._should_send_email(notification_type, recipient): + log.debug("Try to send email") emailed = cls._send_email( recipient=recipient, template=email_template, @@ -121,6 +122,8 @@ class NotificationService: **template_data, }, ) + else: + log.debug("Should not send email") try: response = notify.send( @@ -141,7 +144,7 @@ class NotificationService: except Exception as e: log.error("Failed to send notification", exception=str(e)) else: - log.info("Notification sent successfully") + log.info("Notification sent successfully", emailed=emailed) @staticmethod def _should_send_email( @@ -158,7 +161,7 @@ class NotificationService: template_data: dict, ) -> bool: return send_email( - to_emails=recipient.email, + recipient_email=recipient.email, template=template, template_data=template_data, template_language=recipient.language, From 5790fac78f49ffc35dee3032ba2bfb084fa0d88c Mon Sep 17 00:00:00 2001 From: Daniel Egger Date: Fri, 25 Aug 2023 17:32:17 +0200 Subject: [PATCH 05/11] Add `LoggedCommand` to JobLogs in django admin interface --- server/vbv_lernwelt/core/admin.py | 44 +++++++++++++++++++ server/vbv_lernwelt/core/base.py | 38 ++++++++++++++++ .../core/migrations/0002_joblog.py | 33 ++++++++++++++ server/vbv_lernwelt/core/models.py | 13 ++++++ server/vbv_lernwelt/core/utils.py | 13 ++++++ .../migrations/0005_auto_20230825_1723.py | 24 ++++++++++ .../send_attendance_course_reminders.py | 10 +++-- 7 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 server/vbv_lernwelt/core/base.py create mode 100644 server/vbv_lernwelt/core/migrations/0002_joblog.py create mode 100644 server/vbv_lernwelt/course_session/migrations/0005_auto_20230825_1723.py diff --git a/server/vbv_lernwelt/core/admin.py b/server/vbv_lernwelt/core/admin.py index 4fdb1a9b..f2a65d3b 100644 --- a/server/vbv_lernwelt/core/admin.py +++ b/server/vbv_lernwelt/core/admin.py @@ -2,9 +2,29 @@ from django.contrib import admin from django.contrib.auth import admin as auth_admin, get_user_model from django.utils.translation import gettext_lazy as _ +from vbv_lernwelt.core.models import JobLog +from vbv_lernwelt.core.utils import pretty_print_json + User = get_user_model() +class LogAdmin(admin.ModelAdmin): + def has_add_permission(self, request): + return False + + def has_delete_permission(self, request, obj=None): + return False + + def get_readonly_fields(self, request, obj=None): + # set all fields read only + return [field.name for field in self.opts.local_fields] + [ + field.name for field in self.opts.local_many_to_many + ] + + def pretty_print_json(self, json_string): + return pretty_print_json(json_string) + + @admin.register(User) class UserAdmin(auth_admin.UserAdmin): fieldsets = ( @@ -34,3 +54,27 @@ class UserAdmin(auth_admin.UserAdmin): "sso_id", ] search_fields = ["first_name", "last_name", "email", "username", "sso_id"] + + +@admin.register(JobLog) +class JobLogAdmin(LogAdmin): + date_hierarchy = "started" + list_display = ( + "job_name", + "started", + "ended", + "runtime", + "success", + ) + list_filter = ["job_name", "success"] + + def json_data_pretty(self, instance): + return self.pretty_print_json(instance.json_data) + + def get_readonly_fields(self, request, obj=None): + return super().get_readonly_fields(request, obj) + ["json_data_pretty"] + + def runtime(self, obj): + if obj.ended: + return (obj.ended - obj.started).seconds // 60 + return None diff --git a/server/vbv_lernwelt/core/base.py b/server/vbv_lernwelt/core/base.py new file mode 100644 index 00000000..3eaa12a8 --- /dev/null +++ b/server/vbv_lernwelt/core/base.py @@ -0,0 +1,38 @@ +import sys +import traceback + +import structlog +from django.core.management.base import BaseCommand +from django.utils import timezone + +from vbv_lernwelt.core.models import JobLog + +logger = structlog.get_logger(__name__) + + +# pylint: disable=abstract-method +class LoggedCommand(BaseCommand): + def execute(self, *args, **options): + name = getattr(self, "name", "") + if not name: + name = sys.argv[1] + + logger.info("start LoggedCommand", job_name=name, label="job_log") + + self.job_log = JobLog.objects.create(job_name=name) + try: + super(LoggedCommand, self).execute(*args, **options) + self.job_log.refresh_from_db() + self.job_log.ended = timezone.now() + self.job_log.success = True + self.job_log.save() + logger.info( + "LoggedCommand successfully ended", job_name=name, label="job_log" + ) + except Exception as e: + self.job_log.refresh_from_db() + self.job_log.error_message = str(e) + self.job_log.stack_trace = traceback.format_exc() + self.job_log.save() + logger.error("LoggedCommand failed", job_name=name, label="job_log") + raise e diff --git a/server/vbv_lernwelt/core/migrations/0002_joblog.py b/server/vbv_lernwelt/core/migrations/0002_joblog.py new file mode 100644 index 00000000..f36059f0 --- /dev/null +++ b/server/vbv_lernwelt/core/migrations/0002_joblog.py @@ -0,0 +1,33 @@ +# Generated by Django 3.2.20 on 2023-08-25 15:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("core", "0001_initial"), + ] + + operations = [ + migrations.CreateModel( + name="JobLog", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("started", models.DateTimeField(auto_now_add=True)), + ("ended", models.DateTimeField(blank=True, null=True)), + ("job_name", models.CharField(max_length=255)), + ("success", models.BooleanField(default=False)), + ("error_message", models.TextField(blank=True, default="")), + ("stack_trace", models.TextField(blank=True, default="")), + ("json_data", models.JSONField(default=dict)), + ], + ), + ] diff --git a/server/vbv_lernwelt/core/models.py b/server/vbv_lernwelt/core/models.py index 8f8bcb75..524a2133 100644 --- a/server/vbv_lernwelt/core/models.py +++ b/server/vbv_lernwelt/core/models.py @@ -41,3 +41,16 @@ class SecurityRequestResponseLog(models.Model): response_status_code = models.CharField(max_length=255, blank=True, default="") additional_json_data = JSONField(default=dict, blank=True) + + +class JobLog(models.Model): + started = models.DateTimeField(auto_now_add=True) + ended = models.DateTimeField(blank=True, null=True) + job_name = models.CharField(max_length=255) + success = models.BooleanField(default=False) + error_message = models.TextField(blank=True, default="") + stack_trace = models.TextField(blank=True, default="") + json_data = models.JSONField(default=dict) + + def __str__(self): + return "{job_name} {started:%H:%M %d.%m.%Y}".format(**self.__dict__) diff --git a/server/vbv_lernwelt/core/utils.py b/server/vbv_lernwelt/core/utils.py index 1f4e16fc..75b3e517 100644 --- a/server/vbv_lernwelt/core/utils.py +++ b/server/vbv_lernwelt/core/utils.py @@ -38,3 +38,16 @@ def replace_whitespace(text, replacement=" "): def get_django_content_type(obj): return obj._meta.app_label + "." + type(obj).__name__ + + +def pretty_print_json(json_string): + try: + parsed = json_string + if isinstance(json_string, str): + parsed = json.loads(json_string) + return mark_safe( + "
{}
".format(json.dumps(parsed, indent=4, sort_keys=True)) + ) + # pylint: disable=broad-except + except Exception: + return json_string diff --git a/server/vbv_lernwelt/course_session/migrations/0005_auto_20230825_1723.py b/server/vbv_lernwelt/course_session/migrations/0005_auto_20230825_1723.py new file mode 100644 index 00000000..5840c77a --- /dev/null +++ b/server/vbv_lernwelt/course_session/migrations/0005_auto_20230825_1723.py @@ -0,0 +1,24 @@ +# Generated by Django 3.2.20 on 2023-08-25 15:23 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("course_session", "0004_coursesessionedoniqtest"), + ] + + operations = [ + migrations.AlterModelOptions( + name="coursesessionassignment", + options={"ordering": ["course_session", "submission_deadline__start"]}, + ), + migrations.AlterModelOptions( + name="coursesessionattendancecourse", + options={"ordering": ["course_session", "due_date__start"]}, + ), + migrations.AlterModelOptions( + name="coursesessionedoniqtest", + options={"ordering": ["course_session", "deadline__start"]}, + ), + ] diff --git a/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py b/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py index 2cd8ae45..19c81b8a 100644 --- a/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py +++ b/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py @@ -1,10 +1,10 @@ from datetime import timedelta -import djclick as click import structlog from django.utils import timezone from django.utils.translation import gettext_lazy as _ +from vbv_lernwelt.core.base import LoggedCommand from vbv_lernwelt.core.models import User from vbv_lernwelt.course.models import CourseSessionUser from vbv_lernwelt.course_session.models import CourseSessionAttendanceCourse @@ -55,6 +55,8 @@ def attendance_course_reminder_notification_job(): logger.info("No attendance courses found") -@click.command() -def command(): - attendance_course_reminder_notification_job() +class Command(LoggedCommand): + help = "Sends attendance course reminder notifications to participants" + + def handle(self, *args, **options): + attendance_course_reminder_notification_job() From f6a01b3ad1fde998ea0f0ce38ce26715068a9281 Mon Sep 17 00:00:00 2001 From: Daniel Egger Date: Fri, 25 Aug 2023 18:21:48 +0200 Subject: [PATCH 06/11] Add result and statistics data to reminder job --- .../send_attendance_course_reminders.py | 47 ++++++++-- server/vbv_lernwelt/notify/services.py | 92 ++++++++++++------- 2 files changed, 95 insertions(+), 44 deletions(-) diff --git a/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py b/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py index 19c81b8a..7e6dd80c 100644 --- a/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py +++ b/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py @@ -1,3 +1,4 @@ +from collections import Counter from datetime import timedelta import structlog @@ -21,8 +22,8 @@ PRESENCE_COURSE_REMINDER_LEAD_TIME = timedelta(weeks=2) def send_attendance_course_reminder_notification( recipient: User, attendance_course: CourseSessionAttendanceCourse -): - NotificationService.send_information_notification( +) -> str: + return NotificationService.send_information_notification( recipient=recipient, verb=_("Erinnerung: Bald findet ein Präsenzkurs statt"), target_url=attendance_course.learning_content.get_frontend_url(), @@ -37,26 +38,52 @@ def attendance_course_reminder_notification_job(): """Checks if an attendance course is coming up and sends a reminder to the participants""" start = timezone.now() end = timezone.now() + PRESENCE_COURSE_REMINDER_LEAD_TIME - logger.info( - "Querying for attendance courses in specified time range", - start_time=start, - end_time=end, - ) + results_counter = Counter() attendance_courses = CourseSessionAttendanceCourse.objects.filter( due_date__start__lte=end, due_date__start__gte=start, ) + + logger.info( + "Querying for attendance courses in specified time range", + start_time=start, + end_time=end, + label="attendance_course_reminder_notification_job", + num_attendance_courses=len(attendance_courses), + ) for attendance_course in attendance_courses: cs_id = attendance_course.course_session.id csu = CourseSessionUser.objects.filter(course_session_id=cs_id) + logger.info( + "Sending attendance course reminder notification", + start_time=start, + end_time=end, + label="attendance_course_reminder_notification_job", + num_users=len(csu), + course_session_id=cs_id, + ) for user in csu: - send_attendance_course_reminder_notification(user.user, attendance_course) + result = send_attendance_course_reminder_notification( + user.user, attendance_course + ) + results_counter[result] += 1 if not attendance_courses: - logger.info("No attendance courses found") + logger.info( + "No attendance courses found", + label="attendance_course_reminder_notification_job", + ) + return dict(results_counter) class Command(LoggedCommand): help = "Sends attendance course reminder notifications to participants" def handle(self, *args, **options): - attendance_course_reminder_notification_job() + results = attendance_course_reminder_notification_job() + self.job_log.json_data = results + self.job_log.save() + logger.info( + "Attendance course reminder notification job finished", + label="attendance_course_reminder_notification_job", + results=results, + ) diff --git a/server/vbv_lernwelt/notify/services.py b/server/vbv_lernwelt/notify/services.py index af329da7..c4b5c8bb 100644 --- a/server/vbv_lernwelt/notify/services.py +++ b/server/vbv_lernwelt/notify/services.py @@ -21,8 +21,8 @@ class NotificationService: target_url: str, email_template: EmailTemplate, template_data: dict = None, - ) -> None: - cls._send_notification( + ) -> str: + return cls._send_notification( recipient=recipient, verb=verb, sender=sender, @@ -42,8 +42,8 @@ class NotificationService: target_url: str, email_template: EmailTemplate, template_data: dict = None, - ) -> None: - cls._send_notification( + ) -> str: + return cls._send_notification( recipient=recipient, verb=verb, course=course, @@ -61,8 +61,8 @@ class NotificationService: target_url: str, email_template: EmailTemplate, template_data: dict = None, - ) -> None: - cls._send_notification( + ) -> str: + return cls._send_notification( recipient=recipient, verb=verb, target_url=target_url, @@ -82,10 +82,13 @@ class NotificationService: sender: User | None = None, course: str | None = None, target_url: str | None = None, - ) -> None: + fail_silently: bool = True, + ) -> str: if template_data is None: template_data = {} + notification_identifier = f"{notification_type.name}_{email_template.name}" + actor_avatar_url: Optional[str] = None if not sender: sender = User.objects.get(email="admin") @@ -100,32 +103,42 @@ class NotificationService: target_url=target_url, template_data=template_data, ) - if NotificationService._is_duplicate_notification( - recipient=recipient, - verb=verb, - notification_type=notification_type, - target_url=target_url, - template_name=email_template.name, - template_data=template_data, - ): - log.info("A duplicate notification was omitted from being sent") - return - emailed = False - if cls._should_send_email(notification_type, recipient): - log.debug("Try to send email") - emailed = cls._send_email( - recipient=recipient, - template=email_template, - template_data={ - "target_url": f"https://my.vbv-afa.ch{target_url}", - **template_data, - }, - ) - else: - log.debug("Should not send email") - try: + if NotificationService._is_duplicate_notification( + recipient=recipient, + verb=verb, + notification_type=notification_type, + target_url=target_url, + template_name=email_template.name, + template_data=template_data, + ): + log.info("A duplicate notification was omitted from being sent") + return f"{notification_identifier}_duplicate" + + if cls._should_send_email(notification_type, recipient): + log.debug("Try to send email") + try: + emailed = cls._send_email( + recipient=recipient, + template=email_template, + template_data={ + "target_url": f"https://my.vbv-afa.ch{target_url}", + **template_data, + }, + fail_silently=False, + ) + except Exception as e: + notification_identifier += "_email_error" + if not fail_silently: + raise e + return notification_identifier + + if emailed: + notification_identifier += "_emailed" + else: + log.debug("Should not send email") + response = notify.send( sender=sender, recipient=recipient, @@ -141,10 +154,19 @@ class NotificationService: sent_notification.course = course sent_notification.actor_avatar_url = actor_avatar_url sent_notification.save() - except Exception as e: - log.error("Failed to send notification", exception=str(e)) - else: log.info("Notification sent successfully", emailed=emailed) + return f"{notification_identifier}_success" + except Exception as e: + log.error( + "Failed to send notification", + exception=str(e), + exc_info=True, + stack_info=True, + emailed=emailed, + ) + if not fail_silently: + raise e + return f"{notification_identifier}_error" @staticmethod def _should_send_email( @@ -159,12 +181,14 @@ class NotificationService: recipient: User, template: EmailTemplate, template_data: dict, + fail_silently: bool = True, ) -> bool: return send_email( recipient_email=recipient.email, template=template, template_data=template_data, template_language=recipient.language, + fail_silently=fail_silently, ) @staticmethod From ae9d7cf47137bc8301fb398cd6d3c5732a0136b8 Mon Sep 17 00:00:00 2001 From: Daniel Egger Date: Tue, 29 Aug 2023 14:38:39 +0200 Subject: [PATCH 07/11] Try to fix deadlock import error by lazy loading openpyxl --- server/vbv_lernwelt/core/utils.py | 1 + server/vbv_lernwelt/importer/services.py | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/server/vbv_lernwelt/core/utils.py b/server/vbv_lernwelt/core/utils.py index 75b3e517..e2a012fa 100644 --- a/server/vbv_lernwelt/core/utils.py +++ b/server/vbv_lernwelt/core/utils.py @@ -1,6 +1,7 @@ import json import re +from django.utils.safestring import mark_safe from rest_framework.throttling import UserRateThrottle diff --git a/server/vbv_lernwelt/importer/services.py b/server/vbv_lernwelt/importer/services.py index 912b6145..0677e354 100644 --- a/server/vbv_lernwelt/importer/services.py +++ b/server/vbv_lernwelt/importer/services.py @@ -3,7 +3,6 @@ from typing import Any, Dict, List import structlog from django.utils import timezone -from openpyxl.reader.excel import load_workbook from vbv_lernwelt.assignment.models import AssignmentType from vbv_lernwelt.core.models import User @@ -275,6 +274,8 @@ def import_course_sessions_from_excel( "Basis", "Fahrzeug", ] + from openpyxl.reader.excel import load_workbook + workbook = load_workbook(filename=filename) sheet = workbook["Schulungen Durchführung"] no_course = course is None @@ -521,6 +522,8 @@ def get_uk_course(language: str) -> Course: def import_trainers_from_excel_for_training( filename: str, language="de", course: Course = None ): + from openpyxl.reader.excel import load_workbook + workbook = load_workbook(filename=filename) sheet = workbook["Schulungen Trainer"] @@ -609,6 +612,8 @@ def create_or_update_trainer(course: Course, data: Dict[str, Any], language="de" def import_students_from_excel(filename: str): + from openpyxl.reader.excel import load_workbook + workbook = load_workbook(filename=filename) sheet = workbook.active @@ -671,6 +676,8 @@ def _get_date_of_birth(data: Dict[str, Any]) -> str: def sync_students_from_t2l_excel(filename: str): + from openpyxl.reader.excel import load_workbook + workbook = load_workbook(filename=filename) sheet = workbook.active From 88e7e0edcc648990995335e0166f86abacebe258 Mon Sep 17 00:00:00 2001 From: Daniel Egger Date: Tue, 29 Aug 2023 15:08:12 +0200 Subject: [PATCH 08/11] Add ability to send email, when it was not sent before --- server/vbv_lernwelt/notify/services.py | 59 +++++++++++++++----------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/server/vbv_lernwelt/notify/services.py b/server/vbv_lernwelt/notify/services.py index c4b5c8bb..34143871 100644 --- a/server/vbv_lernwelt/notify/services.py +++ b/server/vbv_lernwelt/notify/services.py @@ -105,18 +105,19 @@ class NotificationService: ) emailed = False try: - if NotificationService._is_duplicate_notification( + notification = NotificationService._find_duplicate_notification( recipient=recipient, verb=verb, notification_type=notification_type, target_url=target_url, template_name=email_template.name, template_data=template_data, - ): - log.info("A duplicate notification was omitted from being sent") - return f"{notification_identifier}_duplicate" + ) + emailed = False + if notification and notification.emailed: + emailed = True - if cls._should_send_email(notification_type, recipient): + if cls._should_send_email(notification_type, recipient) and not emailed: log.debug("Try to send email") try: emailed = cls._send_email( @@ -136,25 +137,35 @@ class NotificationService: if emailed: notification_identifier += "_emailed" + if notification: + notification.emailed = True + notification.save() else: log.debug("Should not send email") - response = notify.send( - sender=sender, - recipient=recipient, - verb=verb, - emailed=emailed, - # The metadata is saved in the 'data' member of the AbstractNotification model - email_template=email_template.name, - template_data=template_data, - ) - sent_notification: Notification = response[0][1][0] # 🫨 - sent_notification.target_url = target_url - sent_notification.notification_type = notification_type - sent_notification.course = course - sent_notification.actor_avatar_url = actor_avatar_url - sent_notification.save() - log.info("Notification sent successfully", emailed=emailed) + if notification: + log.info("Duplicate notification was omitted from being sent") + notification_identifier += "_duplicate" + return notification_identifier + + else: + response = notify.send( + sender=sender, + recipient=recipient, + verb=verb, + emailed=emailed, + # The metadata is saved in the 'data' member of the AbstractNotification model + email_template=email_template.name, + template_data=template_data, + ) + + sent_notification: Notification = response[0][1][0] # 🫨 + sent_notification.target_url = target_url + sent_notification.notification_type = notification_type + sent_notification.course = course + sent_notification.actor_avatar_url = actor_avatar_url + sent_notification.save() + log.info("Notification sent successfully", emailed=emailed) return f"{notification_identifier}_success" except Exception as e: log.error( @@ -192,14 +203,14 @@ class NotificationService: ) @staticmethod - def _is_duplicate_notification( + def _find_duplicate_notification( recipient: User, verb: str, notification_type: NotificationType, template_name: str, template_data: dict, target_url: str | None, - ) -> bool: + ) -> Notification | None: """Check if a notification with the same parameters has already been sent to the recipient. This is to prevent duplicate notifications from being sent and to protect against potential programming errors. """ @@ -212,4 +223,4 @@ class NotificationService: "email_template": template_name, "template_data": template_data, }, - ).exists() + ).first() From d8bce90b8e28d8dd10039c09d5f4af9800bd18cf Mon Sep 17 00:00:00 2001 From: Daniel Egger Date: Wed, 30 Aug 2023 09:48:50 +0200 Subject: [PATCH 09/11] User supervisord in docker to start supercronic and gunicorn --- compose/django/Dockerfile | 8 +++++++- compose/django/docker_start.sh | 9 +++++---- compose/django/send_attendance_course_reminders | 1 - compose/django/sshd_config | 0 compose/django/supercronic_crontab | 8 ++++++++ compose/django/supervisord.conf | 14 ++++++++++++++ .../core/management/commands/simple_dummy_job.py | 15 +++++++++++++++ 7 files changed, 49 insertions(+), 6 deletions(-) delete mode 100644 compose/django/send_attendance_course_reminders delete mode 100644 compose/django/sshd_config create mode 100644 compose/django/supercronic_crontab create mode 100644 compose/django/supervisord.conf create mode 100644 server/vbv_lernwelt/core/management/commands/simple_dummy_job.py diff --git a/compose/django/Dockerfile b/compose/django/Dockerfile index 85e4ba33..0bdccfde 100644 --- a/compose/django/Dockerfile +++ b/compose/django/Dockerfile @@ -31,7 +31,7 @@ RUN curl -fsSLO "$SUPERCRONIC_URL" \ && mv "$SUPERCRONIC" "/usr/local/bin/${SUPERCRONIC}" \ && ln -s "/usr/local/bin/${SUPERCRONIC}" /usr/local/bin/supercronic -COPY ./compose/django/send_attendance_course_reminders /app/send_attendance_course_reminders +COPY ./compose/django/supercronic_crontab /app/supercronic_crontab # Python build stage FROM python as python-build-stage @@ -42,6 +42,7 @@ ARG BUILD_ENVIRONMENT=production RUN apt-get install --no-install-recommends -y \ # dependencies for building Python packages build-essential \ + # supervisor \ # psycopg2 dependencies libpq-dev \ git @@ -119,6 +120,11 @@ RUN chown django:django ${APP_HOME} USER django +RUN pip install supervisor +RUN exit + EXPOSE 7555 +COPY ./compose/django/supervisord.conf /app/supervisord.conf + ENTRYPOINT ["/entrypoint"] diff --git a/compose/django/docker_start.sh b/compose/django/docker_start.sh index 2f359bd2..b6fa79cb 100644 --- a/compose/django/docker_start.sh +++ b/compose/django/docker_start.sh @@ -15,9 +15,10 @@ else python /app/manage.py migrate fi -if [[ $IT_APP_ENVIRONMENT != dev* ]]; then - # Start periodic tasks - /usr/local/bin/supercronic -sentry-dsn "$IT_SENTRY_DSN" -split-logs /app/send_attendance_course_reminders & +# Use sentry for supercronic only in prod* environments +if [[ $IT_APP_ENVIRONMENT == prod* ]]; then + sed -i 's|command=/usr/local/bin/supercronic /app/supercronic_crontab|command=/usr/local/bin/supercronic /app/supercronic_crontab -sentry-dsn "$IT_SENTRY_DSN"|' /app/supervisord.conf fi -newrelic-admin run-program gunicorn config.asgi --bind 0.0.0.0:7555 --chdir=/app -k uvicorn.workers.UvicornWorker +# Set the command to run supervisord +/home/django/.local/bin/supervisord -c /app/supervisord.conf diff --git a/compose/django/send_attendance_course_reminders b/compose/django/send_attendance_course_reminders deleted file mode 100644 index 4d3af4d1..00000000 --- a/compose/django/send_attendance_course_reminders +++ /dev/null @@ -1 +0,0 @@ -@daily /usr/local/bin/python /app/manage.py send_attendance_course_reminders diff --git a/compose/django/sshd_config b/compose/django/sshd_config deleted file mode 100644 index e69de29b..00000000 diff --git a/compose/django/supercronic_crontab b/compose/django/supercronic_crontab new file mode 100644 index 00000000..49452de0 --- /dev/null +++ b/compose/django/supercronic_crontab @@ -0,0 +1,8 @@ +# Run every minute +*/1 * * * * echo "hello" + +# Run every 6 hours +0 */6 * * * /usr/local/bin/python /app/manage.py simple_dummy_job + +# every day at 19:30 +30 19 * * * /usr/local/bin/python /app/manage.py send_attendance_course_reminders diff --git a/compose/django/supervisord.conf b/compose/django/supervisord.conf new file mode 100644 index 00000000..2e668f56 --- /dev/null +++ b/compose/django/supervisord.conf @@ -0,0 +1,14 @@ +[supervisord] +nodaemon=true + +[program:supercronic] +command=/usr/local/bin/supercronic /app/supercronic_crontab +redirect_stderr=true +stdout_logfile=/dev/stdout +stdout_logfile_maxbytes=0 + +[program:gunicorn] +command=newrelic-admin run-program gunicorn config.asgi --bind 0.0.0.0:7555 --chdir=/app -k uvicorn.workers.UvicornWorker +redirect_stderr=true +stdout_logfile=/dev/stdout +stdout_logfile_maxbytes=0 diff --git a/server/vbv_lernwelt/core/management/commands/simple_dummy_job.py b/server/vbv_lernwelt/core/management/commands/simple_dummy_job.py new file mode 100644 index 00000000..9d130a52 --- /dev/null +++ b/server/vbv_lernwelt/core/management/commands/simple_dummy_job.py @@ -0,0 +1,15 @@ +import structlog + +from vbv_lernwelt.core.base import LoggedCommand + +logger = structlog.get_logger(__name__) + + +class Command(LoggedCommand): + help = "Simple Dummy Job to test if supercronic is working" + + def handle(self, *args, **options): + logger.debug( + "Dummy Job finished", + label="job_lob", + ) From b26ec64edb68d09d7eb44a2fa8e1bad6c3be1426 Mon Sep 17 00:00:00 2001 From: Daniel Egger Date: Wed, 30 Aug 2023 13:41:06 +0200 Subject: [PATCH 10/11] Add custom django CustomNotificationAdmin --- compose/django/Dockerfile | 5 ++-- server/vbv_lernwelt/notify/admin.py | 26 +++++++++++++++++++ server/vbv_lernwelt/notify/apps.py | 14 ++++++++++ .../vbv_lernwelt/notify/tests/test_service.py | 3 ++- 4 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 server/vbv_lernwelt/notify/admin.py diff --git a/compose/django/Dockerfile b/compose/django/Dockerfile index 0bdccfde..8c411fa4 100644 --- a/compose/django/Dockerfile +++ b/compose/django/Dockerfile @@ -120,11 +120,10 @@ RUN chown django:django ${APP_HOME} USER django +# pip install under the user django, the scripts will be in /home/django/.local/bin RUN pip install supervisor -RUN exit +COPY ./compose/django/supervisord.conf /app/supervisord.conf EXPOSE 7555 -COPY ./compose/django/supervisord.conf /app/supervisord.conf - ENTRYPOINT ["/entrypoint"] diff --git a/server/vbv_lernwelt/notify/admin.py b/server/vbv_lernwelt/notify/admin.py new file mode 100644 index 00000000..0f667d3c --- /dev/null +++ b/server/vbv_lernwelt/notify/admin.py @@ -0,0 +1,26 @@ +from vbv_lernwelt.core.admin import LogAdmin + + +# admin.register in apps.py +class CustomNotificationAdmin(LogAdmin): + date_hierarchy = "timestamp" + raw_id_fields = ("recipient",) + list_display = ( + "recipient", + "actor", + "notification_type", + "emailed", + "unread", + ) + list_filter = ( + "notification_type", + "emailed", + "unread", + "timestamp", + "level", + "public", + ) + + def get_queryset(self, request): + qs = super(CustomNotificationAdmin, self).get_queryset(request) + return qs.prefetch_related("actor") diff --git a/server/vbv_lernwelt/notify/apps.py b/server/vbv_lernwelt/notify/apps.py index 59aae618..8fd3f8d0 100644 --- a/server/vbv_lernwelt/notify/apps.py +++ b/server/vbv_lernwelt/notify/apps.py @@ -1,6 +1,20 @@ from django.apps import AppConfig +from django.contrib import admin class NotifyConfig(AppConfig): default_auto_field = "django.db.models.BigAutoField" name = "vbv_lernwelt.notify" + + def ready(self): + # Move the admin import here to avoid early imports + from .admin import CustomNotificationAdmin + + # Unregister the default Notification admin if it exists + from vbv_lernwelt.notify.models import Notification + + if admin.site.is_registered(Notification): + admin.site.unregister(Notification) + + # Register the custom admin + admin.site.register(Notification, CustomNotificationAdmin) diff --git a/server/vbv_lernwelt/notify/tests/test_service.py b/server/vbv_lernwelt/notify/tests/test_service.py index 15dbac49..2ec5500c 100644 --- a/server/vbv_lernwelt/notify/tests/test_service.py +++ b/server/vbv_lernwelt/notify/tests/test_service.py @@ -125,7 +125,8 @@ class TestNotificationService(TestCase): self.assertEqual(target_url, notification.target_url) self.assertEqual(course, notification.course) self.assertEqual( - str(NotificationType.USER_INTERACTION), notification.notification_type + str(NotificationType.USER_INTERACTION), + notification.notification_type, ) self.assertTrue(notification.emailed) From da56f2a346c736d1a105f4843c074266c6b8862e Mon Sep 17 00:00:00 2001 From: Daniel Egger Date: Wed, 30 Aug 2023 14:20:36 +0200 Subject: [PATCH 11/11] Refactor Notification model --- .../notifications/NotificationList.vue | 2 +- client/src/types.ts | 4 +- server/vbv_lernwelt/assignment/services.py | 25 +- .../assignment/tests/test_graphql.py | 72 ++++++ server/vbv_lernwelt/feedback/models.py | 39 ++-- .../feedback/tests/test_feedback_api.py | 26 ++- server/vbv_lernwelt/importer/services.py | 7 +- .../importer/tests/test_import_students.py | 2 +- server/vbv_lernwelt/notify/admin.py | 14 +- server/vbv_lernwelt/notify/apps.py | 6 +- .../notify/create_default_notifications.py | 39 ++-- .../send_attendance_course_reminders.py | 22 +- .../migrations/0002_auto_20230830_1606.py | 69 ++++++ .../migrations/0003_truncate_notifications.py | 21 ++ server/vbv_lernwelt/notify/models.py | 34 ++- server/vbv_lernwelt/notify/services.py | 186 +++++++++------ .../notify/tests/test_notify_api.py | 6 +- .../test_send_attendance_course_reminders.py | 123 +++++----- .../vbv_lernwelt/notify/tests/test_service.py | 220 +++++++++++------- server/vbv_lernwelt/notify/views.py | 6 +- 20 files changed, 586 insertions(+), 337 deletions(-) create mode 100644 server/vbv_lernwelt/notify/migrations/0002_auto_20230830_1606.py create mode 100644 server/vbv_lernwelt/notify/migrations/0003_truncate_notifications.py diff --git a/client/src/components/notifications/NotificationList.vue b/client/src/components/notifications/NotificationList.vue index 8d5bd404..e13d5104 100644 --- a/client/src/components/notifications/NotificationList.vue +++ b/client/src/components/notifications/NotificationList.vue @@ -56,7 +56,7 @@ function onNotificationClick(notification: Notification) { >
Notification icon int: verb="Alexandra hat einen neuen Beitrag erfasst", actor_avatar_url=avatar_urls[0], target_url="/", - notification_type=NotificationType.USER_INTERACTION, - course="Versicherungsvermittler/-in", + notification_category=NotificationCategory.USER_INTERACTION, timestamp=timestamps[0], ), NotificationFactory( @@ -38,8 +37,7 @@ def create_default_notifications() -> int: verb="Alexandra hat einen neuen Beitrag erfasst", actor_avatar_url=avatar_urls[0], target_url="/", - notification_type=NotificationType.USER_INTERACTION, - course="Versicherungsvermittler/-in", + notification_category=NotificationCategory.USER_INTERACTION, timestamp=timestamps[1], ), NotificationFactory( @@ -48,8 +46,7 @@ def create_default_notifications() -> int: verb="Alexandra hat einen neuen Beitrag erfasst", actor_avatar_url=avatar_urls[0], target_url="/", - notification_type=NotificationType.USER_INTERACTION, - course="Versicherungsvermittler/-in", + notification_category=NotificationCategory.USER_INTERACTION, timestamp=timestamps[2], ), NotificationFactory( @@ -58,8 +55,7 @@ def create_default_notifications() -> int: verb="Alexandra hat einen neuen Beitrag erfasst", actor_avatar_url=avatar_urls[0], target_url="/", - notification_type=NotificationType.USER_INTERACTION, - course="Versicherungsvermittler/-in", + notification_category=NotificationCategory.USER_INTERACTION, timestamp=timestamps[3], ), NotificationFactory( @@ -68,8 +64,7 @@ def create_default_notifications() -> int: verb="Bianca hat für den Auftrag Autoversicherung 3 eine Lösung abgegeben", actor_avatar_url=avatar_urls[1], target_url="/", - notification_type=NotificationType.USER_INTERACTION, - course="Versicherungsvermittler/-in", + notification_category=NotificationCategory.USER_INTERACTION, timestamp=timestamps[4], ), NotificationFactory( @@ -78,8 +73,7 @@ def create_default_notifications() -> int: verb="Bianca hat für den Auftrag Autoversicherung 1 eine Lösung abgegeben", actor_avatar_url=avatar_urls[1], target_url="/", - notification_type=NotificationType.USER_INTERACTION, - course="Versicherungsvermittler/-in", + notification_category=NotificationCategory.USER_INTERACTION, timestamp=timestamps[5], ), NotificationFactory( @@ -88,8 +82,7 @@ def create_default_notifications() -> int: verb="Bianca hat für den Auftrag Autoversicherung 2 eine Lösung abgegeben", actor_avatar_url=avatar_urls[1], target_url="/", - notification_type=NotificationType.USER_INTERACTION, - course="Versicherungsvermittler/-in", + notification_category=NotificationCategory.USER_INTERACTION, timestamp=timestamps[6], ), NotificationFactory( @@ -98,8 +91,7 @@ def create_default_notifications() -> int: verb="Bianca hat für den Auftrag Autoversicherung 4 eine Lösung abgegeben", actor_avatar_url=avatar_urls[1], target_url="/", - notification_type=NotificationType.USER_INTERACTION, - course="Versicherungsvermittler/-in", + notification_category=NotificationCategory.USER_INTERACTION, timestamp=timestamps[7], ), NotificationFactory( @@ -108,8 +100,7 @@ def create_default_notifications() -> int: verb="Chantal hat eine Bewertung für den Transferauftrag 3 eingegeben", target_url="/", actor_avatar_url=avatar_urls[2], - notification_type=NotificationType.USER_INTERACTION, - course="Versicherungsvermittler/-in", + notification_category=NotificationCategory.USER_INTERACTION, timestamp=timestamps[8], ), NotificationFactory( @@ -118,8 +109,7 @@ def create_default_notifications() -> int: verb="Chantal hat eine Bewertung für den Transferauftrag 4 eingegeben", target_url="/", actor_avatar_url=avatar_urls[2], - notification_type=NotificationType.USER_INTERACTION, - course="Versicherungsvermittler/-in", + notification_category=NotificationCategory.USER_INTERACTION, timestamp=timestamps[9], ), NotificationFactory( @@ -127,22 +117,21 @@ def create_default_notifications() -> int: actor=user, verb="Super, du kommst in deinem Lernpfad gut voran. Schaue dir jetzt die verfügbaren Prüfungstermine an.", target_url="/", - notification_type=NotificationType.PROGRESS, - course="Versicherungsvermittler/-in", + notification_category=NotificationCategory.PROGRESS, timestamp=timestamps[10], ), NotificationFactory( recipient=user, actor=user, verb="Wartungsarbeiten: 20.12.2022 08:00 - 12:00", - notification_type=NotificationType.INFORMATION, + notification_category=NotificationCategory.INFORMATION, timestamp=timestamps[11], ), NotificationFactory( recipient=user, actor=user, verb="Wartungsarbeiten: 31.01.2023 08:00 - 12:00", - notification_type=NotificationType.INFORMATION, + notification_category=NotificationCategory.INFORMATION, timestamp=timestamps[12], ), ) diff --git a/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py b/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py index 7e6dd80c..81e89677 100644 --- a/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py +++ b/server/vbv_lernwelt/notify/management/commands/send_attendance_course_reminders.py @@ -3,16 +3,10 @@ from datetime import timedelta import structlog from django.utils import timezone -from django.utils.translation import gettext_lazy as _ from vbv_lernwelt.core.base import LoggedCommand -from vbv_lernwelt.core.models import User from vbv_lernwelt.course.models import CourseSessionUser from vbv_lernwelt.course_session.models import CourseSessionAttendanceCourse -from vbv_lernwelt.notify.email.email_services import ( - create_template_data_from_course_session_attendance_course, - EmailTemplate, -) from vbv_lernwelt.notify.services import NotificationService logger = structlog.get_logger(__name__) @@ -20,20 +14,6 @@ logger = structlog.get_logger(__name__) PRESENCE_COURSE_REMINDER_LEAD_TIME = timedelta(weeks=2) -def send_attendance_course_reminder_notification( - recipient: User, attendance_course: CourseSessionAttendanceCourse -) -> str: - return NotificationService.send_information_notification( - recipient=recipient, - verb=_("Erinnerung: Bald findet ein Präsenzkurs statt"), - target_url=attendance_course.learning_content.get_frontend_url(), - email_template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, - template_data=create_template_data_from_course_session_attendance_course( - attendance_course=attendance_course - ), - ) - - def attendance_course_reminder_notification_job(): """Checks if an attendance course is coming up and sends a reminder to the participants""" start = timezone.now() @@ -63,7 +43,7 @@ def attendance_course_reminder_notification_job(): course_session_id=cs_id, ) for user in csu: - result = send_attendance_course_reminder_notification( + result = NotificationService.send_attendance_course_reminder_notification( user.user, attendance_course ) results_counter[result] += 1 diff --git a/server/vbv_lernwelt/notify/migrations/0002_auto_20230830_1606.py b/server/vbv_lernwelt/notify/migrations/0002_auto_20230830_1606.py new file mode 100644 index 00000000..d4292aa4 --- /dev/null +++ b/server/vbv_lernwelt/notify/migrations/0002_auto_20230830_1606.py @@ -0,0 +1,69 @@ +# Generated by Django 3.2.20 on 2023-08-30 14:06 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("course", "0004_auto_20230823_1744"), + ("notify", "0001_initial"), + ] + + operations = [ + migrations.RemoveField( + model_name="notification", + name="course", + ), + migrations.RemoveField( + model_name="notification", + name="notification_type", + ), + migrations.AddField( + model_name="notification", + name="course_session", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="course.coursesession", + ), + ), + migrations.AddField( + model_name="notification", + name="notification_category", + field=models.CharField( + choices=[ + ("USER_INTERACTION", "User Interaction"), + ("PROGRESS", "Progress"), + ("INFORMATION", "Information"), + ], + default="INFORMATION", + max_length=255, + ), + ), + migrations.AddField( + model_name="notification", + name="notification_trigger", + field=models.CharField( + choices=[ + ("ATTENDANCE_COURSE_REMINDER", "Attendance Course Reminder"), + ("CASEWORK_SUBMITTED", "Casework Submitted"), + ("CASEWORK_EVALUATED", "Casework Evaluated"), + ("NEW_FEEDBACK", "New Feedback"), + ], + default="ATTENDANCE_COURSE_REMINDER", + max_length=255, + ), + ), + migrations.AlterField( + model_name="notification", + name="actor_avatar_url", + field=models.CharField(blank=True, default="", max_length=2048), + ), + migrations.AlterField( + model_name="notification", + name="target_url", + field=models.CharField(blank=True, default="", max_length=2048), + ), + ] diff --git a/server/vbv_lernwelt/notify/migrations/0003_truncate_notifications.py b/server/vbv_lernwelt/notify/migrations/0003_truncate_notifications.py new file mode 100644 index 00000000..8751eb88 --- /dev/null +++ b/server/vbv_lernwelt/notify/migrations/0003_truncate_notifications.py @@ -0,0 +1,21 @@ +# Generated by Django 3.2.20 on 2023-08-30 14:10 + +from django.db import migrations + + +def init_user_notification_emails(apps=None, schema_editor=None): + User = apps.get_model("core", "User") + for u in User.objects.all(): + u.additional_json_data["email_notification_categories"] = ["NOTIFICATION"] + u.save() + + +class Migration(migrations.Migration): + dependencies = [ + ("notify", "0002_auto_20230830_1606"), + ] + + operations = [ + migrations.RunSQL("truncate table notify_notification cascade;"), + migrations.RunPython(init_user_notification_emails), + ] diff --git a/server/vbv_lernwelt/notify/models.py b/server/vbv_lernwelt/notify/models.py index dd00ed99..dfe6df74 100644 --- a/server/vbv_lernwelt/notify/models.py +++ b/server/vbv_lernwelt/notify/models.py @@ -2,25 +2,43 @@ from django.db import models from django.utils.translation import gettext_lazy as _ from notifications.base.models import AbstractNotification +from vbv_lernwelt.course.models import CourseSession -class NotificationType(models.TextChoices): + +class NotificationCategory(models.TextChoices): USER_INTERACTION = "USER_INTERACTION", _("User Interaction") PROGRESS = "PROGRESS", _("Progress") INFORMATION = "INFORMATION", _("Information") +class NotificationTrigger(models.TextChoices): + ATTENDANCE_COURSE_REMINDER = "ATTENDANCE_COURSE_REMINDER", _( + "Attendance Course Reminder" + ) + CASEWORK_SUBMITTED = "CASEWORK_SUBMITTED", _("Casework Submitted") + CASEWORK_EVALUATED = "CASEWORK_EVALUATED", _("Casework Evaluated") + NEW_FEEDBACK = "NEW_FEEDBACK", _("New Feedback") + + class Notification(AbstractNotification): # UUIDs are not supported by the notifications app... # id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) - notification_type = models.CharField( - max_length=32, - choices=NotificationType.choices, - default=NotificationType.INFORMATION, + notification_category = models.CharField( + max_length=255, + choices=NotificationCategory.choices, + default=NotificationCategory.INFORMATION, + ) + notification_trigger = models.CharField( + max_length=255, + choices=NotificationTrigger.choices, + default="", + ) + target_url = models.CharField(max_length=2048, default="", blank=True) + actor_avatar_url = models.CharField(max_length=2048, default="", blank=True) + course_session = models.ForeignKey( + CourseSession, blank=True, null=True, on_delete=models.SET_NULL ) - target_url = models.URLField(blank=True, null=True) - actor_avatar_url = models.URLField(blank=True, null=True) - course = models.CharField(max_length=32, blank=True, null=True) class Meta(AbstractNotification.Meta): abstract = False diff --git a/server/vbv_lernwelt/notify/services.py b/server/vbv_lernwelt/notify/services.py index 34143871..6cc2848e 100644 --- a/server/vbv_lernwelt/notify/services.py +++ b/server/vbv_lernwelt/notify/services.py @@ -1,74 +1,121 @@ -from typing import Optional +from __future__ import annotations + +from gettext import gettext +from typing import TYPE_CHECKING import structlog +from django.db.models import Model from notifications.signals import notify from vbv_lernwelt.core.models import User -from vbv_lernwelt.notify.email.email_services import EmailTemplate, send_email -from vbv_lernwelt.notify.models import Notification, NotificationType +from vbv_lernwelt.course.models import CourseSession +from vbv_lernwelt.notify.email.email_services import ( + create_template_data_from_course_session_attendance_course, + EmailTemplate, + send_email, +) +from vbv_lernwelt.notify.models import ( + Notification, + NotificationCategory, + NotificationTrigger, +) + +if TYPE_CHECKING: + from vbv_lernwelt.assignment.models import AssignmentCompletion + from vbv_lernwelt.course_session.models import CourseSessionAttendanceCourse + from vbv_lernwelt.feedback.models import FeedbackResponse logger = structlog.get_logger(__name__) class NotificationService: @classmethod - def send_user_interaction_notification( - cls, - recipient: User, - verb: str, - sender: User, - course: str, - target_url: str, - email_template: EmailTemplate, - template_data: dict = None, - ) -> str: + def send_assignment_submitted_notification( + cls, recipient: User, sender: User, assignment_completion: AssignmentCompletion + ): + verb = gettext( + "%(sender)s hat die geleitete Fallarbeit «%(assignment_title)s» abgegeben." + ) % { + "sender": sender.get_full_name(), + "assignment_title": assignment_completion.assignment.title, + } + return cls._send_notification( recipient=recipient, verb=verb, + notification_category=NotificationCategory.USER_INTERACTION, + notification_trigger=NotificationTrigger.CASEWORK_SUBMITTED, sender=sender, - course=course, - target_url=target_url, - notification_type=NotificationType.USER_INTERACTION, - email_template=email_template, - template_data=template_data, + target_url=assignment_completion.get_assignment_evaluation_frontend_url(), + course_session=assignment_completion.course_session, + action_object=assignment_completion, + email_template=EmailTemplate.CASEWORK_SUBMITTED, ) @classmethod - def send_progress_notification( + def send_assignment_evaluated_notification( cls, recipient: User, - verb: str, - course: str, + sender: User, + assignment_completion: AssignmentCompletion, target_url: str, - email_template: EmailTemplate, - template_data: dict = None, - ) -> str: + ): + verb = gettext( + "%(sender)s hat die geleitete Fallarbeit «%(assignment_title)s» bewertet." + ) % { + "sender": sender.get_full_name(), + "assignment_title": assignment_completion.assignment.title, + } + return cls._send_notification( recipient=recipient, verb=verb, - course=course, + notification_category=NotificationCategory.USER_INTERACTION, + notification_trigger=NotificationTrigger.CASEWORK_EVALUATED, + sender=sender, target_url=target_url, - notification_type=NotificationType.PROGRESS, - email_template=email_template, - template_data=template_data, + course_session=assignment_completion.course_session, + action_object=assignment_completion, + email_template=EmailTemplate.CASEWORK_EVALUATED, ) @classmethod - def send_information_notification( + def send_new_feedback_notification( cls, recipient: User, - verb: str, - target_url: str, - email_template: EmailTemplate, - template_data: dict = None, - ) -> str: + feedback_response: FeedbackResponse, + ): + verb = f"New feedback for circle {feedback_response.circle.title}" + return cls._send_notification( recipient=recipient, verb=verb, - target_url=target_url, - notification_type=NotificationType.INFORMATION, - email_template=email_template, - template_data=template_data, + notification_category=NotificationCategory.INFORMATION, + notification_trigger=NotificationTrigger.NEW_FEEDBACK, + target_url=f"/course/{feedback_response.course_session.course.slug}/cockpit/feedback/{feedback_response.circle_id}/", + course_session=feedback_response.course_session, + action_object=feedback_response, + email_template=EmailTemplate.NEW_FEEDBACK, + ) + + @classmethod + def send_attendance_course_reminder_notification( + cls, + recipient: User, + attendance_course: CourseSessionAttendanceCourse, + ): + return cls._send_notification( + recipient=recipient, + verb="Erinnerung: Bald findet ein Präsenzkurs statt", + notification_category=NotificationCategory.INFORMATION, + notification_trigger=NotificationTrigger.ATTENDANCE_COURSE_REMINDER, + target_url=attendance_course.learning_content.get_frontend_url(), + action_object=attendance_course, + course_session=attendance_course.course_session, + email_template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, + template_data=create_template_data_from_course_session_attendance_course( + attendance_course=attendance_course + ), ) @classmethod @@ -76,20 +123,24 @@ class NotificationService: cls, recipient: User, verb: str, - notification_type: NotificationType, - email_template: EmailTemplate, - template_data: dict | None = None, + notification_category: NotificationCategory, + notification_trigger: NotificationTrigger, sender: User | None = None, - course: str | None = None, + action_object: Model | None = None, target_url: str | None = None, + course_session: CourseSession | None = None, + email_template: EmailTemplate | None = None, + template_data: dict | None = None, fail_silently: bool = True, ) -> str: if template_data is None: template_data = {} - notification_identifier = f"{notification_type.name}_{email_template.name}" + notification_identifier = ( + f"{notification_category.name}_{notification_trigger.name}" + ) - actor_avatar_url: Optional[str] = None + actor_avatar_url = "" if not sender: sender = User.objects.get(email="admin") else: @@ -98,8 +149,9 @@ class NotificationService: recipient=recipient.email, sender=sender.email, verb=verb, - notification_type=notification_type, - course=course, + notification_category=notification_category, + notification_trigger=notification_trigger, + course_session=course_session.title if course_session else "", target_url=target_url, template_data=template_data, ) @@ -108,16 +160,20 @@ class NotificationService: notification = NotificationService._find_duplicate_notification( recipient=recipient, verb=verb, - notification_type=notification_type, + notification_category=notification_category, + notification_trigger=notification_trigger, target_url=target_url, - template_name=email_template.name, - template_data=template_data, + course_session=course_session, ) emailed = False if notification and notification.emailed: emailed = True - if cls._should_send_email(notification_type, recipient) and not emailed: + if ( + email_template + and cls._should_send_email(notification_category, recipient) + and not emailed + ): log.debug("Try to send email") try: emailed = cls._send_email( @@ -153,16 +209,18 @@ class NotificationService: sender=sender, recipient=recipient, verb=verb, + action_object=action_object, emailed=emailed, - # The metadata is saved in the 'data' member of the AbstractNotification model - email_template=email_template.name, + # The extra arguments are saved in the 'data' member + email_template=email_template.name if email_template else "", template_data=template_data, ) sent_notification: Notification = response[0][1][0] # 🫨 sent_notification.target_url = target_url - sent_notification.notification_type = notification_type - sent_notification.course = course + sent_notification.notification_category = notification_category + sent_notification.notification_trigger = notification_trigger + sent_notification.course_session = course_session sent_notification.actor_avatar_url = actor_avatar_url sent_notification.save() log.info("Notification sent successfully", emailed=emailed) @@ -181,10 +239,10 @@ class NotificationService: @staticmethod def _should_send_email( - notification_type: NotificationType, recipient: User + notification_category: NotificationCategory, recipient: User ) -> bool: - return str(notification_type) in recipient.additional_json_data.get( - "email_notification_types", [] + return str(notification_category) in recipient.additional_json_data.get( + "email_notification_categories", [] ) @staticmethod @@ -206,10 +264,10 @@ class NotificationService: def _find_duplicate_notification( recipient: User, verb: str, - notification_type: NotificationType, - template_name: str, - template_data: dict, + notification_category: NotificationCategory, + notification_trigger: NotificationTrigger, target_url: str | None, + course_session: CourseSession | None, ) -> Notification | None: """Check if a notification with the same parameters has already been sent to the recipient. This is to prevent duplicate notifications from being sent and to protect against potential programming errors. @@ -217,10 +275,8 @@ class NotificationService: return Notification.objects.filter( recipient=recipient, verb=verb, - notification_type=notification_type, + notification_category=notification_category, + notification_trigger=notification_trigger, target_url=target_url, - data={ - "email_template": template_name, - "template_data": template_data, - }, + course_session=course_session, ).first() diff --git a/server/vbv_lernwelt/notify/tests/test_notify_api.py b/server/vbv_lernwelt/notify/tests/test_notify_api.py index fa9a2863..727841ab 100644 --- a/server/vbv_lernwelt/notify/tests/test_notify_api.py +++ b/server/vbv_lernwelt/notify/tests/test_notify_api.py @@ -4,7 +4,7 @@ from rest_framework.test import APITestCase from vbv_lernwelt.core.admin import User from vbv_lernwelt.core.tests.factories import UserFactory -from vbv_lernwelt.notify.models import Notification, NotificationType +from vbv_lernwelt.notify.models import Notification, NotificationCategory from vbv_lernwelt.notify.tests.factories import NotificationFactory @@ -115,7 +115,7 @@ class TestNotificationSettingsApi(APITestCase): def test_store_retrieve_settings(self): notification_settings = json.dumps( - [NotificationType.INFORMATION, NotificationType.PROGRESS] + [NotificationCategory.INFORMATION, NotificationCategory.PROGRESS] ) api_path = "/api/notify/email_notification_settings/" @@ -128,7 +128,7 @@ class TestNotificationSettingsApi(APITestCase): self.assertEqual(response.json(), notification_settings) self.user.refresh_from_db() self.assertEqual( - self.user.additional_json_data["email_notification_types"], + self.user.additional_json_data["email_notification_categories"], notification_settings, ) diff --git a/server/vbv_lernwelt/notify/tests/test_send_attendance_course_reminders.py b/server/vbv_lernwelt/notify/tests/test_send_attendance_course_reminders.py index 04bc4cc2..de03f261 100644 --- a/server/vbv_lernwelt/notify/tests/test_send_attendance_course_reminders.py +++ b/server/vbv_lernwelt/notify/tests/test_send_attendance_course_reminders.py @@ -1,45 +1,21 @@ -from dataclasses import dataclass from datetime import datetime, timedelta -from unittest.mock import patch -from django.contrib.auth.models import User from django.test import TestCase from django.utils import timezone -from django.utils.translation import gettext_lazy as _ from freezegun import freeze_time from vbv_lernwelt.core.constants import TEST_COURSE_SESSION_BERN_ID from vbv_lernwelt.core.create_default_users import create_default_users from vbv_lernwelt.course.creators.test_course import create_test_course -from vbv_lernwelt.course.models import CourseSession, CourseSessionUser +from vbv_lernwelt.course.models import CourseSession from vbv_lernwelt.course_session.models import CourseSessionAttendanceCourse from vbv_lernwelt.learnpath.models import LearningContentAttendanceCourse -from vbv_lernwelt.notify.email.email_services import EmailTemplate from vbv_lernwelt.notify.management.commands.send_attendance_course_reminders import ( attendance_course_reminder_notification_job, ) +from vbv_lernwelt.notify.models import Notification -@dataclass -class SentNotification: - recipient: User - verb: str - target_url: str - email_template: EmailTemplate - template_data: dict - - -sent_notifications: list[SentNotification] = [] - - -def on_send_notification(**kwargs) -> None: - sent_notifications.append(SentNotification(**kwargs)) - - -@patch( - "vbv_lernwelt.notify.services.NotificationService.send_information_notification", - on_send_notification, -) class TestAttendanceCourseReminders(TestCase): def setUp(self): create_default_users() @@ -57,13 +33,12 @@ class TestAttendanceCourseReminders(TestCase): location="Handelsschule KV Bern, Zimmer 123, Eigerstrasse 16, 3012 Bern", trainer="Roland Grossenbacher", ) - in_one_week = datetime.now() + timedelta(weeks=1) - self.csac.due_date.start = timezone.make_aware( - in_one_week.replace(hour=7, minute=30, second=0, microsecond=0) + ref_date_time = timezone.make_aware(datetime(2023, 8, 29)) + self.csac.due_date.start = ref_date_time.replace( + hour=7, minute=30, second=0, microsecond=0 ) - - self.csac.due_date.end = timezone.make_aware( - in_one_week.replace(hour=16, minute=15, second=0, microsecond=0) + self.csac.due_date.end = ref_date_time.replace( + hour=16, minute=15, second=0, microsecond=0 ) self.csac.due_date.save() @@ -89,41 +64,53 @@ class TestAttendanceCourseReminders(TestCase): self.csac_future.due_date.save() attendance_course_reminder_notification_job() - self.assertEquals(3, len(sent_notifications)) - recipients = CourseSessionUser.objects.filter( - course_session_id=self.csac.course_session.id - ) - self.assertEquals( - set(map(lambda n: n.recipient, sent_notifications)), - set(map(lambda csu: csu.user, recipients)), + + self.assertEquals(3, len(Notification.objects.all())) + notification = Notification.objects.get( + recipient__username="test-student1@example.com" ) - for notification in sent_notifications: - self.assertEquals( - _("Erinnerung: Bald findet ein Präsenzkurs statt"), - notification.verb, - ) - self.assertEquals( - "/course/test-lehrgang/learn/fahrzeug/präsenzkurs-fahrzeug", - notification.target_url, - ) - self.assertEquals( - self.csac.learning_content.title, - notification.template_data["attendance_course"], - ) - self.assertEquals( - self.csac.location, - notification.template_data["location"], - ) - self.assertEquals( - self.csac.trainer, - notification.template_data["trainer"], - ) - self.assertEquals( - self.csac.due_date.start.strftime("%d.%m.%Y %H:%M"), - notification.template_data["start"], - ) - self.assertEquals( - self.csac.due_date.end.strftime("%d.%m.%Y %H:%M"), - notification.template_data["end"], - ) + self.assertEquals( + "Erinnerung: Bald findet ein Präsenzkurs statt", + notification.verb, + ) + self.assertEquals( + "INFORMATION", + notification.notification_category, + ) + self.assertEquals( + "ATTENDANCE_COURSE_REMINDER", + notification.notification_trigger, + ) + self.assertEquals( + self.csac, + notification.action_object, + ) + self.assertEquals( + self.csac.course_session, + notification.course_session, + ) + self.assertEquals( + "/course/test-lehrgang/learn/fahrzeug/präsenzkurs-fahrzeug", + notification.target_url, + ) + self.assertEquals( + self.csac.learning_content.title, + notification.data["template_data"]["attendance_course"], + ) + self.assertEquals( + self.csac.location, + notification.data["template_data"]["location"], + ) + self.assertEquals( + self.csac.trainer, + notification.data["template_data"]["trainer"], + ) + self.assertEquals( + self.csac.due_date.start.strftime("%d.%m.%Y %H:%M"), + notification.data["template_data"]["start"], + ) + self.assertEquals( + self.csac.due_date.end.strftime("%d.%m.%Y %H:%M"), + notification.data["template_data"]["end"], + ) diff --git a/server/vbv_lernwelt/notify/tests/test_service.py b/server/vbv_lernwelt/notify/tests/test_service.py index 2ec5500c..6d8af677 100644 --- a/server/vbv_lernwelt/notify/tests/test_service.py +++ b/server/vbv_lernwelt/notify/tests/test_service.py @@ -5,7 +5,11 @@ from django.test import TestCase from vbv_lernwelt.core.models import User from vbv_lernwelt.core.tests.factories import UserFactory from vbv_lernwelt.notify.email.email_services import EmailTemplate -from vbv_lernwelt.notify.models import Notification, NotificationType +from vbv_lernwelt.notify.models import ( + Notification, + NotificationCategory, + NotificationTrigger, +) from vbv_lernwelt.notify.services import NotificationService @@ -24,142 +28,178 @@ class TestNotificationService(TestCase): self.admin = UserFactory(username="admin", email="admin") self.sender_username = "Bob" - UserFactory(username=self.sender_username, email="bob@gmail.com") + UserFactory(username=self.sender_username, email="bob@example.com") self.sender = User.objects.get(username=self.sender_username) self.recipient_username = "Alice" - UserFactory(username=self.recipient_username, email="alice@gmail.com") + UserFactory(username=self.recipient_username, email="alice@example.com") self.recipient = User.objects.get(username=self.recipient_username) - self.recipient.additional_json_data["email_notification_types"] = json.dumps( - ["USER_INTERACTION", "INFORMATION"] - ) self.recipient.save() self.client.login(username=self.recipient, password="pw") - def test_send_information_notification(self): - verb = "Wartungsarbeiten: 13.12 10:00 - 13:00 Uhr" - target_url = "https://www.vbv.ch" - self.notification_service.send_information_notification( - recipient=self.recipient, - verb=verb, - target_url=target_url, - email_template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, - ) - self.assertEqual(1, Notification.objects.count()) - notification: Notification = Notification.objects.first() - self.assertEqual(self.admin, notification.actor) - self.assertEqual(verb, notification.verb) - self.assertEqual(target_url, notification.target_url) - self.assertEqual( - str(NotificationType.INFORMATION), notification.notification_type - ) - self.assertTrue(notification.emailed) - - def test_send_progress_notification(self): - verb = "Super Fortschritt! Melde dich jetzt an." - target_url = "https://www.vbv.ch" - course = "Versicherungsvermittler/in" - self.notification_service.send_progress_notification( - recipient=self.recipient, - verb=verb, - target_url=target_url, - course=course, - email_template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, - ) - self.assertEqual(1, Notification.objects.count()) - notification: Notification = Notification.objects.first() - self.assertEqual(self.admin, notification.actor) - self.assertEqual(verb, notification.verb) - self.assertEqual(target_url, notification.target_url) - self.assertEqual(course, notification.course) - self.assertEqual(str(NotificationType.PROGRESS), notification.notification_type) - self.assertFalse(notification.emailed) - - def test_send_user_interaction_notification(self): + def test_send_notification_without_email(self): verb = "Anne hat deinen Auftrag bewertet" target_url = "https://www.vbv.ch" - course = "Versicherungsvermittler/in" - self.notification_service.send_user_interaction_notification( + result = self.notification_service._send_notification( sender=self.sender, recipient=self.recipient, verb=verb, target_url=target_url, - course=course, - email_template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, + notification_category=NotificationCategory.USER_INTERACTION, + notification_trigger=NotificationTrigger.CASEWORK_SUBMITTED, + fail_silently=False, ) + + self.assertEqual(result, "USER_INTERACTION_CASEWORK_SUBMITTED_success") + self.assertEqual(1, Notification.objects.count()) notification: Notification = Notification.objects.first() self.assertEqual(self.sender, notification.actor) self.assertEqual(verb, notification.verb) self.assertEqual(target_url, notification.target_url) - self.assertEqual(course, notification.course) self.assertEqual( - str(NotificationType.USER_INTERACTION), notification.notification_type + str(NotificationCategory.USER_INTERACTION), + notification.notification_category, + ) + self.assertFalse(notification.emailed) + + def test_send_notification_with_email(self): + self.recipient.additional_json_data[ + "email_notification_categories" + ] = json.dumps(["USER_INTERACTION"]) + self.recipient.save() + + verb = "Anne hat deinen Auftrag bewertet" + target_url = "https://www.vbv.ch" + result = self.notification_service._send_notification( + sender=self.sender, + recipient=self.recipient, + verb=verb, + target_url=target_url, + notification_category=NotificationCategory.USER_INTERACTION, + notification_trigger=NotificationTrigger.CASEWORK_SUBMITTED, + email_template=EmailTemplate.CASEWORK_SUBMITTED, + fail_silently=False, + ) + + self.assertEqual(result, "USER_INTERACTION_CASEWORK_SUBMITTED_emailed_success") + + self.assertEqual(1, Notification.objects.count()) + notification: Notification = Notification.objects.first() + self.assertEqual(self.sender, notification.actor) + self.assertEqual(verb, notification.verb) + self.assertEqual(target_url, notification.target_url) + self.assertEqual( + str(NotificationCategory.USER_INTERACTION), + notification.notification_category, ) self.assertTrue(notification.emailed) def test_does_not_send_duplicate_notification(self): verb = "Anne hat deinen Auftrag bewertet" target_url = "https://www.vbv.ch" - course = "Versicherungsvermittler/in" - for i in range(2): - self.notification_service.send_user_interaction_notification( - sender=self.sender, - recipient=self.recipient, - verb=verb, - target_url=target_url, - course=course, - email_template=EmailTemplate.ATTENDANCE_COURSE_REMINDER, - template_data={ - "blah": 123, - "foo": "ich habe hunger", - }, - ) + result = self.notification_service._send_notification( + sender=self.sender, + recipient=self.recipient, + verb=verb, + target_url=target_url, + notification_category=NotificationCategory.USER_INTERACTION, + notification_trigger=NotificationTrigger.CASEWORK_SUBMITTED, + email_template=EmailTemplate.CASEWORK_SUBMITTED, + template_data={ + "blah": 123, + "foo": "ich habe hunger", + }, + ) + self.assertEqual(result, "USER_INTERACTION_CASEWORK_SUBMITTED_success") - self.assertEqual(1, Notification.objects.count()) - notification: Notification = Notification.objects.first() - self.assertEqual(self.sender, notification.actor) - self.assertEqual(verb, notification.verb) - self.assertEqual(target_url, notification.target_url) - self.assertEqual(course, notification.course) - self.assertEqual( - str(NotificationType.USER_INTERACTION), - notification.notification_type, - ) - self.assertTrue(notification.emailed) + result = self.notification_service._send_notification( + sender=self.sender, + recipient=self.recipient, + verb=verb, + target_url=target_url, + notification_category=NotificationCategory.USER_INTERACTION, + notification_trigger=NotificationTrigger.CASEWORK_SUBMITTED, + email_template=EmailTemplate.CASEWORK_SUBMITTED, + template_data={ + "blah": 123, + "foo": "ich habe hunger", + }, + ) + self.assertEqual(result, "USER_INTERACTION_CASEWORK_SUBMITTED_duplicate") + + self.assertEqual(1, Notification.objects.count()) + notification: Notification = Notification.objects.first() + self.assertEqual(self.sender, notification.actor) + self.assertEqual(verb, notification.verb) + self.assertEqual(target_url, notification.target_url) + self.assertEqual( + str(NotificationCategory.USER_INTERACTION), + notification.notification_category, + ) + self.assertEqual( + str(NotificationTrigger.CASEWORK_SUBMITTED), + notification.notification_trigger, + ) + self.assertFalse(notification.emailed) + + # when the email was not sent, yet it will still send it afterwards... + self.recipient.additional_json_data[ + "email_notification_categories" + ] = json.dumps(["USER_INTERACTION"]) + self.recipient.save() + + result = self.notification_service._send_notification( + sender=self.sender, + recipient=self.recipient, + verb=verb, + target_url=target_url, + notification_category=NotificationCategory.USER_INTERACTION, + notification_trigger=NotificationTrigger.CASEWORK_SUBMITTED, + email_template=EmailTemplate.CASEWORK_SUBMITTED, + template_data={ + "blah": 123, + "foo": "ich habe hunger", + }, + ) + self.assertEqual( + result, "USER_INTERACTION_CASEWORK_SUBMITTED_emailed_duplicate" + ) + + self.assertEqual(1, Notification.objects.count()) + notification: Notification = Notification.objects.first() + self.assertTrue(notification.emailed) def test_only_sends_email_if_enabled(self): # Assert no mail is sent if corresponding email notification type is not enabled - self.recipient.additional_json_data["email_notification_types"] = json.dumps( - ["INFORMATION"] - ) - self.recipient.save() - self.notification_service.send_user_interaction_notification( + self.notification_service._send_notification( sender=self.sender, recipient=self.recipient, verb="should not be sent", target_url="", - course="", - email_template=EmailTemplate.CASEWORK_EVALUATED, + notification_category=NotificationCategory.USER_INTERACTION, + notification_trigger=NotificationTrigger.CASEWORK_SUBMITTED, + email_template=EmailTemplate.CASEWORK_SUBMITTED, template_data={}, ) self.assertEqual(1, Notification.objects.count()) self.assertFalse(self._has_sent_emails()) # Assert mail is sent if corresponding email notification type is enabled - self.recipient.additional_json_data["email_notification_types"] = json.dumps( - ["USER_INTERACTION"] - ) + self.recipient.additional_json_data[ + "email_notification_categories" + ] = json.dumps(["USER_INTERACTION"]) self.recipient.save() - self.notification_service.send_user_interaction_notification( + self.notification_service._send_notification( sender=self.sender, recipient=self.recipient, verb="should be sent", target_url="", - course="", - email_template=EmailTemplate.CASEWORK_EVALUATED, + notification_category=NotificationCategory.USER_INTERACTION, + notification_trigger=NotificationTrigger.CASEWORK_SUBMITTED, + email_template=EmailTemplate.CASEWORK_SUBMITTED, template_data={}, ) self.assertEqual(2, Notification.objects.count()) diff --git a/server/vbv_lernwelt/notify/views.py b/server/vbv_lernwelt/notify/views.py index b5e2f50b..d4ee3ee9 100644 --- a/server/vbv_lernwelt/notify/views.py +++ b/server/vbv_lernwelt/notify/views.py @@ -4,11 +4,11 @@ from rest_framework.response import Response @api_view(["POST", "GET"]) def email_notification_settings(request): - EMAIL_NOTIFICATION_TYPES = "email_notification_types" + EMAIL_NOTIFICATION_CATEGORIES = "email_notification_categories" if request.method == "POST": - request.user.additional_json_data[EMAIL_NOTIFICATION_TYPES] = request.data + request.user.additional_json_data[EMAIL_NOTIFICATION_CATEGORIES] = request.data request.user.save() return Response( status=200, - data=request.user.additional_json_data.get(EMAIL_NOTIFICATION_TYPES, []), + data=request.user.additional_json_data.get(EMAIL_NOTIFICATION_CATEGORIES, []), )