From e196a04caff9475f05e3ca8b6c818758ce35e4b0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Fri, 16 Apr 2021 10:11:58 +0200
Subject: [PATCH 1/2] api: booking, user_name and first/last_name (#53172)

---
 .../migrations/0086_booking_user_name.py      | 21 +++++++
 chrono/agendas/models.py                      | 10 ++-
 chrono/api/views.py                           |  7 ++-
 chrono/manager/views.py                       |  4 +-
 tests/manager/test_all.py                     |  6 +-
 tests/manager/test_event.py                   | 30 ++++-----
 tests/test_agendas.py                         |  6 +-
 tests/test_api.py                             | 61 +++++++++++++------
 8 files changed, 103 insertions(+), 42 deletions(-)
 create mode 100644 chrono/agendas/migrations/0086_booking_user_name.py

diff --git a/chrono/agendas/migrations/0086_booking_user_name.py b/chrono/agendas/migrations/0086_booking_user_name.py
new file mode 100644
index 0000000..fb24fac
--- /dev/null
+++ b/chrono/agendas/migrations/0086_booking_user_name.py
@@ -0,0 +1,21 @@
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('agendas', '0085_reason_slug'),
+    ]
+
+    operations = [
+        migrations.RenameField(
+            model_name='booking',
+            old_name='user_name',
+            new_name='user_last_name',
+        ),
+        migrations.AddField(
+            model_name='booking',
+            name='user_first_name',
+            field=models.CharField(blank=True, max_length=250),
+        ),
+    ]
diff --git a/chrono/agendas/models.py b/chrono/agendas/models.py
index 934f0a0..b4cfef7 100644
--- a/chrono/agendas/models.py
+++ b/chrono/agendas/models.py
@@ -1596,7 +1596,8 @@ class Booking(models.Model):
         verbose_name=_('Label displayed to user'), max_length=250, blank=True
     )
     user_external_id = models.CharField(max_length=250, blank=True)
-    user_name = models.CharField(max_length=250, blank=True)
+    user_last_name = models.CharField(max_length=250, blank=True)
+    user_first_name = models.CharField(max_length=250, blank=True)
     user_email = models.EmailField(blank=True)
     user_phone_number = models.CharField(max_length=16, blank=True)
     user_was_present = models.NullBooleanField()
@@ -1607,6 +1608,10 @@ class Booking(models.Model):
     cancel_callback_url = models.URLField(blank=True)
     color = models.ForeignKey(BookingColor, null=True, on_delete=models.SET_NULL, related_name='bookings')
 
+    @property
+    def user_name(self):
+        return ('%s %s' % (self.user_first_name, self.user_last_name)).strip()
+
     def save(self, *args, **kwargs):
         with transaction.atomic():
             super(Booking, self).save(*args, **kwargs)
@@ -1643,7 +1648,8 @@ class Booking(models.Model):
             label='',
             user_display_label='',
             user_external_id='',
-            user_name='',
+            user_last_name='',
+            user_first_name='',
             extra_data={},
             anonymization_datetime=now(),
         )
diff --git a/chrono/api/views.py b/chrono/api/views.py
index de00fd1..45be12a 100644
--- a/chrono/api/views.py
+++ b/chrono/api/views.py
@@ -922,7 +922,9 @@ class SlotSerializer(serializers.Serializer):
 
     label = serializers.CharField(max_length=250, allow_blank=True)
     user_external_id = serializers.CharField(max_length=250, allow_blank=True)
-    user_name = serializers.CharField(max_length=250, allow_blank=True)
+    user_name = serializers.CharField(max_length=250, allow_blank=True)  # compatibility
+    user_first_name = serializers.CharField(max_length=250, allow_blank=True)
+    user_last_name = serializers.CharField(max_length=250, allow_blank=True)
     user_display_label = serializers.CharField(max_length=250, allow_blank=True)
     user_email = serializers.CharField(max_length=250, allow_blank=True)
     user_phone_number = serializers.CharField(max_length=16, allow_blank=True)
@@ -1267,7 +1269,8 @@ class Fillslots(APIView):
                         in_waiting_list=in_waiting_list,
                         label=payload.get('label', ''),
                         user_external_id=payload.get('user_external_id', ''),
-                        user_name=payload.get('user_name', ''),
+                        user_first_name=payload.get('user_first_name', ''),
+                        user_last_name=payload.get('user_last_name') or payload.get('user_name') or '',
                         user_email=payload.get('user_email', ''),
                         user_phone_number=payload.get('user_phone_number', ''),
                         form_url=payload.get('form_url', ''),
diff --git a/chrono/manager/views.py b/chrono/manager/views.py
index c9f0041..ab99a64 100644
--- a/chrono/manager/views.py
+++ b/chrono/manager/views.py
@@ -1960,13 +1960,13 @@ class EventCheckView(ViewableAgendaMixin, DetailView):
         event = self.object
         context['booked'] = event.booking_set.filter(
             cancellation_datetime__isnull=True, in_waiting_list=False
-        ).order_by('user_name')
+        ).order_by('user_last_name', 'user_first_name')
         context['booked_without_status'] = any(e.user_was_present is None for e in context['booked'])
         if context['booked_without_status']:
             context['absence_form'] = BookingAbsenceReasonForm(agenda=self.agenda)
         context['waiting'] = event.booking_set.filter(
             cancellation_datetime__isnull=True, in_waiting_list=True
-        ).order_by('user_name')
+        ).order_by('user_last_name', 'user_first_name')
         for booking in context['booked']:
             booking.form = BookingAbsenceReasonForm(
                 agenda=self.agenda, initial={'reason': booking.user_absence_reason}
diff --git a/tests/manager/test_all.py b/tests/manager/test_all.py
index ace00ca..9baddb9 100644
--- a/tests/manager/test_all.py
+++ b/tests/manager/test_all.py
@@ -3233,10 +3233,12 @@ def test_agenda_view_event(app, manager_user):
             booking.creation_datetime = make_aware(datetime.datetime(2019, 12, 21, 14, 0 + i))
         if i == 5:
             booking.creation_datetime = make_aware(datetime.datetime(2019, 12, 21, 15, 0))
-            booking.user_name = 'Foo Bar User'
+            booking.user_first_name = 'Foo Bar'
+            booking.user_last_name = 'User'
         if i == 6:
             booking.creation_datetime = make_aware(datetime.datetime(2019, 12, 21, 16, 0))
-            booking.user_name = 'Foo Bar User 2'
+            booking.user_first_name = 'Foo Bar'
+            booking.user_last_name = 'User 2'
             booking.label = 'Foo Bar Label 2'
         if i == 7:
             booking.creation_datetime = make_aware(datetime.datetime(2019, 12, 21, 17, 0))
diff --git a/tests/manager/test_event.py b/tests/manager/test_event.py
index 5dc79b7..5965bc4 100644
--- a/tests/manager/test_event.py
+++ b/tests/manager/test_event.py
@@ -1068,14 +1068,16 @@ def test_event_check(app, admin_user):
         waiting_list_places=5,
         agenda=agenda,
     )
-    booking1 = Booking.objects.create(event=event, user_name='User 42')
-    Booking.objects.create(event=event, user_name='User 1')
-    Booking.objects.create(event=event, user_name='User 17')
-    Booking.objects.create(event=event, user_name='User 35')
-    Booking.objects.create(event=event, user_name='User 5')
-    booking6 = Booking.objects.create(event=event, user_name='User Cancelled')
+    booking1 = Booking.objects.create(event=event, user_first_name='User', user_last_name='42')
+    Booking.objects.create(event=event, user_first_name='User', user_last_name='01')
+    Booking.objects.create(event=event, user_first_name='User', user_last_name='17')
+    Booking.objects.create(event=event, user_first_name='User', user_last_name='35')
+    Booking.objects.create(event=event, user_first_name='User', user_last_name='05')
+    booking6 = Booking.objects.create(event=event, user_first_name='User', user_last_name='Cancelled')
     booking6.cancel()
-    booking7 = Booking.objects.create(event=event, user_name='User Waiting', in_waiting_list=True)
+    booking7 = Booking.objects.create(
+        event=event, user_first_name='User', user_last_name='Waiting', in_waiting_list=True
+    )
     login(app)
 
     # event not in past
@@ -1098,11 +1100,11 @@ def test_event_check(app, admin_user):
     assert 'Waiting List (1/5)' in resp
     assert (
         resp.text.index('Bookings (5/10)')
-        < resp.text.index('User 1')
+        < resp.text.index('User 01')
+        < resp.text.index('User 05')
         < resp.text.index('User 17')
         < resp.text.index('User 35')
         < resp.text.index('User 42')
-        < resp.text.index('User 5')
         < resp.text.index('Waiting List (1/5)')
         < resp.text.index('User Waiting')
     )  # user ordering is not optimal ...
@@ -1174,7 +1176,7 @@ def test_event_check_booking(app, admin_user):
         waiting_list_places=5,
         agenda=agenda,
     )
-    booking = Booking.objects.create(event=event, user_name='User 42')
+    booking = Booking.objects.create(event=event, user_first_name='User', user_last_name='42')
 
     login(app)
     resp = app.get('/manage/agendas/%s/events/%s/check' % (agenda.pk, event.pk))
@@ -1248,7 +1250,7 @@ def test_event_check_booking_ajax(app, admin_user):
         waiting_list_places=5,
         agenda=agenda,
     )
-    booking = Booking.objects.create(event=event, user_name='User 42')
+    booking = Booking.objects.create(event=event, user_first_name='User', user_last_name='42')
 
     login(app)
     resp = app.get('/manage/agendas/%s/events/%s/check' % (agenda.pk, event.pk))
@@ -1288,7 +1290,7 @@ def test_event_check_all_bookings(app, admin_user):
         waiting_list_places=5,
         agenda=agenda,
     )
-    booking1 = Booking.objects.create(event=event, user_name='User 42')
+    booking1 = Booking.objects.create(event=event, user_first_name='User', user_last_name='42')
 
     login(app)
     resp = app.get('/manage/agendas/%s/events/%s/check' % (agenda.pk, event.pk))
@@ -1309,7 +1311,7 @@ def test_event_check_all_bookings(app, admin_user):
     assert '/manage/agendas/%s/events/%s/presence' % (agenda.pk, event.pk) not in resp
     assert '/manage/agendas/%s/events/%s/absence' % (agenda.pk, event.pk) not in resp
 
-    booking2 = Booking.objects.create(event=event, user_name='User 35')
+    booking2 = Booking.objects.create(event=event, user_first_name='User', user_last_name='35')
     resp = app.get('/manage/agendas/%s/events/%s/check' % (agenda.pk, event.pk))
     assert 'Mark all bookings without status' in resp
     assert '/manage/agendas/%s/events/%s/presence' % (agenda.pk, event.pk) in resp
@@ -1325,7 +1327,7 @@ def test_event_check_all_bookings(app, admin_user):
     assert booking2.user_was_present is True
     assert booking2.user_absence_reason == ''
 
-    booking3 = Booking.objects.create(event=event, user_name='User 51')
+    booking3 = Booking.objects.create(event=event, user_first_name='User', user_last_name='51')
     app.post(
         '/manage/agendas/%s/events/%s/absence' % (agenda.pk, event.pk),
         params={'csrfmiddlewaretoken': token, 'reason': 'Foo reason'},
diff --git a/tests/test_agendas.py b/tests/test_agendas.py
index 88caf3c..0f4f303 100644
--- a/tests/test_agendas.py
+++ b/tests/test_agendas.py
@@ -1831,7 +1831,8 @@ def test_anonymize_bookings(freezer):
             label='john',
             user_display_label='john',
             user_external_id='john',
-            user_name='john',
+            user_first_name='john',
+            user_last_name='doe',
             backoffice_url='https://example.org',
         )
 
@@ -1858,7 +1859,8 @@ def test_anonymize_bookings(freezer):
             label='',
             user_display_label='',
             user_external_id='',
-            user_name='',
+            user_first_name='',
+            user_last_name='',
             backoffice_url='https://example.org',
             extra_data={},
             anonymization_datetime=now(),
diff --git a/tests/test_api.py b/tests/test_api.py
index 295bf2d..cd746f1 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -1259,7 +1259,31 @@ def test_booking_api(app, some_data, user):
     )
     booking = Booking.objects.get(id=resp.json['booking_id'])
     assert booking.label == 'foo'
-    assert booking.user_name == 'bar'
+    assert booking.user_first_name == ''
+    assert booking.user_last_name == 'bar'
+    assert booking.backoffice_url == 'http://example.net/'
+    assert booking.cancel_callback_url == 'http://example.net/jump/trigger/'
+    assert booking.user_email == 'bar@bar.com'
+    assert booking.user_phone_number == '+33123456789'
+    assert booking.form_url == 'http://example.net/'
+
+    resp = app.post_json(
+        '/api/agenda/%s/fillslot/%s/' % (agenda.id, event.id),
+        params={
+            'label': 'foo',
+            'user_first_name': 'foo',
+            'user_last_name': 'bar',
+            'backoffice_url': 'http://example.net/',
+            'cancel_callback_url': 'http://example.net/jump/trigger/',
+            'user_email': 'bar@bar.com',
+            'user_phone_number': '+33123456789',
+            'form_url': 'http://example.net/',
+        },
+    )
+    booking = Booking.objects.get(id=resp.json['booking_id'])
+    assert booking.label == 'foo'
+    assert booking.user_first_name == 'foo'
+    assert booking.user_last_name == 'bar'
     assert booking.backoffice_url == 'http://example.net/'
     assert booking.cancel_callback_url == 'http://example.net/jump/trigger/'
     assert booking.user_email == 'bar@bar.com'
@@ -1269,19 +1293,20 @@ def test_booking_api(app, some_data, user):
     # blank data are OK
     resp = app.post_json(
         '/api/agenda/%s/fillslot/%s/' % (agenda.id, event.id),
-        params={'label': '', 'user_name': '', 'backoffice_url': ''},
+        params={'label': '', 'user_first_name': '', 'user_last_name': '', 'backoffice_url': ''},
     )
     assert Booking.objects.get(id=resp.json['booking_id']).label == ''
-    assert Booking.objects.get(id=resp.json['booking_id']).user_name == ''
+    assert Booking.objects.get(id=resp.json['booking_id']).user_first_name == ''
+    assert Booking.objects.get(id=resp.json['booking_id']).user_last_name == ''
     assert Booking.objects.get(id=resp.json['booking_id']).backoffice_url == ''
 
     # extra data stored in extra_data field
     resp = app.post_json(
         '/api/agenda/%s/fillslot/%s/' % (agenda.id, event.id),
-        params={'label': 'l', 'user_name': 'u', 'backoffice_url': '', 'foo': 'bar'},
+        params={'label': 'l', 'user_last_name': 'u', 'backoffice_url': '', 'foo': 'bar'},
     )
     assert Booking.objects.get(id=resp.json['booking_id']).label == 'l'
-    assert Booking.objects.get(id=resp.json['booking_id']).user_name == 'u'
+    assert Booking.objects.get(id=resp.json['booking_id']).user_last_name == 'u'
     assert Booking.objects.get(id=resp.json['booking_id']).backoffice_url == ''
     assert Booking.objects.get(id=resp.json['booking_id']).extra_data == {'foo': 'bar'}
 
@@ -1293,7 +1318,7 @@ def test_booking_api(app, some_data, user):
     # test invalid data are refused
     resp = app.post_json(
         '/api/agenda/%s/fillslot/%s/' % (agenda.id, event.id),
-        params={'user_name': {'foo': 'bar'}},
+        params={'user_last_name': {'foo': 'bar'}},
         status=400,
     )
     assert resp.json['err'] == 1
@@ -1301,7 +1326,7 @@ def test_booking_api(app, some_data, user):
     assert resp.json['err_class'] == 'invalid payload'
     assert resp.json['err_desc'] == 'invalid payload'
     assert len(resp.json['errors']) == 1
-    assert 'user_name' in resp.json['errors']
+    assert 'user_last_name' in resp.json['errors']
 
     resp = app.post('/api/agenda/foobar/fillslot/%s/' % event.id, status=404)
 
@@ -1447,7 +1472,8 @@ def test_booking_ics(app, some_data, meetings_agenda, user):
         '/api/agenda/%s/fillslot/%s/' % (agenda.id, event.id),
         params={
             'label': 'foo',
-            'user_name': 'bar',
+            'user_first_name': 'foo',
+            'user_last_name': 'bar',
             'backoffice_url': 'http://example.net/',
             'url': 'http://example.com/booking',
             'user_display_label': 'your booking',
@@ -1456,7 +1482,7 @@ def test_booking_ics(app, some_data, meetings_agenda, user):
     assert Booking.objects.count() == 3
     booking_ics = Booking.objects.get(id=resp.json['booking_id']).get_ics()
     assert 'SUMMARY:your booking\r\n' in booking_ics
-    assert 'ATTENDEE:bar\r\n' in booking_ics
+    assert 'ATTENDEE:foo bar\r\n' in booking_ics
     assert 'URL:http://example.com/booking\r\n' in booking_ics
 
     # extra data stored in extra_data field
@@ -1464,7 +1490,6 @@ def test_booking_ics(app, some_data, meetings_agenda, user):
         '/api/agenda/%s/fillslot/%s/' % (agenda.id, event.id),
         params={
             'label': 'l',
-            'user_name': 'u',
             'backoffice_url': '',
             'location': 'bar',
             'comment': 'booking comment',
@@ -1570,7 +1595,7 @@ def test_booking_api_fillslots(app, some_data, user):
             'slots': events_ids,
             'label': 'foo',
             'user_external_id': 'some_external_id',
-            'user_name': 'bar',
+            'user_last_name': 'bar',
             'user_display_label': 'foo',
             'backoffice_url': 'http://example.net/',
         },
@@ -1579,7 +1604,7 @@ def test_booking_api_fillslots(app, some_data, user):
     booking = Booking.objects.get(pk=booking_id)
     assert booking.label == 'foo'
     assert booking.user_external_id == 'some_external_id'
-    assert booking.user_name == 'bar'
+    assert booking.user_last_name == 'bar'
     assert booking.user_display_label == 'foo'
     assert booking.backoffice_url == 'http://example.net/'
     assert Booking.objects.filter(primary_booking=booking_id, label='foo').count() == 2
@@ -1595,10 +1620,10 @@ def test_booking_api_fillslots(app, some_data, user):
     # extra data stored in extra_data field
     resp = app.post_json(
         '/api/agenda/%s/fillslots/' % agenda.id,
-        params={'slots': events_ids, 'label': 'l', 'user_name': 'u', 'backoffice_url': '', 'foo': 'bar'},
+        params={'slots': events_ids, 'label': 'l', 'user_last_name': 'u', 'backoffice_url': '', 'foo': 'bar'},
     )
     assert Booking.objects.get(id=resp.json['booking_id']).label == 'l'
-    assert Booking.objects.get(id=resp.json['booking_id']).user_name == 'u'
+    assert Booking.objects.get(id=resp.json['booking_id']).user_last_name == 'u'
     assert Booking.objects.get(id=resp.json['booking_id']).backoffice_url == ''
     assert Booking.objects.get(id=resp.json['booking_id']).extra_data == {'foo': 'bar'}
     for booking in Booking.objects.filter(primary_booking=resp.json['booking_id']):
@@ -1607,7 +1632,7 @@ def test_booking_api_fillslots(app, some_data, user):
     # test invalid data are refused
     resp = app.post_json(
         '/api/agenda/%s/fillslots/' % agenda.id,
-        params={'slots': events_ids, 'user_name': {'foo': 'bar'}},
+        params={'slots': events_ids, 'user_last_name': {'foo': 'bar'}},
         status=400,
     )
     assert resp.json['err'] == 1
@@ -1615,7 +1640,7 @@ def test_booking_api_fillslots(app, some_data, user):
     assert resp.json['err_class'] == 'invalid payload'
     assert resp.json['err_desc'] == 'invalid payload'
     assert len(resp.json['errors']) == 1
-    assert 'user_name' in resp.json['errors']
+    assert 'user_last_name' in resp.json['errors']
 
     # empty or missing slots
     resp = app.post_json('/api/agenda/%s/fillslots/' % agenda.id, params={'slots': []}, status=400)
@@ -1644,13 +1669,13 @@ def test_booking_api_fillslots(app, some_data, user):
         in_bookable_period.return_value = True
         resp = app.post_json(
             '/api/agenda/%s/fillslots/' % agenda.id,
-            params={'slots': events_ids, 'label': 'l', 'user_name': 'u', 'backoffice_url': '', 'foo': 'bar'},
+            params={'slots': events_ids},
         )
         assert resp.json['err'] == 0
         in_bookable_period.return_value = False
         resp = app.post_json(
             '/api/agenda/%s/fillslots/' % agenda.id,
-            params={'slots': events_ids, 'label': 'l', 'user_name': 'u', 'backoffice_url': '', 'foo': 'bar'},
+            params={'slots': events_ids},
         )
         assert resp.json['err'] == 1
         assert resp.json['reason'] == 'event not bookable'  # legacy
-- 
2.30.2

