From e900a41645e5f33efd0ee7d03ed1d6cea54f87ef Mon Sep 17 00:00:00 2001
From: Konrad Mohrfeldt <konrad.mohrfeldt@farbdev.org>
Date: Wed, 16 Mar 2022 19:24:39 +0100
Subject: [PATCH] refactor: use django_filters FilterSet for APITimeSlotViewSet
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This change re-implements all existing collection filters for the
APITimeSlotViewSet with a FilterSet. No breaking changes are expected,
though there are some changes in semantics:

* The start and end query parameters no longer need to be specified
  together. If users only want to modify the start or end date they
  can now do that.
  If start is specified and end is not, end will be start + 60 days.
* If end was not set it would default to start + 60 days at 00:00.
  This is now fixed and end will be start + 60 days at 23:59:59.
* end now uses time.max, which selects the latest possible time on
  the specified date.
* The surrounding-filter now uses the value of the start filter
  by default. There might not be a use-case for this, but it seemed
  like a good default.
* All filters are now applied in series. This wasn’t the case for
  every filter, e.g. the surrounding-filter would return early.
---
 program/filters.py |  88 +++++++++++++++++++++++++++++++++++
 program/views.py   | 113 +++++++--------------------------------------
 2 files changed, 104 insertions(+), 97 deletions(-)

diff --git a/program/filters.py b/program/filters.py
index c73e507f..8458ebf8 100644
--- a/program/filters.py
+++ b/program/filters.py
@@ -1,5 +1,8 @@
+import datetime
+
 from django_filters import rest_framework as filters
 
+from django import forms
 from django.contrib.auth.models import User
 from django.db.models import Q, QuerySet
 from django.utils import timezone
@@ -90,3 +93,88 @@ class ShowFilterSet(StaticFilterHelpTextMixin, filters.FilterSet):
             "topic",
             "type",
         ]
+
+
+class TimeSlotFilterSet(filters.FilterSet):
+    order = filters.OrderingFilter(
+        fields=[field.name for field in models.TimeSlot._meta.get_fields()]
+    )
+    surrounding = filters.BooleanFilter(
+        method="filter_surrounding",
+        label="Return surrounding timeslots",
+        help_text=(
+            "Returns the 10 nearest timeslots for the current date if set to true. "
+            "No filtering is performed if set to false. "
+            "If specified without a value true is assumed."
+        ),
+    )
+    # The start/end filters will always be applied even if no query parameter has been set.
+    # This is because we enforce a value in the clean_start and clean_end methods
+    # of the filterset form.
+    start = filters.DateFilter(
+        method="filter_start",
+        help_text=(
+            "Only returns timeslots after that start on or after the specified date. "
+            "By default, this is set to the current date."
+        ),
+    )
+    end = filters.DateFilter(
+        method="filter_end",
+        help_text=(
+            "Only returns timeslots that end on or before the specified date. "
+            "By default, this is set to value of the start filter + 60 days."
+        ),
+    )
+
+    def filter_surrounding(self, queryset: QuerySet, name: str, value: bool):
+        if value is not True:
+            return queryset
+        start = self.form.cleaned_data.get("start", None) or timezone.now()
+        nearest_timeslots_in_future = (
+            models.TimeSlot.objects.filter(start__gte=start)
+            .order_by("start")
+            .values_list("id", flat=True)[:5]
+        )
+        nearest_timeslots_in_past = (
+            models.TimeSlot.objects.filter(start__lt=start)
+            .order_by("-start")
+            .values_list("id", flat=True)[:5]
+        )
+        relevant_timeslot_ids = list(nearest_timeslots_in_future) + list(
+            nearest_timeslots_in_past
+        )
+        return queryset.filter(id__in=relevant_timeslot_ids)
+
+    def filter_start(self, queryset: QuerySet, name: str, value: datetime.date):
+        start = timezone.make_aware(datetime.datetime.combine(value, datetime.time.min))
+        return queryset.filter(start__gte=start)
+
+    def filter_end(self, queryset: QuerySet, name: str, value: datetime.date):
+        end = timezone.make_aware(datetime.datetime.combine(value, datetime.time.max))
+        return queryset.filter(end__lte=end)
+
+    def filter_queryset(self, queryset):
+        queryset = super().filter_queryset(queryset)
+        # This is for backwards compatibility as the surrounding-filter was formerly implemented
+        # by just checking for the existence of the query parameter.
+        if self.request.GET.get("surrounding", None) == "":
+            queryset = self.filter_surrounding(queryset, "surrounding", True)
+        return queryset
+
+    class Meta:
+        model = models.TimeSlot
+        fields = [
+            "order",
+            "start",
+            "end",
+            "surrounding",
+        ]
+
+        class form(forms.Form):
+            def clean_start(self):
+                start = self.cleaned_data.get("start", None)
+                return start or timezone.now().date()
+
+            def clean_end(self):
+                end = self.cleaned_data.get("end", None)
+                return end or self.cleaned_data["start"] + datetime.timedelta(days=60)
diff --git a/program/views.py b/program/views.py
index 3a958cfb..9b32e1de 100644
--- a/program/views.py
+++ b/program/views.py
@@ -20,7 +20,7 @@
 
 import json
 import logging
-from datetime import date, datetime, time, timedelta
+from datetime import date, datetime, time
 
 from rest_framework import permissions, status, viewsets
 from rest_framework.pagination import LimitOffsetPagination
@@ -496,110 +496,29 @@ class APIScheduleViewSet(viewsets.ModelViewSet):
 
 class APITimeSlotViewSet(viewsets.ModelViewSet):
     """
-    /timeslots returns timeslots of the next 60 days (GET). Timeslots may only be added by
-     creating/updating a schedule
-    /timeslots/{pk} eturns the given timeslot (GET)
-    /timeslots/?start={start_date}&end={end_date} returns timeslots within the time range (GET)
-    /shows/{show_pk}/timeslots returns timeslots of the show (GET, POST)
-    /shows/{show_pk}/timeslots?surrounding returns the 10 nearest timeslots for the current date
-     (GET)
-    /shows/{show_pk}/timeslots/{pk} returns a timeslots by its ID (GET, PUT, DELETE)
-    /shows/{show_pk}/timeslots/?start={start_date}&end={end_date} returns timeslots of the show
-     within the time range
-    /shows/{show_pk}/schedules/{schedule_pk}/timeslots returns all timeslots of the schedule (GET,
-     POST)
-    /shows/{show_pk}/schedules/{schedule_pk}/timeslots/{pk} returns a timeslot by its ID (GET,
-     DELETE). If PUT, the next repetition is returned or nothing if the next timeslot isn't one
-    /shows/{show_pk}/schedules/{schedule_pk}/timeslots?start={start_date}&end={end_date} returns
-     all timeslots of the schedule within the time range
+    Returns a list of timeslots.
+
+    By default, only timeslots ranging from now + 60 days will be displayed.
+    You may override this default overriding start and/or end parameter.
+
+    Timeslots may only be added by creating/updating a schedule.
     """
 
     permission_classes = [permissions.DjangoModelPermissionsOrAnonReadOnly]
     serializer_class = TimeSlotSerializer
     pagination_class = LimitOffsetPagination
-    queryset = TimeSlot.objects.none()
+    queryset = TimeSlot.objects.order_by("-start")
+    filterset_class = filters.TimeSlotFilterSet
 
     def get_queryset(self):
+        queryset = super().get_queryset()
+        # subroute filters
         show_pk, schedule_pk = get_values(self.kwargs, "show_pk", "schedule_pk")
-        # Filters
-
-        # Return next 60 days by default
-        start = timezone.make_aware(datetime.combine(timezone.now(), time(0, 0)))
-        end = start + timedelta(days=60)
-
-        if ("start" in self.request.query_params) and (
-            "end" in self.request.query_params
-        ):
-            start = timezone.make_aware(
-                datetime.combine(
-                    parse_date(self.request.query_params.get("start")), time(0, 0)
-                )
-            )
-            end = timezone.make_aware(
-                datetime.combine(
-                    parse_date(self.request.query_params.get("end")), time(23, 59)
-                )
-            )
-
-        default_order = "-start"
-        order = self.request.query_params.get("order", default_order)
-
-        # If someone tries to sort by a field that isn't available on the model
-        # we silently ignore that and use the default sort order.
-        model_fields = [field.name for field in TimeSlot._meta.get_fields()]
-        if order.lstrip("-") not in model_fields:
-            order = default_order
-
-        if "surrounding" in self.request.query_params:
-            now = timezone.now()
-
-            nearest_timeslots_in_future = (
-                TimeSlot.objects.filter(start__gte=now)
-                .order_by("start")
-                .values_list("id", flat=True)[:5]
-            )
-            nearest_timeslots_in_past = (
-                TimeSlot.objects.filter(start__lt=now)
-                .order_by("-start")
-                .values_list("id", flat=True)[:5]
-            )
-            relevant_timeslot_ids = list(nearest_timeslots_in_future) + list(
-                nearest_timeslots_in_past
-            )
-
-            return TimeSlot.objects.filter(id__in=relevant_timeslot_ids).order_by(order)
-
-        # Endpoints
-
-        #
-        #     /shows/1/schedules/1/timeslots/
-        #
-        #     Returns timeslots of the given show and schedule
-        #
-        if show_pk and schedule_pk:
-            return TimeSlot.objects.filter(
-                show=show_pk, schedule=schedule_pk, start__gte=start, end__lte=end
-            ).order_by(order)
-
-        #
-        #     /shows/1/timeslots/
-        #
-        #     Returns timeslots of the show
-        #
-        elif show_pk and schedule_pk is None:
-            return TimeSlot.objects.filter(
-                show=show_pk, start__gte=start, end__lte=end
-            ).order_by(order)
-
-        #
-        #     /timeslots/
-        #
-        #     Returns all timeslots
-        #
-        else:
-            return TimeSlot.objects.filter(start__gte=start, end__lte=end).order_by(
-                order
-            )
+        if show_pk:
+            queryset = queryset.filter(show=show_pk)
+        if schedule_pk:
+            queryset = queryset.filter(schedule=schedule_pk)
+        return queryset
 
     def retrieve(self, request, *args, **kwargs):
         pk, show_pk = get_values(self.kwargs, "pk", "show_pk")
-- 
GitLab