add: support for multi-zone scatter tickets (#32)

paikkala/forms.py

@@ -18,8 +18,9 @@ class ReservationForm(forms.ModelForm):
     max_count = 5
     integrity_error_retries = 10
 
-    zone = ReservationZoneChoiceField(queryset=Zone.objects.none(), empty_label=None)
+    zone = ReservationZoneChoiceField(queryset=Zone.objects.none(), required=False, empty_label='Any')
     count = forms.IntegerField(min_value=1, initial=1)
+    allow_scatter = forms.BooleanField(required=True)
 
     class Meta:
         fields = ()
@@ -54,7 +55,11 @@ def mangle_zone_field(self) -> None:
             # This additional magic is required because widgets don't have access to their
             # parent fields.  That would be all too easy.
             # ReservationZoneSelect.create_option will process the `z` object here to something sane.
-            zone_field.choices = [(z.id, z) for z in zone_field.queryset]
+            if self.instance.numbered_seats:
+                zone_field.choices = [('', 'Any')]
+            else:
+                zone_field.choices = []
+            zone_field.choices += [(z.id, z) for z in zone_field.queryset]
             zone_field.populate_reservation_statuses(program=self.instance)
             if len(zone_field.choices) == 1 and not self.instance.numbered_seats:
                 zone_field.widget = HiddenInput()
@@ -87,6 +92,7 @@ def save(self, commit: bool = True) -> List[Ticket]:
                             phone=self.cleaned_data.get('phone'),
                             zone=zone,
                             count=count,
+                            allow_scatter=self.cleaned_data['allow_scatter'],
                         )
                     )
             except IntegrityError:  # pragma: no cover

paikkala/models/programs.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import datetime
 from collections import defaultdict
 from typing import TYPE_CHECKING, Dict, Iterator, List, Optional, Set, Tuple
@@ -13,7 +15,6 @@
     MaxTicketsPerUserReached,
     MaxTicketsReached,
     NoCapacity,
-    NoRowCapacity,
     Unreservable,
     UserRequired,
 )
@@ -143,7 +144,7 @@ def remaining_tickets(self) -> int:
     def reserve(  # noqa: C901
         self,
         *,
-        zone: 'Zone',
+        zone: Optional['Zone'],
         count: int,
         user: Optional[AbstractBaseUser] = None,
         name: Optional[str] = None,
@@ -161,13 +162,16 @@ def reserve(  # noqa: C901
         This method is a generator, so please be sure to fully iterate it
         (i.e. `list(p.reserve())`).  Also, it'd be prudent to run it within a transaction.
 
-        :param zone:
+        :param zone: The zone to use, or None for any zone available
         :param count:
         :param user:
-        :param allow_scatter: Whether to allow allocating tickets from scattered rows.
+        :param allow_scatter: Whether to allow allocating tickets from scattered rows. \
+                              Overrides `attempt_sequential` to False.
         :param attempt_sequential: Attempt allocation of sequential seats from each row.
         :return:
         """
+
+        # Trivial sanity checks
         if user and user.is_anonymous:
             user = None
 
@@ -185,39 +189,84 @@ def reserve(  # noqa: C901
                 f'Can only reserve {self.max_tickets_per_batch} tickets per batch for {self}, {count} attempted'
             )
         self.check_reservable()
-        reservation_status = zone.get_reservation_status(program=self)
-        total_reserved = reservation_status.total_reserved
+
+        # User and program quota checks
+        if allow_scatter:
+            attempt_sequential = False
+
+        total_reserved = sum(z.get_reservation_status(self).total_reserved for z in self.zones)
         if total_reserved + count > self.max_tickets:
             raise MaxTicketsReached(
                 f'Reserving {count} more tickets would overdraw {self}\'s ticket limit {self.max_tickets}'
             )
+
         if user and self.tickets.filter(user=user).count() + count > self.max_tickets_per_user:
             raise MaxTicketsPerUserReached(
                 f'{user} reserving {count} more tickets would overdraw '
                 f'{self}\'s per-user ticket limit {self.max_tickets_per_user}'
             )
-        new_reservations = []
-        reserve_count = count  # Count remaining to reserve
-        for row, row_status in sorted(reservation_status.items(), key=lambda pair: pair[1].remaining):
-            if row_status.remaining >= reserve_count or allow_scatter or not self.numbered_seats:
-                row_count = min(reserve_count, row_status.remaining)
-                new_reservations.append((row, row_count))
-                reserve_count -= row_count
-            if reserve_count <= 0:
-                break
-        if reserve_count > 0:  # Oops, ran out of rows with tickets left unscattered
-            raise NoCapacity(f'Could not allocate {reserve_count} of {count} requested tickets in zone {zone}')
-        if not new_reservations:
-            raise NoRowCapacity(f'No single row in zone {zone} has {count} tickets left (try scatter?)')
-
-        for row, row_count in new_reservations:
-            yield from row.reserve(
-                program=self,
-                count=row_count,
-                user=user,
-                name=name,
-                email=email,
-                phone=phone,
-                attempt_sequential=attempt_sequential,
-                excluded_numbers=reservation_status[row].blocked_set,
-            )
+
+        def _reserve_inner(count: int, zone: 'Zone', allow_partial: bool) -> Iterator['Ticket']:
+            reservation_status = zone.get_reservation_status(self)
+            new_reservations: list[tuple[Row, int]] = []
+            reserve_count = count  # Count remaining to reserve
+            for row, row_status in sorted(reservation_status.items(), key=lambda pair: pair[1].remaining):
+                # Add a reservation if:
+                # 1. we can get all requested seats in a single row; or
+                # 2. scatter is allowed (in which case get as many as we can); or
+                # 3. the seats are not numbered (in which case also get as many as we can)
+                if row_status.remaining >= reserve_count or allow_scatter or not self.numbered_seats:
+                    row_count = min(reserve_count, row_status.remaining)
+                    new_reservations.append((row, row_count))
+                    reserve_count -= row_count
+                if reserve_count <= 0:
+                    break
+
+            if reserve_count > 0 and not allow_partial:
+                if allow_scatter:
+                    raise NoCapacity(f'Could not allocate {reserve_count} of {count} requested tickets in zone {zone}')
+                raise NoCapacity(
+                    f'Could not allocate {reserve_count} of {count} requested tickets in zone {zone} (try scatter?)'
+                )
+
+            for row, row_count in new_reservations:
+                yield from row.reserve(
+                    program=self,
+                    count=row_count,
+                    user=user,
+                    name=name,
+                    email=email,
+                    phone=phone,
+                    attempt_sequential=attempt_sequential and not allow_scatter,
+                    excluded_numbers=reservation_status[row].blocked_set,
+                )
+
+        # Single zone: trivial case, scatter is handled in _reserve_inner
+        if zone is not None:
+            yield from _reserve_inner(count, zone, False)
+
+        # Multiple zones, no scatter: loop through zones, accept the first one that gave us the full ticket amount
+        elif not allow_scatter:
+            tickets = None
+            for try_zone in self.zones.all():
+                try:
+                    tickets = list(_reserve_inner(count, try_zone, False))
+                    break
+                except NoCapacity:
+                    continue
+            if not tickets:
+                raise NoCapacity(f'Unable to allocate {count} tickets from any single zone (no scatter)')
+            yield from tickets
+
+        # Multiple zones with scatter: loop through zones, attempting to get the full ticket amount in total
+        else:
+            reserved: list[Ticket] = []
+            for try_zone in self.zones.all():
+                chunk = list(_reserve_inner(count - len(reserved), try_zone, True))
+                reserved += chunk
+                if len(reserved) >= count:
+                    assert len(reserved) == count
+                    break
+            if len(reserved) < count:
+                raise NoCapacity('Unable to allocate {count} tickets total from any zone with scatter')
+            yield from reserved

paikkala/models/zones.py

@@ -1,3 +1,4 @@
+from dataclasses import dataclass
 from typing import TYPE_CHECKING, Any, Dict, Set
 
 from django.db import models
@@ -8,20 +9,12 @@
     from paikkala.models.rows import Row
 
 
-# TODO(3.7): dataclass-ify
+@dataclass
 class RowReservationStatus:
-    def __init__(
-        self,
-        *,
-        capacity: int,
-        reserved: int,
-        remaining: int,
-        blocked_set: Set[int],
-    ) -> None:
-        self.capacity = capacity
-        self.reserved = reserved
-        self.remaining = remaining
-        self.blocked_set = blocked_set
+    capacity: int
+    reserved: int
+    remaining: int
+    blocked_set: Set[int]
 
 
 class ZoneReservationStatus(dict):

paikkala/tests/conftest.py

@@ -8,6 +8,7 @@
 from paikkala.models import Program, Room, Row, Zone
 from paikkala.tests.demo_data import (
     create_jussi_program,
+    create_scatter_program,
     create_workshop_program,
     create_workshop_room,
     create_workshop_row,
@@ -26,6 +27,11 @@ def jussi_program(sibeliustalo_zones):
     return create_jussi_program(sibeliustalo_zones)
 
 
+@pytest.fixture
+def scatter_program(sibeliustalo_zones):
+    return create_scatter_program(sibeliustalo_zones)
+
+
 @pytest.fixture
 def workshop_room():
     return create_workshop_room()

paikkala/tests/demo_data.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import os
 from datetime import timedelta
 
@@ -40,6 +42,40 @@ def create_jussi_program(zones, room=None):
     return program
 
 
+def create_scatter_program(zones: list[Zone], room=None):
+    if not room:
+        room = zones[0].room
+    program = Program.objects.create(
+        room=room,
+        name='Hyvin suosittu ohjelmanumero',
+        reservation_start=now() - timedelta(hours=1),
+        reservation_end=now() + timedelta(hours=1),
+        max_tickets=1_000_000,
+        max_tickets_per_batch=1_000_000,
+    )
+    program.rows.set(Row.objects.filter(zone__in=zones))
+
+    for zone in zones:
+        row: Row
+        for row in zone.rows.all():
+            # Leave one seat per row
+            _ = list(row.reserve(
+                program=program,
+                count=row.capacity - 1,
+                user=None,
+                name='Señor Developer',
+                email='test@localhost',
+                phone=None,
+                attempt_sequential=False,
+                excluded_numbers=None,
+            ))
+
+        status = zone.get_reservation_status(program)
+        assert status.total_remaining == zone.rows.count()
+
+    return program
+
+
 def create_workshop_room():
     return Room.objects.create(
         name='Pajatila',

paikkala/tests/test_paikkala.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import pytest
 from django.contrib.auth.models import AnonymousUser
 
@@ -23,7 +25,7 @@ def test_is_reservable(jussi_program):
 
 
 @pytest.mark.django_db
-def test_reserve_non_scatter(jussi_program):
+def test_reserve_non_scatter_single_zone(jussi_program):
     zone = jussi_program.zones.get(name='Aitio 1 (vasen)')
     assert zone.capacity == 9
     with pytest.raises(NoCapacity):
@@ -34,6 +36,18 @@ def test_reserve_non_scatter(jussi_program):
     assert rstat[row].reserved == 5
 
 
+@pytest.mark.django_db
+def test_reserve_non_scatter_multi_zone(jussi_program):
+    jussi_program.max_tickets = 1_000
+    max_zone_capacity = max(z.get_reservation_status(jussi_program).total_capacity for z in jussi_program.zones)
+    n_to_reserve = 21
+    with pytest.raises(NoCapacity, match=r'from any single zone \(no scatter\)'):
+        list(jussi_program.reserve(zone=None, count=max_zone_capacity + 1, allow_scatter=False))
+    tickets = list(jussi_program.reserve(zone=None, count=n_to_reserve, allow_scatter=False))
+    assert len(tickets) == n_to_reserve
+    assert all(t.zone == tickets[0].zone for t in tickets)
+
+
 @pytest.mark.django_db
 def test_reserve_limit(jussi_program):
     zone = jussi_program.zones.get(name='Permanto')
@@ -42,13 +56,13 @@ def test_reserve_limit(jussi_program):
 
 
 @pytest.mark.django_db
-def test_reserve_scatter(jussi_program):
-    jussi_program.max_tickets = 1000
+def test_reserve_scatter_single_zone(jussi_program):
+    jussi_program.max_tickets = 1_000
     zone = jussi_program.zones.get(name='Permanto')
     assert zone.capacity == 650
     n_to_reserve = 494
     with pytest.raises(NoCapacity):
-        list(jussi_program.reserve(zone=zone, count=n_to_reserve))
+        list(jussi_program.reserve(zone=zone, count=zone.capacity + 1, allow_scatter=True))
     tickets = list(jussi_program.reserve(zone=zone, count=n_to_reserve, allow_scatter=True))
     assert len(tickets) == n_to_reserve
     rstat = zone.get_reservation_status(program=jussi_program)
@@ -57,6 +71,30 @@ def test_reserve_scatter(jussi_program):
     assert any(r.reserved and r.capacity for r in rstat.values())  # Check that we have semi-reserved rows
 
 
+@pytest.mark.django_db
+def test_reserve_scatter_multi_zone(scatter_program):
+    scatter_program.max_tickets = 1_000_000
+    assert scatter_program.is_reservable()
+
+    rstat_before = {z: z.get_reservation_status(scatter_program) for z in scatter_program.zones}
+    total_reserved_before = sum(rs.total_reserved for rs in rstat_before.values())
+
+    n_to_reserve = sum(z.rows.count() for z in scatter_program.zones)
+    tickets = list(scatter_program.reserve(zone=None, count=n_to_reserve, allow_scatter=True))
+
+    rstat_after = {z: z.get_reservation_status(scatter_program) for z in scatter_program.zones}
+    total_reserved_after = sum(rs.total_reserved for rs in rstat_after.values())
+
+    assert len(tickets) == n_to_reserve
+    assert total_reserved_after == total_reserved_before + n_to_reserve
+
+
+@pytest.mark.django_db
+def test_reserve_scatter_multi_zone_fail(scatter_program):
+    with pytest.raises(NoCapacity):
+        list(scatter_program.reserve(zone=None, count=1_000, allow_scatter=True))
+
+
 @pytest.mark.django_db
 def test_reserve_user_required(jussi_program):
     jussi_program.require_user = True

paikkala/tests/test_views.py

@@ -15,6 +15,7 @@ def test_reserve(jussi_program, user_client):
             {
                 'zone': jussi_program.zones[0].pk,
                 'count': 5,
+                'allow_scatter': 0,
             },
         ).status_code
         == 302