From 885b29cb4b630768615b5cf169842c40d3b8f79a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Tue, 23 Jun 2020 15:15:26 +0200
Subject: [PATCH] agenda: generate event slug if not provided (#44375)

---
 chrono/agendas/migrations/0049_event_slug.py  | 38 +++++++++++++++++++
 chrono/agendas/migrations/0050_event_slug.py  | 26 +++++++++++++
 chrono/agendas/models.py                      | 29 ++++++++++++--
 chrono/api/views.py                           | 17 +++++----
 .../manager_events_agenda_settings.html       |  2 +-
 tests/test_agendas.py                         | 29 ++++++++++++++
 tests/test_api.py                             |  7 +---
 tests/test_manager.py                         |  1 -
 8 files changed, 130 insertions(+), 19 deletions(-)
 create mode 100644 chrono/agendas/migrations/0049_event_slug.py
 create mode 100644 chrono/agendas/migrations/0050_event_slug.py

diff --git a/chrono/agendas/migrations/0049_event_slug.py b/chrono/agendas/migrations/0049_event_slug.py
new file mode 100644
index 0000000..39e2d90
--- /dev/null
+++ b/chrono/agendas/migrations/0049_event_slug.py
@@ -0,0 +1,38 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations
+from django.utils.text import slugify
+
+
+def generate_slug(instance, **query_filters):
+    base_slug = slugify(instance.label or ('%s-event' % instance.agenda.label))
+    slug = base_slug
+    i = 1
+    while True:
+        queryset = instance._meta.model.objects.filter(slug=slug, **query_filters).exclude(pk=instance.pk)
+        if not queryset.exists():
+            break
+        slug = '%s-%s' % (base_slug, i)
+        i += 1
+    return slug
+
+
+def set_slug_on_events(apps, schema_editor):
+    Event = apps.get_model('agendas', 'Event')
+    for event in Event.objects.all().order_by('-pk'):
+        if event.slug and not Event.objects.filter(slug=event.slug).exclude(pk=event.pk).exists():
+            continue
+        event.slug = generate_slug(event)
+        event.save(update_fields=['slug'])
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('agendas', '0048_meeting_type_deleted_flag'),
+    ]
+
+    operations = [
+        migrations.RunPython(set_slug_on_events, lambda x, y: None),
+    ]
diff --git a/chrono/agendas/migrations/0050_event_slug.py b/chrono/agendas/migrations/0050_event_slug.py
new file mode 100644
index 0000000..d8153c2
--- /dev/null
+++ b/chrono/agendas/migrations/0050_event_slug.py
@@ -0,0 +1,26 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+import chrono.agendas.models
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('agendas', '0049_event_slug'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='event',
+            name='slug',
+            field=models.SlugField(
+                blank=True,
+                max_length=160,
+                validators=[chrono.agendas.models.validate_not_digit],
+                verbose_name='Identifier',
+            ),
+            preserve_default=False,
+        ),
+    ]
diff --git a/chrono/agendas/models.py b/chrono/agendas/models.py
index 6beca09..803a666 100644
--- a/chrono/agendas/models.py
+++ b/chrono/agendas/models.py
@@ -60,7 +60,7 @@ def is_midnight(dtime):
 
 
 def generate_slug(instance, **query_filters):
-    base_slug = slugify(instance.label)
+    base_slug = instance.base_slug
     slug = base_slug
     i = 1
     while instance._meta.model.objects.filter(slug=slug, **query_filters).exists():
@@ -154,6 +154,10 @@ class Agenda(models.Model):
                 self.maximal_booking_delay = 8 * 7
         super(Agenda, self).save(*args, **kwargs)
 
+    @property
+    def base_slug(self):
+        return slugify(self.label)
+
     def get_absolute_url(self):
         return reverse('chrono-manager-agenda-view', kwargs={'pk': self.id})
 
@@ -691,6 +695,10 @@ class MeetingType(models.Model):
             self.slug = generate_slug(self, agenda=self.agenda)
         super(MeetingType, self).save(*args, **kwargs)
 
+    @property
+    def base_slug(self):
+        return slugify(self.label)
+
     @classmethod
     def import_json(cls, data):
         data = clean_import_data(cls, data)
@@ -735,9 +743,7 @@ class Event(models.Model):
         blank=True,
         help_text=_('Optional label to identify this date.'),
     )
-    slug = models.SlugField(
-        _('Identifier'), max_length=160, null=True, blank=True, default=None, validators=[validate_not_digit]
-    )
+    slug = models.SlugField(_('Identifier'), max_length=160, blank=True, validators=[validate_not_digit])
     description = models.TextField(
         _('Description'), null=True, blank=True, help_text=_('Optional event description.')
     )
@@ -761,8 +767,15 @@ class Event(models.Model):
         assert self.agenda.kind != 'virtual', "an event can't reference a virtual agenda"
         assert not (self.slug and self.slug.isdigit()), 'slug cannot be a number'
         self.check_full()
+        if not self.slug:
+            self.slug = generate_slug(self, agenda=self.agenda)
         return super(Event, self).save(*args, **kwargs)
 
+    @property
+    def base_slug(self):
+        # label can be empty
+        return slugify(self.label or ('%s-event' % self.agenda.label))
+
     def check_full(self):
         self.full = bool(
             (self.booked_places >= self.places and self.waiting_list_places == 0)
@@ -982,6 +995,10 @@ class Desk(models.Model):
             self.slug = generate_slug(self, agenda=self.agenda)
         super(Desk, self).save(*args, **kwargs)
 
+    @property
+    def base_slug(self):
+        return slugify(self.label)
+
     @classmethod
     def import_json(cls, data):
         timeperiods = data.pop('timeperiods', [])
@@ -1214,6 +1231,10 @@ class Resource(models.Model):
             self.slug = generate_slug(self)
         super().save(*args, **kwargs)
 
+    @property
+    def base_slug(self):
+        return slugify(self.label)
+
 
 def ics_directory_path(instance, filename):
     return 'ics/{0}/{1}'.format(str(uuid.uuid4()), filename)
diff --git a/chrono/api/views.py b/chrono/api/views.py
index 55e9013..33352cd 100644
--- a/chrono/api/views.py
+++ b/chrono/api/views.py
@@ -17,10 +17,10 @@
 import collections
 import datetime
 import itertools
+import uuid
 
 
 from django.db import transaction
-from django.db.models import Q
 from django.http import Http404, HttpResponse
 from django.shortcuts import get_object_or_404
 from django.urls import reverse
@@ -424,13 +424,13 @@ class Datetimes(APIView):
                         'fillslot_url': request.build_absolute_uri(
                             reverse(
                                 'api-fillslot',
-                                kwargs={'agenda_identifier': agenda.slug, 'event_identifier': x.id,},
+                                kwargs={'agenda_identifier': agenda.slug, 'event_identifier': x.slug},
                             )
                         ),
                         'status_url': request.build_absolute_uri(
                             reverse(
                                 'api-event-status',
-                                kwargs={'agenda_identifier': agenda.slug, 'event_identifier': x.id,},
+                                kwargs={'agenda_identifier': agenda.slug, 'event_identifier': x.slug},
                             )
                         ),
                     },
@@ -497,7 +497,7 @@ class MeetingDatetimes(APIView):
         fillslot_url = request.build_absolute_uri(
             reverse(
                 'api-fillslot',
-                kwargs={'agenda_identifier': agenda.slug, 'event_identifier': fake_event_identifier,},
+                kwargs={'agenda_identifier': agenda.slug, 'event_identifier': fake_event_identifier},
             )
         )
         if resources:
@@ -520,7 +520,7 @@ class MeetingDatetimes(APIView):
                     'datetime': format_response_datetime(slot.start_datetime),
                     'text': date_format(slot.start_datetime, format='DATETIME_FORMAT'),
                     'disabled': bool(slot.full),
-                    'api': {'fillslot_url': fillslot_url.replace(fake_event_identifier, slot_id),},
+                    'api': {'fillslot_url': fillslot_url.replace(fake_event_identifier, slot_id)},
                 }
                 for slot in generator_of_unique_slots
                 # we do not have the := operator, so we do that
@@ -723,7 +723,7 @@ class Fillslots(APIView):
                 cancel_error = gettext_noop('cancel booking: booking does no exist')
 
             if cancel_error:
-                return Response({'err': 1, 'err_class': cancel_error, 'err_desc': _(cancel_error),})
+                return Response({'err': 1, 'err_class': cancel_error, 'err_desc': _(cancel_error)})
 
         extra_data = {}
         for k, v in request.data.items():
@@ -860,6 +860,7 @@ class Fillslots(APIView):
             for start_datetime in datetimes:
                 event = Event.objects.create(
                     agenda=available_desk.agenda,
+                    slug=str(uuid.uuid4()),  # set slug to avoid queries during slug generation
                     meeting_type_id=meeting_type_id,
                     start_datetime=start_datetime,
                     full=False,
@@ -871,9 +872,9 @@ class Fillslots(APIView):
                 events.append(event)
         else:
             try:
-                events = Event.objects.filter(id__in=[int(s) for s in slots]).order_by('start_datetime')
+                events = agenda.event_set.filter(id__in=[int(s) for s in slots]).order_by('start_datetime')
             except ValueError:
-                events = Event.objects.filter(slug__in=slots).order_by('start_datetime')
+                events = agenda.event_set.filter(slug__in=slots).order_by('start_datetime')
 
             for event in events:
                 if not event.in_bookable_period():
diff --git a/chrono/manager/templates/chrono/manager_events_agenda_settings.html b/chrono/manager/templates/chrono/manager_events_agenda_settings.html
index dd7c90f..505db6f 100644
--- a/chrono/manager/templates/chrono/manager_events_agenda_settings.html
+++ b/chrono/manager/templates/chrono/manager_events_agenda_settings.html
@@ -24,7 +24,7 @@
           data-total="{{event.waiting_list_places}}" data-booked="{{event.waiting_list_count}}"
         {% endif %}
         ><a rel="popup" href="{% url 'chrono-manager-event-edit' pk=agenda.id event_pk=event.id %}?next=settings">
-        {% if event.label %}{{event.label}} / {% endif %}
+        {% if event.label %}{{event.label}} / {% endif %}[{% trans "identifier:" %} {{ event.slug }}]
         {{ event.start_datetime }}
         {% if event.full %}/ <span class="full">{% trans "full" %}</span>{% endif %}
         (
diff --git a/tests/test_agendas.py b/tests/test_agendas.py
index ff19bf5..d7d020f 100644
--- a/tests/test_agendas.py
+++ b/tests/test_agendas.py
@@ -149,6 +149,35 @@ def test_resource_duplicate_slugs():
     assert resource.slug == 'foo-baz-2'
 
 
+def test_event_slug():
+    other_agenda = Agenda.objects.create(label='Foo bar')
+    Event.objects.create(agenda=other_agenda, places=42, start_datetime=now(), slug='foo-bar')
+
+    agenda = Agenda.objects.create(label='Foo baz')
+    event = Event.objects.create(agenda=agenda, places=42, start_datetime=now(), label='Foo bar')
+    assert event.slug == 'foo-bar'
+    event = Event.objects.create(agenda=agenda, places=42, start_datetime=now(), label='Foo bar')
+    assert event.slug == 'foo-bar-1'
+    event = Event.objects.create(agenda=agenda, places=42, start_datetime=now(), label='Foo bar')
+    assert event.slug == 'foo-bar-2'
+
+    event = Event.objects.create(agenda=agenda, places=42, start_datetime=now())
+    assert event.slug == 'foo-baz-event'
+    event = Event.objects.create(agenda=agenda, places=42, start_datetime=now())
+    assert event.slug == 'foo-baz-event-1'
+    event = Event.objects.create(agenda=agenda, places=42, start_datetime=now())
+    assert event.slug == 'foo-baz-event-2'
+
+
+def test_event_existing_slug():
+    other_agenda = Agenda.objects.create(label='Foo bar')
+    Event.objects.create(agenda=other_agenda, places=42, start_datetime=now(), slug='bar')
+
+    agenda = Agenda.objects.create(label='Foo baz')
+    event = Event.objects.create(agenda=agenda, places=42, start_datetime=now(), label='Foo bar', slug='bar')
+    assert event.slug == 'bar'
+
+
 def test_event_manager():
     agenda = Agenda(label=u'Foo baz')
     agenda.save()
diff --git a/tests/test_api.py b/tests/test_api.py
index cec9f09..6ca681f 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -360,7 +360,7 @@ def test_datetime_api_status_url(app, some_data):
     for datum in resp.json['data']:
         assert urlparse.urlparse(datum['api']['status_url']).path == '/api/agenda/%s/status/%s/' % (
             agenda.slug,
-            datum['slug'] or datum['id'],
+            datum['slug'],
         )
 
 
@@ -706,7 +706,7 @@ def test_booking_api(app, some_data, user):
         ]
         assert urlparse.urlparse(event_fillslot_url).path == '/api/agenda/%s/fillslot/%s/' % (
             agenda.slug,
-            event.slug or event.id,
+            event.slug,
         )
 
     app.authorization = ('Basic', ('john.doe', 'password'))
@@ -889,9 +889,6 @@ def test_booking_ics(app, some_data, meetings_agenda, user):
 def test_booking_api_fillslots(app, some_data, user):
     agenda = Agenda.objects.filter(label=u'Foo bar')[0]
     events_ids = [x.id for x in Event.objects.filter(agenda=agenda) if x.in_bookable_period()]
-    for i, event in enumerate([x for x in Event.objects.filter(agenda=agenda) if x.in_bookable_period()]):
-        event.slug = 'event-%s' % i
-        event.save()
     events_slugs = [x.slug for x in Event.objects.filter(agenda=agenda) if x.in_bookable_period()]
     assert len(events_ids) == 3
     event = [x for x in Event.objects.filter(agenda=agenda) if x.in_bookable_period()][0]  # first event
diff --git a/tests/test_manager.py b/tests/test_manager.py
index 932540f..cdae53f 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -907,7 +907,6 @@ def test_add_event(app, admin_user):
     resp = resp.form.submit()
     resp = resp.follow()
     event = Event.objects.get(places=10)
-    assert event.slug is None
     assert event.publication_date is None
     assert "This agenda doesn't have any event yet." not in resp.text
     assert '/manage/agendas/%s/events/%s/' % (agenda.id, event.id) in resp.text
-- 
2.20.1

