From d69a30d265d834c43bc202eb5d41adaed0802efb Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Mon, 9 Dec 2024 13:32:03 +0100 Subject: [PATCH] :bug: [#4824] Schedule task to create FormVariables after saving FormStep this ensures that the Form cannot be left in a broken state with regards to FormVariables, because every component will always have a FormVariable this is a bandaid fix, because ideally we would have a single endpoint and the FormStep wouldn't be updated if there are errors with variables --- .../forms/api/serializers/form_step.py | 20 +++++++++ src/openforms/forms/tasks.py | 45 +++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/src/openforms/forms/api/serializers/form_step.py b/src/openforms/forms/api/serializers/form_step.py index 8fbf625c49..a9ef370d4a 100644 --- a/src/openforms/forms/api/serializers/form_step.py +++ b/src/openforms/forms/api/serializers/form_step.py @@ -1,9 +1,13 @@ +from functools import partial + +from django.db import transaction from django.db.models import Q from rest_framework import serializers from rest_framework_nested.relations import NestedHyperlinkedRelatedField from openforms.api.serializers import PublicFieldsSerializerMixin +from openforms.forms.tasks import on_formstep_save_event from openforms.translations.api.serializers import ( ComponentTranslationsSerializer, ModelTranslationsSerializer, @@ -170,3 +174,19 @@ def validate_form_definition(self, current_form_definition): ) return current_form_definition + + @transaction.atomic() + def save(self, **kwargs): + """ + Bandaid fix for #4824 + + Ensure that the FormVariables are in line with the state of the FormDefinitions + after saving + """ + instance = super().save(**kwargs) + + transaction.on_commit( + partial(on_formstep_save_event, instance.form.id, countdown=60) + ) + + return instance diff --git a/src/openforms/forms/tasks.py b/src/openforms/forms/tasks.py index c8c21203dd..8f8d382c8a 100644 --- a/src/openforms/forms/tasks.py +++ b/src/openforms/forms/tasks.py @@ -34,6 +34,28 @@ def on_variables_bulk_update_event(form_id: int) -> None: actions_chain.delay() +def on_formstep_save_event(form_id: int, countdown: int) -> None: + """ + Celery chain of tasks to execute after saving a FormStep. + """ + create_form_variables_for_components_task = create_form_variables_for_components.si( + form_id + ) + repopulate_reusable_definition_variables_task = ( + repopulate_reusable_definition_variables_to_form_variables.si(form_id) + ) + recouple_submission_variables_task = ( + recouple_submission_variables_to_form_variables.si(form_id) + ) + + actions_chain = chain( + create_form_variables_for_components_task, + repopulate_reusable_definition_variables_task, + recouple_submission_variables_task, + ) + actions_chain.apply_async(countdown=countdown) + + @app.task(ignore_result=True) def recouple_submission_variables_to_form_variables(form_id: int) -> None: """Recouple SubmissionValueVariable to FormVariable @@ -96,6 +118,29 @@ def repopulate_reusable_definition_variables_to_form_variables(form_id: int) -> FormVariable.objects.create_for_form(form) +@app.task(ignore_result=True) +@transaction.atomic() +def create_form_variables_for_components(form_id: int) -> None: + """Create FormVariables for each component in the Form + + This task is scheduled after creating/updating a FormStep, to ensure that the saved + Form has the appropriate FormVariables, even if errors occur in the variables update + endpoint. This is done to avoid leaving the Form in a broken state. + + See also: https://github.com/open-formulieren/open-forms/issues/4824#issuecomment-2514913073 + """ + from .models import Form, FormVariable # due to circular import + + form = Form.objects.get(id=form_id) + + # delete the existing form variables, we will re-create them + FormVariable.objects.filter( + form=form_id, source=FormVariableSources.component + ).delete() + + FormVariable.objects.create_for_form(form) + + @app.task() def activate_forms(): """Activate all the forms that should be activated by the specific date and time."""