From 474def187b80a5d3230ff50e64dd9726555bd8b2 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Thu, 25 Nov 2021 16:35:55 +0100 Subject: [PATCH 1/4] api: filter by subscriptions in multiple agendas datetimes (#58446) --- chrono/agendas/models.py | 15 +++- chrono/api/serializers.py | 45 +++++++++++- chrono/api/views.py | 14 ++-- tests/api/test_datetimes.py | 138 +++++++++++++++++++++++++++++++++++- tests/api/test_fillslot.py | 2 +- tests/test_api_utils.py | 8 ++- 6 files changed, 207 insertions(+), 15 deletions(-) diff --git a/chrono/agendas/models.py b/chrono/agendas/models.py index 184b600d..60db42d1 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 @@ -898,7 +898,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()), @@ -914,6 +919,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' 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 65d29b46..e1d77cad 100644 --- a/chrono/api/views.py +++ b/chrono/api/views.py @@ -866,9 +866,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')) @@ -876,6 +874,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'), ) @@ -900,8 +899,13 @@ 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]) + ) response = { 'data': [ diff --git a/tests/api/test_datetimes.py b/tests/api/test_datetimes.py index 1bbb6195..824897d2 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}) @@ -1753,6 +1757,12 @@ def test_datetimes_multiple_agendas_sort(app): def test_datetimes_multiple_agendas_queries(app): for i in range(10): agenda = Agenda.objects.create(label=str(i), kind='events') + 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 +1775,125 @@ 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 + + +@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' -- 2.30.2