agendas: add unicity constraint on exception source slug (#47916)

chrono/agendas/migrations/0066_timeperiodexceptionsource_unique_settings_slug.py

@@ -0,0 +1,50 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.18 on 2020-10-21 11:56
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+from django.db.models import Count, F
+
+
+def remove_broken_exceptions(apps, schema_editor):
+    TimePeriodException = apps.get_model('agendas', 'TimePeriodException')
+    qs = TimePeriodException.objects.filter(source__settings_slug__isnull=False)
+    # an exception is broken if its desk in not the same at the desk of its source
+    qs.exclude(source__desk=F('desk')).delete()
+
+
+def remove_duplicate_sources(apps, schema_editor):
+    Desk = apps.get_model('agendas', 'Desk')
+    TimePeriodExceptionSource = apps.get_model('agendas', 'TimePeriodExceptionSource')
+    for desk in Desk.objects.all():
+        duplicate_source_slugs = (
+            desk.timeperiodexceptionsource_set.values('settings_slug')
+            .annotate(count=Count('settings_slug'))
+            .order_by()
+            .filter(count__gt=1)
+        )
+        if not duplicate_source_slugs:
+            continue
+        for source in duplicate_source_slugs:
+            settings_slug = source['settings_slug']
+            duplicate_sources = desk.timeperiodexceptionsource_set.filter(settings_slug=settings_slug)
+            # remove duplicates, keeping the one that has related time period exceptions, if any
+            source_to_keep = duplicate_sources.filter(timeperiodexception__isnull=False).first()
+            if not source_to_keep:
+                # if no source had exceptions, try to keep one that is flagged as disabled
+                source_to_keep = duplicate_sources.filter(enabled=False).first()
+            if not source_to_keep:
+                source_to_keep = duplicate_sources.first()
+            duplicate_sources.exclude(pk=source_to_keep.pk).delete()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('agendas', '0065_unavailability_calendar'),
+    ]
+
+    operations = [
+        migrations.RunPython(remove_broken_exceptions, migrations.RunPython.noop),
+        migrations.RunPython(remove_duplicate_sources, migrations.RunPython.noop),
+    ]

chrono/agendas/migrations/0067_auto_20201021_1746.py

@@ -0,0 +1,18 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.18 on 2020-10-21 15:46
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('agendas', '0066_timeperiodexceptionsource_unique_settings_slug'),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name='timeperiodexceptionsource', unique_together=set([('desk', 'settings_slug')]),
+        ),
+    ]

chrono/agendas/models.py

@@ -1432,6 +1432,9 @@ class TimePeriodExceptionSource(models.Model):
     last_update = models.DateTimeField(auto_now=True, null=True)
     enabled = models.BooleanField(default=True)
 
+    class Meta:
+        unique_together = ['desk', 'settings_slug']
+
     def __str__(self):
         if self.ics_filename is not None:
             return self.ics_filename

tests/test_misc.py

@@ -4,6 +4,7 @@ from django.db import IntegrityError
 from django.db import ProgrammingError
 from django.db import connection
 from django.db import transaction
+from django.db.migrations.executor import MigrationExecutor
 from django.utils.timezone import now
 
 
@@ -209,3 +210,99 @@ def test_meeting_event_exclusion_constraint():
     Event.objects.create(
         start_datetime=event1.start_datetime, meeting_type=meeting_type1, places=10, agenda=agenda, desk=desk1
     )
+
+
+def test_clean_time_period_exceptions(transactional_db):
+    app = 'agendas'
+
+    migrate_from = [(app, '0065_unavailability_calendar')]
+    migrate_to = [(app, '0066_timeperiodexceptionsource_unique_settings_slug')]
+    executor = MigrationExecutor(connection)
+    old_apps = executor.loader.project_state(migrate_from).apps
+    executor.migrate(migrate_from)
+
+    Agenda = old_apps.get_model(app, 'Agenda')
+    Desk = old_apps.get_model(app, 'Desk')
+    TimePeriodException = old_apps.get_model(app, 'TimePeriodException')
+    TimePeriodExceptionSource = old_apps.get_model(app, 'TimePeriodExceptionSource')
+
+    agenda = Agenda.objects.create(label='Agenda')
+    desk = Desk.objects.create(label='Desk', slug='desk', agenda=agenda)
+
+    # add normal time period exception to Desk
+    source_desk = TimePeriodExceptionSource.objects.create(desk=desk, settings_slug='holidays', enabled=True)
+    start_datetime = datetime.datetime(year=2020, month=1, day=2)
+    end_datetime = datetime.datetime(year=2020, month=1, day=3)
+    for i in range(5):
+        TimePeriodException.objects.create(
+            desk=desk,
+            source=source_desk,
+            external=True,
+            start_datetime=start_datetime,
+            end_datetime=end_datetime,
+        )
+
+    # now simulate broken state (desk duplication)
+    new_desk = Desk.objects.create(label='New Desk', slug='new-desk', agenda=agenda)
+
+    # normal source and exceptions
+    source_new_desk = TimePeriodExceptionSource.objects.create(
+        desk=new_desk, settings_slug='holidays', enabled=True
+    )
+    for i in range(5):
+        TimePeriodException.objects.create(
+            desk=new_desk,
+            source=source_new_desk,
+            external=True,
+            start_datetime=start_datetime,
+            end_datetime=end_datetime,
+        )
+
+    # wrong duplicate of source
+    TimePeriodExceptionSource.objects.create(desk=new_desk, settings_slug='holidays', enabled=True)
+
+    # wrong duplicate of exceptions, referencing original desk source
+    for i in range(5):
+        TimePeriodException.objects.create(
+            desk=new_desk,
+            source=source_desk,
+            external=True,
+            start_datetime=start_datetime,
+            end_datetime=end_datetime,
+        )
+
+    # extra data that should not be touched
+    other_exception = TimePeriodException.objects.create(
+        desk=desk, start_datetime=start_datetime, end_datetime=end_datetime,
+    )
+    other_source = TimePeriodExceptionSource.objects.create(desk=desk, ics_file='test.ics')
+    # even if wrong desk, this exception is not from settings thus should not get removed
+    exception_from_ics = TimePeriodException.objects.create(
+        desk=new_desk, start_datetime=start_datetime, end_datetime=end_datetime, source=other_source,
+    )
+
+    # ensure migration fixes state
+    executor = MigrationExecutor(connection)
+    executor.migrate(migrate_to)
+    executor.loader.build_graph()
+
+    apps = executor.loader.project_state(migrate_to).apps
+    Desk = apps.get_model(app, 'Desk')
+    TimePeriodException = apps.get_model(app, 'TimePeriodException')
+    TimePeriodExceptionSource = apps.get_model(app, 'TimePeriodExceptionSource')
+
+    # original desk hasn't been touched
+    desk = Desk.objects.get(pk=desk.pk)
+    assert desk.timeperiodexception_set.filter(source__settings_slug='holidays').count() == 5
+    assert desk.timeperiodexceptionsource_set.filter(settings_slug='holidays').count() == 1
+    assert desk.timeperiodexception_set.filter(pk=other_exception.pk).exists()
+    assert desk.timeperiodexceptionsource_set.filter(pk=other_source.pk).exists()
+
+    # duplicated desk has correct exceptions
+    new_desk = Desk.objects.get(pk=new_desk.pk)
+    assert new_desk.timeperiodexception_set.filter(source__settings_slug='holidays').count() == 5
+    assert new_desk.timeperiodexceptionsource_set.filter(settings_slug='holidays').count() == 1
+    assert new_desk.timeperiodexception_set.filter(pk=exception_from_ics.pk).exists()
+
+    exc = new_desk.timeperiodexception_set.filter(source__settings_slug='holidays').first()
+    assert exc.source == new_desk.timeperiodexceptionsource_set.get(settings_slug='holidays')