Refactor create_or_update_reservation_statistics to celery task

Optimize the function so that it can create statistics in bulk
for many reservations. Optimize queries to be more efficient.

Author: matti-lamppu

api/graphql/types/recurring_reservation/serializers.py

@@ -19,7 +19,7 @@
 from reservation_units.models import ReservationUnit
 from reservations.enums import ReservationStateChoice, ReservationTypeStaffChoice
 from reservations.models import AffectingTimeSpan, RecurringReservation, Reservation
-from reservations.tasks import create_statistics_for_reservations_task
+from reservations.tasks import create_or_update_reservation_statistics
 
 __all__ = [
     "RecurringReservationCreateSerializer",
@@ -228,7 +228,7 @@ def save(self, **kwargs: Any) -> RecurringReservation:
             AffectingTimeSpan.refresh()
 
         if settings.SAVE_RESERVATION_STATISTICS:
-            create_statistics_for_reservations_task.delay(
+            create_or_update_reservation_statistics.delay(
                 reservation_pks=[reservation.pk for reservation in reservations],
             )
 

applications/tasks.py

@@ -13,7 +13,7 @@
 from opening_hours.utils.reservable_time_span_client import ReservableTimeSpanClient
 from reservations.enums import CustomerTypeChoice, ReservationStateChoice, ReservationTypeChoice
 from reservations.models import AffectingTimeSpan, RecurringReservation
-from reservations.tasks import create_statistics_for_reservations_task
+from reservations.tasks import create_or_update_reservation_statistics
 from tilavarauspalvelu.celery import app
 from utils.sentry import SentryLogger
 
@@ -182,4 +182,4 @@ def generate_reservation_series_from_allocations(application_round_id: int) -> N
         AffectingTimeSpan.refresh()
 
     if settings.SAVE_RESERVATION_STATISTICS:
-        create_statistics_for_reservations_task.delay(reservation_pks=list(reservation_pks))
+        create_or_update_reservation_statistics.delay(reservation_pks=list(reservation_pks))

reservations/management/commands/create_missing_statistics.py

@@ -6,7 +6,7 @@
 
 from common.date_utils import local_datetime
 from reservations.models import Reservation
-from reservations.statistic_utils import create_or_update_reservation_statistics
+from reservations.tasks import create_or_update_reservation_statistics
 
 
 class Command(BaseCommand):
@@ -19,11 +19,12 @@ def handle(self, *args: Any, **options: Any) -> None:
 def create_missing_statistics() -> None:
     pruned_cutoff = local_datetime() - relativedelta(years=settings.REMOVE_RESERVATION_STATS_OLDER_THAN_YEARS)
 
-    pks = Reservation.objects.filter(
-        reservationstatistic__isnull=True,
-        # Don't recreate statistics for old reservations
-        reservation_created_at__gt=pruned_cutoff,
-    ).values_list("pk", flat=True)
+    pks: list[int] = list(
+        Reservation.objects.filter(
+            reservationstatistic__isnull=True,
+            # Don't recreate statistics for old reservations
+            created_at__gt=pruned_cutoff,
+        ).values_list("pk", flat=True)
+    )
 
-    for pk in pks:
-        create_or_update_reservation_statistics(pk)
+    create_or_update_reservation_statistics(pks)

reservations/migrations/0051_refesh_reservation_statistics.py

@@ -2,17 +2,18 @@
 
 from django.db import migrations
 
-from reservations.statistic_utils import create_or_update_reservation_statistics
+from reservations.tasks import create_or_update_reservation_statistics
+
 
 def refresh_reservation_statistics(apps, schema_editor):
     ReservationStatistic = apps.get_model("reservations", "ReservationStatistic")
 
     for statistic in ReservationStatistic.objects.filter(primary_reservation_unit__isnull=True).all():
         if statistic.reservation is not None:
-            create_or_update_reservation_statistics(statistic.reservation.pk)
+            create_or_update_reservation_statistics([statistic.reservation.pk])
 
-class Migration(migrations.Migration):
 
+class Migration(migrations.Migration):
     dependencies = [
         ("reservations", "0050_non_subsidised_price_net_decimals"),
     ]

reservations/models/reservation_statistic.py

@@ -1,9 +1,19 @@
+from __future__ import annotations
+
 import datetime
-from decimal import Decimal
+from typing import TYPE_CHECKING
 
-from django.db import models
+from django.db import models, transaction
 from django.utils import timezone
 
+from reservations.enums import CustomerTypeChoice
+
+if TYPE_CHECKING:
+    from decimal import Decimal
+
+    from reservations.models import Reservation
+
+
 __all__ = [
     "ReservationStatistic",
     "ReservationStatisticsReservationUnit",
@@ -137,6 +147,80 @@ class Meta:
     def __str__(self) -> str:
         return f"{self.reservee_uuid} - {self.begin} - {self.end}"
 
+    @classmethod
+    def for_reservation(cls, reservation: Reservation, *, save: bool = True) -> ReservationStatistic:  # noqa: PLR0915
+        recurring_reservation = getattr(reservation, "recurring_reservation", None)
+        ability_group = getattr(recurring_reservation, "ability_group", None)
+        allocated_time_slot = getattr(recurring_reservation, "allocated_time_slot", None)
+
+        requires_org_name = reservation.reservee_type != CustomerTypeChoice.INDIVIDUAL
+        requires_org_id = not reservation.reservee_is_unregistered_association and requires_org_name
+        by_profile_user = bool(getattr(reservation.user, "profile_id", ""))
+
+        statistic = (  #
+            ReservationStatistic.objects.filter(reservation=reservation).first()
+            or ReservationStatistic(reservation=reservation)
+        )
+
+        statistic.ability_group = ability_group
+        statistic.age_group = reservation.age_group
+        statistic.age_group_name = str(reservation.age_group)
+        statistic.applying_for_free_of_charge = reservation.applying_for_free_of_charge
+        statistic.begin = reservation.begin
+        statistic.buffer_time_after = reservation.buffer_time_after
+        statistic.buffer_time_before = reservation.buffer_time_before
+        statistic.cancel_reason = reservation.cancel_reason
+        statistic.cancel_reason_text = getattr(reservation.cancel_reason, "reason", "")
+        statistic.deny_reason = reservation.deny_reason
+        statistic.deny_reason_text = getattr(reservation.deny_reason, "reason", "")
+        statistic.duration_minutes = (reservation.end - reservation.begin).total_seconds() / 60
+        statistic.end = reservation.end
+        statistic.home_city = reservation.home_city
+        statistic.home_city_municipality_code = getattr(reservation.home_city, "municipality_code", "")
+        statistic.home_city_name = reservation.home_city.name if reservation.home_city else ""
+        statistic.is_applied = allocated_time_slot is not None
+        statistic.is_recurring = recurring_reservation is not None
+        statistic.is_subsidised = reservation.price < reservation.non_subsidised_price
+        statistic.non_subsidised_price = reservation.non_subsidised_price
+        statistic.non_subsidised_price_net = reservation.non_subsidised_price_net
+        statistic.num_persons = reservation.num_persons
+        statistic.price = reservation.price
+        statistic.price_net = reservation.price_net
+        statistic.purpose = reservation.purpose
+        statistic.purpose_name = reservation.purpose.name if reservation.purpose else ""
+        statistic.recurrence_begin_date = getattr(recurring_reservation, "begin_date", None)
+        statistic.recurrence_end_date = getattr(recurring_reservation, "end_date", None)
+        statistic.recurrence_uuid = getattr(recurring_reservation, "uuid", "")
+        statistic.reservation = reservation
+        statistic.reservation_confirmed_at = reservation.confirmed_at
+        statistic.reservation_created_at = reservation.created_at
+        statistic.reservation_handled_at = reservation.handled_at
+        statistic.reservation_type = reservation.type
+        statistic.reservee_address_zip = reservation.reservee_address_zip if by_profile_user else ""
+        statistic.reservee_id = reservation.reservee_id if requires_org_id else ""
+        statistic.reservee_is_unregistered_association = reservation.reservee_is_unregistered_association
+        statistic.reservee_language = reservation.reservee_language
+        statistic.reservee_organisation_name = reservation.reservee_organisation_name if requires_org_name else ""
+        statistic.reservee_type = reservation.reservee_type
+        statistic.reservee_uuid = str(reservation.user.tvp_uuid) if reservation.user else ""
+        statistic.state = reservation.state
+        statistic.tax_percentage_value = reservation.tax_percentage_value
+
+        for res_unit in reservation.reservation_unit.all():
+            statistic.primary_reservation_unit = res_unit
+            statistic.primary_reservation_unit_name = res_unit.name
+            statistic.primary_unit_name = getattr(res_unit.unit, "name", "")
+            statistic.primary_unit_tprek_id = getattr(res_unit.unit, "tprek_id", "")
+            break
+
+        if statistic.is_applied and ability_group:
+            statistic.ability_group_name = ability_group.name
+
+        if save:
+            statistic.save()
+
+        return statistic
+
 
 class ReservationStatisticsReservationUnit(models.Model):
     name = models.CharField(max_length=255)
@@ -163,3 +247,32 @@ class Meta:
 
     def __str__(self) -> str:
         return f"{self.reservation_statistics} - {self.reservation_unit}"
+
+    @classmethod
+    def for_statistic(
+        cls,
+        statistic: ReservationStatistic,
+        *,
+        save: bool = True,
+    ) -> list[ReservationStatisticsReservationUnit]:
+        to_save: list[ReservationStatisticsReservationUnit] = []
+        for reservation_unit in statistic.reservation.reservation_unit.all():
+            stat_unit = ReservationStatisticsReservationUnit(
+                reservation_statistics=statistic,
+                reservation_unit=reservation_unit,
+            )
+
+            stat_unit.name = reservation_unit.name
+            stat_unit.reservation_statistics = statistic
+            stat_unit.reservation_unit = reservation_unit
+            stat_unit.unit_name = getattr(reservation_unit.unit, "name", "")
+            stat_unit.unit_tprek_id = getattr(reservation_unit.unit, "tprek_id", "")
+
+            to_save.append(stat_unit)
+
+        if save:
+            with transaction.atomic():
+                ReservationStatisticsReservationUnit.objects.filter(reservation_statistics=statistic).delete()
+                ReservationStatisticsReservationUnit.objects.bulk_create(to_save)
+
+        return to_save

reservations/signals.py

@@ -5,7 +5,7 @@
 from django.dispatch import receiver
 
 from reservations.models import AffectingTimeSpan, Reservation
-from reservations.statistic_utils import create_or_update_reservation_statistics
+from reservations.tasks import create_or_update_reservation_statistics
 
 type M2MAction = Literal["pre_add", "post_add", "pre_remove", "post_remove", "pre_clear", "post_clear"]
 
@@ -29,7 +29,7 @@ def reservation_create(
         # one can be defined. If in the future we truly support reservations with multiple reservation units,
         # we need to change this implementation so that we either have multiple reservation units in the statistics
         # or we have better way to indicate which one is the primary unit.
-        create_or_update_reservation_statistics(instance.pk)
+        create_or_update_reservation_statistics([instance.pk])
 
     if not raw and settings.UPDATE_AFFECTING_TIME_SPANS:
         AffectingTimeSpan.refresh(using=kwargs.get("using"))
@@ -57,7 +57,7 @@ def reservations_reservation_units_m2m(
     **kwargs: Any,
 ) -> None:
     if action == "post_add" and not raw and not reverse and settings.SAVE_RESERVATION_STATISTICS:
-        create_or_update_reservation_statistics(instance.pk)
+        create_or_update_reservation_statistics([instance.pk])
 
     if not raw and settings.UPDATE_AFFECTING_TIME_SPANS:
         AffectingTimeSpan.refresh(using=kwargs.get("using"))