diff --git a/backend/api/permissions/course_permissions.py b/backend/api/permissions/course_permissions.py index 71e3d04c..7c24fa2c 100644 --- a/backend/api/permissions/course_permissions.py +++ b/backend/api/permissions/course_permissions.py @@ -1,3 +1,8 @@ +from typing import cast + +from django.contrib.auth.base_user import AbstractBaseUser +from django.contrib.auth.models import AbstractUser + from api.models.course import Course from api.permissions.role_permissions import (is_assistant, is_student, is_teacher) @@ -12,7 +17,7 @@ class CoursePermission(BasePermission): def has_permission(self, request: Request, view: ViewSet) -> bool: """Check if user has permission to view a general course endpoint.""" - user: User = request.user + user: AbstractBaseUser = request.user # Logged-in users can fetch course information. if request.method in SAFE_METHODS: @@ -23,7 +28,7 @@ def has_permission(self, request: Request, view: ViewSet) -> bool: def has_object_permission(self, request: Request, view: ViewSet, course: Course) -> bool: """Check if user has permission to view a detailed course endpoint""" - user = request.user + user: User = cast(User, request.user) # Logged-in users can fetch course details. if request.method in SAFE_METHODS: diff --git a/backend/api/permissions/group_permissions.py b/backend/api/permissions/group_permissions.py index 29280cdf..08ab0641 100644 --- a/backend/api/permissions/group_permissions.py +++ b/backend/api/permissions/group_permissions.py @@ -65,14 +65,7 @@ def has_object_permission(self, request: Request, view: ViewSet, group) -> bool: class GroupSubmissionPermission(BasePermission): """Permission class for submission related group endpoints""" - def has_permission(self, request: Request, view: APIView) -> bool: - """Check if user has permission to view a general group submission endpoint.""" - user = request.user - - # Get the individual permission clauses. - return request.method in SAFE_METHODS or is_teacher(user) or is_assistant(user) - - def had_object_permission(self, request: Request, view: ViewSet, group: Group) -> bool: + def had_object_permission(self, request: Request, _: ViewSet, group: Group) -> bool: """Check if user has permission to view a detailed group submission endpoint""" user = request.user course = group.project.course diff --git a/backend/api/signals.py b/backend/api/signals.py index ba034ca4..f7e42d89 100644 --- a/backend/api/signals.py +++ b/backend/api/signals.py @@ -7,6 +7,7 @@ from api.models.student import Student from api.models.submission import (ExtraCheckResult, StateEnum, StructureCheckResult, Submission) +from api.models.teacher import Teacher from api.tasks.docker_image import (task_docker_image_build, task_docker_image_remove) from api.tasks.extra_check import task_extra_check_start @@ -35,8 +36,11 @@ def _user_creation(user: User, attributes: dict, **_): """Upon user creation, auto-populate additional properties""" student_id: str = cast(str, attributes.get("ugentStudentID")) - if student_id is not None: + if student_id is None: Student.create(user, student_id=student_id) + else: + # For now, we assume that everyone without a student ID is a teacher. + Teacher.create(user) @receiver(run_docker_image_build) @@ -120,12 +124,12 @@ 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={} - ) + notification_create.send( + sender=Submission, + type=NotificationType.SUBMISSION_RECEIVED, + queryset=list(instance.group.students.all()), + arguments={} + ) @receiver(post_save, sender=DockerImage) diff --git a/backend/api/tasks/docker_image.py b/backend/api/tasks/docker_image.py index 246d5934..b4a1eb9f 100644 --- a/backend/api/tasks/docker_image.py +++ b/backend/api/tasks/docker_image.py @@ -21,7 +21,7 @@ def task_docker_image_build(docker_image: DockerImage): client.images.build(path=MEDIA_ROOT, dockerfile=docker_image.file.path, tag=get_docker_image_tag(docker_image), rm=True, quiet=True, forcerm=True) docker_image.state = StateEnum.READY - except (docker.errors.APIError, docker.errors.BuildError, TypeError): + except (docker.errors.BuildError, docker.errors.APIError): docker_image.state = StateEnum.ERROR notification_type = NotificationType.DOCKER_IMAGE_BUILD_ERROR finally: diff --git a/backend/api/views/group_view.py b/backend/api/views/group_view.py index b4c67383..0484ce5f 100644 --- a/backend/api/views/group_view.py +++ b/backend/api/views/group_view.py @@ -70,6 +70,26 @@ def submissions(self, request, **_): ) return Response(serializer.data) + @submissions.mapping.post + @submissions.mapping.put + @swagger_auto_schema(request_body=SubmissionSerializer) + def _add_submission(self, request: Request, **_): + """Add a submission to the group""" + group: Group = self.get_object() + + # Add submission to course + serializer = SubmissionSerializer( + data=request.data, context={"group": group, "request": request} + ) + + if serializer.is_valid(raise_exception=True): + serializer.save(group=group) + + return Response({ + "message": gettext("group.success.submissions.add"), + "submission": serializer.data + }) + @students.mapping.post @students.mapping.put @swagger_auto_schema(request_body=StudentJoinGroupSerializer) @@ -110,23 +130,3 @@ def _remove_student(self, request, **_): return Response({ "message": gettext("group.success.students.remove"), }) - - @submissions.mapping.post - @submissions.mapping.put - @swagger_auto_schema(request_body=SubmissionSerializer) - def _add_submission(self, request: Request, **_): - """Add a submission to the group""" - group: Group = self.get_object() - - # Add submission to course - serializer = SubmissionSerializer( - data=request.data, context={"group": group, "request": request} - ) - - if serializer.is_valid(raise_exception=True): - serializer.save(group=group) - - return Response({ - "message": gettext("group.success.submissions.add"), - "submission": serializer.data - }) diff --git a/backend/api/views/user_view.py b/backend/api/views/user_view.py index 9d8b33a2..b4ae6c93 100644 --- a/backend/api/views/user_view.py +++ b/backend/api/views/user_view.py @@ -79,22 +79,28 @@ def search(self, request: Request) -> Response: @action(detail=True, methods=["get"], permission_classes=[NotificationPermission]) def notifications(self, request: Request, pk: str): """Returns a list of notifications for the given user""" - notifications = Notification.objects.filter(user=pk) + count = min( + int(request.query_params.get("count", 10)), 30 + ) + + # Get the notifications for the user + notifications = Notification.objects.filter(user=pk, is_read=False).order_by("-created_at") + + if notifications.count() < count: + notifications = list(notifications) + list( + Notification.objects.filter(user=pk, is_read=True).order_by("-created_at")[:count - notifications.count()] + ) + + # Serialize the notifications serializer = NotificationSerializer( notifications, many=True, context={"request": request} ) return Response(serializer.data) - @action( - detail=True, - methods=["post"], - permission_classes=[NotificationPermission], - url_path="notifications/read", - ) - def read(self, _: Request, pk: str): + @notifications.mapping.patch + def _read_notifications(self, _: Request, pk: str): """Marks all notifications as read for the given user""" notifications = Notification.objects.filter(user=pk) notifications.update(is_read=True) - return Response(status=HTTP_200_OK) diff --git a/backend/authentication/serializers.py b/backend/authentication/serializers.py index f7d9297b..45b6f343 100644 --- a/backend/authentication/serializers.py +++ b/backend/authentication/serializers.py @@ -1,5 +1,7 @@ from typing import Tuple +from rest_framework.relations import HyperlinkedIdentityField + from authentication.cas.client import client from authentication.models import User from authentication.signals import user_created, user_login @@ -35,7 +37,7 @@ def validate(self, data): # Update the user's last login. if api_settings.UPDATE_LAST_LOGIN: - update_last_login(self, user) + update_last_login(CASTokenObtainSerializer, user) # Login and send authentication signals. if "request" in self.context: @@ -95,11 +97,13 @@ class UserSerializer(ModelSerializer): This serializer validates the user fields for creation and updating. """ faculties = HyperlinkedRelatedField( - many=True, read_only=True, view_name="faculty-detail" + view_name="faculty-detail", + many=True, + read_only=True ) - notifications = HyperlinkedRelatedField( - view_name="notification-detail", + notifications = HyperlinkedIdentityField( + view_name="user-notifications", read_only=True, ) diff --git a/backend/notifications/locale/en/LC_MESSAGES/django.po b/backend/notifications/locale/en/LC_MESSAGES/django.po index d9ee882e..bff878ed 100644 --- a/backend/notifications/locale/en/LC_MESSAGES/django.po +++ b/backend/notifications/locale/en/LC_MESSAGES/django.po @@ -34,7 +34,7 @@ msgstr "Your score has been updated.\nNew score: %(score)s" 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" +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" @@ -44,7 +44,7 @@ msgstr "Failed to build your docker image, %(name)s" 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" +msgstr "Your submission passed the extra check, %(name)s" # Extra Check Error msgid "Title: Extra check error" msgstr "Failed an extra check" diff --git a/backend/notifications/locale/nl/LC_MESSAGES/django.po b/backend/notifications/locale/nl/LC_MESSAGES/django.po index 7d9633d0..6796a0d0 100644 --- a/backend/notifications/locale/nl/LC_MESSAGES/django.po +++ b/backend/notifications/locale/nl/LC_MESSAGES/django.po @@ -34,7 +34,7 @@ msgstr "Je score is geupdate.\nNieuwe score: %(score)s" 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" +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" @@ -44,7 +44,7 @@ msgstr "Gefaald om jouw docker image, %(name)s, te bouwen" 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" +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" diff --git a/backend/notifications/logic.py b/backend/notifications/logic.py index 0f7133a3..a9544995 100644 --- a/backend/notifications/logic.py +++ b/backend/notifications/logic.py @@ -11,8 +11,8 @@ from ypovoli.settings import EMAIL_CUSTOM -# Returns a dictionary with the title and description of the notification def get_message_dict(notification: Notification) -> Dict[str, str]: + """Get the message from the template and arguments.""" return { "title": _(notification.template_id.title_key), "description": _(notification.template_id.description_key) @@ -20,17 +20,17 @@ 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(): + """Schedule the sending of emails.""" if not cache.get("notifications_send_mails", False): cache.set("notifications_send_mails", True) _send_mails.apply_async(countdown=60) -# Try to send one email and set the result -def _send_mail(mail: mail.EmailMessage, result: List[bool]): +def _send_mail(message: mail.EmailMessage, result: List[bool]): + """Try to send one email and set the result.""" try: - mail.send(fail_silently=False) + message.send(fail_silently=False) result[0] = True except SMTPException: result[0] = False @@ -40,11 +40,13 @@ def _send_mail(mail: mail.EmailMessage, result: List[bool]): # TODO: Move to tasks module # TODO: Retry 3 # https://docs.celeryq.dev/en/v5.3.6/getting-started/next-steps.html#next-steps -# Send all unsent emails @shared_task() def _send_mails(): + """Send all unsent emails.""" + # All notifications that need to be sent notifications = Notification.objects.filter(is_sent=False) + # Dictionary with the number of errors for each email errors: DefaultDict[str, int] = cache.get( "notifications_send_mails_errors", defaultdict(int) @@ -105,5 +107,6 @@ def _send_mails(): # Restart the process if there are any notifications left that were not sent unsent_notifications = Notification.objects.filter(is_sent=False) cache.set("notifications_send_mails", False) + if unsent_notifications.count() > 0: schedule_send_mails() diff --git a/backend/notifications/migrations/0002_alter_notification_user.py b/backend/notifications/migrations/0002_alter_notification_user.py new file mode 100644 index 00000000..1ddeae05 --- /dev/null +++ b/backend/notifications/migrations/0002_alter_notification_user.py @@ -0,0 +1,21 @@ +# Generated by Django 5.0.4 on 2024-05-23 10:51 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('notifications', '0001_initial'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AlterField( + model_name='notification', + name='user', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='notifications', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/backend/notifications/models.py b/backend/notifications/models.py index c9c2fbde..9ff69cc1 100644 --- a/backend/notifications/models.py +++ b/backend/notifications/models.py @@ -3,27 +3,53 @@ class NotificationTemplate(models.Model): - id = models.AutoField(auto_created=True, primary_key=True) - title_key = models.CharField(max_length=255) # Key used to get translated title + """This model represents a template for a notification.""" + id = models.AutoField( + auto_created=True, + primary_key=True + ) + title_key = models.CharField( + max_length=255 + ) description_key = models.CharField( max_length=511 - ) # Key used to get translated description + ) class Notification(models.Model): - id = models.AutoField(auto_created=True, primary_key=True) - user = models.ForeignKey(User, on_delete=models.CASCADE) - template_id = models.ForeignKey(NotificationTemplate, on_delete=models.CASCADE) - created_at = models.DateTimeField(auto_now_add=True) - arguments = models.JSONField(default=dict) # Arguments to be used in the template + """This model represents a notification.""" + id = models.AutoField( + auto_created=True, + primary_key=True + ) + + user = models.ForeignKey( + User, + related_name="notifications", + on_delete=models.CASCADE + ) + + template_id = models.ForeignKey( + NotificationTemplate, + on_delete=models.CASCADE + ) + created_at = models.DateTimeField( + auto_now_add=True + ) + # Arguments to be used in the template + arguments = models.JSONField( + default=dict + ) + # Whether the notification has been read is_read = models.BooleanField( default=False - ) # Whether the notification has been read + ) + # Whether the notification has been sent (email) is_sent = models.BooleanField( default=False - ) # Whether the notification has been sent (email) + ) - # Mark the notification as read def sent(self): + """Mark the notification as sent""" self.is_sent = True self.save() diff --git a/backend/notifications/serializers.py b/backend/notifications/serializers.py index 2e3d7c75..351c9c16 100644 --- a/backend/notifications/serializers.py +++ b/backend/notifications/serializers.py @@ -1,7 +1,5 @@ -import re - from authentication.models import User -from notifications.logic import get_message_dict +from django.utils.translation import gettext as _ from notifications.models import Notification, NotificationTemplate from rest_framework import serializers @@ -15,58 +13,31 @@ class Meta: class NotificationSerializer(serializers.ModelSerializer): # Hyper linked user field user = serializers.HyperlinkedRelatedField( - view_name="user-detail", queryset=User.objects.all() + view_name="user-detail", + queryset=User.objects.all() ) - # Translate template and arguments into a message - 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) - missing_keys = [key for key in required_keys if key not in arguments] - - return missing_keys - - 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: - data["arguments"] = {} - - title_missing = self._get_missing_keys( - data["template_id"].title_key, data["arguments"] - ) - description_missing = self._get_missing_keys( - data["template_id"].description_key, data["arguments"] - ) - - if title_missing or description_missing: - raise serializers.ValidationError( - { - "missing arguments": { - "title": title_missing, - "description": description_missing, - } - } - ) + title = serializers.SerializerMethodField() + content = serializers.SerializerMethodField() + arguments = serializers.JSONField(write_only=True) - return data + def get_content(self, notification: Notification) -> str: + """Get the content from the template and arguments.""" + return _(notification.template_id.description_key) % notification.arguments - # Get the message from the template and arguments - def get_message(self, obj: Notification) -> dict[str, str]: - return get_message_dict(obj) + def get_title(self, notification: Notification) -> str: + """Get the title from the template and arguments.""" + return _(notification.template_id.title_key) class Meta: model = Notification fields = [ "id", "user", - "template_id", - "arguments", - "message", + "content", + "title", "created_at", "is_read", "is_sent", + "arguments" ] diff --git a/backend/notifications/signals.py b/backend/notifications/signals.py index 979184bb..15cad5e0 100644 --- a/backend/notifications/signals.py +++ b/backend/notifications/signals.py @@ -4,11 +4,11 @@ from typing import Dict, List, Union from authentication.models import User -from django.db.models.query import QuerySet from django.dispatch import Signal, receiver from django.urls import reverse from notifications.logic import schedule_send_mails from notifications.serializers import NotificationSerializer +from ypovoli.settings import TESTING notification_create = Signal() @@ -21,13 +21,15 @@ def notification_creation( arguments: Dict[str, str], **kwargs, # Required by django ) -> bool: + if TESTING: + return True + data: List[Dict[str, Union[str, int, Dict[str, str]]]] = [] for user in queryset: if user: data.append( { - "template_id": type.value, "user": reverse("user-detail", kwargs={"pk": user.id}), "arguments": arguments, } @@ -38,7 +40,7 @@ def notification_creation( if not serializer.is_valid(raise_exception=False): return False - serializer.save() + serializer.save(template_id_id=type.value) schedule_send_mails() diff --git a/backend/ypovoli/settings.py b/backend/ypovoli/settings.py index 36527c9f..223fcc2f 100644 --- a/backend/ypovoli/settings.py +++ b/backend/ypovoli/settings.py @@ -27,6 +27,7 @@ MEDIA_ROOT = os.path.normpath(os.path.join("data")) # TESTING +TESTING = environ.get("DJANGO_TESTING", "False").lower() in ["true", "1", "t"] TESTING_BASE_LINK = "http://testserver" TEST_ADMIN_DATA = { "id": "0", diff --git a/frontend/src/assets/lang/app/en.json b/frontend/src/assets/lang/app/en.json index 9b41cc18..93cc9cb9 100644 --- a/frontend/src/assets/lang/app/en.json +++ b/frontend/src/assets/lang/app/en.json @@ -14,6 +14,13 @@ "language": { "nl": "Nederlands", "en": "English" + }, + "notifications": { + "new": "New", + "title": "Notifications", + "markAsRead": "Mark as read", + "loadMore": "Load more", + "noNotifications": "No notifications available" } }, "footer": { diff --git a/frontend/src/assets/lang/app/nl.json b/frontend/src/assets/lang/app/nl.json index 7b4d89aa..99cd899d 100644 --- a/frontend/src/assets/lang/app/nl.json +++ b/frontend/src/assets/lang/app/nl.json @@ -14,6 +14,13 @@ "language": { "nl": "Nederlands", "en": "English" + }, + "notifications": { + "new": "Nieuw", + "title": "Notificaties", + "markAsRead": "Markeer alles als gelezen", + "loadMore": "Laad meer notificaties", + "noNotifications": "Geen notificaties" } }, "footer": { diff --git a/frontend/src/components/courses/CourseForm.vue b/frontend/src/components/courses/CourseForm.vue index 47114855..f8bf714e 100644 --- a/frontend/src/components/courses/CourseForm.vue +++ b/frontend/src/components/courses/CourseForm.vue @@ -31,7 +31,7 @@ const form = ref(new Course()); const rules = computed(() => { return { name: { required: helpers.withMessage(t('validations.required'), required) }, - faculty: { required: helpers.withMessage(t('validations.required'), required) }, + faculty: { required: helpers.withMessage(t('validations.required'), (faculty: Faculty) => faculty.id !== '') }, excerpt: { required: helpers.withMessage(t('validations.required'), required) }, }; }); diff --git a/frontend/src/components/notifications/NotificationCard.vue b/frontend/src/components/notifications/NotificationCard.vue new file mode 100644 index 00000000..5557e554 --- /dev/null +++ b/frontend/src/components/notifications/NotificationCard.vue @@ -0,0 +1,39 @@ + + + + + diff --git a/frontend/src/components/notifications/NotificationsOverlay.vue b/frontend/src/components/notifications/NotificationsOverlay.vue new file mode 100644 index 00000000..17e9819f --- /dev/null +++ b/frontend/src/components/notifications/NotificationsOverlay.vue @@ -0,0 +1,74 @@ + + + + + diff --git a/frontend/src/components/notifications/NotificationsScrollPanel.vue b/frontend/src/components/notifications/NotificationsScrollPanel.vue new file mode 100644 index 00000000..6f8aeec5 --- /dev/null +++ b/frontend/src/components/notifications/NotificationsScrollPanel.vue @@ -0,0 +1,35 @@ + + + + + diff --git a/frontend/src/components/projects/groups/GroupsManageTable.vue b/frontend/src/components/projects/groups/GroupsManageTable.vue index 65b00c68..72304094 100644 --- a/frontend/src/components/projects/groups/GroupsManageTable.vue +++ b/frontend/src/components/projects/groups/GroupsManageTable.vue @@ -2,7 +2,7 @@ import DataTable, { type DataTableRowExpandEvent } from 'primevue/datatable'; import Column from 'primevue/column'; import AllSubmission from '@/components/submissions/AllSubmission.vue'; -import InputNumber from 'primevue/inputnumber'; +import InputNumber, { type InputNumberBlurEvent } from 'primevue/inputnumber'; import { type Project } from '@/types/Project.ts'; import { useStudents } from '@/composables/services/student.service.ts'; import { ref } from 'vue'; @@ -35,7 +35,7 @@ const editingGroup = ref(); * @param group The group to update. * @param score The new score. */ -function updateGroupScore(group: Group, score: number): void { +function updateGroupScore(group: Group, score: number | string): void { emit('update:group-score', group, score); editingGroup.value = null; } @@ -81,9 +81,10 @@ async function expandGroup(event: DataTableRowExpandEvent): Promise {