From 63c18d2e28e39fafae0c26b7facf4f7d9189bf20 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Benjamin=20H=C3=A4ttasch?=
<benjamin.haettasch@fachschaft.informatik.tu-darmstadt.de>
Date: Tue, 15 Aug 2023 21:52:31 +0200
Subject: [PATCH] Improve AKScheduling
Add or complete docstrings
Remove code smells
Disable irrelevant warnings
Remove empty admin.py (to disable doc generation for an empty module)
Add additional test cases and improve basic test interface (support both 403 and 302 in case the user lacks rights to see a view through optional configuration)
Improve usage of types for API endpoints (e.g., restrict writing endpoints to writing only)
---
AKModel/tests.py | 19 +++--
AKScheduling/admin.py | 1 -
AKScheduling/api.py | 88 +++++++++++++++++----
AKScheduling/apps.py | 3 +
AKScheduling/forms.py | 3 +
AKScheduling/models.py | 171 ++++++++++++++++++++++-------------------
AKScheduling/tests.py | 73 +++++++++++++++++-
AKScheduling/views.py | 68 +++++++++++++++-
8 files changed, 320 insertions(+), 106 deletions(-)
delete mode 100644 AKScheduling/admin.py
diff --git a/AKModel/tests.py b/AKModel/tests.py
index 5b353c30..fb24dc08 100644
--- a/AKModel/tests.py
+++ b/AKModel/tests.py
@@ -106,15 +106,17 @@ class BasicViewTests:
"""
# Not logged in? Views should not be visible
self.client.logout()
- for view_name in self.VIEWS_STAFF_ONLY:
- view_name_with_prefix, url = self._name_and_url(view_name)
+ for view_name_info in self.VIEWS_STAFF_ONLY:
+ expected_response_code = 302 if len(view_name_info) == 2 else view_name_info[2]
+ view_name_with_prefix, url = self._name_and_url(view_name_info)
response = self.client.get(url)
- self.assertEqual(response.status_code, 302, msg=f"{view_name_with_prefix} ({url}) accessible by non-staff")
+ self.assertEqual(response.status_code, expected_response_code,
+ msg=f"{view_name_with_prefix} ({url}) accessible by non-staff")
# Logged in? Views should be visible
self.client.force_login(self.staff_user)
- for view_name in self.VIEWS_STAFF_ONLY:
- view_name_with_prefix, url = self._name_and_url(view_name)
+ for view_name_info in self.VIEWS_STAFF_ONLY:
+ view_name_with_prefix, url = self._name_and_url(view_name_info)
try:
response = self.client.get(url)
self.assertEqual(response.status_code, 200,
@@ -125,10 +127,11 @@ class BasicViewTests:
# Disabled user? Views should not be visible
self.client.force_login(self.deactivated_user)
- for view_name in self.VIEWS_STAFF_ONLY:
- view_name_with_prefix, url = self._name_and_url(view_name)
+ for view_name_info in self.VIEWS_STAFF_ONLY:
+ expected_response_code = 302 if len(view_name_info) == 2 else view_name_info[2]
+ view_name_with_prefix, url = self._name_and_url(view_name_info)
response = self.client.get(url)
- self.assertEqual(response.status_code, 302,
+ self.assertEqual(response.status_code, expected_response_code,
msg=f"{view_name_with_prefix} ({url}) still accessible for deactivated user")
def _to_sendable_value(self, val):
diff --git a/AKScheduling/admin.py b/AKScheduling/admin.py
deleted file mode 100644
index 846f6b40..00000000
--- a/AKScheduling/admin.py
+++ /dev/null
@@ -1 +0,0 @@
-# Register your models here.
diff --git a/AKScheduling/api.py b/AKScheduling/api.py
index 7155e6fe..61c939ed 100644
--- a/AKScheduling/api.py
+++ b/AKScheduling/api.py
@@ -12,20 +12,32 @@ from AKModel.metaviews.admin import EventSlugMixin
class ResourceSerializer(serializers.ModelSerializer):
+ """
+ REST Framework Serializer for Rooms to produce format required for fullcalendar resources
+ """
class Meta:
model = Room
fields = ['id', 'title']
title = serializers.SerializerMethodField('transform_title')
- def transform_title(self, obj):
+ @staticmethod
+ def transform_title(obj):
+ """
+ Adapt title, add capacity information if room has a restriction (capacity is not -1)
+ """
if obj.capacity > 0:
return f"{obj.title} [{obj.capacity}]"
return obj.title
-class ResourcesViewSet(EventSlugMixin, mixins.RetrieveModelMixin, mixins.ListModelMixin, viewsets.GenericViewSet):
- permission_classes = (permissions.DjangoModelPermissionsOrAnonReadOnly,)
+class ResourcesViewSet(EventSlugMixin, mixins.ListModelMixin, viewsets.GenericViewSet):
+ """
+ API View: Rooms (resources to schedule for in fullcalendar)
+
+ Read-only, adaption to fullcalendar format through :class:`ResourceSerializer`
+ """
+ permission_classes = (permissions.DjangoModelPermissions,)
serializer_class = ResourceSerializer
def get_queryset(self):
@@ -33,6 +45,13 @@ class ResourcesViewSet(EventSlugMixin, mixins.RetrieveModelMixin, mixins.ListMod
class EventsView(LoginRequiredMixin, EventSlugMixin, ListView):
+ """
+ API View: Slots (events to schedule in fullcalendar)
+
+ Read-only, JSON formatted response is created manually since it requires a bunch of "custom" fields that have
+ different names compared to the normal model or are not present at all and need to be computed to create the
+ required format for fullcalendar.
+ """
model = AKSlot
def get_queryset(self):
@@ -42,13 +61,16 @@ class EventsView(LoginRequiredMixin, EventSlugMixin, ListView):
return JsonResponse(
[{
"slotID": slot.pk,
- "title": f'{slot.ak.short_name}: \n{slot.ak.owners_list}',
+ "title": f'{slot.ak.short_name}:\n{slot.ak.owners_list}',
"description": slot.ak.details,
"resourceId": slot.room.id,
"start": timezone.localtime(slot.start, self.event.timezone).strftime("%Y-%m-%d %H:%M:%S"),
"end": timezone.localtime(slot.end, self.event.timezone).strftime("%Y-%m-%d %H:%M:%S"),
"backgroundColor": slot.ak.category.color,
- "borderColor": "#2c3e50" if slot.fixed else '#e74c3c' if slot.constraintviolation_set.count() > 0 else slot.ak.category.color,
+ "borderColor":
+ "#2c3e50" if slot.fixed
+ else '#e74c3c' if slot.constraintviolation_set.count() > 0
+ else slot.ak.category.color,
"constraint": 'roomAvailable',
"editable": not slot.fixed,
'url': str(reverse('admin:AKModel_akslot_change', args=[slot.pk])),
@@ -59,6 +81,13 @@ class EventsView(LoginRequiredMixin, EventSlugMixin, ListView):
class RoomAvailabilitiesView(LoginRequiredMixin, EventSlugMixin, ListView):
+ """
+ API view: Availabilities of rooms
+
+ Read-only, JSON formatted response is created manually since it requires a bunch of "custom" fields that have
+ different names compared to the normal model or are not present at all and need to be computed to create the
+ required format for fullcalendar.
+ """
model = Availability
context_object_name = "availabilities"
@@ -81,6 +110,13 @@ class RoomAvailabilitiesView(LoginRequiredMixin, EventSlugMixin, ListView):
class DefaultSlotsView(LoginRequiredMixin, EventSlugMixin, ListView):
+ """
+ API view: default slots
+
+ Read-only, JSON formatted response is created manually since it requires a bunch of "custom" fields that have
+ different names compared to the normal model or are not present at all and need to be computed to create the
+ required format for fullcalendar.
+ """
model = DefaultSlot
context_object_name = "default_slots"
@@ -105,6 +141,9 @@ class DefaultSlotsView(LoginRequiredMixin, EventSlugMixin, ListView):
class EventSerializer(serializers.ModelSerializer):
+ """
+ REST framework serializer to adapt between AKSlot model and the event format of fullcalendar
+ """
class Meta:
model = AKSlot
fields = ['id', 'start', 'end', 'roomId']
@@ -114,17 +153,31 @@ class EventSerializer(serializers.ModelSerializer):
roomId = serializers.IntegerField(source='room.pk')
def update(self, instance, validated_data):
+ # Ignore timezone of input (treat it as timezone-less) and set the event timezone
+ # By working like this, the client does not need to know about timezones, since every timestamp it deals with
+ # has the timezone offsets already applied
start = timezone.make_aware(timezone.make_naive(validated_data.get('start')), instance.event.timezone)
end = timezone.make_aware(timezone.make_naive(validated_data.get('end')), instance.event.timezone)
instance.start = start
- instance.room = get_object_or_404(Room, pk=validated_data.get('room')["pk"])
+ # Also, adapt from start & end format of fullcalendar to our start & duration model
diff = end - start
instance.duration = round(diff.days * 24 + (diff.seconds / 3600), 2)
+
+ # Updated room if needed (pk changed -- otherwise, no need for an additional database lookup)
+ new_room_id = validated_data.get('room')["pk"]
+ if instance.room.pk != new_room_id:
+ instance.room = get_object_or_404(Room, pk=new_room_id)
+
instance.save()
return instance
-class EventsViewSet(EventSlugMixin, viewsets.ModelViewSet):
+class EventsViewSet(EventSlugMixin, mixins.UpdateModelMixin, viewsets.GenericViewSet):
+ """
+ API view: Update scheduling of a slot (event in fullcalendar format)
+
+ Write-only (will however reply with written values to PUT request)
+ """
permission_classes = (permissions.DjangoModelPermissions,)
serializer_class = EventSerializer
@@ -136,17 +189,26 @@ class EventsViewSet(EventSlugMixin, viewsets.ModelViewSet):
class ConstraintViolationSerializer(serializers.ModelSerializer):
+ """
+ REST Framework Serializer for constraint violations
+ """
class Meta:
model = ConstraintViolation
- fields = ['pk', 'type_display', 'aks', 'ak_slots', 'ak_owner', 'room', 'requirement', 'category', 'comment', 'timestamp_display', 'manually_resolved', 'level_display', 'details', 'edit_url']
+ fields = ['pk', 'type_display', 'aks', 'ak_slots', 'ak_owner', 'room', 'requirement', 'category', 'comment',
+ 'timestamp_display', 'manually_resolved', 'level_display', 'details', 'edit_url']
+
+class ConstraintViolationsViewSet(EventSlugMixin, mixins.ListModelMixin, viewsets.GenericViewSet):
+ """
+ API View: Constraint Violations of an event
-class ConstraintViolationsViewSet(EventSlugMixin, viewsets.ModelViewSet):
+ Read-only, fields and model selected in :class:`ConstraintViolationSerializer`
+ """
permission_classes = (permissions.DjangoModelPermissions,)
serializer_class = ConstraintViolationSerializer
- def get_object(self):
- return get_object_or_404(ConstraintViolation, pk=self.kwargs["pk"])
-
def get_queryset(self):
- return ConstraintViolation.objects.select_related('event', 'room').prefetch_related('aks', 'ak_slots', 'ak_owner', 'requirement', 'category').filter(event=self.event).order_by('manually_resolved', '-type', '-timestamp')
+ # Optimize query to reduce database load
+ return (ConstraintViolation.objects.select_related('event', 'room')
+ .prefetch_related('aks', 'ak_slots', 'ak_owner', 'requirement', 'category')
+ .filter(event=self.event).order_by('manually_resolved', '-type', '-timestamp'))
diff --git a/AKScheduling/apps.py b/AKScheduling/apps.py
index c76ceac5..13a065bf 100644
--- a/AKScheduling/apps.py
+++ b/AKScheduling/apps.py
@@ -2,4 +2,7 @@ from django.apps import AppConfig
class AkschedulingConfig(AppConfig):
+ """
+ App configuration (default, only specifies name of the app)
+ """
name = 'AKScheduling'
diff --git a/AKScheduling/forms.py b/AKScheduling/forms.py
index 00a9b5bc..47a19f78 100644
--- a/AKScheduling/forms.py
+++ b/AKScheduling/forms.py
@@ -5,6 +5,9 @@ from AKModel.models import AK
class AKInterestForm(forms.ModelForm):
+ """
+ Form for quickly changing the interest count and notes of an AK
+ """
required_css_class = 'required'
class Meta:
diff --git a/AKScheduling/models.py b/AKScheduling/models.py
index 6664a8ae..769527f7 100644
--- a/AKScheduling/models.py
+++ b/AKScheduling/models.py
@@ -1,9 +1,15 @@
+# This file mainly contains signal receivers, which follow a very strong interface, having e.g., a sender attribute
+# that is hardly used by us. Nevertheless, to follow the django receiver coding style and since changes might
+# cause issues when loading fixtures or model dumps, it is not wise to replace that attribute with "_".
+# Therefore, the check that finds unused arguments is disabled for this whole file:
+# pylint: disable=unused-argument
+
from django.db.models.signals import post_save, m2m_changed, pre_delete
from django.dispatch import receiver
from django.utils.translation import gettext_lazy as _
from AKModel.availability.models import Availability
-from AKModel.models import AK, AKSlot, Room, Event, AKOwner, ConstraintViolation
+from AKModel.models import AK, AKSlot, Room, Event, ConstraintViolation
def update_constraint_violations(new_violations, existing_violations_to_check):
@@ -43,11 +49,15 @@ def update_cv_reso_deadline_for_slot(slot):
:type slot: AKSlot
"""
event = slot.event
+
+ # Update only if reso_deadline exists
+ # if event was changed and reso_deadline is removed, CVs will be deleted by event changed handler
+ # Update only has to be done for already scheduled slots with reso intention
if slot.ak.reso and slot.event.reso_deadline and slot.start:
- # Update only if reso_deadline exists
- # if event was changed and reso_deadline is removed, CVs will be deleted by event changed handler
violation_type = ConstraintViolation.ViolationType.AK_AFTER_RESODEADLINE
new_violations = []
+
+ # Violation?
if slot.end > event.reso_deadline:
c = ConstraintViolation(
type=violation_type,
@@ -69,38 +79,47 @@ def check_capacity_for_slot(slot: AKSlot):
:return: Violation (if any) or None
:rtype: ConstraintViolation or None
"""
- if slot.room:
- if slot.room.capacity >= 0:
- if slot.room.capacity < slot.ak.interest:
- c = ConstraintViolation(
- type=ConstraintViolation.ViolationType.ROOM_CAPACITY_EXCEEDED,
- level=ConstraintViolation.ViolationLevel.VIOLATION,
- event=slot.event,
- room=slot.room,
- comment=_("Not enough space for AK interest (Interest: %(interest)d, Capacity: %(capacity)d)")
- % {'interest': slot.ak.interest, 'capacity': slot.room.capacity},
- )
- c.ak_slots_tmp.add(slot)
- c.aks_tmp.add(slot.ak)
- return c
- elif slot.room.capacity < slot.ak.interest + 5 or slot.room.capacity < slot.ak.interest * 1.25:
- c = ConstraintViolation(
- type=ConstraintViolation.ViolationType.ROOM_CAPACITY_EXCEEDED,
- level=ConstraintViolation.ViolationLevel.WARNING,
- event=slot.event,
- room=slot.room,
- comment=_("Space is too close to AK interest (Interest: %(interest)d, Capacity: %(capacity)d)")
- % {'interest': slot.ak.interest, 'capacity': slot.room.capacity}
- )
- c.ak_slots_tmp.add(slot)
- c.aks_tmp.add(slot.ak)
- return c
- return None
+
+ # If slot is scheduled in a room
+ if slot.room and slot.room.capacity >= 0:
+ # Create a violation if interest exceeds room capacity
+ if slot.room.capacity < slot.ak.interest:
+ c = ConstraintViolation(
+ type=ConstraintViolation.ViolationType.ROOM_CAPACITY_EXCEEDED,
+ level=ConstraintViolation.ViolationLevel.VIOLATION,
+ event=slot.event,
+ room=slot.room,
+ comment=_("Not enough space for AK interest (Interest: %(interest)d, Capacity: %(capacity)d)")
+ % {'interest': slot.ak.interest, 'capacity': slot.room.capacity},
+ )
+ c.ak_slots_tmp.add(slot)
+ c.aks_tmp.add(slot.ak)
+ return c
+
+ # Create a warning if interest is close to room capacity
+ if slot.room.capacity < slot.ak.interest + 5 or slot.room.capacity < slot.ak.interest * 1.25:
+ c = ConstraintViolation(
+ type=ConstraintViolation.ViolationType.ROOM_CAPACITY_EXCEEDED,
+ level=ConstraintViolation.ViolationLevel.WARNING,
+ event=slot.event,
+ room=slot.room,
+ comment=_("Space is too close to AK interest (Interest: %(interest)d, Capacity: %(capacity)d)")
+ % {'interest': slot.ak.interest, 'capacity': slot.room.capacity}
+ )
+ c.ak_slots_tmp.add(slot)
+ c.aks_tmp.add(slot.ak)
+ return c
+
+ return None
@receiver(post_save, sender=AK)
def ak_changed_handler(sender, instance: AK, **kwargs):
- # Changes might affect: Reso intention, Category, Interest
+ """
+ Signal receiver: Check for violations after AK changed
+
+ Changes might affect: Reso intention, Category, Interest
+ """
# TODO Reso intention changes
# Check room capacities
@@ -118,14 +137,12 @@ def ak_changed_handler(sender, instance: AK, **kwargs):
@receiver(m2m_changed, sender=AK.owners.through)
def ak_owners_changed_handler(sender, instance: AK, action: str, **kwargs):
"""
- Owners of AK changed
+ Signal receiver: Owners of AK changed
"""
# Only signal after change (post_add, post_delete, post_clear) are relevant
if not action.startswith("post"):
return
- # print(f"{instance} changed")
-
event = instance.event
# Owner(s) changed: Might affect multiple AKs by the same owner(s) at the same time
@@ -157,8 +174,6 @@ def ak_owners_changed_handler(sender, instance: AK, action: str, **kwargs):
c.ak_slots_tmp.add(other_slot)
new_violations.append(c)
- #print(f"{owner} has the following conflicts: {new_violations}")
-
# ... and compare to/update list of existing violations of this type
# belonging to the AK that was recently changed (important!)
existing_violations_to_check = list(instance.constraintviolation_set.filter(type=violation_type))
@@ -169,14 +184,12 @@ def ak_owners_changed_handler(sender, instance: AK, action: str, **kwargs):
@receiver(m2m_changed, sender=AK.conflicts.through)
def ak_conflicts_changed_handler(sender, instance: AK, action: str, **kwargs):
"""
- Conflicts of AK changed
+ Signal receiver: Conflicts of AK changed
"""
# Only signal after change (post_add, post_delete, post_clear) are relevant
if not action.startswith("post"):
return
- # print(f"{instance} changed")
-
event = instance.event
# Conflict(s) changed: Might affect multiple AKs that are conflicts of each other
@@ -186,6 +199,7 @@ def ak_conflicts_changed_handler(sender, instance: AK, action: str, **kwargs):
slots_of_this_ak: [AKSlot] = instance.akslot_set.filter(start__isnull=False)
conflicts_of_this_ak: [AK] = instance.conflicts.all()
+ # Loop over all existing conflicts
for ak in conflicts_of_this_ak:
if ak != instance:
for other_slot in ak.akslot_set.filter(start__isnull=False):
@@ -203,8 +217,6 @@ def ak_conflicts_changed_handler(sender, instance: AK, action: str, **kwargs):
c.ak_slots_tmp.add(other_slot)
new_violations.append(c)
- # print(f"{instance} has the following conflicts: {new_violations}")
-
# ... and compare to/update list of existing violations of this type
# belonging to the AK that was recently changed (important!)
existing_violations_to_check = list(instance.constraintviolation_set.filter(type=violation_type))
@@ -215,23 +227,22 @@ def ak_conflicts_changed_handler(sender, instance: AK, action: str, **kwargs):
@receiver(m2m_changed, sender=AK.prerequisites.through)
def ak_prerequisites_changed_handler(sender, instance: AK, action: str, **kwargs):
"""
- Prerequisites of AK changed
+ Signal receiver: Prerequisites of AK changed
"""
# Only signal after change (post_add, post_delete, post_clear) are relevant
if not action.startswith("post"):
return
- # print(f"{instance} changed")
-
event = instance.event
- # Conflict(s) changed: Might affect multiple AKs that are conflicts of each other
+ # Prerequisite(s) changed: Might affect multiple AKs that should have a certain order
violation_type = ConstraintViolation.ViolationType.AK_BEFORE_PREREQUISITE
new_violations = []
slots_of_this_ak: [AKSlot] = instance.akslot_set.filter(start__isnull=False)
prerequisites_of_this_ak: [AK] = instance.prerequisites.all()
+ # Loop over all prerequisites
for ak in prerequisites_of_this_ak:
if ak != instance:
for other_slot in ak.akslot_set.filter(start__isnull=False):
@@ -249,8 +260,6 @@ def ak_prerequisites_changed_handler(sender, instance: AK, action: str, **kwargs
c.ak_slots_tmp.add(other_slot)
new_violations.append(c)
- # print(f"{instance} has the following conflicts: {new_violations}")
-
# ... and compare to/update list of existing violations of this type
# belonging to the AK that was recently changed (important!)
existing_violations_to_check = list(instance.constraintviolation_set.filter(type=violation_type))
@@ -261,14 +270,12 @@ def ak_prerequisites_changed_handler(sender, instance: AK, action: str, **kwargs
@receiver(m2m_changed, sender=AK.requirements.through)
def ak_requirements_changed_handler(sender, instance: AK, action: str, **kwargs):
"""
- Requirements of AK changed
+ Signal receiver: Requirements of AK changed
"""
# Only signal after change (post_add, post_delete, post_clear) are relevant
if not action.startswith("post"):
return
- # print(f"{instance} changed")
-
event = instance.event
# Requirement(s) changed: Might affect slots and rooms
@@ -298,8 +305,6 @@ def ak_requirements_changed_handler(sender, instance: AK, action: str, **kwargs)
c.ak_slots_tmp.add(slot)
new_violations.append(c)
- # print(f"{instance} has the following conflicts: {new_violations}")
-
# ... and compare to/update list of existing violations of this type
# belonging to the AK that was recently changed (important!)
existing_violations_to_check = list(instance.constraintviolation_set.filter(type=violation_type))
@@ -309,8 +314,13 @@ def ak_requirements_changed_handler(sender, instance: AK, action: str, **kwargs)
@receiver(post_save, sender=AKSlot)
def akslot_changed_handler(sender, instance: AKSlot, **kwargs):
- # Changes might affect: Duplicate parallel, Two in room, Resodeadline
- # print(f"{sender} changed")
+ """
+ Signal receiver: AKSlot changed
+
+ Changes might affect: Duplicate parallel, Two in room, Resodeadline
+ """
+ # TODO Consider rewriting this very long and complex method to resolve several (style) issues:
+ # pylint: disable=too-many-nested-blocks,too-many-locals,too-many-branches,too-many-statements
event = instance.event
# == Check for two parallel slots by one of the owners ==
@@ -341,8 +351,6 @@ def akslot_changed_handler(sender, instance: AKSlot, **kwargs):
c.ak_slots_tmp.add(other_slot)
new_violations.append(c)
- # print(f"{owner} has the following conflicts: {new_violations}")
-
# ... and compare to/update list of existing violations of this type
# belonging to the AK that was recently changed (important!)
existing_violations_to_check = list(instance.constraintviolation_set.filter(type=violation_type))
@@ -373,8 +381,6 @@ def akslot_changed_handler(sender, instance: AKSlot, **kwargs):
c.ak_slots_tmp.add(other_slot)
new_violations.append(c)
- # print(f"Multiple slots in room {instance.room}: {new_violations}")
-
# ... and compare to/update list of existing violations of this type
# belonging to the slot that was recently changed (important!)
existing_violations_to_check = list(instance.constraintviolation_set.filter(type=violation_type))
@@ -437,8 +443,6 @@ def akslot_changed_handler(sender, instance: AKSlot, **kwargs):
c.ak_slots_tmp.add(instance)
new_violations.append(c)
- # print(f"{instance.ak} has the following slots outside availabilities: {new_violations}")
-
# ... and compare to/update list of existing violations of this type
# belonging to the AK that was recently changed (important!)
existing_violations_to_check = list(instance.constraintviolation_set.filter(type=violation_type))
@@ -470,8 +474,6 @@ def akslot_changed_handler(sender, instance: AKSlot, **kwargs):
c.ak_slots_tmp.add(instance)
new_violations.append(c)
- # print(f"{instance} has the following conflicts: {new_violations}")
-
# ... and compare to/update list of existing violations of this type
# belonging to the AK that was recently changed (important!)
existing_violations_to_check = list(instance.constraintviolation_set.filter(type=violation_type))
@@ -502,8 +504,6 @@ def akslot_changed_handler(sender, instance: AKSlot, **kwargs):
c.ak_slots_tmp.add(other_slot)
new_violations.append(c)
- # print(f"{instance} has the following conflicts: {new_violations}")
-
# ... and compare to/update list of existing violations of this type
# belonging to the AK that was recently changed (important!)
existing_violations_to_check = list(instance.ak.constraintviolation_set.filter(type=violation_type))
@@ -534,8 +534,6 @@ def akslot_changed_handler(sender, instance: AKSlot, **kwargs):
c.ak_slots_tmp.add(other_slot)
new_violations.append(c)
- # print(f"{instance} has the following conflicts: {new_violations}")
-
# ... and compare to/update list of existing violations of this type
# belonging to the AK that was recently changed (important!)
existing_violations_to_check = list(instance.ak.constraintviolation_set.filter(type=violation_type))
@@ -547,15 +545,21 @@ def akslot_changed_handler(sender, instance: AKSlot, **kwargs):
new_violations = [cv] if cv is not None else []
# Compare to/update list of existing violations of this type for this slot
- existing_violations_to_check = list(instance.constraintviolation_set.filter(type=ConstraintViolation.ViolationType.ROOM_CAPACITY_EXCEEDED))
+ existing_violations_to_check = list(
+ instance.constraintviolation_set.filter(type=ConstraintViolation.ViolationType.ROOM_CAPACITY_EXCEEDED)
+ )
update_constraint_violations(new_violations, existing_violations_to_check)
@receiver(pre_delete, sender=AKSlot)
def akslot_deleted_handler(sender, instance: AKSlot, **kwargs):
- # Manually clean up or remove constraint violations that belong to this slot since there is no cascade deletion
- # for many2many relationships. Explicitly listening for AK deletion signals is not necessary since they will
- # transitively trigger this signal and we always set both AK and AKSlot references in a constraint violation
+ """
+ Signal receiver: AKSlot deleted
+
+ Manually clean up or remove constraint violations that belong to this slot since there is no cascade deletion
+ for many2many relationships. Explicitly listening for AK deletion signals is not necessary since they will
+ transitively trigger this signal and we always set both AK and AKSlot references in a constraint violation
+ """
# print(f"{instance} deleted")
for cv in instance.constraintviolation_set.all():
@@ -566,8 +570,11 @@ def akslot_deleted_handler(sender, instance: AKSlot, **kwargs):
@receiver(post_save, sender=Room)
def room_changed_handler(sender, instance: Room, **kwargs):
- # Changes might affect: Room size
+ """
+ Signal receiver: Room changed
+ Changes might affect: Room size
+ """
# Check room capacities
violation_type = ConstraintViolation.ViolationType.ROOM_CAPACITY_EXCEEDED
new_violations = []
@@ -583,24 +590,23 @@ def room_changed_handler(sender, instance: Room, **kwargs):
@receiver(m2m_changed, sender=Room.properties.through)
def room_requirements_changed_handler(sender, instance: Room, action: str, **kwargs):
"""
- Requirements of room changed
+ Signal Receiver: Requirements of room changed
"""
# Only signal after change (post_add, post_delete, post_clear) are relevant
if not action.startswith("post"):
return
- # print(f"{instance} changed")
-
- event = instance.event
-
+ # event = instance.event
# TODO React to changes
@receiver(post_save, sender=Availability)
def availability_changed_handler(sender, instance: Availability, **kwargs):
- # Changes might affect: category availability, AK availability, Room availability
- # print(f"{instance} changed")
+ """
+ Signal receiver: Availalability changed
+ Changes might affect: category availability, AK availability, Room availability
+ """
event = instance.event
# An AK's availability changed: Might affect AK slots scheduled outside the permitted time
@@ -627,8 +633,6 @@ def availability_changed_handler(sender, instance: Availability, **kwargs):
c.ak_slots_tmp.add(slot)
new_violations.append(c)
- # print(f"{instance.ak} has the following slots outside availabilities: {new_violations}")
-
# ... and compare to/update list of existing violations of this type
# belonging to the AK that was recently changed (important!)
existing_violations_to_check = list(instance.ak.constraintviolation_set.filter(type=violation_type))
@@ -638,7 +642,12 @@ def availability_changed_handler(sender, instance: Availability, **kwargs):
@receiver(post_save, sender=Event)
def event_changed_handler(sender, instance: Event, **kwargs):
- # == Check for reso ak after reso deadline (which might have changed) ==
+ """
+ Signal receiver: Event changed
+
+ Changes might affect: Reso deadline
+ """
+ # Check for reso ak after reso deadline (which might have changed)
if instance.reso_deadline:
for slot in instance.akslot_set.filter(start__isnull=False, ak__reso=True):
update_cv_reso_deadline_for_slot(slot)
diff --git a/AKScheduling/tests.py b/AKScheduling/tests.py
index 8b7bcf91..0996eedd 100644
--- a/AKScheduling/tests.py
+++ b/AKScheduling/tests.py
@@ -1,11 +1,20 @@
+import json
+from datetime import timedelta
+
from django.test import TestCase
-from AKModel.tests import BasicViewTests
+from django.utils import timezone
+from AKModel.tests import BasicViewTests
+from AKModel.models import AKSlot, Event, Room
class ModelViewTests(BasicViewTests, TestCase):
+ """
+ Tests for AKScheduling
+ """
fixtures = ['model.json']
VIEWS_STAFF_ONLY = [
+ # Views
('admin:schedule', {'event_slug': 'kif42'}),
('admin:slots_unscheduled', {'event_slug': 'kif42'}),
('admin:constraint-violations', {'slug': 'kif42'}),
@@ -14,4 +23,66 @@ class ModelViewTests(BasicViewTests, TestCase):
('admin:autocreate-availabilities', {'event_slug': 'kif42'}),
('admin:tracks_manage', {'event_slug': 'kif42'}),
('admin:enter-interest', {'event_slug': 'kif42', 'pk': 1}),
+ # API (Read)
+ ('model:scheduling-resources-list', {'event_slug': 'kif42'}, 403),
+ ('model:scheduling-constraint-violations-list', {'event_slug': 'kif42'}, 403),
+ ('model:scheduling-events', {'event_slug': 'kif42'}),
+ ('model:scheduling-room-availabilities', {'event_slug': 'kif42'}),
+ ('model:scheduling-default-slots', {'event_slug': 'kif42'}),
]
+
+ def test_scheduling_of_slot_update(self):
+ """
+ Test rescheduling a slot to a different time or room
+ """
+ self.client.force_login(self.admin_user)
+
+ event = Event.get_by_slug('kif42')
+
+ # Get the first already scheduled slot belonging to this event
+ slot = event.akslot_set.filter(start__isnull=False).first()
+ pk = slot.pk
+ room_id = slot.room_id
+ events_api_url = f"/kif42/api/scheduling-event/{pk}/"
+
+ # Create updated time
+ offset = timedelta(hours=1)
+ new_start_time = slot.start + offset
+ new_end_time = slot.end + offset
+ new_start_time_string = timezone.localtime(new_start_time, event.timezone).strftime("%Y-%m-%d %H:%M:%S")
+ new_end_time_string = timezone.localtime(new_end_time, event.timezone).strftime("%Y-%m-%d %H:%M:%S")
+
+ # Try API call
+ response = self.client.put(
+ events_api_url,
+ json.dumps({
+ 'start': new_start_time_string,
+ 'end': new_end_time_string,
+ 'roomId': room_id,
+ }),
+ content_type = 'application/json'
+ )
+ self.assertEqual(response.status_code, 200, "PUT to API endpoint did not work")
+
+ # Make sure API call did update the slot as expected
+ slot = AKSlot.objects.get(pk=pk)
+ self.assertEqual(new_start_time, slot.start, "Update did not work")
+
+ # Test updating room
+ new_room = Room.objects.exclude(pk=room_id).first()
+
+ # Try second API call
+ response = self.client.put(
+ events_api_url,
+ json.dumps({
+ 'start': new_start_time_string,
+ 'end': new_end_time_string,
+ 'roomId': new_room.pk,
+ }),
+ content_type = 'application/json'
+ )
+ self.assertEqual(response.status_code, 200, "Second PUT to API endpoint did not work")
+
+ # Make sure API call did update the slot as expected
+ slot = AKSlot.objects.get(pk=pk)
+ self.assertEqual(new_room.pk, slot.room.pk, "Update did not work")
diff --git a/AKScheduling/views.py b/AKScheduling/views.py
index 2c8f275f..f0be637f 100644
--- a/AKScheduling/views.py
+++ b/AKScheduling/views.py
@@ -11,6 +11,9 @@ from AKScheduling.forms import AKInterestForm, AKAddSlotForm
class UnscheduledSlotsAdminView(AdminViewMixin, FilterByEventSlugMixin, ListView):
+ """
+ Admin view: Get a list of all unscheduled slots
+ """
template_name = "admin/AKScheduling/unscheduled.html"
model = AKSlot
context_object_name = "akslots"
@@ -25,6 +28,12 @@ class UnscheduledSlotsAdminView(AdminViewMixin, FilterByEventSlugMixin, ListView
class SchedulingAdminView(AdminViewMixin, FilterByEventSlugMixin, ListView):
+ """
+ Admin view: Scheduler
+
+ View and adapt the schedule of an event. This view heavily uses JavaScript to display a calendar view plus
+ a list of unscheduled slots and to allow dragging slots in and into the calendar
+ """
template_name = "admin/AKScheduling/scheduling.html"
model = AKSlot
context_object_name = "slots_unscheduled"
@@ -46,6 +55,12 @@ class SchedulingAdminView(AdminViewMixin, FilterByEventSlugMixin, ListView):
class TrackAdminView(AdminViewMixin, FilterByEventSlugMixin, ListView):
+ """
+ Admin view: Distribute AKs to tracks
+
+ Again using JavaScript, the user can here see a list of all AKs split-up by tracks and can move them to other or
+ even new tracks using drag and drop. The state is then automatically synchronized via API calls in the background
+ """
template_name = "admin/AKScheduling/manage_tracks.html"
model = AKTrack
context_object_name = "tracks"
@@ -57,6 +72,12 @@ class TrackAdminView(AdminViewMixin, FilterByEventSlugMixin, ListView):
class ConstraintViolationsAdminView(AdminViewMixin, DetailView):
+ """
+ Admin view: Inspect and adjust all constraint violations of the event
+
+ This view populates a table of constraint violations via background API call (JavaScript), offers the option to
+ see details or edit each of them and provides an auto-reload feature.
+ """
template_name = "admin/AKScheduling/constraint_violations.html"
model = Event
context_object_name = "event"
@@ -68,6 +89,10 @@ class ConstraintViolationsAdminView(AdminViewMixin, DetailView):
class SpecialAttentionAKsAdminView(AdminViewMixin, DetailView):
+ """
+ Admin view: List all AKs that require special attention via scheduling, e.g., because of free-form comments,
+ since there are slots even though it is a wish, or no slots even though it is an AK etc.
+ """
template_name = "admin/AKScheduling/special_attention.html"
model = Event
context_object_name = "event"
@@ -76,12 +101,16 @@ class SpecialAttentionAKsAdminView(AdminViewMixin, DetailView):
context = super().get_context_data(**kwargs)
context["title"] = f"{_('AKs requiring special attention for')} {context['event']}"
- aks = AK.objects.filter(event=context["event"]).annotate(Count('owners', distinct=True)).annotate(Count('akslot', distinct=True)).annotate(Count('availabilities', distinct=True))
+ # Load all "special" AKs from the database using annotations to reduce the amount of necessary queries
+ aks = (AK.objects.filter(event=context["event"]).annotate(Count('owners', distinct=True))
+ .annotate(Count('akslot', distinct=True)).annotate(Count('availabilities', distinct=True)))
aks_with_comment = []
ak_wishes_with_slots = []
aks_without_availabilities = []
aks_without_slots = []
+ # Loop over all AKs of this event and identify all relevant factors that make the AK "special" and add them to
+ # the respective lists if the AK fullfills an condition
for ak in aks:
if ak.notes != "":
aks_with_comment.append(ak)
@@ -104,6 +133,14 @@ class SpecialAttentionAKsAdminView(AdminViewMixin, DetailView):
class InterestEnteringAdminView(SuccessMessageMixin, AdminViewMixin, EventSlugMixin, UpdateView):
+ """
+ Admin view: Form view to quickly store information about the interest in an AK
+ (e.g., during presentation of the AK list)
+
+ The view offers a field to update interest and manually set a comment for the current AK, but also features links
+ to the AKs before and probably coming up next, as well as links to other AKs sorted by category, for quick
+ and hazzle-free navigation during the AK presentation
+ """
template_name = "admin/AKScheduling/interest.html"
model = AK
context_object_name = "ak"
@@ -126,6 +163,8 @@ class InterestEnteringAdminView(SuccessMessageMixin, AdminViewMixin, EventSlugMi
last_ak = None
next_is_next = False
+ # Building the right navigation is a bit tricky since wishes have to be treated as an own category here
+ # Hence, depending on the AK we are currently at (displaying the form for) we need to either:
# Find other AK wishes (regardless of the category)...
if context['ak'].wish:
other_aks = [ak for ak in context['event'].ak_set.prefetch_related('owners').all() if ak.wish]
@@ -133,6 +172,7 @@ class InterestEnteringAdminView(SuccessMessageMixin, AdminViewMixin, EventSlugMi
else:
other_aks = [ak for ak in context['ak'].category.ak_set.prefetch_related('owners').all() if not ak.wish]
+ # Use that list of other AKs belonging to this category to identify the previous and next AK (if any)
for other_ak in other_aks:
if next_is_next:
context['next_ak'] = other_ak
@@ -142,6 +182,7 @@ class InterestEnteringAdminView(SuccessMessageMixin, AdminViewMixin, EventSlugMi
next_is_next = True
last_ak = other_ak
+ # Gather information for link lists for all categories (and wishes)
for category in context['event'].akcategory_set.prefetch_related('ak_set').all():
aks_for_category = []
for ak in category.ak_set.prefetch_related('owners').all():
@@ -151,6 +192,8 @@ class InterestEnteringAdminView(SuccessMessageMixin, AdminViewMixin, EventSlugMi
aks_for_category.append(ak)
categories_with_aks.append((category, aks_for_category))
+ # Make sure wishes have the right order (since the list was filled category by category before, this requires
+ # explicitly reordering them by their primary key)
ak_wishes.sort(key=lambda x: x.pk)
categories_with_aks.append(
(AKCategory(name=_("Wishes"), pk=0, description="-"), ak_wishes))
@@ -161,6 +204,16 @@ class InterestEnteringAdminView(SuccessMessageMixin, AdminViewMixin, EventSlugMi
class WishSlotCleanupView(EventSlugMixin, IntermediateAdminView):
+ """
+ Admin action view: Allow to delete all unscheduled slots for wishes
+
+ The view will render a preview of all slots that are affected by this. It is not possible to manually choose
+ which slots should be deleted (either all or none) and the functionality will therefore delete slots that were
+ created in the time between rendering of the preview and running the action ofter confirmation as well.
+
+ Due to the automated slot cleanup functionality for wishes in the AKSubmission app, this functionality should be
+ rarely needed/used
+ """
title = _('Cleanup: Delete unscheduled slots for wishes')
def get_success_url(self):
@@ -180,6 +233,15 @@ class WishSlotCleanupView(EventSlugMixin, IntermediateAdminView):
class AvailabilityAutocreateView(EventSlugMixin, IntermediateAdminView):
+ """
+ Admin action view: Allow to automatically create default availabilities (event start to end) for all AKs without
+ any manually specified availability information
+
+ The view will render a preview of all AKs that are affected by this. It is not possible to manually choose
+ which AKs should be affected (either all or none) and the functionality will therefore create availability entries
+ for AKs that were created in the time between rendering of the preview and running the action ofter confirmation
+ as well.
+ """
title = _('Create default availabilities for AKs')
def get_success_url(self):
@@ -194,6 +256,8 @@ class AvailabilityAutocreateView(EventSlugMixin, IntermediateAdminView):
)
def form_valid(self, form):
+ # Local import to prevent cyclic imports
+ # pylint: disable=import-outside-toplevel
from AKModel.availability.models import Availability
success_count = 0
@@ -202,7 +266,7 @@ class AvailabilityAutocreateView(EventSlugMixin, IntermediateAdminView):
availability = Availability.with_event_length(event=self.event, ak=ak)
availability.save()
success_count += 1
- except:
+ except: # pylint: disable=bare-except
messages.add_message(
self.request, messages.WARNING,
_("Could not create default availabilities for AK: {ak}").format(ak=ak)
--
GitLab