diff --git a/AKModel/tests.py b/AKModel/tests.py index 5b353c30d56d23c95459c09eca0e5163b3fbfeff..fb24dc08a7891425c24d6017be0a51e25566e52d 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 846f6b4061a68eda58bc9c76c36603d1e7721ee8..0000000000000000000000000000000000000000 --- a/AKScheduling/admin.py +++ /dev/null @@ -1 +0,0 @@ -# Register your models here. diff --git a/AKScheduling/api.py b/AKScheduling/api.py index 7155e6fe6e47081ccc6101699eba02bab78274ec..61c939ede1526504409a90ff52bcd55f66c76302 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 c76ceac5ba250b7c429226de5f803871102554d8..13a065bfd3fb836dcdfa898c55f1252e79dd2cba 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 00a9b5bc04aa62f1acd7ecd8477a84adcd49402f..47a19f7857c3c9e68d2c6e2b59cecf0d79fbcebf 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 6664a8aee83ddf108b04666a5e99a5fea077b4e7..769527f7aec7792c565e7a18bdbb888c938eeb59 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 8b7bcf91eeb40abe1185b814b0018c2f36715c51..0996eedd905259f0f463589a89f68bde055bc01a 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 2c8f275f3d262060cd511cc29dfa071f3ee1551d..f0be637f0ec4129507d08129e4b2801eeb671f90 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)