From e588f5623ad0b08d0cba8dc17703727326d05ede Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= <fpeters@entrouvert.com>
Date: Wed, 27 Nov 2019 09:27:44 +0100
Subject: [PATCH] general: make event slugs optional (#37987)

---
 chrono/agendas/migrations/0028_event_slug.py   | 18 +-----------------
 .../migrations/0029_auto_20191106_1320.py      |  2 +-
 chrono/agendas/models.py                       |  7 +++----
 chrono/manager/forms.py                        |  7 ++++++-
 .../chrono/manager_agenda_settings.html        |  2 +-
 chrono/manager/views.py                        |  9 +++++++++
 tests/test_agendas.py                          | 13 -------------
 tests/test_api.py                              | 10 ++++++++--
 tests/test_manager.py                          |  3 ++-
 9 files changed, 31 insertions(+), 40 deletions(-)

diff --git a/chrono/agendas/migrations/0028_event_slug.py b/chrono/agendas/migrations/0028_event_slug.py
index e720f31..912b229 100644
--- a/chrono/agendas/migrations/0028_event_slug.py
+++ b/chrono/agendas/migrations/0028_event_slug.py
@@ -5,21 +5,6 @@ from django.db import migrations, models
 from django.utils.text import slugify
 
 
-def set_slug_on_events(apps, schema_editor):
-    Event = apps.get_model('agendas', 'Event')
-    for event in Event.objects.filter(slug=''):
-        base_slug = slugify(event.label)
-        slug = base_slug
-        i = 1
-        while True:
-            if not Event.objects.filter(slug=slug).exists():
-                break
-            slug = '%s-%s' % (base_slug, i)
-            i += 1
-        event.slug = slug
-        event.save(update_fields=['slug'])
-
-
 class Migration(migrations.Migration):
 
     dependencies = [
@@ -30,8 +15,7 @@ class Migration(migrations.Migration):
         migrations.AddField(
             model_name='event',
             name='slug',
-            field=models.SlugField(default='', max_length=160, verbose_name='Identifier'),
+            field=models.SlugField(default=None, null=True, blank=True, max_length=160, verbose_name='Identifier'),
             preserve_default=False,
         ),
-        migrations.RunPython(set_slug_on_events, lambda x, y: None),
     ]
diff --git a/chrono/agendas/migrations/0029_auto_20191106_1320.py b/chrono/agendas/migrations/0029_auto_20191106_1320.py
index 14dfe0c..78a6971 100644
--- a/chrono/agendas/migrations/0029_auto_20191106_1320.py
+++ b/chrono/agendas/migrations/0029_auto_20191106_1320.py
@@ -15,6 +15,6 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='event',
             name='slug',
-            field=models.SlugField(max_length=160, unique=True, verbose_name='Identifier'),
+            field=models.SlugField(default=None, null=True, blank=True, max_length=160, verbose_name='Identifier'),
         ),
     ]
diff --git a/chrono/agendas/models.py b/chrono/agendas/models.py
index 5c347f7..12e88da 100644
--- a/chrono/agendas/models.py
+++ b/chrono/agendas/models.py
@@ -288,7 +288,7 @@ class Event(models.Model):
             _('Places in waiting list'), default=0)
     label = models.CharField(_('Label'), max_length=150, null=True, blank=True,
             help_text=_('Optional label to identify this date.'))
-    slug = models.SlugField(_('Identifier'), max_length=160, unique=True)
+    slug = models.SlugField(_('Identifier'), max_length=160, null=True, blank=True, default=None)
     description = models.TextField(_('Description'), null=True, blank=True,
             help_text=_('Optional event description.'))
     full = models.BooleanField(default=False)
@@ -297,6 +297,7 @@ class Event(models.Model):
 
     class Meta:
         ordering = ['agenda', 'start_datetime', 'label']
+        unique_together = ('agenda', 'slug')
 
     def __str__(self):
         if self.label:
@@ -305,8 +306,6 @@ class Event(models.Model):
 
     def save(self, *args, **kwargs):
         self.check_full()
-        if not self.slug:
-            self.slug = generate_slug(self)
         return super(Event, self).save(*args, **kwargs)
 
     def check_full(self):
@@ -347,7 +346,7 @@ class Event(models.Model):
     def import_json(cls, data):
         data['start_datetime'] = make_aware(datetime.datetime.strptime(
             data['start_datetime'], '%Y-%m-%d %H:%M:%S'))
-        if 'slug' in data:
+        if data.get('slug'):
             event, created = cls.objects.get_or_create(slug=data['slug'], defaults=data)
             if not created:
                 for k, v in data.items():
diff --git a/chrono/manager/forms.py b/chrono/manager/forms.py
index cbd66c0..19b4214 100644
--- a/chrono/manager/forms.py
+++ b/chrono/manager/forms.py
@@ -174,6 +174,10 @@ class ImportEventsForm(forms.Form):
                         'as columns.'))
     events = None
 
+    def __init__(self, agenda_pk, **kwargs):
+        self.agenda_pk = agenda_pk
+        super(ImportEventsForm, self).__init__(**kwargs)
+
     def clean_events_csv_file(self):
         content = self.cleaned_data['events_csv_file'].read()
         if b'\0' in content:
@@ -209,6 +213,7 @@ class ImportEventsForm(forms.Form):
             if i == 0 and csvline[0].strip('#') in ('date', 'Date', _('date'), _('Date')):
                 continue
             event = Event()
+            event.agenda_id = self.agenda_pk
             for datetime_fmt in ('%Y-%m-%d %H:%M', '%d/%m/%Y %H:%M',
                     '%d/%m/%Y %Hh%M', '%Y-%m-%d %H:%M:%S', '%d/%m/%Y %H:%M:%S'):
                 try:
@@ -231,7 +236,7 @@ class ImportEventsForm(forms.Form):
                     raise ValidationError(_('Invalid file format. (number of places in waiting list, line %d)') % (i+1))
             if len(csvline) >= 5:
                 event.label = force_text(csvline[4])
-            exclude = ['agenda', 'desk', 'meeting_type']
+            exclude = ['desk', 'meeting_type']
             if len(csvline) >= 6:
                 event.slug = ' '.join([force_text(x) for x in csvline[5:]])
             else:
diff --git a/chrono/manager/templates/chrono/manager_agenda_settings.html b/chrono/manager/templates/chrono/manager_agenda_settings.html
index 56aa3e9..f9f3839 100644
--- a/chrono/manager/templates/chrono/manager_agenda_settings.html
+++ b/chrono/manager/templates/chrono/manager_agenda_settings.html
@@ -50,7 +50,7 @@
           data-total="{{event.waiting_list_places}}" data-booked="{{event.waiting_list}}"
         {% endif %}
         ><a rel="popup" href="{% if user_can_manage %}{% url 'chrono-manager-event-edit' pk=event.id %}{% else %}#{% endif %}">
-        {% if event.label %}{{event.label}} {% endif %}[{% trans "identifier:" %} {{ event.slug }}] /
+        {% if event.label %}{{event.label}} / {% endif %}
         {{ event.start_datetime }}
         {% if event.full %}/ <span class="full">{% trans "full" %}</span>{% endif %}
         (
diff --git a/chrono/manager/views.py b/chrono/manager/views.py
index daa85cf..41a0674 100644
--- a/chrono/manager/views.py
+++ b/chrono/manager/views.py
@@ -591,10 +591,19 @@ class AgendaImportEventsView(ManagedAgendaMixin, FormView):
     template_name = 'chrono/manager_import_events.html'
     agenda = None
 
+    def get_form_kwargs(self):
+        kwargs = super(AgendaImportEventsView, self).get_form_kwargs()
+        kwargs['agenda_pk'] = self.kwargs['pk']
+        return kwargs
+
     def form_valid(self, form):
         if form.events:
             for event in form.events:
                 event.agenda_id = self.kwargs['pk']
+                if event.slug and Event.objects.filter(
+                        agenda_id=event.agenda_id,
+                        slug=event.slug).exists():
+                    raise ValidationError(_('Duplicated event identifier'))
                 event.save()
             messages.info(self.request, _('%d events have been imported.') % len(form.events))
         return super(AgendaImportEventsView, self).form_valid(form)
diff --git a/tests/test_agendas.py b/tests/test_agendas.py
index 60d7ffa..820d8a2 100644
--- a/tests/test_agendas.py
+++ b/tests/test_agendas.py
@@ -123,18 +123,12 @@ def test_slug():
     agenda.save()
     assert agenda.slug == 'foo-bar'
 
-    event = Event.objects.create(start_datetime=now(), places=42, agenda=agenda, label='Foo bar')
-    assert event.slug == 'foo-bar'
-
 
 def test_existing_slug():
     agenda = Agenda(label=u'Foo bar', slug='bar')
     agenda.save()
     assert agenda.slug == 'bar'
 
-    event = Event.objects.create(start_datetime=now(), places=42, agenda=agenda, label='Foo bar', slug='bar')
-    assert event.slug == 'bar'
-
 
 def test_duplicate_slugs():
     agenda = Agenda(label=u'Foo baz')
@@ -147,13 +141,6 @@ def test_duplicate_slugs():
     agenda.save()
     assert agenda.slug == 'foo-baz-2'
 
-    event = Event.objects.create(start_datetime=now(), places=42, agenda=agenda, label='Foo bar')
-    assert event.slug == 'foo-bar'
-    event = Event.objects.create(start_datetime=now(), places=42, agenda=agenda, label='Foo bar')
-    assert event.slug == 'foo-bar-1'
-    event = Event.objects.create(start_datetime=now(), places=42, agenda=agenda, label='Foo bar')
-    assert event.slug == 'foo-bar-2'
-
 
 def test_event_manager():
     agenda = Agenda(label=u'Foo baz')
diff --git a/tests/test_api.py b/tests/test_api.py
index c6d939b..48ea345 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -351,6 +351,8 @@ def test_booking_api(app, some_data, user):
     assert Booking.objects.count() == 1
 
     # access by slug
+    event.slug = 'bar'
+    event.save()
     resp = app.post('/api/agenda/%s/fillslot/%s/' % (agenda.id, event.slug))
     assert Booking.objects.count() == 2
     assert Booking.objects.filter(event__agenda=agenda).count() == 2
@@ -474,6 +476,9 @@ 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
@@ -992,6 +997,8 @@ def test_status(app, some_data, user):
     assert resp.json['places']['waiting_list_reserved'] == 1
 
     # access by slug
+    event.slug = 'bar'
+    event.save()
     resp = app.get('/api/agenda/%s/status/%s/' % (agenda_id, event.slug))
     # not found event
     resp = app.get('/api/agenda/%s/status/%s/' % (agenda_id, 'unknown'), status=404)
@@ -1453,8 +1460,7 @@ def test_agenda_meeting_api_multiple_desk(app, meetings_agenda, user):
         booking_url = event_data['api']['fillslot_url']
         with CaptureQueriesContext(connection) as ctx:
             app.post(booking_url)
-            # 2 + idx: because of slug unicity
-            assert len(ctx.captured_queries) == queries_count_fillslot1 + 2 + idx
+            assert len(ctx.captured_queries) == queries_count_fillslot1
 
     with CaptureQueriesContext(connection) as ctx:
         app.get('/api/agenda/meetings/%s/datetimes/' % meeting_type.id)
diff --git a/tests/test_manager.py b/tests/test_manager.py
index 8f1dfb7..dc4db09 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -328,6 +328,7 @@ 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 not "This agenda doesn't have any event yet." in resp.text
     assert '/manage/events/%s/' % event.id in resp.text
     assert ('Feb. 15, %s, 5 p.m.' % year) in resp.text
@@ -651,7 +652,7 @@ def test_import_events(app, admin_user):
     resp = app.get('/manage/agendas/%s/import-events' % agenda.id, status=200)
     resp.form['events_csv_file'] = Upload('t.csv', b'2016-09-16,18:00,10,5,label,slug', 'text/csv')
     resp = resp.form.submit(status=200)
-    assert 'Invalid file format. (slug: Event with this Identifier already exists.' in resp.text
+    assert 'Invalid file format. (__all__: Event with this Agenda and Identifier already exists.' in resp.text
 
 
 def test_add_meetings_agenda(app, admin_user):
-- 
2.24.0

