Importers: Disallow course merging for published evaluations and sing…

…le results (#1978)

Importers: Disallow course merging if it would break _participant_count. Fixes #1976.

Give an error for state >= IN_EVALUATION or for single result evaluations.
Also fixes the superfluous "will be merged" warning in cases with merge hindrances.

Co-authored-by: Richard Ebeling <[email protected]>

evap/staff/importers/enrollment.py

@@ -3,7 +3,7 @@
 from collections.abc import Iterable
 from dataclasses import dataclass, fields
 from datetime import date, datetime
-from typing import TypeAlias, TypeGuard, TypeVar
+from typing import NoReturn, TypeAlias, TypeGuard, TypeVar
 
 from django.conf import settings
 from django.db import transaction
@@ -39,7 +39,9 @@
 @dataclass(frozen=True)
 class InvalidValue:
     # We make this a dataclass to make sure all instances compare equal.
-    pass
+
+    def __bool__(self) -> NoReturn:
+        raise NotImplementedError("Bool conversion of InvalidValue is likely a bug")
 
 
 invalid_value = InvalidValue()
@@ -326,6 +328,7 @@ def __init__(self, semester: Semester):
     @staticmethod
     def get_merge_hindrances(course_data: CourseData, merge_candidate: Course) -> list[str]:
         hindrances = []
+
         if merge_candidate.type != course_data.course_type:
             hindrances.append(_("the course type does not match"))
 
@@ -336,9 +339,24 @@ def get_merge_hindrances(course_data: CourseData, merge_candidate: Course) -> li
         merge_candidate_evaluations = merge_candidate.evaluations.all()
         if len(merge_candidate_evaluations) != 1:
             hindrances.append(_("the existing course does not have exactly one evaluation"))
-        elif merge_candidate_evaluations[0].wait_for_grade_upload_before_publishing != course_data.is_graded:
+            return hindrances
+
+        merge_candidate_evaluation: Evaluation = merge_candidate_evaluations[0]
+
+        if merge_candidate_evaluation.wait_for_grade_upload_before_publishing != course_data.is_graded:
             hindrances.append(_("the evaluation of the existing course has a mismatching grading specification"))
 
+        if merge_candidate_evaluation.is_single_result:
+            hindrances.append(_("the evaluation of the existing course is a single result"))
+            return hindrances
+
+        if merge_candidate_evaluation.state >= Evaluation.State.IN_EVALUATION:
+            hindrances.append(
+                _("the import would add participants to the existing evaluation but the evaluation is already running")
+            )
+        else:
+            assert merge_candidate_evaluation._participant_count is None
+
         return hindrances
 
     def set_course_merge_target(self, course_data: CourseData) -> None:
@@ -407,7 +425,7 @@ def check_course_data(self, course_data: CourseData, location: ExcelFileLocation
         except CourseMergeLogic.NameEnCollisionException:
             self.name_en_collision_tracker.add_location_for_key(location, course_data.name_en)
 
-        if course_data.merge_into_course:
+        if course_data.merge_into_course != invalid_value and course_data.merge_into_course:
             self.course_merged_tracker.add_location_for_key(location, course_data.name_en)
 
         self.name_en_by_name_de.setdefault(course_data.name_de, course_data.name_en)

evap/staff/tests/test_importers.py

@@ -306,6 +306,8 @@ def test_wrong_column_count(self):
 
 
 class TestEnrollmentImport(ImporterTestCase):
+    semester: Semester
+
     @classmethod
     def setUpTestData(cls):
         cls.random_excel_file_content = excel_data.random_file_content
@@ -320,7 +322,7 @@ def setUpTestData(cls):
         cls.default_excel_content = excel_data.create_memory_excel_file(excel_data.test_enrollment_data_filedata)
         cls.empty_excel_content = excel_data.create_memory_excel_file(excel_data.test_enrollment_data_empty_filedata)
 
-    def create_existing_course(self):
+    def create_existing_course(self) -> tuple[Course, Evaluation]:
         existing_course = baker.make(
             Course,
             name_de="Schütteln",
@@ -710,6 +712,7 @@ def test_existing_course_different_attributes(self):
             self.default_excel_content, self.semester, self.vote_start_datetime, self.vote_end_date, test_run=False
         )
 
+        self.assertEqual({}, importer_log.warnings_by_category())
         self.assertErrorIs(
             importer_log,
             ImporterLogEntry.Category.COURSE,
@@ -723,6 +726,67 @@ def test_existing_course_different_attributes(self):
         existing_course.refresh_from_db()
         self.assertEqual(old_dict, model_to_dict(existing_course))
 
+    def test_existing_course_with_published_evaluation(self):
+        __, existing_evaluation = self.create_existing_course()
+
+        # Attempt with state = Published
+        Evaluation.objects.filter(pk=existing_evaluation.pk).update(state=Evaluation.State.PUBLISHED)
+        existing_evaluation = Evaluation.objects.get(pk=existing_evaluation.pk)
+
+        importer_log = import_enrollments(
+            self.default_excel_content, self.semester, self.vote_start_datetime, self.vote_end_date, test_run=False
+        )
+
+        self.assertEqual({}, importer_log.warnings_by_category())
+        self.assertErrorIs(
+            importer_log,
+            ImporterLogEntry.Category.COURSE,
+            "Sheet "BA Belegungen", row 2 and 1 other place: "
+            + "Course &quot;Shake&quot; already exists in this semester, but the courses can not be merged for the following reasons:<br /> "
+            + "- the import would add participants to the existing evaluation but the evaluation is already running",
+        )
+
+        # Attempt with earlier state but set _participant_count
+        Evaluation.objects.filter(pk=existing_evaluation.pk).update(state=Evaluation.State.APPROVED)
+        existing_evaluation = Evaluation.objects.get(pk=existing_evaluation.pk)
+        existing_evaluation._participant_count = existing_evaluation.participants.count()
+        existing_evaluation._voter_count = existing_evaluation.voters.count()
+        existing_evaluation.save()
+
+        with override_settings(DEBUG=False):
+            importer_log = import_enrollments(
+                self.default_excel_content, self.semester, self.vote_start_datetime, self.vote_end_date, test_run=False
+            )
+        self.assertEqual(
+            [msg.message for msg in importer_log.errors_by_category()[ImporterLogEntry.Category.GENERAL]],
+            ["Import aborted after exception: ''. No data was imported."],
+        )
+
+    def test_existing_course_with_single_result(self):
+        __, existing_evaluation = self.create_existing_course()
+        existing_evaluation.is_single_result = True
+        existing_evaluation.save()
+
+        old_evaluation_count = Evaluation.objects.count()
+        old_dict = model_to_dict(existing_evaluation)
+
+        importer_log = import_enrollments(
+            self.default_excel_content, self.semester, self.vote_start_datetime, self.vote_end_date, test_run=False
+        )
+
+        self.assertEqual({}, importer_log.warnings_by_category())
+        self.assertErrorIs(
+            importer_log,
+            ImporterLogEntry.Category.COURSE,
+            "Sheet &quot;BA Belegungen&quot;, row 2 and 1 other place: "
+            + "Course &quot;Shake&quot; already exists in this semester, but the courses can not be merged for the following reasons:<br /> "
+            + "- the evaluation of the existing course is a single result",
+        )
+
+        self.assertEqual(Evaluation.objects.count(), old_evaluation_count)
+        existing_evaluation = Evaluation.objects.get(pk=existing_evaluation.pk)
+        self.assertEqual(old_dict, model_to_dict(existing_evaluation))
+
     def test_existing_course_equal_except_evaluations(self):
         existing_course, __ = self.create_existing_course()
         baker.make(Evaluation, course=existing_course, name_de="Zweite Evaluation", name_en="Second Evaluation")
@@ -734,6 +798,7 @@ def test_existing_course_equal_except_evaluations(self):
             self.default_excel_content, self.semester, self.vote_start_datetime, self.vote_end_date, test_run=False
         )
 
+        self.assertEqual({}, importer_log.warnings_by_category())
         self.assertErrorIs(
             importer_log,
             ImporterLogEntry.Category.COURSE,
@@ -758,6 +823,7 @@ def test_existing_course_different_grading(self):
             self.default_excel_content, self.semester, self.vote_start_datetime, self.vote_end_date, test_run=False
         )
 
+        self.assertEqual({}, importer_log.warnings_by_category())
         self.assertErrorIs(
             importer_log,
             ImporterLogEntry.Category.COURSE,