From d778ca51dc41ce2440ff4712c8f940fd1ee4c818 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 | 2 + chrono/api/views.py | 42 ++++++- tests/api/datetimes/test_recurring_events.py | 73 +++++++++++ tests/api/fillslot/test_recurring_events.py | 125 ++++++++++++++++++- 5 files changed, 279 insertions(+), 3 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 84c53baf..10058394 100644 --- a/chrono/api/serializers.py +++ b/chrono/api/serializers.py @@ -127,6 +127,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(): @@ -348,6 +349,7 @@ class RecurringFillslotsQueryStringSerializer(AgendaOrSubscribedSlugsSerializer) class RecurringEventsListSerializer(AgendaOrSubscribedSlugsSerializer): sort = serializers.ChoiceField(required=False, choices=['day']) + check_overlaps = serializers.BooleanField(default=False) class AgendaSlugsSerializer(serializers.Serializer): diff --git a/chrono/api/views.py b/chrono/api/views.py index d944fb87..24d6aab0 100644 --- a/chrono/api/views.py +++ b/chrono/api/views.py @@ -1119,6 +1119,7 @@ class RecurringEventsList(APIView): if not serializer.is_valid(): raise APIErrorBadRequest(N_('invalid payload'), errors=serializer.errors) data = serializer.validated_data + check_overlaps = bool(data.get('check_overlaps')) guardian_external_id = data.get('guardian_external_id') if guardian_external_id: @@ -1136,6 +1137,8 @@ class RecurringEventsList(APIView): recurring_events = Event.objects.filter(pk__in=days_by_event).select_related( 'agenda', 'agenda__category' ) + if check_overlaps: + recurring_events = Event.annotate_recurring_events_with_overlaps(recurring_events) events = [] for event in recurring_events: for day in days_by_event[event.pk]: @@ -1143,7 +1146,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=check_overlaps) events = [] for agenda in agendas: for event in agenda.get_open_recurring_events(): @@ -1152,6 +1155,15 @@ class RecurringEventsList(APIView): event.day = day events.append(event) + if check_overlaps: + 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']) @@ -1191,6 +1203,7 @@ class RecurringEventsList(APIView): 'description': event.description, 'pricing': event.pricing, 'url': event.url, + 'overlaps': event.overlaps if check_overlaps else None, } for event in events ] @@ -1704,6 +1717,9 @@ class RecurringFillslots(APIView): guardian_external_id, ).values_list('pk', flat=True) + if payload.get('check_overlaps'): + 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, @@ -1821,6 +1837,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..9be6c1fa 100644 --- a/tests/api/datetimes/test_recurring_events.py +++ b/tests/api/datetimes/test_recurring_events.py @@ -434,3 +434,76 @@ 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&check_overlaps=true' + ) + 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']), + ] + + resp = app.get('/api/agendas/recurring-events/?agendas=first-agenda,second-agenda&sort=day') + assert ['overlaps' not in x for x in resp.json['data']] diff --git a/tests/api/fillslot/test_recurring_events.py b/tests/api/fillslot/test_recurring_events.py index c7f76056..1e6f522f 100644 --- a/tests/api/fillslot/test_recurring_events.py +++ b/tests/api/fillslot/test_recurring_events.py @@ -1288,10 +1288,11 @@ def test_recurring_events_api_fillslots_multiple_agendas_queries(app, user): 'slots': events_to_book, 'user_external_id': 'user', 'include_booked_events_detail': True, + 'check_overlaps': True, }, ) 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') @@ -1371,3 +1372,125 @@ 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', + 'check_overlaps': True, + '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', 'check_overlaps': True, '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', + 'check_overlaps': True, + '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', + 'check_overlaps': True, + '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', + 'check_overlaps': True, + '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' + ) + + # overlaps check is disabled by default + 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'] == 0 + assert resp.json['booking_count'] == 10 -- 2.30.2