✅ [#4824] Tests for task to create FormVariables for components

src/openforms/forms/tests/test_api_formsteps.py

@@ -856,6 +856,103 @@ def test_with_non_unique_fd_slugs(self):
         slugs = {form_step.slug for form_step in form_steps}
         self.assertEqual(len(slugs), 2)  # we expect two unique slugs
 
+    @patch(
+        "openforms.forms.api.serializers.form_step.on_formstep_save_event",
+        autospec=True,
+    )
+    @tag("gh-4824")
+    def test_create_form_step_schedules_task_chain_to_fix_form_variables(
+        self, mock_on_formstep_save_event
+    ):
+        """
+        Regression test for https://github.com/open-formulieren/open-forms/issues/4824
+        """
+        assign_change_form_permissions(self.user)
+        form = FormFactory.create()
+        url = reverse("api:form-steps-list", kwargs={"form_uuid_or_slug": form.uuid})
+        form_definition = FormDefinitionFactory.create(
+            configuration={
+                "display": "form",
+                "components": [
+                    {
+                        "key": "lastName",
+                        "type": "textfield",
+                        "label": "Last Name",
+                    },
+                ],
+            }
+        )
+        formdefinition_detail_url = reverse(
+            "api:formdefinition-detail",
+            kwargs={"uuid": form_definition.uuid},
+        )
+        data = {
+            "formDefinition": f"http://testserver{formdefinition_detail_url}",
+            "index": 0,
+        }
+        self.client.force_login(user=self.user)
+
+        with self.captureOnCommitCallbacks(execute=True):
+            response = self.client.post(url, data=data)
+
+        mock_on_formstep_save_event.assert_called_once_with(form.id, 60)
+
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self.assertEqual(
+            FormStep.objects.filter(form_definition=form_definition).count(),
+            1,
+        )
+
+    @patch(
+        "openforms.forms.api.serializers.form_step.on_formstep_save_event",
+        autospec=True,
+    )
+    @tag("gh-4824")
+    def test_update_form_step_schedules_task_chain_to_fix_form_variables(
+        self, mock_on_formstep_save_event
+    ):
+        """
+        Regression test for https://github.com/open-formulieren/open-forms/issues/4824
+        """
+        assign_change_form_permissions(self.user)
+        form_step = FormStepFactory.create()
+        form_definition = FormDefinitionFactory.create(
+            configuration={
+                "display": "form",
+                "components": [
+                    {
+                        "key": "lastName",
+                        "type": "textfield",
+                        "label": "Last Name",
+                    },
+                ],
+            }
+        )
+        url = reverse(
+            "api:form-steps-detail",
+            kwargs={"form_uuid_or_slug": form_step.form.uuid, "uuid": form_step.uuid},
+        )
+        formdefinition_detail_url = reverse(
+            "api:formdefinition-detail",
+            kwargs={"uuid": form_definition.uuid},
+        )
+        data = {
+            "formDefinition": f"http://testserver{formdefinition_detail_url}",
+            "index": 0,
+        }
+        self.client.force_login(user=self.user)
+
+        with self.captureOnCommitCallbacks(execute=True):
+            response = self.client.put(url, data=data)
+
+        mock_on_formstep_save_event.assert_called_once_with(form_step.form.id, 60)
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(
+            FormStep.objects.filter(form_definition=form_definition).count(),
+            1,
+        )
+
 
 class FormStepsAPITranslationTests(APITestCase):
     maxDiff = None

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

@@ -3,7 +3,7 @@
 from unittest.mock import patch
 
 from django.db import close_old_connections
-from django.test import TestCase, TransactionTestCase, tag
+from django.test import TestCase, TransactionTestCase, override_settings, tag
 
 from openforms.forms.tasks import (
     recouple_submission_variables_to_form_variables,
@@ -12,6 +12,7 @@
 from openforms.forms.tests.factories import (
     FormDefinitionFactory,
     FormFactory,
+    FormStepFactory,
     FormVariableFactory,
 )
 from openforms.submissions.models import SubmissionValueVariable
@@ -21,6 +22,9 @@
 )
 from openforms.variables.constants import FormVariableSources
 
+from ...models import FormVariable
+from ...tasks import create_form_variables_for_components, on_formstep_save_event
+
 
 class FormVariableTasksTest(TestCase):
     def test_recouple_submission_variables(self):
@@ -205,3 +209,143 @@ def race_condition():
 
         race_condition_thread.join()
         recouple_variables_thread.join()
+
+
+@tag("gh-4824")
+class CreateFormVariablesForComponentsTests(TestCase):
+    def test_create_form_variables_for_components(self):
+        form = FormFactory.create()
+        form_definition = FormDefinitionFactory.create(
+            configuration={
+                "display": "form",
+                "components": [
+                    {
+                        "key": "lastName",
+                        "type": "textfield",
+                        "label": "Last Name",
+                    },
+                ],
+            }
+        )
+        FormStepFactory.create(form=form, form_definition=form_definition)
+
+        # Form is in a broken state, because no FormVariable exists for `lastName`
+        FormVariable.objects.filter(form=form).delete()
+
+        with self.subTest("create variables for components"):
+            create_form_variables_for_components(form.id)
+
+            variables = FormVariable.objects.filter(form=form)
+
+            self.assertEqual(variables.count(), 1)
+
+            [variable] = variables
+
+            self.assertEqual(variable.form_definition, form_definition)
+            self.assertEqual(variable.source, "component")
+            self.assertEqual(variable.key, "lastName")
+
+        with self.subTest("task is idempotent"):
+            create_form_variables_for_components(form.id)
+
+            variables = FormVariable.objects.filter(form=form)
+
+            self.assertEqual(variables.count(), 1)
+
+
+@tag("gh-4824")
+@override_settings(CELERY_TASK_ALWAYS_EAGER=True)
+class OnFormStepSaveEventTests(TestCase):
+    def test_on_formstep_save_event_fixes_broken_form_variables_state(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"},
+        )
+
+        # Form is in a broken state, because no FormVariable exists for `lastName`
+        # Simulating the scenario where `lastName` was added but no variable was created
+        # because there are errors in the variables tab
+        FormVariable.objects.filter(form=form, key="lastName").delete()
+        FormVariable.objects.filter(form=other_form, key="lastName").delete()
+
+        on_formstep_save_event(form.id, 60)
+
+        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)
+
+        #     [first_name_submission_var2] = submission_vars2
+
+        #     self.assertEqual(first_name_submission_var2.form_variable, first_name_var2)