Skip to content

Commit

Permalink
🐛 [#5058] Removed the submission value variable recoupling
Browse files Browse the repository at this point in the history
Before this patch, we ran some reconciliation when a form is edited to
ensure that existing SubmissionValueVariable instances don't have
'null' form_variable foreign keys because the FormVariable set of a
form was updated after editing (a) step(s) - the idea here was to be
eventually consistent. This turns out not to be necessary (if we can
trust our test suite) because the load_submission_value_variables_state
method on the Submission operates on the variable keys rather than
the FK relation and is able to properly resolve everything. This is
also the interface that developers should use when accessing the
submission values and it appears to be done properly in the
registration backends, otherwise tests would likely fail.

This re-coupling was extended in #4900, after it was noticed in #4824
that the re-coupling didn't happen for other forms that use the same
re-usable form definition. At the time, we didn't understand how this
seemingly didn't cause issues or at least didn't result in issues
being reported to us, but we can now conclude that it just wasn't a
problem in the first place because the proper interfaces/service layer
are/were being used and everything is done/reconciled in-memory when
comparing/populating submission variables with form variables.

A further cleanup step will then also be to remove this FK field from
the submission value variable model, as it is not necessary.
  • Loading branch information
sergei-maertens committed Jan 31, 2025
1 parent 9631c60 commit 9c61599
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 405 deletions.
67 changes: 2 additions & 65 deletions src/openforms/forms/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from functools import partial

from django.db import DatabaseError, transaction
from django.db.utils import IntegrityError
from django.utils import timezone

from celery import chain
Expand All @@ -18,20 +17,9 @@

def on_variables_bulk_update_event(form_id: int) -> None:
"""
Celery chain of tasks to execute on a bulk update of variables event.
Celery tasks to execute on a "bulk update of variables" event.
"""
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(
repopulate_reusable_definition_variables_task,
recouple_submission_variables_task,
)
actions_chain.delay()
repopulate_reusable_definition_variables_to_form_variables.delay(form_id=form_id)


def on_formstep_save_event(form_id: int, countdown: int) -> None:
Expand All @@ -44,65 +32,14 @@ def on_formstep_save_event(form_id: int, countdown: int) -> None:
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
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 and does the same for
other Forms in case of reusable FormDefinitions
"""
from openforms.submissions.models import SubmissionValueVariable

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()
}

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.")

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)
@transaction.atomic()
def repopulate_reusable_definition_variables_to_form_variables(form_id: int) -> None:
Expand Down
Loading

0 comments on commit 9c61599

Please sign in to comment.