From 4c7074829a27d597dc230d4855261ace9f78b7fc Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Thu, 21 Oct 2021 11:15:38 +0200 Subject: [PATCH 1/5] api: move MultipleAgendasEventsFillslots validation to serializers (#57957) --- chrono/api/serializers.py | 30 ++++++++++++++++++++++++ chrono/api/views.py | 48 +++++++++++++++++++++----------------- tests/api/test_fillslot.py | 31 ++++++++++++++++++------ 3 files changed, 81 insertions(+), 28 deletions(-) diff --git a/chrono/api/serializers.py b/chrono/api/serializers.py index c3783131..07b5aaf3 100644 --- a/chrono/api/serializers.py +++ b/chrono/api/serializers.py @@ -64,6 +64,30 @@ class EventsSlotsSerializer(SlotSerializer): return attrs +class MultipleAgendasEventsSlotsSerializer(EventsSlotsSerializer): + def validate_slots(self, value): + allowed_agenda_slugs = self.context['allowed_agenda_slugs'] + slots_agenda_slugs = set() + for slot in value: + try: + agenda_slug, event_slug = slot.split('@') + except ValueError: + raise ValidationError(_('Invalid format for slot %s') % slot) + if not agenda_slug: + raise ValidationError(_('Missing agenda slug in slot %s') % slot) + if not event_slug: + raise ValidationError(_('Missing event slug in slot %s') % slot) + slots_agenda_slugs.add(agenda_slug) + + extra_agendas = slots_agenda_slugs - set(allowed_agenda_slugs) + if extra_agendas: + extra_agendas = ', '.join(sorted(extra_agendas)) + raise ValidationError( + _('Some events belong to agendas that are not present in querystring: %s' % extra_agendas) + ) + return value + + class BookingSerializer(serializers.ModelSerializer): user_absence_reason = serializers.CharField(required=False, allow_blank=True, allow_null=True) @@ -137,6 +161,12 @@ class MultipleAgendasDatetimesSerializer(DatetimesSerializer): show_past_events = serializers.BooleanField(default=False) +class AgendaSlugsSerializer(serializers.Serializer): + agendas = CommaSeparatedStringField( + required=True, child=serializers.SlugField(max_length=160, allow_blank=False) + ) + + class EventSerializer(serializers.ModelSerializer): recurrence_days = CommaSeparatedStringField( required=False, child=serializers.IntegerField(min_value=0, max_value=6) diff --git a/chrono/api/views.py b/chrono/api/views.py index ac28b80c..a585126c 100644 --- a/chrono/api/views.py +++ b/chrono/api/views.py @@ -675,6 +675,19 @@ def get_start_and_end_datetime_from_request(request): return serializer.validated_data.get('date_start'), serializer.validated_data.get('date_end') +def get_agendas_from_request(request): + serializer = serializers.AgendaSlugsSerializer(data=request.query_params) + if not serializer.is_valid(): + raise APIError( + _('invalid payload'), + err_class='invalid payload', + errors=serializer.errors, + http_status=status.HTTP_400_BAD_REQUEST, + ) + + return serializer.validated_data.get('agendas') + + def make_booking(event, payload, extra_data, primary_booking=None, in_waiting_list=False, color=None): return Booking( event_id=event.pk, @@ -1664,6 +1677,7 @@ recurring_fillslots = RecurringFillslots.as_view() class EventsFillslots(APIView): permission_classes = (permissions.IsAuthenticated,) serializer_class = serializers.EventsSlotsSerializer + serializer_extra_context = None def post(self, request, agenda_identifier): self.agenda = get_object_or_404(Agenda, slug=agenda_identifier, kind='events') @@ -1672,7 +1686,9 @@ class EventsFillslots(APIView): def fillslots(self, request): start_datetime, end_datetime = get_start_and_end_datetime_from_request(request) - serializer = self.serializer_class(data=request.data, partial=True) + serializer = self.serializer_class( + data=request.data, partial=True, context=self.serializer_extra_context + ) if not serializer.is_valid(): raise APIError( _('invalid payload'), @@ -1736,15 +1752,10 @@ events_fillslots = EventsFillslots.as_view() class MultipleAgendasEventsFillslots(EventsFillslots): + serializer_class = serializers.MultipleAgendasEventsSlotsSerializer + def post(self, request): - self.agendas = None - if 'agendas' not in request.GET: - raise APIError( - _('Missing agendas list in querystring'), - err_class='Missing agendas list in querystring', - http_status=status.HTTP_400_BAD_REQUEST, - ) - self.agenda_slugs = [s for s in request.GET.get('agendas', '').split(',') if s] + self.agenda_slugs = get_agendas_from_request(request) self.agendas = get_objects_from_slugs(self.agenda_slugs, qs=Agenda.objects.filter(kind='events')) return self.fillslots(request) @@ -1754,18 +1765,9 @@ class MultipleAgendasEventsFillslots(EventsFillslots): agenda, event = slot.split('@') events_by_agenda[agenda].append(event) - agendas = get_objects_from_slugs(events_by_agenda.keys(), qs=Agenda.objects.filter(kind='events')) - agendas_by_slug = {agenda.slug: agenda for agenda in agendas} - - if not set(agendas_by_slug).issubset(self.agenda_slugs): - extra_agendas = set(agendas_by_slug) - set(self.agenda_slugs) - extra_agendas = ','.join(extra_agendas) - raise APIError( - _('Some events belong to agendas that are not present in querystring: %s' % extra_agendas), - err_class='Some events belong to agendas that are not present in querystring: %s' - % extra_agendas, - http_status=status.HTTP_400_BAD_REQUEST, - ) + agendas_by_slug = get_objects_from_slugs( + events_by_agenda.keys(), qs=Agenda.objects.filter(kind='events') + ).in_bulk(field_name='slug') events = Event.objects.none() for agenda_slug, event_slugs in events_by_agenda.items(): @@ -1776,6 +1778,10 @@ class MultipleAgendasEventsFillslots(EventsFillslots): def get_already_booked_events(self, user_external_id): return Event.objects.filter(agenda__in=self.agendas, booking__user_external_id=user_external_id) + @property + def serializer_extra_context(self): + return {'allowed_agenda_slugs': self.agenda_slugs} + agendas_events_fillslots = MultipleAgendasEventsFillslots.as_view() diff --git a/tests/api/test_fillslot.py b/tests/api/test_fillslot.py index 545571bb..afc5a857 100644 --- a/tests/api/test_fillslot.py +++ b/tests/api/test_fillslot.py @@ -2487,29 +2487,46 @@ def test_api_events_fillslots_multiple_agendas(app, user): assert resp.json['err'] == 1 assert resp.json['err_desc'] == 'some events are full: Event' - # invalid agenda slugs + # invalid agenda slugs in querystring 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' + # invalid agenda slugs in payload params = {'user_external_id': 'user_id_3', 'slots': 'first-agenda@event,xxx@event,yyy@event'} resp = app.post_json( '/api/agendas/events/fillslots/?agendas=%s' % agenda_slugs, params=params, status=400 ) - assert resp.json['err_desc'] == 'invalid slugs: xxx, yyy' + assert resp.json['errors']['slots'] == [ + 'Some events belong to agendas that are not present in querystring: xxx, yyy' + ] # missing agendas parameter resp = app.post_json('/api/agendas/events/fillslots/', params=params, status=400) - assert resp.json['err_desc'] == 'Missing agendas list in querystring' + assert resp.json['errors']['agendas'] == ['This field is required.'] # valid agendas parameter and event slugs, but mismatch between the two params = {'user_external_id': 'user_id_3', 'slots': event_slugs} resp = app.post_json('/api/agendas/events/fillslots/?agendas=first-agenda', params=params, status=400) - assert ( - resp.json['err_desc'] - == 'Some events belong to agendas that are not present in querystring: second-agenda' - ) + assert resp.json['errors']['slots'] == [ + 'Some events belong to agendas that are not present in querystring: second-agenda' + ] + + # missing @ in slot + params['slots'] = 'first-agenda' + resp = app.post_json('/api/agendas/events/fillslots/?agendas=first-agenda', params=params, status=400) + assert resp.json['errors']['slots'] == ['Invalid format for slot first-agenda'] + + # empty event slug + params['slots'] = 'first-agenda@' + resp = app.post_json('/api/agendas/events/fillslots/?agendas=first-agenda', params=params, status=400) + assert resp.json['errors']['slots'] == ['Missing event slug in slot first-agenda@'] + + # empty agenda slug + params['slots'] = '@event' + resp = app.post_json('/api/agendas/events/fillslots/?agendas=first-agenda', params=params, status=400) + assert resp.json['errors']['slots'] == ['Missing agenda slug in slot @event'] def test_url_translation(app, some_data, user): -- 2.30.2