Skip to content

Commit

Permalink
🚧 [#5139] Remove DataContainer usage from iter_evaluate_rules
Browse files Browse the repository at this point in the history
It's a quick test to see what happens if we were to remove the DataContainer from iter_evaluate_rules, and replace it with a FormioData instance.

Also added support for FormioData in recursive_apply to fix several failing tests.

test_two_actions_on_the_same_variable illustrates what happens when two actions are performed on the same variable.
  • Loading branch information
viktorvanwijk committed Mar 6, 2025
1 parent 1fc9168 commit e5db653
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 7 deletions.
5 changes: 5 additions & 0 deletions src/openforms/formio/datastructures.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,14 @@ class FormioData(UserDict):

data: dict[str, JSONValue]
_keys: set[str]
updates: dict[str, JSONValue]
"""
A collection of flattened key names, for quicker __contains__ access
"""

def __init__(self, *args, **kwargs):
self._keys = set()
self.updates = {}
super().__init__(*args, **kwargs)

def __getitem__(self, key: str):
Expand Down Expand Up @@ -206,3 +208,6 @@ def __contains__(self, key: object) -> bool:
return True
except KeyError:
return False

def track_updates(self, m: dict[str, JSONValue]) -> None:
self.updates.update(m)
3 changes: 2 additions & 1 deletion src/openforms/formio/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ def recursive_apply(
JSON serialization unless transform_leaf flag is set to True where func is
applied to the nested value as well.
"""
from openforms.formio.datastructures import FormioData
match input:
# string primitive - we can throw it into the template engine
case str():
Expand All @@ -460,7 +461,7 @@ def recursive_apply(
]

# mapping - map every key/value pair recursively
case dict():
case dict() | FormioData():
return {
key: recursive_apply(nested_bit, func, transform_leaf, *args, **kwargs)
for key, nested_bit in input.items()
Expand Down
12 changes: 10 additions & 2 deletions src/openforms/submissions/form_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,21 @@ def evaluate_form_logic(
# 5.1 - if the action type is to set a variable, update the variable state. This
# happens inside of iter_evaluate_rules. This is the ONLY operation that is allowed
# to execute while we're looping through the rules.
# TODO-5139: just a simple test by wrapping the data property of the container with
# a FormioData instance
data_for_logic = FormioData(data_container.data_without_to_python)
with elasticapm.capture_span(
name="collect_logic_operations", span_type="app.submissions.logic"
):
for operation in iter_evaluate_rules(
rules,
data_container,
data_for_logic,
submission=submission,
):
mutation_operations.append(operation)

submission_variables_state.set_values(data_for_logic.updates)

# 6. The variable state is now completely resolved - we can start processing the
# dynamic configuration and side effects.

Expand Down Expand Up @@ -209,9 +214,12 @@ def check_submission_logic(
data_container = DataContainer(state=submission_variables_state)

mutation_operations: list[ActionOperation] = []
for operation in iter_evaluate_rules(rules, data_container, submission):
data_for_logic = FormioData(data_container.data_without_to_python)
for operation in iter_evaluate_rules(rules, data_for_logic, submission):
mutation_operations.append(operation)

submission_variables_state.set_values(data_for_logic.updates)

# we loop over all steps because we have validations that ensure unique component
# keys across multiple steps for the whole form.
#
Expand Down
11 changes: 11 additions & 0 deletions src/openforms/submissions/logic/datastructures.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,24 @@ def data(self) -> DataMapping:
:return: A datamapping (key: variable key, value: variable value) ready for
(template context) evaluation.
"""
# TODO: this .to_python is needed for conversion of date/datetimes. Not sure yet
# if logic evaluation uses this in any way, though. Maybe we can get rid of it.
dynamic_values = {
key: variable.to_python() for key, variable in self.state.variables.items()
}
static_values = self.state.static_data()
nested_data = FormioData({**dynamic_values, **static_values})
return nested_data.data

@property
def data_without_to_python(self):
dynamic_values = {
key: variable.value for key, variable in self.state.variables.items()
}
static_values = self.state.static_data()
nested_data = FormioData({**dynamic_values, **static_values})
return nested_data.data

def update(self, updates: DataMapping) -> None:
"""
Update the dynamic data state.
Expand Down
11 changes: 7 additions & 4 deletions src/openforms/submissions/logic/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
import elasticapm
from json_logic import jsonLogic

from openforms.formio.datastructures import FormioData
from openforms.forms.models import FormLogic, FormStep

from ..models import Submission, SubmissionStep
from .actions import ActionOperation
from .datastructures import DataContainer
from .log_utils import log_errors


Expand Down Expand Up @@ -108,7 +108,7 @@ def get_current_step(submission: Submission) -> SubmissionStep | None:

def iter_evaluate_rules(
rules: Iterable[FormLogic],
data_container: DataContainer,
data_container: FormioData,
submission: Submission,
) -> Iterator[ActionOperation]:
"""
Expand Down Expand Up @@ -137,15 +137,18 @@ def iter_evaluate_rules(
triggered = False
with log_errors(rule.json_logic_trigger, rule):
triggered = bool(
jsonLogic(rule.json_logic_trigger, data_container.data)
jsonLogic(rule.json_logic_trigger, data_container)
)

if not triggered:
continue

for operation in rule.action_operations:
if mutations := operation.eval(
data_container.data, submission=submission
data_container, submission=submission
):
# TODO-5139: not sure if FormioData should track the changes, seems
# like a task for iter_evaluate_rules itself.
data_container.update(mutations)
data_container.track_updates(mutations)
yield operation
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,50 @@


class VariableModificationTests(TestCase):
def test_two_actions_on_the_same_variable(self):
form = FormFactory.create()
form_step = FormStepFactory.create(
form=form,
form_definition__configuration={
"components": [
{"type": "date", "key": "date"},
]
},
)
submission = SubmissionFactory.create(form=form)
submission_step = SubmissionStepFactory.create(
submission=submission,
form_step=form_step,
data={},
)

FormLogicFactory.create(
form=form,
json_logic_trigger=True,
actions=[
{
"variable": "date",
"action": {
"type": "variable",
"value": "2025-06-06",
},
},
{
"variable": "date",
"action": {
"type": "variable",
"value": {"+": [{"var": "date"}, {"duration": "P1M"}]},
},
}
],
)

evaluate_form_logic(submission, submission_step, submission.data)

variables_state = submission.load_submission_value_variables_state()

self.assertEqual(str(variables_state.variables["date"].value), "2025-07-06")

def test_modify_variable_related_to_step_being_edited(self):
form = FormFactory.create()
step1 = FormStepFactory.create(
Expand Down

0 comments on commit e5db653

Please sign in to comment.