Merge pull request #4964 from open-formulieren/issue/4900-recouple-re…

…usable-fd-submission-value-vars

🐛 [#4900] Recouple SubmissionValueVars for other forms for reusable FormDefinitions

src/openforms/forms/tasks.py

@@ -62,33 +62,45 @@ def recouple_submission_variables_to_form_variables(form_id: int) -> None:
 
     When the FormVariable bulk create/update endpoint is called, all existing FormVariable related to the form are
     deleted and new are created. If there are existing submissions for this form, the SubmissionValueVariables don't
-    have a related FormVariable anymore. This task tries to recouple them.
+    have a related FormVariable anymore. This task tries to recouple them and does the same for
+    other Forms in case of reusable FormDefinitions
     """
     from openforms.submissions.models import SubmissionValueVariable
 
-    form = Form.objects.get(id=form_id)
-    submission_variables_to_recouple = SubmissionValueVariable.objects.filter(
-        form_variable__isnull=True, submission__form=form
-    )
+    from .models import FormDefinition  # due to circular import
+
+    def recouple(form):
+        submission_variables_to_recouple = SubmissionValueVariable.objects.filter(
+            form_variable__isnull=True, submission__form=form
+        )
 
-    form_variables = {
-        variable.key: variable for variable in form.formvariable_set.all()
-    }
+        form_variables = {
+            variable.key: variable for variable in form.formvariable_set.all()
+        }
 
-    submission_variables_to_update = []
-    for submission_variable in submission_variables_to_recouple:
-        if form_variable := form_variables.get(submission_variable.key):
-            submission_variable.form_variable = form_variable
-            submission_variables_to_update.append(submission_variable)
+        submission_variables_to_update = []
+        for submission_variable in submission_variables_to_recouple:
+            if form_variable := form_variables.get(submission_variable.key):
+                submission_variable.form_variable = form_variable
+                submission_variables_to_update.append(submission_variable)
 
-    try:
-        SubmissionValueVariable.objects.bulk_update(
-            submission_variables_to_update, fields=["form_variable"]
-        )
-    except IntegrityError:
-        # Issue #1970: If the form is saved again from the form editor while this task was running, the form variables
-        # retrieved don't exist anymore. Another task will be scheduled from the endpoint, so nothing more to do here.
-        logger.info("Form variables were updated while this task was runnning.")
+        try:
+            SubmissionValueVariable.objects.bulk_update(
+                submission_variables_to_update, fields=["form_variable"]
+            )
+        except IntegrityError:
+            # Issue #1970: If the form is saved again from the form editor while this task was running, the form variables
+            # retrieved don't exist anymore. Another task will be scheduled from the endpoint, so nothing more to do here.
+            logger.info("Form variables were updated while this task was runnning.")
+
+    recouple(Form.objects.get(id=form_id))
+
+    fds = FormDefinition.objects.filter(formstep__form=form_id, is_reusable=True)
+    other_forms = Form.objects.filter(formstep__form_definition__in=fds).exclude(
+        id=form_id
+    )
+    for form in other_forms:
+        recouple(form)
 
 
 @app.task(ignore_result=True)

src/openforms/forms/tests/variables/test_tasks.py

@@ -3,6 +3,7 @@
 from unittest.mock import patch
 
 from django.db import close_old_connections
+from django.db.models import Q
 from django.test import TestCase, TransactionTestCase, override_settings, tag
 
 from openforms.forms.tasks import (
@@ -19,11 +20,16 @@
 from openforms.submissions.tests.factories import (
     SubmissionFactory,
     SubmissionStepFactory,
+    SubmissionValueVariableFactory,
 )
 from openforms.variables.constants import FormVariableSources
 
 from ...models import FormVariable
-from ...tasks import create_form_variables_for_components, on_formstep_save_event
+from ...tasks import (
+    create_form_variables_for_components,
+    on_formstep_save_event,
+    on_variables_bulk_update_event,
+)
 
 
 class FormVariableTasksTest(TestCase):
@@ -253,6 +259,88 @@ def test_create_form_variables_for_components(self):
             self.assertEqual(variables.count(), 1)
 
 
+@override_settings(CELERY_TASK_ALWAYS_EAGER=True)
+class RecoupleSubmissionVariablesToFormVariables(TestCase):
+    def test_recouple(self):
+        form = FormFactory.create()
+        other_form = FormFactory.create()
+        form_definition = FormDefinitionFactory.create(
+            configuration={
+                "display": "form",
+                "components": [
+                    {
+                        "key": "firstName",
+                        "type": "textfield",
+                        "label": "First Name",
+                    },
+                    {
+                        "key": "lastName",
+                        "type": "textfield",
+                        "label": "Last Name",
+                    },
+                ],
+            },
+            is_reusable=True,
+        )
+        form_step1 = FormStepFactory.create(form=form, form_definition=form_definition)
+        form_step2 = FormStepFactory.create(
+            form=other_form, form_definition=form_definition
+        )
+
+        submission1 = SubmissionFactory.create(form=form)
+        submission2 = SubmissionFactory.create(form=other_form)
+
+        SubmissionStepFactory.create(
+            submission=submission1,
+            form_step=form_step1,
+            data={"firstName": "John"},
+        )
+        SubmissionStepFactory.create(
+            submission=submission2,
+            form_step=form_step2,
+            data={"firstName": "John"},
+        )
+
+        # Task should ignore keys for which there is no FormVariable
+        SubmissionValueVariableFactory.create(
+            submission=submission1, key="non-existent", form_variable=None
+        )
+        SubmissionValueVariable.objects.filter(
+            Q(submission__form=form) | Q(submission__form=other_form),
+        ).update(form_variable=None)
+
+        first_name_var1 = FormVariable.objects.get(form=form, key="firstName")
+        first_name_var2 = FormVariable.objects.get(form=other_form, key="firstName")
+
+        recouple_submission_variables_to_form_variables(form.id)
+
+        with self.subTest(
+            "Existing submission values are recoupled with new variables"
+        ):
+            submission_vars1 = SubmissionValueVariable.objects.filter(
+                submission__form=form
+            )
+
+            self.assertEqual(submission_vars1.count(), 2)
+
+            first_name_submission_var1, _ = submission_vars1.order_by("key")
+
+            self.assertEqual(first_name_submission_var1.form_variable, first_name_var1)
+
+        with self.subTest(
+            "Existing submission values for other form are recoupled with new variables"
+        ):
+            submission_vars2 = SubmissionValueVariable.objects.filter(
+                submission__form=other_form
+            )
+
+            self.assertEqual(submission_vars2.count(), 1)
+
+            [first_name_submission_var2] = submission_vars2
+
+            self.assertEqual(first_name_submission_var2.form_variable, first_name_var2)
+
+
 @tag("gh-4824")
 @override_settings(CELERY_TASK_ALWAYS_EAGER=True)
 class OnFormStepSaveEventTests(TestCase):
@@ -337,15 +425,106 @@ def test_on_formstep_save_event_fixes_broken_form_variables_state(self):
 
             self.assertEqual(first_name_submission_var1.form_variable, first_name_var1)
 
-        # with self.subTest(
-        #     "Existing submission values for other form are recoupled with new variables"
-        # ):
-        #     submission_vars2 = SubmissionValueVariable.objects.filter(
-        #         submission__form=other_form
-        #     )
+        with self.subTest(
+            "Existing submission values for other form are recoupled with new variables"
+        ):
+            submission_vars2 = SubmissionValueVariable.objects.filter(
+                submission__form=other_form
+            )
+
+            self.assertEqual(submission_vars2.count(), 1)
+
+            [first_name_submission_var2] = submission_vars2
+
+            self.assertEqual(first_name_submission_var2.form_variable, first_name_var2)
+
+
+@override_settings(CELERY_TASK_ALWAYS_EAGER=True)
+class OnVariablesBulkUpdateEventTests(TestCase):
+    def test_on_variables_bulk_update_event(self):
+        form = FormFactory.create()
+        other_form = FormFactory.create()
+        form_definition = FormDefinitionFactory.create(
+            configuration={
+                "display": "form",
+                "components": [
+                    {
+                        "key": "firstName",
+                        "type": "textfield",
+                        "label": "First Name",
+                    },
+                    {
+                        "key": "lastName",
+                        "type": "textfield",
+                        "label": "Last Name",
+                    },
+                ],
+            },
+            is_reusable=True,
+        )
+        form_step1 = FormStepFactory.create(form=form, form_definition=form_definition)
+        form_step2 = FormStepFactory.create(
+            form=other_form, form_definition=form_definition
+        )
+
+        submission1 = SubmissionFactory.create(form=form)
+        submission2 = SubmissionFactory.create(form=other_form)
+
+        SubmissionStepFactory.create(
+            submission=submission1,
+            form_step=form_step1,
+            data={"firstName": "John"},
+        )
+        SubmissionStepFactory.create(
+            submission=submission2,
+            form_step=form_step2,
+            data={"firstName": "John"},
+        )
+
+        on_variables_bulk_update_event(form.id)
+
+        with self.subTest("Form has appropriate FormVariables"):
+            self.assertEqual(FormVariable.objects.filter(form=form).count(), 2)
+            first_name_var1, last_name_var1 = FormVariable.objects.filter(
+                form=form
+            ).order_by("pk")
+            self.assertEqual(first_name_var1.key, "firstName")
+            self.assertEqual(last_name_var1.key, "lastName")
+
+        with self.subTest(
+            "other Form that reuses this FormDefinition has appropriate FormVariables"
+        ):
+            self.assertEqual(FormVariable.objects.filter(form=other_form).count(), 2)
+
+            first_name_var2, last_name_var2 = FormVariable.objects.filter(
+                form=other_form
+            ).order_by("pk")
+
+            self.assertEqual(first_name_var2.key, "firstName")
+            self.assertEqual(last_name_var2.key, "lastName")
+
+        with self.subTest(
+            "Existing submission values are recoupled with new variables"
+        ):
+            submission_vars1 = SubmissionValueVariable.objects.filter(
+                submission__form=form
+            )
+
+            self.assertEqual(submission_vars1.count(), 1)
+
+            [first_name_submission_var1] = submission_vars1
+
+            self.assertEqual(first_name_submission_var1.form_variable, first_name_var1)
+
+        with self.subTest(
+            "Existing submission values for other form are recoupled with new variables"
+        ):
+            submission_vars2 = SubmissionValueVariable.objects.filter(
+                submission__form=other_form
+            )
 
-        #     self.assertEqual(submission_vars1.count(), 1)
+            self.assertEqual(submission_vars2.count(), 1)
 
-        #     [first_name_submission_var2] = submission_vars2
+            [first_name_submission_var2] = submission_vars2
 
-        #     self.assertEqual(first_name_submission_var2.form_variable, first_name_var2)
+            self.assertEqual(first_name_submission_var2.form_variable, first_name_var2)