Skip to content

Commit

Permalink
🐛 [#4824] Schedule task to create FormVariables after saving FormStep
Browse files Browse the repository at this point in the history
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
  • Loading branch information
stevenbal committed Dec 10, 2024
1 parent 3e7b959 commit c98a928
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 0 deletions.
20 changes: 20 additions & 0 deletions src/openforms/forms/api/serializers/form_step.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
from functools import partial

from django.db import transaction
from django.db.models import Q

from rest_framework import serializers
Expand All @@ -10,6 +13,7 @@
)

from ...models import FormDefinition, FormStep
from ...tasks import on_formstep_save_event
from ...validators import validate_no_duplicate_keys_across_steps
from ..validators import FormStepIsApplicableIfFirstValidator
from .button_text import ButtonTextSerializer
Expand Down Expand Up @@ -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
45 changes: 45 additions & 0 deletions src/openforms/forms/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."""
Expand Down

0 comments on commit c98a928

Please sign in to comment.