From 97b0b899afb066021a95212ba0c5bc31d864a3b7 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Thu, 25 Nov 2021 16:35:55 +0100 Subject: [PATCH] api: filter by subscriptions in multiple agendas datetimes (#58446) --- chrono/agendas/models.py | 16 ++- chrono/api/serializers.py | 45 ++++++++- chrono/api/views.py | 29 +++++- tests/api/test_datetimes.py | 191 ++++++++++++++++++++++++++++++++++-- tests/api/test_fillslot.py | 2 +- tests/test_api_utils.py | 8 +- 6 files changed, 272 insertions(+), 19 deletions(-) diff --git a/chrono/agendas/models.py b/chrono/agendas/models.py index 052a8cb4..73eb76cb 100644 --- a/chrono/agendas/models.py +++ b/chrono/agendas/models.py @@ -33,7 +33,7 @@ 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 IntegrityError, connection, models, transaction -from django.db.models import Count, Max, Prefetch, Q +from django.db.models import Count, F, Max, Prefetch, Q from django.template import Context, Template, TemplateSyntaxError, VariableDoesNotExist, engines from django.urls import reverse from django.utils import functional @@ -901,7 +901,12 @@ class Agenda(models.Model): @staticmethod def prefetch_events_and_exceptions( - qs, user_external_id=None, show_past_events=False, min_start=None, max_start=None + qs, + user_external_id=None, + show_past_events=False, + show_only_subscribed=False, + min_start=None, + max_start=None, ): event_queryset = Event.objects.filter( Q(publication_datetime__isnull=True) | Q(publication_datetime__lte=now()), @@ -917,6 +922,12 @@ class Agenda(models.Model): event_queryset = event_queryset.filter(start_datetime__gte=min_start) if max_start: event_queryset = event_queryset.filter(start_datetime__lt=max_start) + if show_only_subscribed: + event_queryset = event_queryset.filter( + agenda__subscriptions__user_external_id=user_external_id, + agenda__subscriptions__date_start__lt=F('start_datetime'), + agenda__subscriptions__date_end__gt=F('start_datetime'), + ) exceptions_desk = Desk.objects.filter(slug='_exceptions_holder').prefetch_related( 'unavailability_calendars' @@ -934,6 +945,7 @@ class Agenda(models.Model): ), ) qs = Agenda.prefetch_recurring_events(qs) + qs = qs.select_related('category') agendas_exceptions = TimePeriodException.objects.filter( Q(desk__slug='_exceptions_holder', desk__agenda__in=qs) | Q( diff --git a/chrono/api/serializers.py b/chrono/api/serializers.py index 294d7352..6f5fb5ef 100644 --- a/chrono/api/serializers.py +++ b/chrono/api/serializers.py @@ -8,6 +8,16 @@ from rest_framework.exceptions import ValidationError from chrono.agendas.models import AbsenceReason, Agenda, Booking, Category, Event, Subscription +def get_objects_from_slugs(slugs, qs): + slugs = set(slugs) + objects = qs.filter(slug__in=slugs) + if len(objects) != len(slugs): + unknown_slugs = sorted(slugs - {obj.slug for obj in objects}) + unknown_slugs = ', '.join(unknown_slugs) + raise ValidationError(('invalid slugs: %s') % unknown_slugs) + return objects + + class StringOrListField(serializers.ListField): def to_internal_value(self, data): if isinstance(data, str): @@ -190,10 +200,41 @@ class DatetimesSerializer(DateRangeSerializer): return attrs -class MultipleAgendasDatetimesSerializer(DatetimesSerializer): +class AgendaOrSubscribedSlugsMixin(metaclass=serializers.SerializerMetaclass): agendas = CommaSeparatedStringField( - required=True, child=serializers.SlugField(max_length=160, allow_blank=False) + required=False, child=serializers.SlugField(max_length=160, allow_blank=False) ) + subscribed = CommaSeparatedStringField( + required=False, child=serializers.SlugField(max_length=160, allow_blank=False) + ) + + def validate(self, attrs): + super().validate(attrs) + if 'agendas' not in attrs and 'subscribed' not in attrs: + raise ValidationError(_('Either "agendas" or "subscribed" parameter is required.')) + if 'agendas' in attrs and 'subscribed' in attrs: + raise ValidationError(_('"agendas" and "subscribed" parameters are mutually exclusive.')) + user_external_id = attrs.get('user_external_id') + if 'subscribed' in attrs and not user_external_id: + raise ValidationError( + {'user_external_id': _('This field is required when using "subscribed" parameter.')} + ) + + if 'subscribed' in attrs: + agendas = Agenda.objects.filter(subscriptions__user_external_id=user_external_id).distinct() + if attrs['subscribed'] != ['all']: + agendas = agendas.filter(category__slug__in=attrs['subscribed']) + attrs['agendas'] = agendas + else: + attrs['agenda_slugs'] = self.agenda_slugs + return attrs + + def validate_agendas(self, value): + self.agenda_slugs = value + return get_objects_from_slugs(value, qs=Agenda.objects.filter(kind='events')) + + +class MultipleAgendasDatetimesSerializer(AgendaOrSubscribedSlugsMixin, DatetimesSerializer): show_past_events = serializers.BooleanField(default=False) diff --git a/chrono/api/views.py b/chrono/api/views.py index 845164c8..05cc82f9 100644 --- a/chrono/api/views.py +++ b/chrono/api/views.py @@ -875,9 +875,7 @@ class MultipleAgendasDatetimes(APIView): raise APIErrorBadRequest(N_('invalid payload'), errors=serializer.errors) payload = serializer.validated_data - agenda_slugs = payload['agendas'] - agendas = get_objects_from_slugs(agenda_slugs, qs=Agenda.objects.filter(kind='events')) - + agendas = payload['agendas'] user_external_id = payload.get('user_external_id') or payload.get('exclude_user_external_id') disable_booked = bool(payload.get('exclude_user_external_id')) show_past_events = bool(payload.get('show_past_events')) @@ -885,6 +883,7 @@ class MultipleAgendasDatetimes(APIView): agendas, user_external_id=user_external_id, show_past_events=show_past_events, + show_only_subscribed=bool('subscribed' in payload), min_start=payload.get('date_start'), max_start=payload.get('date_end'), ) @@ -909,8 +908,28 @@ class MultipleAgendasDatetimes(APIView): ) ) - agenda_querystring_indexes = {agenda_slug: i for i, agenda_slug in enumerate(agenda_slugs)} - entries.sort(key=lambda event: (event.start_datetime, agenda_querystring_indexes[event.agenda.slug])) + if 'agendas' in request.query_params: + agenda_querystring_indexes = { + agenda_slug: i for i, agenda_slug in enumerate(payload['agenda_slugs']) + } + entries.sort( + key=lambda event: ( + event.start_datetime, + agenda_querystring_indexes[event.agenda.slug], + event.slug, + ) + ) + elif 'subscribed' in request.query_params: + category_querystring_indexes = {category: i for i, category in enumerate(payload['subscribed'])} + sort_by_category = bool(payload['subscribed'] != ['all']) + entries.sort( + key=lambda event: ( + event.start_datetime, + category_querystring_indexes[event.agenda.category.slug] if sort_by_category else None, + event.agenda.slug, + event.slug, + ) + ) response = { 'data': [ diff --git a/tests/api/test_datetimes.py b/tests/api/test_datetimes.py index 1bbb6195..1454b593 100644 --- a/tests/api/test_datetimes.py +++ b/tests/api/test_datetimes.py @@ -7,7 +7,7 @@ from django.test import override_settings from django.test.utils import CaptureQueriesContext from django.utils.timezone import localtime, make_aware, make_naive, now -from chrono.agendas.models import Agenda, Booking, Desk, Event, TimePeriodException +from chrono.agendas.models import Agenda, Booking, Category, Desk, Event, Subscription, TimePeriodException pytestmark = pytest.mark.django_db @@ -1584,10 +1584,14 @@ def test_datetimes_multiple_agendas(app): # invalid slugs resp = app.get('/api/agendas/datetimes/', params={'agendas': 'xxx'}, status=400) - assert resp.json['err_desc'] == 'invalid slugs: xxx' + assert resp.json['errors']['agendas'][0] == 'invalid slugs: xxx' resp = app.get('/api/agendas/datetimes/', params={'agendas': 'first-agenda,xxx,yyy'}, status=400) - assert resp.json['err_desc'] == 'invalid slugs: xxx, yyy' + assert resp.json['errors']['agendas'][0] == 'invalid slugs: xxx, yyy' + + # missing agendas parameter + resp = app.get('/api/agendas/datetimes/', params={}, status=400) + assert resp.json['err_desc'] == 'invalid payload' # it's possible to show past events resp = app.get('/api/agendas/datetimes/', params={'agendas': agenda_slugs, 'show_past_events': True}) @@ -1720,17 +1724,26 @@ def test_datetimes_multiple_agendas(app): @pytest.mark.freeze_time('2021-05-06 14:00') def test_datetimes_multiple_agendas_sort(app): - first_agenda = Agenda.objects.create(label='First agenda', kind='events') + category = Category.objects.create(label='Category A') + first_agenda = Agenda.objects.create(label='First agenda', kind='events', category=category) Desk.objects.create(agenda=first_agenda, slug='_exceptions_holder') Event.objects.create(label='10-05', start_datetime=now().replace(day=10), places=5, agenda=first_agenda) - second_agenda = Agenda.objects.create(label='Second agenda', kind='events') + second_agenda = Agenda.objects.create(label='Second agenda', kind='events', category=category) Desk.objects.create(agenda=second_agenda, slug='_exceptions_holder') Event.objects.create(label='09-05', start_datetime=now().replace(day=9), places=5, agenda=second_agenda) Event.objects.create(label='04-05', start_datetime=now().replace(day=4), places=5, agenda=second_agenda) - third_agenda = Agenda.objects.create(label='Third agenda', kind='events') + category = Category.objects.create(label='Category B') + third_agenda = Agenda.objects.create(label='Third agenda', kind='events', category=category) Desk.objects.create(agenda=third_agenda, slug='_exceptions_holder') Event.objects.create(label='09-05', start_datetime=now().replace(day=9), places=5, agenda=third_agenda) Event.objects.create(label='04-05', start_datetime=now().replace(day=4), places=5, agenda=third_agenda) + for agenda in Agenda.objects.all(): + Subscription.objects.create( + agenda=agenda, + user_external_id='xxx', + date_start=now(), + date_end=now() + datetime.timedelta(days=30), + ) # check events are ordered by start_datetime and then by agenda order in querystring agenda_slugs = ','.join((first_agenda.slug, third_agenda.slug, second_agenda.slug)) @@ -1748,11 +1761,45 @@ def test_datetimes_multiple_agendas_sort(app): assert resp.json['data'][3]['id'] == 'second-agenda@09-05' assert resp.json['data'][4]['id'] == 'first-agenda@10-05' + # check subscribed events are ordered by start_datetime + resp = app.get('/api/agendas/datetimes/', params={'subscribed': 'all', 'user_external_id': 'xxx'}) + assert len(resp.json['data']) == 3 + assert resp.json['data'][0]['id'] == 'second-agenda@09-05' + assert resp.json['data'][1]['id'] == 'third-agenda@09-05' + assert resp.json['data'][2]['id'] == 'first-agenda@10-05' + + # and by category slug if specified in querystring + resp = app.get( + '/api/agendas/datetimes/', params={'subscribed': 'category-b,category-a', 'user_external_id': 'xxx'} + ) + assert len(resp.json['data']) == 3 + assert resp.json['data'][0]['id'] == 'third-agenda@09-05' + assert resp.json['data'][1]['id'] == 'second-agenda@09-05' + assert resp.json['data'][2]['id'] == 'first-agenda@10-05' + + # order is stable if same date events are in the same category + Event.objects.create(label='09-05', start_datetime=now().replace(day=9), places=5, agenda=first_agenda) + resp = app.get( + '/api/agendas/datetimes/', params={'subscribed': 'category-b,category-a', 'user_external_id': 'xxx'} + ) + assert len(resp.json['data']) == 4 + assert resp.json['data'][0]['id'] == 'third-agenda@09-05' + assert resp.json['data'][1]['id'] == 'first-agenda@09-05' + assert resp.json['data'][2]['id'] == 'second-agenda@09-05' + assert resp.json['data'][3]['id'] == 'first-agenda@10-05' + @pytest.mark.freeze_time('2021-05-06 14:00') def test_datetimes_multiple_agendas_queries(app): + category = Category.objects.create(label='Category A') for i in range(10): - agenda = Agenda.objects.create(label=str(i), kind='events') + agenda = Agenda.objects.create(label=str(i), kind='events', category=category) + Subscription.objects.create( + agenda=agenda, + user_external_id='xxx', + date_start=now() - datetime.timedelta(days=10), + date_end=now() + datetime.timedelta(days=10), + ) Desk.objects.create(agenda=agenda, slug='_exceptions_holder') Event.objects.create(start_datetime=now() - datetime.timedelta(days=5), places=5, agenda=agenda) Event.objects.create(start_datetime=now() + datetime.timedelta(days=5), places=5, agenda=agenda) @@ -1765,3 +1812,133 @@ def test_datetimes_multiple_agendas_queries(app): ) assert len(resp.json['data']) == 30 assert len(ctx.captured_queries) == 7 + + with CaptureQueriesContext(connection) as ctx: + resp = app.get( + '/api/agendas/datetimes/', + params={'subscribed': 'all', 'user_external_id': 'xxx', 'show_past_events': True}, + ) + assert len(resp.json['data']) == 30 + assert len(ctx.captured_queries) == 7 + + with CaptureQueriesContext(connection) as ctx: + resp = app.get( + '/api/agendas/datetimes/', + params={'subscribed': 'category-a', 'user_external_id': 'xxx', 'show_past_events': True}, + ) + assert len(resp.json['data']) == 30 + assert len(ctx.captured_queries) == 7 + + +@pytest.mark.freeze_time('2021-05-06 14:00') +def test_datetimes_multiple_agendas_subscribed(app): + first_agenda = Agenda.objects.create(label='First agenda', kind='events') + Desk.objects.create(agenda=first_agenda, slug='_exceptions_holder') + Event.objects.create( + slug='event', + start_datetime=now() + datetime.timedelta(days=5), + places=5, + agenda=first_agenda, + ) + Event.objects.create( + slug='event-2', + start_datetime=now() + datetime.timedelta(days=20), + places=5, + agenda=first_agenda, + ) + category = Category.objects.create(label='Category A') + second_agenda = Agenda.objects.create( + label='Second agenda', kind='events', category=category, maximal_booking_delay=400 + ) + Desk.objects.create(agenda=second_agenda, slug='_exceptions_holder') + Event.objects.create( + slug='event', + start_datetime=now() + datetime.timedelta(days=20), + places=5, + agenda=second_agenda, + ) + Event.objects.create( + slug='next-year-event', + start_datetime=now() + datetime.timedelta(days=365), + places=5, + agenda=second_agenda, + ) + Subscription.objects.create( + agenda=first_agenda, + user_external_id='yyy', + date_start=now(), + date_end=now() + datetime.timedelta(days=10), + ) + + # no subscription for user xxx + resp = app.get('/api/agendas/datetimes/', params={'subscribed': 'all', 'user_external_id': 'xxx'}) + assert len(resp.json['data']) == 0 + + # add subscription to first agenda + Subscription.objects.create( + agenda=first_agenda, + user_external_id='xxx', + date_start=now(), + date_end=now() + datetime.timedelta(days=10), + ) + + resp = app.get('/api/agendas/datetimes/', params={'subscribed': 'all', 'user_external_id': 'xxx'}) + assert len(resp.json['data']) == 1 + assert resp.json['data'][0]['id'] == 'first-agenda@event' + + # no subscription to second agenda + resp = app.get('/api/agendas/datetimes/', params={'subscribed': 'category-a', 'user_external_id': 'xxx'}) + assert len(resp.json['data']) == 0 + + # add subscription to second agenda + Subscription.objects.create( + agenda=second_agenda, + user_external_id='xxx', + date_start=now() + datetime.timedelta(days=15), + date_end=now() + datetime.timedelta(days=25), + ) + + resp = app.get('/api/agendas/datetimes/', params={'subscribed': 'category-a', 'user_external_id': 'xxx'}) + assert len(resp.json['data']) == 1 + assert resp.json['data'][0]['id'] == 'second-agenda@event' + + # add new subscription (disjoint) to second agenda + Subscription.objects.create( + agenda=second_agenda, + user_external_id='xxx', + date_start=now() + datetime.timedelta(days=355), + date_end=now() + datetime.timedelta(days=375), + ) + + resp = app.get('/api/agendas/datetimes/', params={'subscribed': 'category-a', 'user_external_id': 'xxx'}) + assert len(resp.json['data']) == 2 + assert resp.json['data'][0]['id'] == 'second-agenda@event' + assert resp.json['data'][1]['id'] == 'second-agenda@next-year-event' + + # view events from all subscriptions + resp = app.get('/api/agendas/datetimes/', params={'subscribed': 'all', 'user_external_id': 'xxx'}) + assert len(resp.json['data']) == 3 + assert resp.json['data'][0]['id'] == 'first-agenda@event' + assert resp.json['data'][1]['id'] == 'second-agenda@event' + assert resp.json['data'][2]['id'] == 'second-agenda@next-year-event' + + # overlapping subscription changes nothing + Subscription.objects.create( + agenda=first_agenda, + user_external_id='xxx', + date_start=now() + datetime.timedelta(days=1), + date_end=now() + datetime.timedelta(days=11), + ) + resp = app.get('/api/agendas/datetimes/', params={'subscribed': 'all', 'user_external_id': 'xxx'}) + assert len(resp.json['data']) == 3 + + # check errors + resp = app.get('/api/agendas/datetimes/', params={'subscribed': 'all'}, status=400) + assert 'required' in resp.json['errors']['user_external_id'][0] + + resp = app.get( + '/api/agendas/datetimes/', + params={'subscribed': 'all', 'agendas': 'first-agenda', 'user_external_id': 'xxx'}, + status=400, + ) + assert 'mutually exclusive' in resp.json['errors']['non_field_errors'][0] diff --git a/tests/api/test_fillslot.py b/tests/api/test_fillslot.py index 66b26627..f0d20fac 100644 --- a/tests/api/test_fillslot.py +++ b/tests/api/test_fillslot.py @@ -2664,7 +2664,7 @@ def test_api_events_fillslots_multiple_agendas(app, user): resp = app.post_json( '/api/agendas/events/fillslots/?agendas=first-agenda,xxx,yyy', params=params, status=400 ) - assert resp.json['err_desc'] == 'invalid slugs: xxx, yyy' + assert resp.json['errors']['agendas'][0] == 'invalid slugs: xxx, yyy' # invalid agenda slugs in payload params = {'user_external_id': 'user_id_3', 'slots': 'first-agenda@event,xxx@event,yyy@event'} diff --git a/tests/test_api_utils.py b/tests/test_api_utils.py index 05e59a08..d7e56d15 100644 --- a/tests/test_api_utils.py +++ b/tests/test_api_utils.py @@ -1,6 +1,6 @@ import pytest -from chrono.agendas.models import Agenda +from chrono.agendas.models import Agenda, MeetingType from chrono.api.utils import Response @@ -31,6 +31,10 @@ def test_err_desc_translation(db, app, settings): assert resp.json['err_desc'] == 'contenu de requête invalide' assert resp.json['err_class'] == 'invalid payload' - resp = app.get('/api/agendas/datetimes/?agendas=hop', status=400) + agenda = Agenda.objects.create(label='Foo bar', kind='meetings') + meeting_type = MeetingType.objects.create(agenda=agenda, slug='foo', duration=60) + resp = app.get( + '/api/agenda/%s/meetings/%s/datetimes/?resources=hop' % (agenda.slug, meeting_type.slug), status=400 + ) assert resp.json['err_desc'] == 'slugs invalides : hop' assert resp.json['err_class'] == 'invalid slugs: hop'