From 3f82f4dae59ec8123b36ea4feae661a7c0eec87f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Mon, 20 Jul 2020 17:39:45 +0200
Subject: [PATCH] api: fix resize endpoint (#44739)

---
 chrono/agendas/models.py |   3 +-
 chrono/api/views.py      |  80 ++++++++----
 tests/test_api.py        | 256 +++++++++++++++++++++++++++------------
 3 files changed, 234 insertions(+), 105 deletions(-)

diff --git a/chrono/agendas/models.py b/chrono/agendas/models.py
index 21091f2..f6ad5f0 100644
--- a/chrono/agendas/models.py
+++ b/chrono/agendas/models.py
@@ -966,10 +966,9 @@ class Booking(models.Model):
         ics.add(vevent)
         return ics.serialize()
 
-    def clone(self, in_waiting_list=False, primary_booking=None, save=True):
+    def clone(self, primary_booking=None, save=True):
         new_booking = copy.deepcopy(self)
         new_booking.id = None
-        new_booking.in_waiting_list = in_waiting_list
         new_booking.primary_booking = primary_booking
         if save:
             new_booking.save()
diff --git a/chrono/api/views.py b/chrono/api/views.py
index 284ea1d..82ebf12 100644
--- a/chrono/api/views.py
+++ b/chrono/api/views.py
@@ -1247,9 +1247,11 @@ class ResizeBooking(APIView):
             }
             return Response(response)
         event_ids = set([event.pk])
+        in_waiting_list = set([booking.in_waiting_list])
         secondary_bookings = booking.secondary_booking_set.all().order_by('-creation_datetime')
         for secondary in secondary_bookings:
             event_ids.add(secondary.event_id)
+            in_waiting_list.add(secondary.in_waiting_list)
         if len(event_ids) > 1:
             response = {
                 'err': 4,
@@ -1257,41 +1259,69 @@ class ResizeBooking(APIView):
                 'err_desc': _('can not resize multi event booking'),
             }
             return Response(response)
+        if len(in_waiting_list) > 1:
+            response = {
+                'err': 5,
+                'err_class': 'can not resize booking: waiting list inconsistency',
+                'err_desc': _('can not resize booking: waiting list inconsistency'),
+            }
+            return Response(response)
 
-        count = payload['count']
-        booked_places = event.waiting_list_places and event.waiting_list or event.booked_places
-        if booked_places < count:
-            if (
-                event.waiting_list
-                and count > event.waiting_list_places
-                or not event.waiting_list
-                and count > event.places
-            ):
+        # total places for the event (in waiting or main list, depending on the primary booking location)
+        places = booking.in_waiting_list and event.waiting_list_places or event.places
+        # total booked places for the event (in waiting or main list, depending on the primary booking location)
+        booked_places = booking.in_waiting_list and event.waiting_list or event.booked_places
+
+        # places to book for this primary booking
+        primary_wanted_places = payload['count']
+        # already booked places for this primary booking
+        primary_booked_places = 1 + len(secondary_bookings)
+
+        if primary_booked_places > primary_wanted_places:
+            # it is always ok to decrease booking
+            return self.decrease(booking, secondary_bookings, primary_booked_places, primary_wanted_places)
+        if primary_booked_places == primary_wanted_places:
+            # it is always ok to do nothing
+            return self.success(booking)
+
+        # else, increase places if allowed
+        if booked_places - primary_booked_places + primary_wanted_places > places:
+            # oversized request
+            if booking.in_waiting_list:
+                # booking in waiting list: can not be overbooked
+                response = {
+                    'err': 3,
+                    'err_class': 'sold out',
+                    'err_desc': _('sold out'),
+                }
+                return Response(response)
+            if event.booked_places <= event.places:
+                # in main list and no overbooking for the moment: can not be overbooked
                 response = {
                     'err': 3,
                     'err_class': 'sold out',
                     'err_desc': _('sold out'),
                 }
                 return Response(response)
+        return self.increase(booking, secondary_bookings, primary_booked_places, primary_wanted_places)
 
+    def increase(self, booking, secondary_bookings, primary_booked_places, primary_wanted_places):
         with transaction.atomic():
-            if booked_places > count:
-                # decrease places
-                for secondary in secondary_bookings[: booked_places - count]:
-                    secondary.delete()
-            elif booked_places < count:
-                # increase places
-                bulk_bookings = []
-                for i in range(0, count - booked_places):
-                    bulk_bookings.append(
-                        booking.clone(
-                            in_waiting_list=bool(event.waiting_list_places and event.waiting_list),
-                            primary_booking=booking,
-                            save=False,
-                        )
-                    )
-                Booking.objects.bulk_create(bulk_bookings)
+            bulk_bookings = []
+            for i in range(0, primary_wanted_places - primary_booked_places):
+                bulk_bookings.append(booking.clone(primary_booking=booking, save=False,))
+            Booking.objects.bulk_create(bulk_bookings)
+
+        return self.success(booking)
+
+    def decrease(self, booking, secondary_bookings, primary_booked_places, primary_wanted_places):
+        with transaction.atomic():
+            for secondary in secondary_bookings[: primary_booked_places - primary_wanted_places]:
+                secondary.delete()
+
+        return self.success(booking)
 
+    def success(self, booking):
         response = {'err': 0, 'booking_id': booking.pk}
         return Response(response)
 
diff --git a/tests/test_api.py b/tests/test_api.py
index b6db0aa..34e1aeb 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -2009,121 +2009,221 @@ def test_suspend_booking(app, some_data, user):
     assert primary.in_waiting_list is False
 
 
-def test_resize_booking(app, some_data, user):
-    agenda_id = Agenda.objects.filter(label=u'Foo bar')[0].id
-    event = Event.objects.filter(agenda_id=agenda_id).exclude(start_datetime__lt=now())[0]
-    event.places = 5
-    event.waiting_list_places = 5
-    event.save()
-    event2 = Event.objects.filter(agenda_id=agenda_id).exclude(start_datetime__lt=now())[1]
+def test_resize_booking_invalid_payload(app, user):
+    app.authorization = ('Basic', ('john.doe', 'password'))
+    # decrease a booking to 0
+    resp = app.post('/api/booking/0/resize/', params={'count': 0}, status=400)
+    assert resp.json['err'] == 1
+    assert resp.json['err_desc'] == 'invalid payload'
 
-    # create a booking not on the waiting list
-    primary = Booking.objects.create(event=event, in_waiting_list=False)
-    secondary = Booking.objects.create(event=event, in_waiting_list=False, primary_booking=primary)
 
-    assert Booking.objects.filter(in_waiting_list=False).count() == 2
+def test_resize_booking_not_found(app, user):
+    app.authorization = ('Basic', ('john.doe', 'password'))
+    app.post('/api/booking/0/resize/', params={'count': 42}, status=404)
+
 
+def test_resize_booking_not_primary(app, user):
+    agenda = Agenda.objects.create(label='Foo bar', kind='events')
+    event = Event.objects.create(
+        slug='event', start_datetime=now(), places=5, waiting_list_places=5, agenda=agenda,
+    )
+    primary = Booking.objects.create(event=event)
+    secondary = Booking.objects.create(event=event, primary_booking=primary)
     app.authorization = ('Basic', ('john.doe', 'password'))
 
+    # resize a booking that is not primary
     resp = app.post('/api/booking/%s/resize/' % secondary.pk, params={'count': 42})
     assert resp.json['err'] == 2
-    assert resp.json['reason'] == 'secondary booking'  # legacy
-    assert resp.json['err_class'] == 'secondary booking'
     assert resp.json['err_desc'] == 'secondary booking'
 
-    # resize a booking that doesn't exist
-    resp = app.post('/api/booking/0/resize/', params={'count': 42}, status=404)
 
-    # decrease a booking to 0
-    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 0}, status=400)
+def test_resize_booking_cancelled(app, user):
+    agenda = Agenda.objects.create(label='Foo bar', kind='events')
+    event = Event.objects.create(
+        slug='event', start_datetime=now(), places=5, waiting_list_places=5, agenda=agenda,
+    )
+    primary = Booking.objects.create(event=event)
+    primary.cancel()
+
+    # resize a booking that was cancelled before
+    app.authorization = ('Basic', ('john.doe', 'password'))
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 42})
     assert resp.json['err'] == 1
+    assert resp.json['err_desc'] == 'booking is cancelled'
 
-    # decrease a booking not in waiting list
-    assert Booking.objects.filter(in_waiting_list=False).count() == 2
-    assert Booking.objects.filter().count() == 2
-    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 1})
-    assert resp.json['err'] == 0
-    assert Booking.objects.filter(in_waiting_list=False).count() == 1
-    assert Booking.objects.filter(primary_booking__isnull=True).count() == 1
-    assert Booking.objects.filter().count() == 1
 
-    # no changes
-    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 1})
-    assert resp.json['err'] == 0
-    assert Booking.objects.filter(in_waiting_list=False).count() == 1
-    assert Booking.objects.filter().count() == 1
+def test_resize_booking_multi_events(app, user):
+    agenda = Agenda.objects.create(label='Foo bar', kind='events')
+    event1 = Event.objects.create(
+        slug='event1', start_datetime=now(), places=5, waiting_list_places=5, agenda=agenda,
+    )
+    event2 = Event.objects.create(
+        slug='event2', start_datetime=now(), places=5, waiting_list_places=5, agenda=agenda,
+    )
+    primary = Booking.objects.create(event=event1, in_waiting_list=False)
+    Booking.objects.create(event=event2, in_waiting_list=False, primary_booking=primary)
+    app.authorization = ('Basic', ('john.doe', 'password'))
+
+    # resize a booking that is on multi events
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 42})
+    assert resp.json['err'] == 4
+    assert resp.json['err_desc'] == 'can not resize multi event booking'
+
+
+def test_resize_booking_mixed_waiting_list(app, user):
+    agenda = Agenda.objects.create(label='Foo bar', kind='events')
+    event = Event.objects.create(
+        slug='event', start_datetime=now(), places=5, waiting_list_places=5, agenda=agenda,
+    )
+    primary = Booking.objects.create(event=event, in_waiting_list=False)
+    Booking.objects.create(event=event, primary_booking=primary, in_waiting_list=True)
+    app.authorization = ('Basic', ('john.doe', 'password'))
+
+    # booking is in main list and waiting list in the same time
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 42})
+    assert resp.json['err'] == 5
+    assert resp.json['err_desc'] == 'can not resize booking: waiting list inconsistency'
+
 
-    # increase a booking not in waiting list
+def test_resize_booking(app, user):
+    agenda = Agenda.objects.create(label='Foo bar', kind='events')
+    event = Event.objects.create(
+        slug='event', start_datetime=now(), places=5, waiting_list_places=5, agenda=agenda,
+    )
+    primary = Booking.objects.create(event=event, in_waiting_list=False)
+    # there is other bookings
+    Booking.objects.create(event=event, in_waiting_list=False)
+    app.authorization = ('Basic', ('john.doe', 'password'))
+
+    # increase
     resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 2})
     assert resp.json['err'] == 0
-    assert Booking.objects.filter(in_waiting_list=False).count() == 2
-    assert Booking.objects.filter(primary_booking__isnull=True).count() == 1
-    assert Booking.objects.filter().count() == 2
-    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 6})
+    assert Booking.objects.filter(in_waiting_list=False).count() == 3
+    assert Booking.objects.filter(primary_booking__isnull=True).count() == 2
+    assert Booking.objects.count() == 3
+    assert primary.secondary_booking_set.count() == 1
+    # too much
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 5})
     assert resp.json['err'] == 3
-    assert resp.json['reason'] == 'sold out'  # legacy
-    assert resp.json['err_class'] == 'sold out'
     assert resp.json['err_desc'] == 'sold out'
-    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 5})
+    # max
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 4})
     assert resp.json['err'] == 0
     assert Booking.objects.filter(in_waiting_list=False).count() == 5
-    assert Booking.objects.filter(primary_booking__isnull=True).count() == 1
-    assert Booking.objects.filter().count() == 5
-    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 6})
-    assert resp.json['err'] == 3
-    assert resp.json['reason'] == 'sold out'  # legacy
-    assert resp.json['err_class'] == 'sold out'
-    assert resp.json['err_desc'] == 'sold out'
+    assert Booking.objects.filter(primary_booking__isnull=True).count() == 2
+    assert Booking.objects.count() == 5
+    assert primary.secondary_booking_set.count() == 3
 
-    # decrease a booking in waiting list
-    primary.suspend()
-    assert Booking.objects.filter(in_waiting_list=True).count() == 5
-    assert Booking.objects.filter().count() == 5
+    # booking is overbooked - it is ok to increase
+    event.places = 4
+    event.save()
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 5})
+    assert resp.json['err'] == 0
+    assert Booking.objects.filter(in_waiting_list=False).count() == 6
+    assert Booking.objects.filter(primary_booking__isnull=True).count() == 2
+    assert Booking.objects.count() == 6
+    assert primary.secondary_booking_set.count() == 4
+
+    # decrease
+    # booking is overbooked, but it is always ok to do nothing
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 5})
+    assert resp.json['err'] == 0
+    assert Booking.objects.filter(in_waiting_list=False).count() == 6
+    assert Booking.objects.filter(primary_booking__isnull=True).count() == 2
+    assert Booking.objects.count() == 6
+    assert primary.secondary_booking_set.count() == 4
+    # or to decrease
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 4})
+    assert resp.json['err'] == 0
+    assert Booking.objects.filter(in_waiting_list=False).count() == 5
+    assert Booking.objects.filter(primary_booking__isnull=True).count() == 2
+    assert Booking.objects.count() == 5
+    assert primary.secondary_booking_set.count() == 3
+    # again
     resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 1})
     assert resp.json['err'] == 0
-    assert Booking.objects.filter(in_waiting_list=True).count() == 1
-    assert Booking.objects.filter(primary_booking__isnull=True).count() == 1
-    assert Booking.objects.filter().count() == 1
+    assert Booking.objects.filter(in_waiting_list=False).count() == 2
+    assert Booking.objects.filter(primary_booking__isnull=True).count() == 2
+    assert Booking.objects.count() == 2
+    assert primary.secondary_booking_set.count() == 0
 
     # no changes
     resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 1})
     assert resp.json['err'] == 0
-    assert Booking.objects.filter(in_waiting_list=True).count() == 1
-    assert Booking.objects.filter().count() == 1
+    assert Booking.objects.filter(in_waiting_list=False).count() == 2
+    assert Booking.objects.filter(primary_booking__isnull=True).count() == 2
+    assert Booking.objects.count() == 2
+    assert primary.secondary_booking_set.count() == 0
+
+
+def test_resize_booking_in_waiting_list(app, user):
+    agenda = Agenda.objects.create(label='Foo bar', kind='events')
+    event = Event.objects.create(
+        slug='event', start_datetime=now(), places=5, waiting_list_places=5, agenda=agenda,
+    )
+    primary = Booking.objects.create(event=event, in_waiting_list=True)
+    # there is other bookings
+    Booking.objects.create(event=event, in_waiting_list=True)
+    app.authorization = ('Basic', ('john.doe', 'password'))
 
-    # increase a booking in waiting list
+    # increase
     resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 2})
     assert resp.json['err'] == 0
-    assert Booking.objects.filter(in_waiting_list=True).count() == 2
-    assert Booking.objects.filter(primary_booking__isnull=True).count() == 1
-    assert Booking.objects.filter().count() == 2
-    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 6})
+    assert Booking.objects.filter(in_waiting_list=True).count() == 3
+    assert Booking.objects.filter(primary_booking__isnull=True).count() == 2
+    assert Booking.objects.count() == 3
+    assert primary.secondary_booking_set.count() == 1
+    # too much
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 5})
     assert resp.json['err'] == 3
-    assert resp.json['reason'] == 'sold out'  # legacy
-    assert resp.json['err_class'] == 'sold out'
     assert resp.json['err_desc'] == 'sold out'
-    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 5})
+    # max
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 4})
     assert resp.json['err'] == 0
     assert Booking.objects.filter(in_waiting_list=True).count() == 5
-    assert Booking.objects.filter(primary_booking__isnull=True).count() == 1
-    assert Booking.objects.filter().count() == 5
-    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 6})
+    assert Booking.objects.filter(primary_booking__isnull=True).count() == 2
+    assert Booking.objects.count() == 5
+    assert primary.secondary_booking_set.count() == 3
+
+    # booking is overbooked (on waiting list, that shoud not happen)
+    event.waiting_list_places = 4
+    event.save()
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 5})
     assert resp.json['err'] == 3
-    assert resp.json['reason'] == 'sold out'  # legacy
-    assert resp.json['err_class'] == 'sold out'
     assert resp.json['err_desc'] == 'sold out'
 
-    # resize a booking that is on multi events
-    secondary = Booking.objects.create(event=event2, in_waiting_list=False, primary_booking=primary)
-    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 42})
-    assert resp.json['err'] == 4
-    assert resp.json['reason'] == 'can not resize multi event booking'  # legacy
-    assert resp.json['err_class'] == 'can not resize multi event booking'
-    assert resp.json['err_desc'] == 'can not resize multi event booking'
-    # resize a booking that was cancelled before
-    primary.cancel()
-    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 42})
-    assert resp.json['err'] == 1
+    # decrease
+    # booking is overbooked, but it is always ok to do nothing
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 4})
+    assert resp.json['err'] == 0
+    assert Booking.objects.filter(in_waiting_list=True).count() == 5
+    assert Booking.objects.filter(primary_booking__isnull=True).count() == 2
+    assert Booking.objects.count() == 5
+    assert primary.secondary_booking_set.count() == 3
+    # or to decrease
+    event.waiting_list_places = 3
+    event.save()
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 3})
+    assert resp.json['err'] == 0
+    assert Booking.objects.filter(in_waiting_list=True).count() == 4
+    assert Booking.objects.filter(primary_booking__isnull=True).count() == 2
+    assert Booking.objects.count() == 4
+    assert primary.secondary_booking_set.count() == 2
+    # again
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 1})
+    assert resp.json['err'] == 0
+    assert Booking.objects.filter(in_waiting_list=True).count() == 2
+    assert Booking.objects.filter(primary_booking__isnull=True).count() == 2
+    assert Booking.objects.count() == 2
+    assert primary.secondary_booking_set.count() == 0
+
+    # no changes
+    resp = app.post('/api/booking/%s/resize/' % primary.pk, params={'count': 1})
+    assert resp.json['err'] == 0
+    assert Booking.objects.filter(in_waiting_list=True).count() == 2
+    assert Booking.objects.filter(primary_booking__isnull=True).count() == 2
+    assert Booking.objects.count() == 2
+    assert primary.secondary_booking_set.count() == 0
 
 
 def test_multiple_booking_api(app, some_data, user):
-- 
2.20.1

