From f2583f049eda42a74b4ba6c99123ce534f3d9185 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 16 Mar 2026 20:42:21 +0100 Subject: [PATCH 1/2] Run QuestionAssignment's delete garbage collection in signal The model's `.delete` method is not called when an object is deleted from a queryset or in a cascading delete. Instead, the `post_delete` signal can be used. See https://docs.djangoproject.com/en/6.0/topics/db/models/#overriding-model-methods --- evap/evaluation/models.py | 19 ++++++++----------- evap/evaluation/tests/test_models.py | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index d3fff0f2ec..e558def6f3 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -23,6 +23,7 @@ from django.db import IntegrityError, models, transaction from django.db.models import CheckConstraint, Count, Exists, F, Manager, OuterRef, Q, QuerySet, Subquery, Value from django.db.models.functions import Coalesce, Lower, NullIf, TruncDate +from django.db.models.signals import post_delete from django.dispatch import Signal, receiver from django.http import HttpRequest from django.template import Context, Template @@ -1373,18 +1374,14 @@ class Meta: ordering = ["order"] unique_together = [("question", "questionnaire")] - def delete(self, using=None, keep_parents=False) -> tuple[int, dict[str, int]]: - if not self.question.is_heading_question: - assert not self.question.answer_class.objects.filter(assignment=self).exists(), ( - "cannot delete question with answers" - ) - count = 0 - meta: dict[str, int] = {} - if not self.question.questionnaires.exclude(pk=self.questionnaire.pk).exists(): - count, meta = self.question.delete(using=using, keep_parents=False) # garbage-collect unused questions - self_count, self_meta = super().delete(using=using, keep_parents=keep_parents) - return count + self_count, meta | self_meta +@receiver(post_delete, sender=QuestionAssignment) +@transaction.atomic +def _question_assignment_post_delete(*, instance: QuestionAssignment, **_kwargs) -> None: + """Garbage-collect unused questions once no questionnaires reference them anymore.""" + + if not QuestionAssignment.objects.filter(question=instance.question.pk).exists(): + Question.objects.filter(pk=instance.question.pk).delete() @dataclass diff --git a/evap/evaluation/tests/test_models.py b/evap/evaluation/tests/test_models.py index 431d8dfa9a..30e7c6b38d 100644 --- a/evap/evaluation/tests/test_models.py +++ b/evap/evaluation/tests/test_models.py @@ -17,6 +17,7 @@ Evaluation, NotArchivableError, Question, + QuestionAssignment, Questionnaire, QuestionType, Semester, @@ -67,6 +68,26 @@ def test_can_be_deleted_by_manager(self): self.assertFalse(questionnaire.can_be_deleted_by_manager) +class TestQuestionAssignment(TestCase): + @classmethod + def setUpTestData(cls): + cls.assignment = baker.make(QuestionAssignment) + cls.question = cls.assignment.question + cls.questionnaire = cls.assignment.questionnaire + + def test_assignment_delete_gc(self): + self.assignment.delete() + self.assertRaises(Question.DoesNotExist, self.question.refresh_from_db) + + def test_assignment_queryset_delete_gc(self): + QuestionAssignment.objects.filter(pk=self.assignment.pk).delete() + self.assertRaises(Question.DoesNotExist, self.question.refresh_from_db) + + def test_questionnaire_cascading_delete_gc(self): + self.questionnaire.delete() + self.assertRaises(Question.DoesNotExist, self.question.refresh_from_db) + + @override_settings(EVALUATION_END_OFFSET_HOURS=0) class TestEvaluations(WebTest): def test_approved_to_in_evaluation(self): From 615a5698ee559fca8bab432d39bbe204c2b0b1ac Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 16 Mar 2026 20:46:28 +0100 Subject: [PATCH 2/2] Merge `TestQuestionnaire` and `QuestionnaireTests`, and downgrade to TestCase --- evap/evaluation/tests/test_models.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/evap/evaluation/tests/test_models.py b/evap/evaluation/tests/test_models.py index 30e7c6b38d..416a027220 100644 --- a/evap/evaluation/tests/test_models.py +++ b/evap/evaluation/tests/test_models.py @@ -59,7 +59,7 @@ def test_can_be_deleted_by_manager(self): self.assertTrue(semester.can_be_deleted_by_manager) -class TestQuestionnaire(WebTest): +class TestQuestionnaire(TestCase): def test_can_be_deleted_by_manager(self): questionnaire = baker.make(Questionnaire) self.assertTrue(questionnaire.can_be_deleted_by_manager) @@ -67,6 +67,10 @@ def test_can_be_deleted_by_manager(self): baker.make(Contribution, questionnaires=[questionnaire]) self.assertFalse(questionnaire.can_be_deleted_by_manager) + def test_locked_contributor_questionnaire(self): + questionnaire = baker.prepare(Questionnaire, is_locked=True, type=Questionnaire.Type.CONTRIBUTOR) + self.assertRaises(ValidationError, questionnaire.clean) + class TestQuestionAssignment(TestCase): @classmethod @@ -1169,9 +1173,3 @@ def test_recipient_list_filtering(self): evaluation, [EmailTemplate.Recipients.CONTRIBUTORS], filter_users_in_cc=True ) self.assertCountEqual(recipient_list, [contributor2, contributor3]) - - -class QuestionnaireTests(TestCase): - def test_locked_contributor_questionnaire(self): - questionnaire = baker.prepare(Questionnaire, is_locked=True, type=Questionnaire.Type.CONTRIBUTOR) - self.assertRaises(ValidationError, questionnaire.clean)