Use atomic transaction for whole import; use Enums instead of magic values

Also, throws exceptions on unrecoverable errors to terminate and rollback the transaction cleanly

aleksis/apps/untis/util/mysql/importers/absences.py

+from enum import Enum
 import logging
 
 from tqdm import tqdm
@@ -14,6 +15,10 @@ from ..util import (
 logger = logging.getLogger(__name__)
 unknown_reason, _ = chronos_models.AbsenceReason.objects.get_or_create(short_name="?")
 
+class AbsenceType(Enum):
+    GROUP = 100
+    TEACHER = 101
+    ROOM = 102
 
 def import_absences(
     absence_reasons_ref, time_periods_ref, teachers_ref, classes_ref, rooms_ref
@@ -73,12 +78,11 @@ def import_absences(
         teacher = None
         room = None
 
-        # 100, 101 and 102 are UNTIS constants
-        if type_ == 100:
+        if type_ == AbsenceType.GROUP:
             group = classes_ref[absence.ida]
-        elif type_ == 101:
+        elif type_ == AbsenceType.TEACHER:
             teacher = teachers_ref[absence.ida]
-        elif type == 102:
+        elif type == AbsenceType.ROOM:
             room = rooms_ref[absence.ida]
 
         new_absence, created = chronos_models.Absence.objects.get_or_create(

aleksis/apps/untis/util/mysql/importers/common_data.py

+from enum import Enum
 import logging
 from datetime import time
 from typing import List, Dict
@@ -15,6 +16,10 @@ from ..util import run_default_filter, untis_colour_to_hex, untis_split_first, c
 logger = logging.getLogger(__name__)
 
 
+class CommonDataId(Enum):
+    PERIOD = 40
+
+
 def import_subjects() -> Dict[int, chronos_models.Subject]:
     """ Import subjects """
 
@@ -26,12 +31,11 @@ def import_subjects() -> Dict[int, chronos_models.Subject]:
     for subject in tqdm(subjects, desc="Import subjects", **TQDM_DEFAULTS):
         # Check if needed data are provided
         if not subject.name:
-            logger.error(
+            raise RuntimeException(
                 "Subject ID {}: Cannot import subject without short name.".format(
                     subject.subject_id
                 )
             )
-            continue
 
         # Build values
         short_name = subject.name[:10]
@@ -95,12 +99,11 @@ def import_teachers() -> Dict[int, core_models.Person]:
     for teacher in tqdm(teachers, desc="Import teachers", **TQDM_DEFAULTS):
         # Check if needed data are provided
         if not teacher.name:
-            logger.error(
+            raise RuntimeException(
                 "Teacher ID {}: Cannot import teacher without short name.".format(
                     teacher.teacher_id
                 )
             )
-            continue
 
         # Build values
         short_name = teacher.name
@@ -165,12 +168,11 @@ def import_classes(
     for class_ in tqdm(course_classes, desc="Import classes", **TQDM_DEFAULTS):
         # Check if needed data are provided
         if not class_.name:
-            logger.error(
+            raise RuntimeException(
                 "Class ID {}: Cannot import class without short name.".format(
                     class_.teacher_id
                 )
             )
-            continue
 
         # Build values
         short_name = class_.name[:16]
@@ -234,12 +236,11 @@ def import_rooms() -> Dict[int, chronos_models.Room]:
 
     for room in tqdm(rooms, desc="Import rooms", **TQDM_DEFAULTS):
         if not room.name:
-            logger.error(
+            raise RuntimeException(
                 "Room ID {}: Cannot import room without short name.".format(
                     room.room_id
                 )
             )
-            continue
 
         # Build values
         short_name = room.name[:10]
@@ -288,12 +289,11 @@ def import_supervision_areas(
 
     for area in tqdm(areas, desc="Import supervision areas", **TQDM_DEFAULTS):
         if not area.name:
-            logger.error(
+            raise RuntimeException(
                 "Supervision area ID {}: Cannot import supervision area without short name.".format(
                     area.corridor_id
                 )
             )
-            continue
 
         short_name = area.name[:10]
         name = area.longname[:50] if area.longname else short_name
@@ -417,7 +417,7 @@ def import_time_periods() -> Dict[int, Dict[int, chronos_models.TimePeriod]]:
 
     periods = (
         run_default_filter(mysql_models.Commondata.objects, filter_term=False)
-        .filter(id=40)  # Fixed UNTIS constant
+        .filter(id=CommonDataId.PERIOD)
         .order_by("number", "number1")
     )
 
@@ -510,12 +510,11 @@ def import_absence_reasons() -> Dict[int, chronos_models.AbsenceReason]:
 
     for reason in tqdm(reasons, desc="Import absence reasons", **TQDM_DEFAULTS):
         if not reason.name:
-            logger.error(
+            raise RuntimeException(
                 "Absence reason ID {}: Cannot import absence reason without short name.".format(
                     reason.absence_reason_id
                 )
             )
-            continue
 
         # Build values
         short_name = reason.name

aleksis/apps/untis/util/mysql/importers/holidays.py

@@ -23,12 +23,11 @@ def import_holidays() -> Dict[int, chronos_models.Holiday]:
 
         # Check if needed data are provided
         if not holiday.name:
-            logger.error(
+            raise RuntimeException(
                 "Holiday ID {}: Cannot import holiday without short name.".format(
                     import_ref
                 )
             )
-            continue
 
         title = holiday.name[:50]
         comments = holiday.longname

aleksis/apps/untis/util/mysql/importers/lessons.py

@@ -116,6 +116,7 @@ def import_lessons(
                 course_classes.append(c)
 
             if config.UNTIS_IMPORT_MYSQL_USE_COURSE_GROUPS:
+                # Negative import_ref denotes a course group
                 group_import_ref = -int("{}{}".format(lesson_id, i))
                 subject_ref = subject.abbrev
 
@@ -223,7 +224,7 @@ def import_lessons(
                 logger.info("    New lesson created")
 
             # Sync groups
-            lesson.groupsset(groups)
+            lesson.groups.set(groups)
 
             # Sync teachers
             lesson.teachers.set(teachers)

aleksis/apps/untis/util/mysql/importers/substitutions.py

+from enum import Enum
 import logging
 
 from calendarweek import CalendarWeek
@@ -15,6 +16,9 @@ from .... import models as mysql_models
 
 logger = logging.getLogger(__name__)
 
+class SubstitutionFlag(Enum):
+    CANCELLED = "E"
+    CANCELLED_FOR_TEACHERS = "F"
 
 def import_substitutions(
     teachers_ref, subjects_ref, rooms_ref, classes_ref, supervision_areas_ref
@@ -58,9 +62,9 @@ def import_substitutions(
 
         # Cancellation?
         cancelled, cancelled_for_teachers = False, False
-        if "E" in sub.flags:
+        if SubstitutionFlag.CANCELLED in sub.flags:
             cancelled = True
-        elif "F" in sub.flags:
+        elif SubstitutionFlag.CANCELLED_FOR_TEACHERS in sub.flags:
             cancelled_for_teachers = True
 
         # Comment
@@ -88,6 +92,7 @@ def import_substitutions(
                 period__period=period,
                 period__weekday=weekday,
             ).on_day(date)
+
             if lesson_periods.exists():
                 lesson_period = lesson_periods[0]
                 logger.info("  Matching lesson period found ({})".format(lesson_period))
@@ -106,7 +111,7 @@ def import_substitutions(
             if cancelled:
                 subject_new = None
 
-            # # Room
+            # Room
             room_old = lesson_period.room if lesson_period else None
             room_new = None
             if sub.room_idsubst != 0:
@@ -115,7 +120,7 @@ def import_substitutions(
                 if room_old is not None and room_old.id == room_new.id:
                     room_new = None
 
-            # # Classes
+            # Classes
             classes = []
             class_ids = untis_split_first(sub.classids, conv=int)
 
@@ -194,13 +199,6 @@ def import_substitutions(
                         logger.info("  Supervision substitution updated")
 
     # Delete all no longer existing substitutions
-    for s in chronos_models.LessonSubstitution.objects.within_dates(date_start, date_end):
-        if s.import_ref_untis and s.import_ref_untis not in existing_subs:
-            logger.info("Substitution {} deleted".format(s.id))
-            s.delete()
-
-    # Delete all no longer existing supervision substitutions
-    for s in chronos_models.SupervisionSubstitution.objects.filter(date__gte=date_start, date__lte=date_end):
-        if s.import_ref_untis and s.import_ref_untis not in existing_subs:
-            logger.info("Supervision substitution {} deleted".format(s.id))
-            s.delete()
+    chronos_models.LessonSubstitution.objects.within_dates(date_start, date_end).exclude(import_ref_untis__in=existing_subs).delete()
+    chronos_models.SupervisionSubstitution.objects.filter(date__gte=date_start, date__lte=date_end).exclude(existing_subs__in=existing_subs).delete()
+    logger.info("Left-over substitutions deleted")

aleksis/apps/untis/util/mysql/main.py

+from django.db import transaction
+
 from .importers.absences import import_absences
 from .importers.common_data import (
     import_subjects,
@@ -15,6 +17,7 @@ from .importers.lessons import import_lessons
 from .importers.substitutions import import_substitutions
 
 
+@transaction.atomic
 def untis_import_mysql():
     # Coomon data for Chronos
     subjects_ref = import_subjects()