From acc311026ff2f05e6e085a20f1534ad3e8d2e7b3 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Thu, 21 Apr 2022 11:48:57 +0200 Subject: [PATCH 1/2] api: forbid overlapping events booking (#64383) --- chrono/agendas/models.py | 29 +++++- chrono/api/views.py | 11 +++ chrono/utils/db.py | 11 ++- .../datetimes/test_events_multiple_agendas.py | 49 ++++++++++ tests/api/fillslot/test_events.py | 91 ++++++++++++++++++- .../fillslot/test_events_multiple_agendas.py | 43 ++++++++- 6 files changed, 228 insertions(+), 6 deletions(-) diff --git a/chrono/agendas/models.py b/chrono/agendas/models.py index 7bb3e658..6692e6e7 100644 --- a/chrono/agendas/models.py +++ b/chrono/agendas/models.py @@ -36,8 +36,8 @@ from django.contrib.postgres.fields import ArrayField, JSONField from django.core.exceptions import FieldDoesNotExist, ValidationError from django.core.validators import MaxValueValidator, MinValueValidator from django.db import connection, models, transaction -from django.db.models import Count, Exists, F, Func, Max, OuterRef, Prefetch, Q -from django.db.models.functions import Cast, Coalesce, ExtractWeek +from django.db.models import Count, Exists, ExpressionWrapper, F, Func, Max, OuterRef, Prefetch, Q, Value +from django.db.models.functions import Cast, Coalesce, Concat, ExtractWeek from django.template import Context, Template, TemplateSyntaxError, VariableDoesNotExist, engines from django.urls import reverse from django.utils import functional @@ -55,7 +55,7 @@ from django.utils.translation import ungettext from chrono.interval import Interval, IntervalSet from chrono.utils.date import get_weekday_index -from chrono.utils.db import SumCardinality +from chrono.utils.db import ArraySubquery, SumCardinality from chrono.utils.publik_urls import translate_from_publik_url from chrono.utils.requests_wrapper import requests as requests_wrapper @@ -1562,6 +1562,29 @@ class Event(models.Model): ) return qs + @staticmethod + def annotate_queryset_with_overlaps(qs): + qs = qs.annotate( + computed_end_datetime=ExpressionWrapper( + F('start_datetime') + datetime.timedelta(minutes=1) * F('duration'), + output_field=models.DateTimeField(), + ), + computed_slug=Concat('agenda__slug', Value('@'), 'slug'), + ) + + overlapping_events = qs.filter( + start_datetime__lt=OuterRef('computed_end_datetime'), + computed_end_datetime__gt=OuterRef('start_datetime'), + ).exclude(pk=OuterRef('pk')) + + return qs.annotate( + overlaps=ArraySubquery( + overlapping_events.values('computed_slug'), + output_field=ArrayField(models.CharField()), + ), + has_overlap=Exists(overlapping_events), + ) + @property def remaining_places(self): return max(0, self.places - self.booked_places) diff --git a/chrono/api/views.py b/chrono/api/views.py index 83a8e15d..1516ee4d 100644 --- a/chrono/api/views.py +++ b/chrono/api/views.py @@ -596,6 +596,8 @@ def get_event_detail( details['status'] = 'cancelled' else: details['status'] = 'free' + if hasattr(event, 'overlaps'): + details['overlaps'] = event.overlaps return details @@ -926,6 +928,7 @@ class MultipleAgendasDatetimes(APIView): show_out_of_minimal_delay=show_past_events, ) entries = Event.annotate_queryset_for_user(entries, user_external_id, with_status=with_status) + entries = Event.annotate_queryset_with_overlaps(entries) if show_only_subscribed: entries = entries.filter( agenda__subscriptions__user_external_id=user_external_id, @@ -1846,6 +1849,13 @@ class EventsFillslots(APIView): events = self.get_events(request, payload) + overlapping_events = Event.annotate_queryset_with_overlaps(events).filter(has_overlap=True) + if overlapping_events: + raise APIError( + N_('Some events occur at the same time: %s'), + ', '.join(sorted(str(x) for x in overlapping_events)), + ) + already_booked_events = self.get_already_booked_events(user_external_id) already_booked_events = already_booked_events.filter(start_datetime__gt=now()) if start_datetime: @@ -1877,6 +1887,7 @@ class EventsFillslots(APIView): booking__user_external_id=user_external_id, booking__cancellation_datetime__isnull=False, ) + # book only events without active booking for the user events = events.exclude( pk__in=Booking.objects.filter( diff --git a/chrono/utils/db.py b/chrono/utils/db.py index 2f202643..bbf3c83a 100644 --- a/chrono/utils/db.py +++ b/chrono/utils/db.py @@ -15,13 +15,22 @@ # along with this program. If not, see . from django.db.migrations.operations.base import Operation -from django.db.models import Aggregate +from django.db.models import Aggregate, Subquery class SumCardinality(Aggregate): template = 'SUM(CARDINALITY(%(expressions)s))' +class ArraySubquery(Subquery): + ''' + Available in Django 4.0 + https://docs.djangoproject.com/en/4.0/ref/contrib/postgres/expressions/#arraysubquery-expressions + ''' + + template = 'ARRAY(%(subquery)s)' + + class EnsureJsonbType(Operation): reversible = True diff --git a/tests/api/datetimes/test_events_multiple_agendas.py b/tests/api/datetimes/test_events_multiple_agendas.py index c932b4c2..cc7f8a70 100644 --- a/tests/api/datetimes/test_events_multiple_agendas.py +++ b/tests/api/datetimes/test_events_multiple_agendas.py @@ -1352,3 +1352,52 @@ def test_datetimes_multiple_agendas_with_status(app): status=400, ) assert 'required' in resp.json['errors']['user_external_id'][0] + + +@pytest.mark.freeze_time('2021-09-06 12:00') +def test_datetimes_multiple_agendas_overlapping_events(app): + agenda = Agenda.objects.create(label='Foo bar', kind='events') + Event.objects.create( + label='Event', + start_datetime=now() + datetime.timedelta(days=5), + duration=120, + places=5, + agenda=agenda, + ) + Event.objects.create( + label='Event containing all events', + start_datetime=now() + datetime.timedelta(days=4, hours=23), + duration=440, + places=5, + agenda=agenda, + ) + second_agenda = Agenda.objects.create(label='Foo bar 2', kind='events') + Event.objects.create( + label='Event', + start_datetime=now() + datetime.timedelta(days=5, hours=1), + duration=120, + places=5, + agenda=second_agenda, + ) + Event.objects.create( + label='Event', + start_datetime=now() + datetime.timedelta(days=5, hours=2), + duration=120, + places=5, + agenda=second_agenda, + ) + Event.objects.create( + label='Event no duration', + start_datetime=now() + datetime.timedelta(days=5, hours=1), + places=5, + agenda=second_agenda, + ) + + resp = app.get('/api/agendas/datetimes/', params={'agendas': 'foo-bar,foo-bar-2'}) + assert [(x['id'], x['overlaps']) for x in resp.json['data']] == [ + ('foo-bar@event-containing-all-events', ['foo-bar@event', 'foo-bar-2@event', 'foo-bar-2@event-1']), + ('foo-bar@event', ['foo-bar@event-containing-all-events', 'foo-bar-2@event']), + ('foo-bar-2@event', ['foo-bar@event-containing-all-events', 'foo-bar@event', 'foo-bar-2@event-1']), + ('foo-bar-2@event-no-duration', []), + ('foo-bar-2@event-1', ['foo-bar@event-containing-all-events', 'foo-bar-2@event']), + ] diff --git a/tests/api/fillslot/test_events.py b/tests/api/fillslot/test_events.py index 1c9cf504..0e2df887 100644 --- a/tests/api/fillslot/test_events.py +++ b/tests/api/fillslot/test_events.py @@ -33,7 +33,7 @@ def test_api_events_fillslots(app, user): params = {'user_external_id': 'user_id', 'slots': 'event,event-2'} with CaptureQueriesContext(connection) as ctx: resp = app.post_json(fillslots_url, params=params) - assert len(ctx.captured_queries) == 11 + assert len(ctx.captured_queries) == 12 assert resp.json['booking_count'] == 2 assert len(resp.json['booked_events']) == 2 assert resp.json['booked_events'][0]['id'] == 'event' @@ -402,3 +402,92 @@ def test_api_events_fillslots_preserve_out_of_delays_bookings(app, user, freezer assert resp.json['cancelled_booking_count'] == 0 assert event.booking_set.filter(cancellation_datetime__isnull=True).count() == 1 assert second_event.booking_set.filter(cancellation_datetime__isnull=True).count() == 0 + + +@pytest.mark.freeze_time('2021-09-06 12:00') +def test_api_events_fillslots_overlapping_events(app, user, freezer): + agenda = Agenda.objects.create(label='Foo bar', kind='events') + first_event = Event.objects.create( + label='Event', + start_datetime=now() + datetime.timedelta(days=5), + duration=120, + places=5, + agenda=agenda, + ) + second_event = Event.objects.create( + label='Event 2', + start_datetime=now() + datetime.timedelta(days=5, hours=1), + duration=120, + places=5, + agenda=agenda, + ) + Event.objects.create( + label='Event no duration', + start_datetime=now() + datetime.timedelta(days=5, hours=1), + places=5, + agenda=agenda, + ) + + app.authorization = ('Basic', ('john.doe', 'password')) + fillslots_url = '/api/agenda/foo-bar/events/fillslots/' + resp = app.post_json(fillslots_url, params={'user_external_id': 'user_id', 'slots': 'event'}) + assert resp.json['booking_count'] == 1 + + # booking the same event is still allowed + resp = app.post_json(fillslots_url, params={'user_external_id': 'user_id', 'slots': 'event'}) + assert resp.json['err'] == 0 + assert resp.json['booking_count'] == 0 + + # changing booking to second event is allowed + resp = app.post_json(fillslots_url, params={'user_external_id': 'user_id', 'slots': 'event-2'}) + assert resp.json['err'] == 0 + assert resp.json['booking_count'] == 1 + assert resp.json['cancelled_booking_count'] == 1 + + # booking overlapping events is allowed if one has no duration + resp = app.post_json( + fillslots_url, params={'user_external_id': 'user_id', 'slots': 'event,event-no-duration'} + ) + assert resp.json['err'] == 0 + assert resp.json['booking_count'] == 2 + assert resp.json['cancelled_booking_count'] == 1 + + # booking overlapping events with durations is forbidden + resp = app.post_json(fillslots_url, params={'user_external_id': 'user_id', 'slots': 'event,event-2'}) + assert resp.json['err'] == 1 + assert resp.json['err_desc'] == 'Some events occur at the same time: Event, Event 2' + + # still overlaps but start before + second_event.start_datetime -= datetime.timedelta(hours=2) + second_event.save() + + resp = app.post_json(fillslots_url, params={'user_external_id': 'user_id', 'slots': 'event,event-2'}) + assert resp.json['err'] == 1 + assert resp.json['err_desc'] == 'Some events occur at the same time: Event, Event 2' + + # still overlaps but contains first event + second_event.start_datetime = first_event.start_datetime - datetime.timedelta(minutes=10) + second_event.save() + second_event.duration = first_event.duration + 10 + second_event.save() + + resp = app.post_json(fillslots_url, params={'user_external_id': 'user_id', 'slots': 'event,event-2'}) + assert resp.json['err'] == 1 + assert resp.json['err_desc'] == 'Some events occur at the same time: Event, Event 2' + + # still overlaps but contained by first event + second_event.start_datetime = first_event.start_datetime + datetime.timedelta(minutes=10) + second_event.save() + second_event.duration = first_event.duration - 10 + second_event.save() + + resp = app.post_json(fillslots_url, params={'user_external_id': 'user_id', 'slots': 'event,event-2'}) + assert resp.json['err'] == 1 + assert resp.json['err_desc'] == 'Some events occur at the same time: Event, Event 2' + + # no more overlap + second_event.start_datetime -= datetime.timedelta(hours=5) + second_event.save() + + resp = app.post_json(fillslots_url, params={'user_external_id': 'user_id', 'slots': 'event,event-2'}) + assert resp.json['booking_count'] == 1 diff --git a/tests/api/fillslot/test_events_multiple_agendas.py b/tests/api/fillslot/test_events_multiple_agendas.py index 4d68e67c..24aeafb5 100644 --- a/tests/api/fillslot/test_events_multiple_agendas.py +++ b/tests/api/fillslot/test_events_multiple_agendas.py @@ -163,7 +163,7 @@ def test_api_events_fillslots_multiple_agendas(app, user): params = {'user_external_id': 'user_id', 'slots': event_slugs} with CaptureQueriesContext(connection) as ctx: resp = app.post_json('/api/agendas/events/fillslots/?agendas=%s' % agenda_slugs, params=params) - assert len(ctx.captured_queries) == 16 + assert len(ctx.captured_queries) == 17 assert resp.json['booking_count'] == 2 assert len(resp.json['booked_events']) == 2 assert resp.json['booked_events'][0]['id'] == 'first-agenda@event' @@ -648,3 +648,44 @@ def test_api_events_fillslots_multiple_agendas_shared_custody(app, user): ) assert resp.json['err'] == 1 assert resp.json['err_desc'] == 'Some events are outside guardian custody: first-agenda@event-thursday' + + +@pytest.mark.freeze_time('2021-09-06 12:00') +def test_api_events_fillslots_multiple_agendas_overlapping_events(app, user, freezer): + agenda = Agenda.objects.create(label='Foo bar', kind='events') + Event.objects.create( + label='Event', + start_datetime=now() + datetime.timedelta(days=5), + duration=120, + places=5, + agenda=agenda, + ) + second_agenda = Agenda.objects.create(label='Foo bar 2', kind='events') + Event.objects.create( + label='Event 2', + start_datetime=now() + datetime.timedelta(days=5, hours=1), + duration=120, + places=5, + agenda=second_agenda, + ) + + app.authorization = ('Basic', ('john.doe', 'password')) + fillslots_url = '/api/agendas/events/fillslots/?agendas=%s' + resp = app.post_json( + fillslots_url % ','.join((agenda.slug, second_agenda.slug)), + params={'user_external_id': 'user_id', 'slots': 'foo-bar@event,foo-bar-2@event-2'}, + ) + assert resp.json['err'] == 1 + assert resp.json['err_desc'] == 'Some events occur at the same time: Event, Event 2' + + # events can be booked separately + resp = app.post_json( + fillslots_url % agenda.slug, params={'user_external_id': 'user_id', 'slots': 'foo-bar@event'} + ) + assert resp.json['booking_count'] == 1 + + resp = app.post_json( + fillslots_url % second_agenda.slug, + params={'user_external_id': 'user_id', 'slots': 'foo-bar-2@event-2'}, + ) + assert resp.json['booking_count'] == 1 -- 2.30.2