From 0efb57745e89d18e8a61b113212d2a365e052063 Mon Sep 17 00:00:00 2001 From: Topvennie Date: Mon, 20 May 2024 17:08:43 +0200 Subject: [PATCH 1/2] chore: send notifications --- backend/api/serializers/group_serializer.py | 15 ++++++- backend/api/signals.py | 8 ++++ backend/api/tasks/docker_image.py | 17 +++++-- backend/api/tasks/extra_check.py | 28 +++++++++++- backend/api/tasks/structure_check.py | 15 +++++++ .../fixtures/realistic/realistic.yaml | 45 +++++++++++++++++++ .../locale/en/LC_MESSAGES/django.po | 35 +++++++++++++++ .../locale/nl/LC_MESSAGES/django.po | 35 +++++++++++++++ backend/notifications/logic.py | 4 +- backend/notifications/serializers.py | 11 +++-- backend/notifications/signals.py | 27 +++++++---- development.sh | 2 +- 12 files changed, 218 insertions(+), 24 deletions(-) diff --git a/backend/api/serializers/group_serializer.py b/backend/api/serializers/group_serializer.py index 3865f7a2..b6f95d61 100644 --- a/backend/api/serializers/group_serializer.py +++ b/backend/api/serializers/group_serializer.py @@ -1,11 +1,13 @@ +from api.models.assistant import Assistant from api.models.group import Group from api.models.student import Student -from api.models.assistant import Assistant from api.models.teacher import Teacher -from api.permissions.role_permissions import is_student, is_assistant, is_teacher +from api.permissions.role_permissions import (is_assistant, is_student, + is_teacher) from api.serializers.project_serializer import ProjectSerializer from api.serializers.student_serializer import StudentIDSerializer from django.utils.translation import gettext +from notifications.signals import NotificationType, notification_create from rest_framework import serializers from rest_framework.exceptions import ValidationError @@ -55,6 +57,15 @@ def validate(self, attrs): if "score" in attrs and attrs["score"] > group.project.max_score: raise ValidationError(gettext("group.errors.score_exceeds_max")) + if "score" in attrs and (group.score is None or attrs["score"] != group.score): + # Score is updated -> send notification + notification_create.send( + sender=Group, + type=NotificationType.SCORE_UPDATED, + queryset=list(group.students.all()), + arguments={"score": attrs["score"]}, + ) + return attrs diff --git a/backend/api/signals.py b/backend/api/signals.py index 82bc905c..ba034ca4 100644 --- a/backend/api/signals.py +++ b/backend/api/signals.py @@ -15,6 +15,7 @@ from authentication.signals import user_created from django.db.models.signals import post_delete, post_save, pre_delete from django.dispatch import Signal, receiver +from notifications.signals import NotificationType, notification_create # MARK: Signals @@ -119,6 +120,13 @@ def hook_submission(sender, instance: Submission, created: bool, **kwargs): run_all_checks.send(sender=Submission, submission=instance) pass + notification_create.send( + sender=Submission, + type=NotificationType.SUBMISSION_RECEIVED, + queryset=list(instance.group.students.all()), + arguments={} + ) + @receiver(post_save, sender=DockerImage) def hook_docker_image(sender, instance: DockerImage, created: bool, **kwargs): diff --git a/backend/api/tasks/docker_image.py b/backend/api/tasks/docker_image.py index 3636ef36..246d5934 100644 --- a/backend/api/tasks/docker_image.py +++ b/backend/api/tasks/docker_image.py @@ -3,6 +3,7 @@ from api.logic.get_file_path import get_docker_image_tag from api.models.docker import DockerImage, StateEnum from celery import shared_task +from notifications.signals import NotificationType, notification_create from ypovoli.settings import MEDIA_ROOT @@ -12,6 +13,8 @@ def task_docker_image_build(docker_image: DockerImage): docker_image.state = StateEnum.BUILDING docker_image.save() + notification_type = NotificationType.DOCKER_IMAGE_BUILD_SUCCESS + # Build the image try: client = docker.from_env() @@ -20,10 +23,18 @@ def task_docker_image_build(docker_image: DockerImage): docker_image.state = StateEnum.READY except (docker.errors.APIError, docker.errors.BuildError, TypeError): docker_image.state = StateEnum.ERROR - # TODO: Sent notification + notification_type = NotificationType.DOCKER_IMAGE_BUILD_ERROR + finally: + # Update the state + docker_image.save() - # Update the state - docker_image.save() + # Send notification + notification_create.send( + sender=DockerImage, + type=notification_type, + queryset=[docker_image.owner], + arguments={"name": docker_image.name}, + ) @shared_task diff --git a/backend/api/tasks/extra_check.py b/backend/api/tasks/extra_check.py index 5ddb1565..1f63e87a 100644 --- a/backend/api/tasks/extra_check.py +++ b/backend/api/tasks/extra_check.py @@ -13,10 +13,10 @@ from api.models.docker import StateEnum as DockerStateEnum from api.models.submission import ErrorMessageEnum, ExtraCheckResult, StateEnum from celery import shared_task -from django.core.files import File from django.core.files.base import ContentFile from docker.models.containers import Container from docker.types import LogConfig +from notifications.signals import NotificationType, notification_create from requests.exceptions import ConnectionError @@ -36,12 +36,22 @@ def task_extra_check_start(structure_check_result: bool, extra_check_result: Ext extra_check_result.error_message = ErrorMessageEnum.DOCKER_IMAGE_ERROR extra_check_result.save() + notification_create.send( + sender=ExtraCheckResult, + type=NotificationType.EXTRA_CHECK_FAIL, + queryset=list(extra_check_result.submission.group.students.all()), + arguments={"name": extra_check_result.extra_check.name}, + ) + return structure_check_result # Will probably never happen but doesn't hurt to check while extra_check_result.submission.running_checks: sleep(1) + # Notification type + notification_type = NotificationType.EXTRA_CHECK_SUCCESS + # Lock extra_check_result.submission.running_checks = True @@ -114,41 +124,49 @@ def task_extra_check_start(structure_check_result: bool, extra_check_result: Ext case 1: extra_check_result.result = StateEnum.FAILED extra_check_result.error_message = ErrorMessageEnum.CHECK_ERROR + notification_type = NotificationType.EXTRA_CHECK_FAIL # Time limit case 2: extra_check_result.result = StateEnum.FAILED extra_check_result.error_message = ErrorMessageEnum.TIME_LIMIT + notification_type = NotificationType.EXTRA_CHECK_FAIL # Memory limit case 3: extra_check_result.result = StateEnum.FAILED extra_check_result.error_message = ErrorMessageEnum.MEMORY_LIMIT + notification_type = NotificationType.EXTRA_CHECK_FAIL # Catch all non zero exit codes case _: extra_check_result.result = StateEnum.FAILED extra_check_result.error_message = ErrorMessageEnum.RUNTIME_ERROR + notification_type = NotificationType.EXTRA_CHECK_FAIL # Docker image error except (docker.errors.APIError, docker.errors.ImageNotFound): extra_check_result.result = StateEnum.FAILED extra_check_result.error_message = ErrorMessageEnum.DOCKER_IMAGE_ERROR + notification_type = NotificationType.EXTRA_CHECK_FAIL # Runtime error except docker.errors.ContainerError: extra_check_result.result = StateEnum.FAILED extra_check_result.error_message = ErrorMessageEnum.RUNTIME_ERROR + notification_type = NotificationType.EXTRA_CHECK_FAIL # Timeout error except ConnectionError: extra_check_result.result = StateEnum.FAILED extra_check_result.error_message = ErrorMessageEnum.TIME_LIMIT + notification_type = NotificationType.EXTRA_CHECK_FAIL # Unknown error except Exception: extra_check_result.result = StateEnum.FAILED extra_check_result.error_message = ErrorMessageEnum.UNKNOWN + notification_type = NotificationType.EXTRA_CHECK_FAIL # Cleanup and data saving # Start by saving any logs @@ -165,6 +183,14 @@ def task_extra_check_start(structure_check_result: bool, extra_check_result: Ext extra_check_result.log_file.save(submission_uuid, content=ContentFile(logs), save=False) + # Send notification + notification_create.send( + sender=ExtraCheckResult, + type=notification_type, + queryset=list(extra_check_result.submission.group.students.all()), + arguments={"name": extra_check_result.extra_check.name}, + ) + # Zip and save any possible artifacts memory_zip = io.BytesIO() if os.listdir(artifacts_directory): diff --git a/backend/api/tasks/structure_check.py b/backend/api/tasks/structure_check.py index 8adce52a..d22cd866 100644 --- a/backend/api/tasks/structure_check.py +++ b/backend/api/tasks/structure_check.py @@ -5,6 +5,7 @@ from api.models.submission import (ErrorMessageEnum, StateEnum, StructureCheckResult) from celery import shared_task +from notifications.signals import NotificationType, notification_create @shared_task() @@ -19,6 +20,9 @@ def task_structure_check_start(structure_check_results: list[StructureCheckResul # Lock structure_check_results[0].submission.running_checks = True + # Notification type + notification_type = NotificationType.STRUCTURE_CHECK_SUCCESS + all_checks_passed = True # Boolean to check if all structure checks passed name_ext = _get_all_name_ext(structure_check_results[0].submission.zip.path) # Dict with file name and extension @@ -38,6 +42,7 @@ def task_structure_check_start(structure_check_results: list[StructureCheckResul if len(extensions) == 0: structure_check_result.result = StateEnum.FAILED structure_check_result.error_message = ErrorMessageEnum.FILE_DIR_NOT_FOUND + notification_type = NotificationType.STRUCTURE_CHECK_FAIL # Check if no blocked extension is present if structure_check_result.result == StateEnum.SUCCESS: @@ -45,6 +50,7 @@ def task_structure_check_start(structure_check_results: list[StructureCheckResul if extension.extension in extensions: structure_check_result.result = StateEnum.FAILED structure_check_result.error_message = ErrorMessageEnum.BLOCKED_EXTENSION + notification_type = NotificationType.STRUCTURE_CHECK_FAIL # Check if all obligated extensions are present if structure_check_result.result == StateEnum.SUCCESS: @@ -52,6 +58,7 @@ def task_structure_check_start(structure_check_results: list[StructureCheckResul if extension.extension not in extensions: structure_check_result.result = StateEnum.FAILED structure_check_result.error_message = ErrorMessageEnum.OBLIGATED_EXTENSION_NOT_FOUND + notification_type = NotificationType.STRUCTURE_CHECK_FAIL all_checks_passed = all_checks_passed and structure_check_result.result == StateEnum.SUCCESS structure_check_result.save() @@ -59,6 +66,14 @@ def task_structure_check_start(structure_check_results: list[StructureCheckResul # Release structure_check_results[0].submission.running_checks = False + # Send notification + notification_create.send( + sender=StructureCheckResult, + type=notification_type, + queryset=list(structure_check_results[0].submission.group.students.all()), + arguments={}, + ) + return all_checks_passed diff --git a/backend/notifications/fixtures/realistic/realistic.yaml b/backend/notifications/fixtures/realistic/realistic.yaml index e69de29b..0e37a039 100644 --- a/backend/notifications/fixtures/realistic/realistic.yaml +++ b/backend/notifications/fixtures/realistic/realistic.yaml @@ -0,0 +1,45 @@ +- model: notifications.notificationtemplate + pk: 1 + fields: + title_key: "Title: Score added" + description_key: "Description: Score added %(score)s" +- model: notifications.notificationtemplate + pk: 2 + fields: + title_key: "Title: Score updated" + description_key: "Description: Score updated %(score)s" +- model: notifications.notificationtemplate + pk: 3 + fields: + title_key: "Title: Docker image build success" + description_key: "Description: Docker image build success %(name)s" +- model: notifications.notificationtemplate + pk: 4 + fields: + title_key: "Title: Docker image build error" + description_key: "Description: Docker image build error %(name)s" +- model: notifications.notificationtemplate + pk: 5 + fields: + title_key: "Title: Extra check success" + description_key: "Description: Extra check success %(name)s" +- model: notifications.notificationtemplate + pk: 6 + fields: + title_key: "Title: Extra check error" + description_key: "Description: Extra check error %(name)s" +- model: notifications.notificationtemplate + pk: 7 + fields: + title_key: "Title: Structure checks success" + description_key: "Description: Structure checks success" +- model: notifications.notificationtemplate + pk: 8 + fields: + title_key: "Title: Structure checks error" + description_key: "Description: Structure checks" +- model: notifications.notificationtemplate + pk: 9 + fields: + title_key: "Title: Submission received" + description_key: "Description: Submission received" diff --git a/backend/notifications/locale/en/LC_MESSAGES/django.po b/backend/notifications/locale/en/LC_MESSAGES/django.po index 465520e9..d9ee882e 100644 --- a/backend/notifications/locale/en/LC_MESSAGES/django.po +++ b/backend/notifications/locale/en/LC_MESSAGES/django.po @@ -30,3 +30,38 @@ msgid "Title: Score updated" msgstr "New score" msgid "Description: Score updated %(score)s" msgstr "Your score has been updated.\nNew score: %(score)s" +# Docker Image Build Succes +msgid "Title: Docker image build success" +msgstr "Docker image successfully build" +msgid "Description: Docker image build success %(name)s" +msgstr "Your docker image, $(name)s, has successfully been build" +# Docker Image Build Error +msgid "Title: Docker image build error" +msgstr "Docker image failed to build" +msgid "Description: Docker image build error %(name)s" +msgstr "Failed to build your docker image, %(name)s" +# Extra Check Succes +msgid "Title: Extra check success" +msgstr "Passed an extra check" +msgid "Description: Extra check success %(name)s" +msgstr "Your submission passed the extra check, $(name)s" +# Extra Check Error +msgid "Title: Extra check error" +msgstr "Failed an extra check" +msgid "Description: Extra check error %(name)s" +msgstr "Your submission failed to pass the extra check, %(name)s" +# Structure Checks Succes +msgid "Title: Structure checks success" +msgstr "Passed all structure checks" +msgid "Description: Structure checks success" +msgstr "Your submission passed all structure checks" +# Structure Checks Error +msgid "Title: Structure checks error" +msgstr "Failed a structure check" +msgid "Description: Structure checks" +msgstr "Your submission failed one or more structure checks" +# Submission received +msgid "Title: Submission received" +msgstr "Received submission" +msgid "Description: Submission received" +msgstr "We have received your submission" diff --git a/backend/notifications/locale/nl/LC_MESSAGES/django.po b/backend/notifications/locale/nl/LC_MESSAGES/django.po index 5a854108..7d9633d0 100644 --- a/backend/notifications/locale/nl/LC_MESSAGES/django.po +++ b/backend/notifications/locale/nl/LC_MESSAGES/django.po @@ -30,3 +30,38 @@ msgid "Title: Score updated" msgstr "Nieuwe score" msgid "Description: Score updated %(score)s" msgstr "Je score is geupdate.\nNieuwe score: %(score)s" +# Docker Image Build Succes +msgid "Title: Docker image build success" +msgstr "Docker image succesvol gebouwd" +msgid "Description: Docker image build success %(name)s" +msgstr "Jouw docker image, $(name)s, is succesvol gebouwd" +# Docker Image Build Error +msgid "Title: Docker image build error" +msgstr "Docker image is gefaald om te bouwen" +msgid "Description: Docker image build error %(name)s" +msgstr "Gefaald om jouw docker image, %(name)s, te bouwen" +# Extra Check Succes +msgid "Title: Extra check success" +msgstr "Geslaagd voor een extra check" +msgid "Description: Extra check success %(name)s" +msgstr "Jouw indiening is geslaagd voor de extra check: $(name)s" +# Extra Check Error +msgid "Title: Extra check error" +msgstr "Gefaald voor een extra check" +msgid "Description: Extra check error %(name)s" +msgstr "Jouw indiening is gefaald voor de extra check: %(name)s" +# Structure Checks Succes +msgid "Title: Structure checks success" +msgstr "Geslaagd voor de structuur checks" +msgid "Description: Structure checks success" +msgstr "Jouw indiening is geslaagd voor alle structuur checks" +# Structure Checks Error +msgid "Title: Structure checks error" +msgstr "Gefaald voor een structuur check" +msgid "Description: Structure checks" +msgstr "Jouw indiening is gefaald voor een structuur check" +# Submission received +msgid "Title: Submission received" +msgstr "Indiening ontvangen" +msgid "Description: Submission received" +msgstr "We hebben jouw indiening ontvangen" diff --git a/backend/notifications/logic.py b/backend/notifications/logic.py index e2061a87..0f7133a3 100644 --- a/backend/notifications/logic.py +++ b/backend/notifications/logic.py @@ -22,7 +22,7 @@ def get_message_dict(notification: Notification) -> Dict[str, str]: # Call the function after 60 seconds and no more than once in that period def schedule_send_mails(): - if not cache.get("notifications_send_mails"): + if not cache.get("notifications_send_mails", False): cache.set("notifications_send_mails", True) _send_mails.apply_async(countdown=60) @@ -41,7 +41,7 @@ def _send_mail(mail: mail.EmailMessage, result: List[bool]): # TODO: Retry 3 # https://docs.celeryq.dev/en/v5.3.6/getting-started/next-steps.html#next-steps # Send all unsent emails -@shared_task(ignore_result=True) +@shared_task() def _send_mails(): # All notifications that need to be sent notifications = Notification.objects.filter(is_sent=False) diff --git a/backend/notifications/serializers.py b/backend/notifications/serializers.py index d4c488ba..2e3d7c75 100644 --- a/backend/notifications/serializers.py +++ b/backend/notifications/serializers.py @@ -1,5 +1,4 @@ import re -from typing import Dict, List from authentication.models import User from notifications.logic import get_message_dict @@ -23,14 +22,14 @@ class NotificationSerializer(serializers.ModelSerializer): message = serializers.SerializerMethodField() # Check if the required arguments are present - def _get_missing_keys(self, string: str, arguments: Dict[str, str]) -> List[str]: - required_keys: List[str] = re.findall(r"%\((\w+)\)", string) + def _get_missing_keys(self, string: str, arguments: dict[str, str]) -> list[str]: + required_keys: list[str] = re.findall(r"%\((\w+)\)", string) missing_keys = [key for key in required_keys if key not in arguments] return missing_keys - def validate(self, data: Dict[str, str]) -> Dict[str, str]: - data: Dict[str, str] = super().validate(data) + def validate(self, data: dict[str, str | int | dict[str, str]]) -> dict[str, str]: + data: dict[str, str] = super().validate(data) # Validate the arguments if "arguments" not in data: @@ -56,7 +55,7 @@ def validate(self, data: Dict[str, str]) -> Dict[str, str]: return data # Get the message from the template and arguments - def get_message(self, obj: Notification) -> Dict[str, str]: + def get_message(self, obj: Notification) -> dict[str, str]: return get_message_dict(obj) class Meta: diff --git a/backend/notifications/signals.py b/backend/notifications/signals.py index f8203f39..979184bb 100644 --- a/backend/notifications/signals.py +++ b/backend/notifications/signals.py @@ -15,25 +15,27 @@ @receiver(notification_create) def notification_creation( + sender: type, type: NotificationType, - queryset: QuerySet[User], + queryset: list[User], arguments: Dict[str, str], **kwargs, # Required by django ) -> bool: data: List[Dict[str, Union[str, int, Dict[str, str]]]] = [] for user in queryset: - data.append( - { - "template_id": type.value, - "user": reverse("user-detail", kwargs={"pk": user.id}), - "arguments": arguments, - } - ) + if user: + data.append( + { + "template_id": type.value, + "user": reverse("user-detail", kwargs={"pk": user.id}), + "arguments": arguments, + } + ) serializer = NotificationSerializer(data=data, many=True) - if not serializer.is_valid(): + if not serializer.is_valid(raise_exception=False): return False serializer.save() @@ -46,3 +48,10 @@ def notification_creation( class NotificationType(Enum): SCORE_ADDED = 1 # Arguments: {"score": int} SCORE_UPDATED = 2 # Arguments: {"score": int} + DOCKER_IMAGE_BUILD_SUCCESS = 3 # Arguments: {"name": str} + DOCKER_IMAGE_BUILD_ERROR = 4 # Arguments: {"name": str} + EXTRA_CHECK_SUCCESS = 5 # Arguments: {"name": str} + EXTRA_CHECK_FAIL = 6 # Arguments: {"name": str} + STRUCTURE_CHECK_SUCCESS = 7 # Arguments: {} + STRUCTURE_CHECK_FAIL = 8 # Arguments: {} + SUBMISSION_RECEIVED = 9 # Arguments: {} diff --git a/development.sh b/development.sh index 70dfe9fe..5a83cf9d 100755 --- a/development.sh +++ b/development.sh @@ -135,7 +135,7 @@ if [ "$data" != "" ]; then echo "Clearing, Migrating & Populating the database" # We have nog fixtures for notification yet. - docker-compose -f development.yml run backend sh -c "python manage.py flush --no-input; python manage.py migrate; python manage.py loaddata authentication/fixtures/$data/*; python manage.py loaddata api/fixtures/$data/*;" + docker-compose -f development.yml run backend sh -c "python manage.py flush --no-input; python manage.py migrate; python manage.py loaddata notifications/fixtures/$data/*; python manage.py loaddata authentication/fixtures/$data/*; python manage.py loaddata api/fixtures/$data/*;" echo "Stopping the services" docker-compose -f development.yml down From ff67b4bfbcefbf0eb7654bbc965c3b5a3f5eef8d Mon Sep 17 00:00:00 2001 From: Topvennie Date: Tue, 21 May 2024 15:28:39 +0200 Subject: [PATCH 2/2] chore: moved notification sender --- backend/api/serializers/group_serializer.py | 11 ----------- backend/api/views/group_view.py | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/backend/api/serializers/group_serializer.py b/backend/api/serializers/group_serializer.py index b6f95d61..b262889c 100644 --- a/backend/api/serializers/group_serializer.py +++ b/backend/api/serializers/group_serializer.py @@ -1,7 +1,5 @@ -from api.models.assistant import Assistant from api.models.group import Group from api.models.student import Student -from api.models.teacher import Teacher from api.permissions.role_permissions import (is_assistant, is_student, is_teacher) from api.serializers.project_serializer import ProjectSerializer @@ -57,15 +55,6 @@ def validate(self, attrs): if "score" in attrs and attrs["score"] > group.project.max_score: raise ValidationError(gettext("group.errors.score_exceeds_max")) - if "score" in attrs and (group.score is None or attrs["score"] != group.score): - # Score is updated -> send notification - notification_create.send( - sender=Group, - type=NotificationType.SCORE_UPDATED, - queryset=list(group.students.all()), - arguments={"score": attrs["score"]}, - ) - return attrs diff --git a/backend/api/views/group_view.py b/backend/api/views/group_view.py index 7c118bcd..cf300cfd 100644 --- a/backend/api/views/group_view.py +++ b/backend/api/views/group_view.py @@ -9,6 +9,7 @@ from api.serializers.submission_serializer import SubmissionSerializer from django.utils.translation import gettext from drf_yasg.utils import swagger_auto_schema +from notifications.signals import NotificationType, notification_create from rest_framework.decorators import action from rest_framework.mixins import (CreateModelMixin, DestroyModelMixin, RetrieveModelMixin, UpdateModelMixin) @@ -28,6 +29,22 @@ class GroupViewSet(CreateModelMixin, serializer_class = GroupSerializer permission_classes = [IsAdminUser | GroupPermission] + def update(self, request, *args, **kwargs): + old_group = self.get_object() + response = super().update(request, *args, **kwargs) + if response.status_code == 200: + new_group = self.get_object() + if "score" in request.data and old_group.score != new_group.score: + # Partial updates end up in the update function as well + notification_create.send( + sender=Group, + type=NotificationType.SCORE_UPDATED, + queryset=list(new_group.students.all()), + arguments={"score": str(new_group.score)}, + ) + + return response + @action(detail=True, methods=["get"], permission_classes=[IsAdminUser | GroupStudentPermission]) def students(self, request, **_): """Returns a list of students for the given group"""