From c1d5eb32ee80623810c8a016d33e612bec0b1ffb Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Wed, 21 Oct 2020 17:47:38 +0200 Subject: [PATCH] agendas: add unicity constraint on exception source slug (#47916) --- ...iodexceptionsource_unique_settings_slug.py | 50 ++++++++++ .../migrations/0067_auto_20201021_1746.py | 18 ++++ chrono/agendas/models.py | 3 + tests/test_misc.py | 97 +++++++++++++++++++ 4 files changed, 168 insertions(+) create mode 100644 chrono/agendas/migrations/0066_timeperiodexceptionsource_unique_settings_slug.py create mode 100644 chrono/agendas/migrations/0067_auto_20201021_1746.py diff --git a/chrono/agendas/migrations/0066_timeperiodexceptionsource_unique_settings_slug.py b/chrono/agendas/migrations/0066_timeperiodexceptionsource_unique_settings_slug.py new file mode 100644 index 00000000..199b3cf1 --- /dev/null +++ b/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), + ] diff --git a/chrono/agendas/migrations/0067_auto_20201021_1746.py b/chrono/agendas/migrations/0067_auto_20201021_1746.py new file mode 100644 index 00000000..c2bb38d4 --- /dev/null +++ b/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')]), + ), + ] diff --git a/chrono/agendas/models.py b/chrono/agendas/models.py index d5f445ae..ee9b9d0c 100644 --- a/chrono/agendas/models.py +++ b/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 diff --git a/tests/test_misc.py b/tests/test_misc.py index 662fafbc..543c0f0d 100644 --- a/tests/test_misc.py +++ b/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')