diff --git a/paikkala/forms.py b/paikkala/forms.py index 5c22227..97ab112 100644 --- a/paikkala/forms.py +++ b/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 diff --git a/paikkala/models/programs.py b/paikkala/models/programs.py index d1315bf..a998d82 100644 --- a/paikkala/models/programs.py +++ b/paikkala/models/programs.py @@ -143,7 +143,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 +161,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 +188,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}' + f'Reserving {count} more tickets would overdraw zone\'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): + 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: + # 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: # Could not find any tickets whatsoever + if not allow_scatter: + raise NoRowCapacity( + f'Unable to reserve {count} tickets in a single row for zone {zone} (try scatter?)' + ) + raise NoCapacity(f'Unable to reserve any tickets in zone {zone}, requested {count}') + + 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 = _reserve_inner(count, try_zone, False) + break + except NoCapacity: + continue + if not tickets: + raise NoCapacity(f'Unable to reserve a total of {count} across all zones') + yield from tickets + + # Multiple zones with scatter: loop through zones, attempting to get the full ticket amount in total + else: + reserved = [] + 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('Not enough free tickets left in the whole program') + yield from reserved diff --git a/paikkala/models/zones.py b/paikkala/models/zones.py index 968a6f0..9874e99 100644 --- a/paikkala/models/zones.py +++ b/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): diff --git a/paikkala/tests/conftest.py b/paikkala/tests/conftest.py index 73940ab..ca79448 100644 --- a/paikkala/tests/conftest.py +++ b/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() diff --git a/paikkala/tests/demo_data.py b/paikkala/tests/demo_data.py index 391f9e5..f510222 100644 --- a/paikkala/tests/demo_data.py +++ b/paikkala/tests/demo_data.py @@ -40,6 +40,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 zone + _ = 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', diff --git a/paikkala/tests/test_paikkala.py b/paikkala/tests/test_paikkala.py index af07344..3246da8 100644 --- a/paikkala/tests/test_paikkala.py +++ b/paikkala/tests/test_paikkala.py @@ -23,7 +23,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 +34,14 @@ 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): + n_to_reserve = 21 + tickets = list(jussi_program.reserve(zone=None, count=n_to_reserve)) + 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,7 +50,7 @@ def test_reserve_limit(jussi_program): @pytest.mark.django_db -def test_reserve_scatter(jussi_program): +def test_reserve_scatter_single_zone(jussi_program): jussi_program.max_tickets = 1000 zone = jussi_program.zones.get(name='Permanto') assert zone.capacity == 650 @@ -57,6 +65,24 @@ 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_user_required(jussi_program): jussi_program.require_user = True diff --git a/paikkala/tests/test_views.py b/paikkala/tests/test_views.py index 120c9b5..92328d9 100644 --- a/paikkala/tests/test_views.py +++ b/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