diff --git a/src/openforms/data_removal/tests/test_tasks.py b/src/openforms/data_removal/tests/test_tasks.py index 98945155af..34b4e87ed9 100644 --- a/src/openforms/data_removal/tests/test_tasks.py +++ b/src/openforms/data_removal/tests/test_tasks.py @@ -1,10 +1,13 @@ from datetime import timedelta from django.core.exceptions import ObjectDoesNotExist -from django.test import TestCase +from django.test import TestCase, tag from django.utils import timezone +from freezegun import freeze_time + from openforms.config.models import GlobalConfiguration +from openforms.forms.models.form_step import FormStep from openforms.forms.tests.factories import ( FormDefinitionFactory, FormFactory, @@ -57,6 +60,32 @@ def test_successful_submissions_correctly_deleted(self): with self.assertRaises(ObjectDoesNotExist): submission_to_be_deleted.refresh_from_db() + @tag("gh-2632") + def test_delete_successful_submission_with_deleted_form_step(self): + config = GlobalConfiguration.get_solo() + + # submission to be deleted + before_limit = timezone.now() - timedelta( + days=config.successful_submissions_removal_limit + 1 + ) + with freeze_time(before_limit): + sub = SubmissionFactory.create(completed=True) + + # create a form step to then delete which introduces the RecursionError + form_step = FormStepFactory.create(form=sub.form) + SubmissionStepFactory.create(submission=sub, form_step=form_step) + + # delete form step - this causes the recursion error + form_step.delete() + + assert sub.submissionstep_set.count() == 1 + assert isinstance(sub.submissionstep_set.get().form_step, FormStep) + assert not FormStep.objects.exists() + + delete_submissions() + + self.assertFalse(Submission.objects.exists()) + def test_successful_submissions_with_form_settings_override_global_configuration( self, ): diff --git a/src/openforms/submissions/models/submission_step.py b/src/openforms/submissions/models/submission_step.py index 7e021982fe..188448c30c 100644 --- a/src/openforms/submissions/models/submission_step.py +++ b/src/openforms/submissions/models/submission_step.py @@ -129,16 +129,37 @@ class Meta: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - # load the form step from historical data if the FK has been broken (due to a - # deleted form step in the form designer/import functionality) - if not self.form_step_id: - self.form_step = self._load_form_step_from_history() + # See #2632, and a long story here on what happens: + # + # When the Submission queryset is deleted, this happens through the + # django.db.models.deletion.Collector which takes care of finding all related + # objects to manage the cascade delete. + # + # It does this (apparently) in an optimized way where only the primary keys of + # records (SubmssionStep in this case) are being loaded (via + # ``get_candidate_relations_to_delete`` and Collector.related_objects). The + # details are not exactly clear, but it *appears* that the form_step_id + # field/description is deferred. This means that when you *access* this field, + # Django will actually perform a separate query to load the (full) record with + # the additional information from the database row, which in turn leads to a new + # instance of the model being created, which in turn causes the __init__ method + # to be called again and that cycle then repeats (since that field is probably + # again deferred?). + # + # We can check if the field was loaded or not by checking in self.__dict__. Note + # that this is far from public API and a structural fix is still recommended. + if "form_step_id" in self.__dict__: + # load the form step from historical data if the FK has been broken (due to a + # deleted form step in the form designer/import functionality) + if not self.form_step_id: + self.form_step = self._load_form_step_from_history() def __str__(self): return f"SubmissionStep {self.pk}: Submission {self.submission_id} submitted on {self.created_on}" def _load_form_step_from_history(self): history = deepcopy(self.form_step_history) + if not history: return None