From d45835f82dfbc5adcf4191cd3a29363684174d4f Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Mon, 2 May 2022 11:29:39 +0200 Subject: [PATCH 2/2] api: forbid overlapping recurring events booking (#64383) --- chrono/agendas/models.py | 40 ++++++- chrono/api/serializers.py | 1 + chrono/api/views.py | 35 +++++- tests/api/datetimes/test_recurring_events.py | 68 +++++++++++ tests/api/fillslot/test_recurring_events.py | 115 ++++++++++++++++++- 5 files changed, 254 insertions(+), 5 deletions(-) diff --git a/chrono/agendas/models.py b/chrono/agendas/models.py index 6692e6e7..901ca923 100644 --- a/chrono/agendas/models.py +++ b/chrono/agendas/models.py @@ -924,11 +924,15 @@ class Agenda(models.Model): ) @staticmethod - def prefetch_recurring_events(qs): + def prefetch_recurring_events(qs, with_overlaps=False): recurring_event_queryset = Event.objects.filter( Q(publication_datetime__isnull=True) | Q(publication_datetime__lte=now()), recurrence_days__isnull=False, ) + + if with_overlaps: + recurring_event_queryset = Event.annotate_recurring_events_with_overlaps(recurring_event_queryset) + qs = qs.prefetch_related( Prefetch( 'event_set', @@ -1585,6 +1589,40 @@ class Event(models.Model): has_overlap=Exists(overlapping_events), ) + @staticmethod + def annotate_recurring_events_with_overlaps(qs): + qs = qs.annotate( + start_hour=Cast('start_datetime', models.TimeField()), + computed_end_datetime=ExpressionWrapper( + F('start_datetime') + datetime.timedelta(minutes=1) * F('duration'), + output_field=models.DateTimeField(), + ), + end_hour=Cast('computed_end_datetime', models.TimeField()), + computed_slug=Concat('agenda__slug', Value('@'), 'slug'), + ) + + overlapping_events = qs.filter( + start_hour__lt=OuterRef('end_hour'), + end_hour__gt=OuterRef('start_hour'), + recurrence_days__overlap=F('recurrence_days'), + ).exclude(pk=OuterRef('pk')) + + json_object = Func( + Value('slug'), + 'computed_slug', + Value('days'), + 'recurrence_days', + function='jsonb_build_object', + output_field=JSONField(), + ) # use django.db.models.functions.JSONObject in Django>=3.2 + + return qs.annotate( + overlaps=ArraySubquery( + overlapping_events.values(json=json_object), + output_field=ArrayField(models.CharField()), + ) + ) + @property def remaining_places(self): return max(0, self.places - self.booked_places) diff --git a/chrono/api/serializers.py b/chrono/api/serializers.py index 3f7606a9..8368c377 100644 --- a/chrono/api/serializers.py +++ b/chrono/api/serializers.py @@ -126,6 +126,7 @@ class RecurringFillslotsSerializer(MultipleAgendasEventsSlotsSerializer): def validate_slots(self, value): super().validate_slots(value) + self.initial_slots = value open_event_slugs = collections.defaultdict(set) for agenda in self.context['agendas']: for event in agenda.get_open_recurring_events(): diff --git a/chrono/api/views.py b/chrono/api/views.py index 1516ee4d..97c5852f 100644 --- a/chrono/api/views.py +++ b/chrono/api/views.py @@ -1134,6 +1134,7 @@ class RecurringEventsList(APIView): recurring_events = Event.objects.filter(pk__in=days_by_event).select_related( 'agenda', 'agenda__category' ) + recurring_events = Event.annotate_recurring_events_with_overlaps(recurring_events) events = [] for event in recurring_events: for day in days_by_event[event.pk]: @@ -1141,7 +1142,7 @@ class RecurringEventsList(APIView): event.day = day events.append(event) else: - agendas = Agenda.prefetch_recurring_events(data['agendas']) + agendas = Agenda.prefetch_recurring_events(data['agendas'], with_overlaps=True) events = [] for agenda in agendas: for event in agenda.get_open_recurring_events(): @@ -1150,6 +1151,11 @@ class RecurringEventsList(APIView): event.day = day events.append(event) + for event in events: + event.overlaps = [ + '%s:%s' % (x['slug'], day) for x in event.overlaps for day in x['days'] if day == event.day + ] + if 'agendas' in request.query_params: agenda_querystring_indexes = { agenda_slug: i for i, agenda_slug in enumerate(data['agenda_slugs']) @@ -1189,6 +1195,7 @@ class RecurringEventsList(APIView): 'description': event.description, 'pricing': event.pricing, 'url': event.url, + 'overlaps': event.overlaps, } for event in events ] @@ -1702,6 +1709,8 @@ class RecurringFillslots(APIView): guardian_external_id, ).values_list('pk', flat=True) + self.check_for_overlaps(events_to_book, serializer.initial_slots) + # outdated bookings to remove (cancelled bookings to replace by an active booking) events_cancelled_to_delete = events_to_book.filter( booking__user_external_id=user_external_id, @@ -1819,6 +1828,30 @@ class RecurringFillslots(APIView): ] return events_to_unbook + @staticmethod + def check_for_overlaps(events, slots): + def get_slug(event, day): + slug = event['slug'] if isinstance(event, dict) else '%s@%s' % (event.agenda.slug, event.slug) + return '%s:%s' % (slug, day) + + recurring_events = Event.objects.filter(pk__in=events.values('primary_event_id')) + recurring_events = Event.annotate_recurring_events_with_overlaps(recurring_events) + + overlaps = set() + for event in recurring_events.select_related('agenda'): + overlaps.update( + tuple(sorted((get_slug(event, d), get_slug(x, d)))) + for x in event.overlaps + for d in x['days'] + if get_slug(x, d) in slots and get_slug(event, d) in slots + ) + + if overlaps: + raise APIError( + N_('Some events occur at the same time: %s') + % ', '.join(sorted('%s / %s' % (x, y) for x, y in overlaps)) + ) + recurring_fillslots = RecurringFillslots.as_view() diff --git a/tests/api/datetimes/test_recurring_events.py b/tests/api/datetimes/test_recurring_events.py index f62ae188..69053d80 100644 --- a/tests/api/datetimes/test_recurring_events.py +++ b/tests/api/datetimes/test_recurring_events.py @@ -434,3 +434,71 @@ def test_recurring_events_api_list_subscribed(app, user): resp = app.get('/api/agendas/recurring-events/?subscribed=category-b,category-a&user_external_id=xxx') event_ids = [x['id'] for x in resp.json['data']] assert event_ids.index('first-agenda@event:0') > event_ids.index('second-agenda@event:0') + + +@pytest.mark.freeze_time('2021-09-06 12:00') +def test_recurring_events_api_list_overlapping_events(app): + agenda = Agenda.objects.create(label='First Agenda', kind='events') + Desk.objects.create(agenda=agenda, slug='_exceptions_holder') + start, end = now(), now() + datetime.timedelta(days=30) + Event.objects.create( + label='Event 12-14', + start_datetime=start, + duration=120, + places=2, + recurrence_end_date=end, + recurrence_days=[1], + agenda=agenda, + ) + Event.objects.create( + label='Event 14-15', + start_datetime=start + datetime.timedelta(hours=2), + duration=60, + places=2, + recurrence_end_date=end, + recurrence_days=[1], + agenda=agenda, + ) + Event.objects.create( + label='Event 15-17', + start_datetime=start + datetime.timedelta(hours=3), + duration=120, + places=2, + recurrence_end_date=end, + recurrence_days=[1, 3, 5], + agenda=agenda, + ) + agenda2 = Agenda.objects.create(label='Second Agenda', kind='events') + Desk.objects.create(agenda=agenda2, slug='_exceptions_holder') + Event.objects.create( + label='Event 12-18', + start_datetime=start, + duration=360, + places=2, + recurrence_end_date=end, + recurrence_days=[1, 5], + agenda=agenda2, + ) + Event.objects.create( + label='No duration', + start_datetime=start, + places=2, + recurrence_end_date=end, + recurrence_days=[5], + agenda=agenda2, + ) + + resp = app.get('/api/agendas/recurring-events/?agendas=first-agenda,second-agenda&sort=day') + assert [(x['id'], x['overlaps']) for x in resp.json['data']] == [ + ('first-agenda@event-12-14:1', ['second-agenda@event-12-18:1']), + ( + 'second-agenda@event-12-18:1', + ['first-agenda@event-12-14:1', 'first-agenda@event-14-15:1', 'first-agenda@event-15-17:1'], + ), + ('first-agenda@event-14-15:1', ['second-agenda@event-12-18:1']), + ('first-agenda@event-15-17:1', ['second-agenda@event-12-18:1']), + ('first-agenda@event-15-17:3', []), + ('second-agenda@event-12-18:5', ['first-agenda@event-15-17:5']), + ('second-agenda@no-duration:5', []), + ('first-agenda@event-15-17:5', ['second-agenda@event-12-18:5']), + ] diff --git a/tests/api/fillslot/test_recurring_events.py b/tests/api/fillslot/test_recurring_events.py index c7f76056..35bf745e 100644 --- a/tests/api/fillslot/test_recurring_events.py +++ b/tests/api/fillslot/test_recurring_events.py @@ -106,7 +106,7 @@ def test_recurring_events_api_fillslots(app, user, freezer, action): params['user_external_id'] = 'user_id_3' with CaptureQueriesContext(connection) as ctx: resp = app.post_json(fillslots_url, params=params) - assert len(ctx.captured_queries) in [12, 13] + assert len(ctx.captured_queries) in [13, 14] # everything goes in waiting list assert events.filter(booked_waiting_list_places=1).count() == 6 # but an event was full @@ -1291,7 +1291,7 @@ def test_recurring_events_api_fillslots_multiple_agendas_queries(app, user): }, ) assert resp.json['booking_count'] == 180 - assert len(ctx.captured_queries) == 13 + assert len(ctx.captured_queries) == 14 father = Person.objects.create(user_external_id='father_id', first_name='John', last_name='Doe') mother = Person.objects.create(user_external_id='mother_id', first_name='Jane', last_name='Doe') @@ -1309,7 +1309,7 @@ def test_recurring_events_api_fillslots_multiple_agendas_queries(app, user): params={'slots': events_to_book, 'user_external_id': 'xxx'}, ) assert resp.json['booking_count'] == 100 - assert len(ctx.captured_queries) == 13 + assert len(ctx.captured_queries) == 14 @pytest.mark.freeze_time('2022-03-07 14:00') # Monday of 10th week @@ -1371,3 +1371,112 @@ def test_recurring_events_api_fillslots_shared_custody(app, user, freezer): '2022-03-19', '2022-03-20', ] + + +@pytest.mark.freeze_time('2021-09-06 12:00') +def test_recurring_events_api_fillslots_overlapping_events(app, user): + agenda = Agenda.objects.create(label='First Agenda', kind='events') + Desk.objects.create(agenda=agenda, slug='_exceptions_holder') + start, end = now(), now() + datetime.timedelta(days=30) + Event.objects.create( + label='Event 12-14', + start_datetime=start, + duration=120, + places=2, + recurrence_end_date=end, + recurrence_days=[1], + agenda=agenda, + ).create_all_recurrences() + Event.objects.create( + label='Event 14-15', + start_datetime=start + datetime.timedelta(hours=2), + duration=60, + places=2, + recurrence_end_date=end, + recurrence_days=[1], + agenda=agenda, + ).create_all_recurrences() + Event.objects.create( + label='Event 15-17', + start_datetime=start + datetime.timedelta(hours=3), + duration=120, + places=2, + recurrence_end_date=end, + recurrence_days=[1, 3, 5], + agenda=agenda, + ).create_all_recurrences() + agenda2 = Agenda.objects.create(label='Second Agenda', kind='events') + Desk.objects.create(agenda=agenda2, slug='_exceptions_holder') + Event.objects.create( + label='Event 12-18', + start_datetime=start, + duration=360, + places=2, + recurrence_end_date=end, + recurrence_days=[1, 5], + agenda=agenda2, + ).create_all_recurrences() + Event.objects.create( + label='No duration', + start_datetime=start, + places=2, + recurrence_end_date=end, + recurrence_days=[5], + agenda=agenda2, + ).create_all_recurrences() + + app.authorization = ('Basic', ('john.doe', 'password')) + fillslots_url = '/api/agendas/recurring-events/fillslots/?action=%s&agendas=%s' + + # booking without overlap + params = { + 'user_external_id': 'user_id', + 'slots': 'first-agenda@event-12-14:1,first-agenda@event-14-15:1,second-agenda@event-12-18:5', + } + resp = app.post_json(fillslots_url % ('update', 'first-agenda,second-agenda'), params=params) + assert resp.json['booking_count'] == 14 + + # book again + resp = app.post_json(fillslots_url % ('update', 'first-agenda,second-agenda'), params=params) + assert resp.json['booking_count'] == 0 + + # change bookings + params = {'user_external_id': 'user_id', 'slots': 'second-agenda@event-12-18:1'} + resp = app.post_json(fillslots_url % ('update', 'first-agenda,second-agenda'), params=params) + assert resp.json['booking_count'] == 5 + assert resp.json['cancelled_booking_count'] == 14 + + # booking overlapping events is allowed if one has no duration + params = { + 'user_external_id': 'user_id', + 'slots': 'second-agenda@event-12-18:5,second-agenda@no-duration:5', + } + resp = app.post_json(fillslots_url % ('update', 'first-agenda,second-agenda'), params=params) + assert resp.json['booking_count'] == 8 + assert resp.json['cancelled_booking_count'] == 5 + + # booking overlapping events with durations is forbidden + params = { + 'user_external_id': 'user_id', + 'slots': 'first-agenda@event-12-14:1,second-agenda@event-12-18:1', + } + resp = app.post_json(fillslots_url % ('update', 'first-agenda,second-agenda'), params=params) + assert resp.json['err'] == 1 + assert ( + resp.json['err_desc'] + == 'Some events occur at the same time: first-agenda@event-12-14:1 / second-agenda@event-12-18:1' + ) + + params = { + 'user_external_id': 'user_id', + 'slots': ( + 'first-agenda@event-12-14:1,first-agenda@event-15-17:1,first-agenda@event-15-17:3,first-agenda@event-15-17:5,second-agenda@event-12-18:1,' + 'second-agenda@event-12-18:5,second-agenda@no-duration:5' + ), + } + resp = app.post_json(fillslots_url % ('update', 'first-agenda,second-agenda'), params=params) + assert resp.json['err'] == 1 + assert resp.json['err_desc'] == ( + 'Some events occur at the same time: first-agenda@event-12-14:1 / second-agenda@event-12-18:1, ' + 'first-agenda@event-15-17:1 / second-agenda@event-12-18:1, first-agenda@event-15-17:5 / second-agenda@event-12-18:5' + ) -- 2.30.2