From b9c5fe37740a126428075a908f5b606508d33bef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= 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 e720f317..912b2299 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 14dfe0c6..78a6971f 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 5c347f7e..12e88dac 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 cbd66c0c..19b42147 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 56aa3e96..f9f38392 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 %} > - {% if event.label %}{{event.label}} {% endif %}[{% trans "identifier:" %} {{ event.slug }}] / + {% if event.label %}{{event.label}} / {% endif %} {{ event.start_datetime }} {% if event.full %}/ {% trans "full" %}{% endif %} ( diff --git a/chrono/manager/views.py b/chrono/manager/views.py index daa85cf1..41a06747 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 60d7ffa1..820d8a27 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 c6d939b5..48ea3457 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 8f1dfb74..dc4db09f 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):