From 4690fec59ebf05d7a93269c4e6ca03f7c9d1df6c Mon Sep 17 00:00:00 2001 From: Viktor van Wijk Date: Fri, 1 May 2026 16:04:28 +0200 Subject: [PATCH 01/10] :fire: [#6164] Remove new_logic_evaluation_enabled feature flag This will be the default in 4.0 :) Note that we want to execute `report_invalid_form_logic.py` as an upgrade check, which means it has to work in both 3.5 and 4.0, so we cannot remove the model field completely yet. --- docs/installation/observability/metrics.rst | 6 +- src/openforms/forms/api/serializers/form.py | 2 - .../forms/api/serializers/logic/form_logic.py | 15 +--- .../forms/api/v3/serializers/form.py | 1 - src/openforms/forms/api/v3/typing.py | 2 - src/openforms/forms/api/validators.py | 13 --- src/openforms/forms/logic_analysis.py | 5 +- src/openforms/forms/metrics.py | 4 - src/openforms/forms/models/form.py | 1 + src/openforms/forms/tests/admin/test_form.py | 2 +- .../tests/e2e_tests/test_form_designer.py | 6 +- src/openforms/forms/tests/factories.py | 6 +- .../forms/tests/test_api_form_logic_bulk.py | 12 +-- src/openforms/forms/tests/test_metrics.py | 19 ++--- src/openforms/forms/tests/test_models.py | 6 -- .../forms/tests/v3/test_endpoints.py | 1 - src/openforms/submissions/api/serializers.py | 3 +- src/openforms/submissions/logic/rules.py | 85 ------------------- .../submissions/rendering/renderer.py | 3 +- .../tests/form_logic/test_endpoint.py | 4 +- .../form_logic/test_evaluation_determinism.py | 2 +- .../form_logic/test_get_rules_to_evaluate.py | 41 ++------- .../tests/form_logic/test_modify_variables.py | 16 ++-- .../tests/form_logic/test_side_effects.py | 3 +- .../tests/form_logic/test_submission_logic.py | 11 ++- .../tests/test_get_submission_step.py | 11 +-- .../test_variables/test_update_with_logic.py | 2 +- 27 files changed, 49 insertions(+), 233 deletions(-) diff --git a/docs/installation/observability/metrics.rst b/docs/installation/observability/metrics.rst index 08ece78750..8d2c01e0c7 100644 --- a/docs/installation/observability/metrics.rst +++ b/docs/installation/observability/metrics.rst @@ -102,9 +102,9 @@ see the :ref:`Submission ` metri moved into the trash. Additional attributes are: - ``scope`` - fixed, set to ``global`` to enable de-duplication. - - ``type`` - one of ``total``, ``live``, ``new_logic_evaluation_enabled``, - ``translation_enabled``, ``is_regular``, ``is_appointment`` , ``is_single_step`` - or ``trash``. For all but ``trash`` the forms in the trash are excluded. + - ``type`` - one of ``total``, ``live``, ``translation_enabled``, ``is_regular``, + ``is_appointment`` , ``is_single_step`` or ``trash``. For all but ``trash`` the + forms in the trash are excluded. ``openforms.form_component_count`` Keeps track of how often a Formio component type is used in a form. This is only diff --git a/src/openforms/forms/api/serializers/form.py b/src/openforms/forms/api/serializers/form.py index bae18855bc..f1a4a499d7 100644 --- a/src/openforms/forms/api/serializers/form.py +++ b/src/openforms/forms/api/serializers/form.py @@ -306,7 +306,6 @@ class Meta: "submission_statements_configuration", "submission_report_download_link_title", "brp_personen_request_options", - "new_logic_evaluation_enabled", ) # allowlist for anonymous users public_fields = ( @@ -342,7 +341,6 @@ class Meta: "cosign_has_link_in_email", "submission_statements_configuration", "submission_report_download_link_title", - "new_logic_evaluation_enabled", ) extra_kwargs = { "uuid": { diff --git a/src/openforms/forms/api/serializers/logic/form_logic.py b/src/openforms/forms/api/serializers/logic/form_logic.py index a7a3bdc056..7e35150ab5 100644 --- a/src/openforms/forms/api/serializers/logic/form_logic.py +++ b/src/openforms/forms/api/serializers/logic/form_logic.py @@ -36,11 +36,7 @@ def validate(self, attrs): raise serializers.ValidationError( _("Appointment forms cannot have logic rules.") ) - if ( - not form.new_logic_evaluation_enabled - or is_appointment - or not form.form_step_map - ): + if is_appointment or not form.form_step_map: return super().validate(attrs) # This will only run when the logic rules have passed individual serializer @@ -96,11 +92,7 @@ def validate(self, attrs): def create(self, validated_data): rules = super().create(validated_data) form: Form = self.context["form"] - if ( - not form.new_logic_evaluation_enabled - or form.type == FormTypeChoices.appointment - or not form.form_step_map - ): + if form.type == FormTypeChoices.appointment or not form.form_step_map: return rules # the (auto-created) through model @@ -163,8 +155,7 @@ class FormLogicSerializer( label=_("form steps"), help_text=_( "Form steps on which the rule will be executed, determined by logic rule " - "analysis. Note that this is only relevant when the " - "`new_logic_evaluation_enabled` feature flag on the form is set to `True`." + "analysis." ), ) diff --git a/src/openforms/forms/api/v3/serializers/form.py b/src/openforms/forms/api/v3/serializers/form.py index 7b1a1ca801..d9fe8e1aa2 100644 --- a/src/openforms/forms/api/v3/serializers/form.py +++ b/src/openforms/forms/api/v3/serializers/form.py @@ -150,7 +150,6 @@ class Meta: # pyright: ignore[reportIncompatibleVariableOverride] "display_main_website_link", "include_confirmation_page_content_in_pdf", "translations", - "new_logic_evaluation_enabled", ) extra_kwargs = { "uuid": { # retrieved from the context passed through from the view diff --git a/src/openforms/forms/api/v3/typing.py b/src/openforms/forms/api/v3/typing.py index b949160c29..06a6224a92 100644 --- a/src/openforms/forms/api/v3/typing.py +++ b/src/openforms/forms/api/v3/typing.py @@ -186,6 +186,4 @@ class FormValidatedData(TypedDict): display_main_website_link: NotRequired[bool] include_confirmation_page_content_in_pdf: NotRequired[bool] - new_logic_evaluation_enabled: NotRequired[bool] - translations: NotRequired[FormTranslationsData] diff --git a/src/openforms/forms/api/validators.py b/src/openforms/forms/api/validators.py index 124689c5c4..836eaf313c 100644 --- a/src/openforms/forms/api/validators.py +++ b/src/openforms/forms/api/validators.py @@ -181,19 +181,6 @@ def __call__(self, attrs: dict, serializer: serializers.Serializer): if not form or not trigger_from_step: return - if form.new_logic_evaluation_enabled: - raise serializers.ValidationError( - { - "trigger_from_step": serializers.ErrorDetail( - _( - "'Trigger from step' is not supported when the new logic " - "evaluation is enabled." - ), - code="invalid", - ) - } - ) - if trigger_from_step.form != form: raise serializers.ValidationError( { diff --git a/src/openforms/forms/logic_analysis.py b/src/openforms/forms/logic_analysis.py index 02469d1f68..6e4e3be7cb 100644 --- a/src/openforms/forms/logic_analysis.py +++ b/src/openforms/forms/logic_analysis.py @@ -213,7 +213,7 @@ def analyze_rules( """ Determine which form steps are relevant for each logic rule in the form. - :param form: The form to analyze. It must have the new logic evaluation enabled. + :param form: The form to analyze. :param rules: Input rules for the form to analyze. If not provided, the current set of rules on the form will be used. The instances can be unsaved, in-memory instances. @@ -223,9 +223,6 @@ def analyze_rules( re-ordered according to their dependencies. The collection of steps specifies on which steps the rule must be executed. """ - if not form.new_logic_evaluation_enabled: - raise ValueError("Form to analyze must have the new logic evaluation enabled.") - if rules is None: rules = list(form.formlogic_set.order_by("order")) diff --git a/src/openforms/forms/metrics.py b/src/openforms/forms/metrics.py index 7c0c16ac1e..b53ba9d82c 100644 --- a/src/openforms/forms/metrics.py +++ b/src/openforms/forms/metrics.py @@ -16,10 +16,6 @@ def count_forms(options: metrics.CallbackOptions) -> Collection[metrics.Observat counts: dict[str, int] = Form.objects.aggregate( total=Count("id", filter=Q(_is_deleted=False)), live=Count("id", filter=Q(_is_deleted=False, active=True)), - new_logic_evaluation_enabled=Count( - "id", - filter=Q(_is_deleted=False, new_logic_evaluation_enabled=True), - ), translation_enabled=Count( "id", filter=Q(_is_deleted=False, translation_enabled=True) ), diff --git a/src/openforms/forms/models/form.py b/src/openforms/forms/models/form.py index 57a9c341d8..7527b434be 100644 --- a/src/openforms/forms/models/form.py +++ b/src/openforms/forms/models/form.py @@ -403,6 +403,7 @@ class Form(models.Model): ) # feature flags + # DeprecationWarning: remove in OF 4.1 new_logic_evaluation_enabled = models.BooleanField( _("enable new logic rule evaluation"), default=True, diff --git a/src/openforms/forms/tests/admin/test_form.py b/src/openforms/forms/tests/admin/test_form.py index 44a71a125a..79c83bb65e 100644 --- a/src/openforms/forms/tests/admin/test_form.py +++ b/src/openforms/forms/tests/admin/test_form.py @@ -538,7 +538,7 @@ def test_copy_form_with_trigger_from_step_in_logic(self): user = SuperUserFactory.create() self.client.force_login(user) - form_step = FormStepFactory.create(form__new_logic_evaluation_enabled=False) + form_step = FormStepFactory.create() FormLogicFactory.create(form=form_step.form, trigger_from_step=form_step) admin_url = reverse("admin:forms_form_change", args=(form_step.form.pk,)) diff --git a/src/openforms/forms/tests/e2e_tests/test_form_designer.py b/src/openforms/forms/tests/e2e_tests/test_form_designer.py index 7399900700..0d220d1228 100644 --- a/src/openforms/forms/tests/e2e_tests/test_form_designer.py +++ b/src/openforms/forms/tests/e2e_tests/test_form_designer.py @@ -876,7 +876,6 @@ def setUpTestData(): name_nl="Playwright test", generate_minimal_setup=True, formstep__form_definition__name_nl="Playwright test", - new_logic_evaluation_enabled=False, ) return form @@ -1235,10 +1234,7 @@ async def test_removing_step_doesnt_break_form(self): @sync_to_async def setUpTestData(): # set up a form - form = FormFactory.create( - name="Form with 2 steps", - new_logic_evaluation_enabled=False, - ) + form = FormFactory.create(name="Form with 2 steps") FormStepFactory.create( form=form, form_definition__configuration={ diff --git a/src/openforms/forms/tests/factories.py b/src/openforms/forms/tests/factories.py index d2557a0a5b..913c265cf8 100644 --- a/src/openforms/forms/tests/factories.py +++ b/src/openforms/forms/tests/factories.py @@ -267,10 +267,8 @@ class Params: @classmethod def _generate(cls, strategy, params): instance = super()._generate(strategy, params) - if instance.form.new_logic_evaluation_enabled and instance.trigger_from_step_id: - raise ValueError( - "Can't set trigger_from_step when new logic evaluation is enabled." - ) + if instance.trigger_from_step_id: + raise ValueError("Can't set trigger_from_step") return instance @factory.lazy_attribute diff --git a/src/openforms/forms/tests/test_api_form_logic_bulk.py b/src/openforms/forms/tests/test_api_form_logic_bulk.py index 67909d1ef2..ce24fcd5df 100644 --- a/src/openforms/forms/tests/test_api_form_logic_bulk.py +++ b/src/openforms/forms/tests/test_api_form_logic_bulk.py @@ -1124,7 +1124,7 @@ def test_mark_step_as_not_applicable_action_works_with_uuid(self): def test_create_form_logic_with_trigger_from_step(self): user = SuperUserFactory.create(username="test", password="test") self.client.force_authenticate(user=user) - form1, form2 = FormFactory.create_batch(2, new_logic_evaluation_enabled=False) + form1, form2 = FormFactory.create_batch(2) step1 = FormStepFactory.create( form=form1, form_definition__configuration={ @@ -1364,7 +1364,6 @@ def test_performance_validation_bulk_create_data(self): self.client.force_authenticate(user=user) form = FormFactory.create( generate_minimal_setup=True, - new_logic_evaluation_enabled=True, formstep__form_definition__configuration={ "components": [ {"type": "textfield", "key": "variable1"}, @@ -1430,7 +1429,7 @@ def test_performance_bulk_create_data_with_step_applicable_and_disable_next_acti """ user = SuperUserFactory.create() self.client.force_authenticate(user=user) - form = FormFactory.create(new_logic_evaluation_enabled=True) + form = FormFactory.create() step_1 = FormStepFactory.create( form=form, form_definition__configuration={ @@ -1758,7 +1757,7 @@ def setUp(self): self.client.force_authenticate(user=user) def test_create(self): - form = FormFactory.create(new_logic_evaluation_enabled=True) + form = FormFactory.create() step_1 = FormStepFactory.create( form=form, form_definition__configuration={ @@ -1875,7 +1874,6 @@ def test_create(self): def test_with_cycles(self): form = FormFactory.create( generate_minimal_setup=True, - new_logic_evaluation_enabled=True, formstep__form_definition__configuration={ "components": [ { @@ -1993,9 +1991,7 @@ def test_with_cycles(self): @tag("gh-6213") def test_create_without_steps(self): """Ensure there is no crash when a form is saved without any steps.""" - form = FormFactory.create( - new_logic_evaluation_enabled=True, generate_minimal_setup=False - ) + form = FormFactory.create(generate_minimal_setup=False) url = reverse("api:form-logic-rules", kwargs={"uuid_or_slug": form.uuid}) response = self.client.put(url, data=[]) diff --git a/src/openforms/forms/tests/test_metrics.py b/src/openforms/forms/tests/test_metrics.py index 9f354c3c87..fe055a85c6 100644 --- a/src/openforms/forms/tests/test_metrics.py +++ b/src/openforms/forms/tests/test_metrics.py @@ -25,12 +25,6 @@ def test_count_forms_by_type(self): FormFactory.create(deleted_=False, active=True, translation_enabled=True) FormFactory.create(deleted_=False, active=False, translation_enabled=True) FormFactory.create(deleted_=True, translation_enabled=True) - # without new renderer or new logic evaluation - FormFactory.create( - deleted_=False, - active=True, - new_logic_evaluation_enabled=False, - ) result = count_forms(CallbackOptions()) @@ -42,15 +36,12 @@ def test_count_forms_by_type(self): self.assertEqual( counts_by_type, { - # 1 appointment, 1 single step, 2 live, 2 with translations, 1 without - # new logic evaluation - "total": 1 + 1 + 2 + 2 + 1, - # 1 appointment, 1 single step, 2 active and not deleted, 1 active with translations, - # 1 without new renderer and logic evaluation - "live": 1 + 1 + 2 + 1 + 1, - "new_logic_evaluation_enabled": 6, # doesn't matter if active or not + # 1 appointment, 1 single step, 2 live, 2 with translations + "total": 1 + 1 + 2 + 2, + # 1 appointment, 1 single step, 2 active and not deleted, 1 active with translations + "live": 1 + 1 + 2 + 1, "translation_enabled": 2, # doesn't matter if they're active or not - "is_regular": 5, # don't consider deleted forms + "is_regular": 4, # don't consider deleted forms "is_appointment": 1, # don't consider deleted forms "is_single_step": 1, # don't consider deleted forms "trash": 1 + 1 + 1 + 1, diff --git a/src/openforms/forms/tests/test_models.py b/src/openforms/forms/tests/test_models.py index e308ae1f31..f91ece352d 100644 --- a/src/openforms/forms/tests/test_models.py +++ b/src/openforms/forms/tests/test_models.py @@ -294,12 +294,6 @@ def test_iter_components_ordering(self): self.assertEqual(actual, expected) - def test_analyse_logic_rules_for_legacy_logic_evaluation(self): - form = FormFactory.create(new_logic_evaluation_enabled=False) - - with self.assertRaises(ValueError): - form.apply_logic_analysis() - class RegressionTests(HypothesisTestCase): @given( diff --git a/src/openforms/forms/tests/v3/test_endpoints.py b/src/openforms/forms/tests/v3/test_endpoints.py index 75e8603725..797f7191bc 100644 --- a/src/openforms/forms/tests/v3/test_endpoints.py +++ b/src/openforms/forms/tests/v3/test_endpoints.py @@ -273,7 +273,6 @@ def test_create_detailed_form(self): "explanationTemplate": "Wees klaar om voor koekjes te vragen", }, }, - "newLogicEvaluationEnabled": True, } response = self.client.put(url, data=data) diff --git a/src/openforms/submissions/api/serializers.py b/src/openforms/submissions/api/serializers.py index ec1209dcdb..637aba3de9 100644 --- a/src/openforms/submissions/api/serializers.py +++ b/src/openforms/submissions/api/serializers.py @@ -388,8 +388,7 @@ def to_representation(self, instance): # configuration. logic_rules = list(instance.form_step.logic_rules.all()) require_backend = ( - not instance.form_step.form.new_logic_evaluation_enabled # backend is always required if the feature flag is disabled - or in_form_logic_evaluation # not relevant for the check-logic endpoint + in_form_logic_evaluation # not relevant for the check-logic endpoint or instance.form_step.is_backend_logic_evaluation_required or any(rule.is_backend_logic_evaluation_required for rule in logic_rules) ) diff --git a/src/openforms/submissions/logic/rules.py b/src/openforms/submissions/logic/rules.py index c94832e3d2..bd42cddfe4 100644 --- a/src/openforms/submissions/logic/rules.py +++ b/src/openforms/submissions/logic/rules.py @@ -19,19 +19,6 @@ tracer = trace.get_tracer("openforms.submissions.logic.rules") -def _include_rule(form_steps: list[FormStep], rule: FormLogic, step_index: int) -> bool: - # rules that always apply - if not rule.trigger_from_step: - return True - - trigger_from_index = form_steps.index(rule.trigger_from_step) - # if the current step is before the trigger-from step, do not include the rule - if step_index < trigger_from_index: - return False - - return True - - def get_rules_to_evaluate( submission: Submission, current_step: SubmissionStep | None = None, @@ -41,78 +28,6 @@ def get_rules_to_evaluate( """ Given a submission, return the logic rules ready for evaluation. - :param submission: A submission instance to retrieve the rules for - :param current_step: The (optional) step at which the rules need to be evaluated. - :param evaluate_all_rules: If ``True``, don't limit the returned rules to the - current submission step. Only relevant when the new logic evaluation is enabled. - :returns: An iterable of :class`openforms.forms.models.FormLogic` instances. This - may be a queryset, but could also be a list. - """ - if submission.form.new_logic_evaluation_enabled: - return get_rules_to_evaluate_new( - submission, current_step, evaluate_all_rules=evaluate_all_rules - ) - else: - return get_rules_to_evaluate_old(submission, current_step) - - -def get_rules_to_evaluate_old( - submission: Submission, current_step: SubmissionStep | None = None -) -> Iterable[FormLogic]: - """ - Given a submission, return the logic rules ready for evaluation. - - As a side-effect, the form logic query results are cached on ``submission.form``. - - :param submission: A submission instance to retrieve the rules for - :param current_step: The (optional) step at which the rules need to be evaluated. If - not provided, the step following the last completed step is used, or the first - step in the form if there are no completed steps. - :returns: An iterable of :class`openforms.forms.models.FormLogic` instances. This - may be a queryset, but could also be a list. Typically cached on the form instance - for performance reasons. - """ - # some callers evaluate logic for all steps at once, so we can avoid repeated queries - # by caching the rules on the form instance. - # Note that form.formlogic_set.all() is never cached by django, so we can't rely - # on that. - rules = getattr(submission.form, "_cached_logic_rules", None) - if rules is None: - rules = FormLogic.objects.select_related("trigger_from_step").filter( - form=submission.form - ) - submission.form._cached_logic_rules = rules - - submission_state = submission.load_execution_state() - # if there are no form steps, there is no usable form -> there are no logic rules - # to evaluate - if not submission_state.form_steps: - return [] - - # filter down the rules that are only applicable in the current step context - current_step = current_step or get_current_step(submission) - step_index = ( - submission_state.form_steps.index(current_step.form_step) - if current_step - else -1 - ) - - return [ - rule - for rule in rules - if _include_rule(submission_state.form_steps, rule, step_index) - ] - - -def get_rules_to_evaluate_new( - submission: Submission, - current_step: SubmissionStep | None = None, - *, - evaluate_all_rules: bool = False, -) -> Iterable[FormLogic]: - """ - Given a submission, return the logic rules ready for evaluation. - The logic rules are fetched from a many-to-many field on the form step. :param submission: A submission instance to retrieve the rules for diff --git a/src/openforms/submissions/rendering/renderer.py b/src/openforms/submissions/rendering/renderer.py index 8e728962aa..34d19a6c6e 100644 --- a/src/openforms/submissions/rendering/renderer.py +++ b/src/openforms/submissions/rendering/renderer.py @@ -66,8 +66,7 @@ def get_children(self) -> Iterator[SubmissionStepNode | VariablesNode]: """ Produce only the direct child nodes. """ - if self.submission.form.new_logic_evaluation_enabled: - prefetch_related_objects(self.steps, "form_step__logic_rules") + prefetch_related_objects(self.steps, "form_step__logic_rules") for step in self.steps: new_configuration = evaluate_form_logic( submission=self.submission, step=step diff --git a/src/openforms/submissions/tests/form_logic/test_endpoint.py b/src/openforms/submissions/tests/form_logic/test_endpoint.py index 19f7bcdf58..512962bed0 100644 --- a/src/openforms/submissions/tests/form_logic/test_endpoint.py +++ b/src/openforms/submissions/tests/form_logic/test_endpoint.py @@ -1289,8 +1289,8 @@ def test_already_submitted_value_is_used_when_component_is_hidden_with_clear_on_ # Logic rule should be triggered self.assertEqual(step_data["canSubmit"], False) - def test_step_data_with_new_logic_evaluation_flag_enabled(self): - form = FormFactory.create(new_logic_evaluation_enabled=True) + def test_step_data_does_not_contain_irrelevant_fields(self): + form = FormFactory.create() step = FormStepFactory.create( form=form, form_definition__configuration={ diff --git a/src/openforms/submissions/tests/form_logic/test_evaluation_determinism.py b/src/openforms/submissions/tests/form_logic/test_evaluation_determinism.py index 538720d0ba..4727038c68 100644 --- a/src/openforms/submissions/tests/form_logic/test_evaluation_determinism.py +++ b/src/openforms/submissions/tests/form_logic/test_evaluation_determinism.py @@ -81,7 +81,7 @@ def test_evaluate_rules_when_trigger_step_reached(self): Set up creates a form with three steps, the logic rule may only kick in from step2 onwards (i.e. - evaluate for step 2 and for step 3, but not step 1). """ - form = FormFactory.create(new_logic_evaluation_enabled=False) + form = FormFactory.create() step1 = FormStepFactory.create( form=form, form_definition__configuration={ diff --git a/src/openforms/submissions/tests/form_logic/test_get_rules_to_evaluate.py b/src/openforms/submissions/tests/form_logic/test_get_rules_to_evaluate.py index 7ab7aa30ad..1cfeb8c51a 100644 --- a/src/openforms/submissions/tests/form_logic/test_get_rules_to_evaluate.py +++ b/src/openforms/submissions/tests/form_logic/test_get_rules_to_evaluate.py @@ -17,11 +17,7 @@ class GetRulesToEvaluateTests(TestCase): def test_get_rules_to_evaluate(self): - """ - Ensure that the logic analysis feature flag influences which rules to execute - for each step. - """ - form = FormFactory.create(new_logic_evaluation_enabled=False) + form = FormFactory.create() step_1 = FormStepFactory.create( form=form, form_definition__configuration={ @@ -61,10 +57,6 @@ def test_get_rules_to_evaluate(self): } ], ) - # We set a value to the number component, so step 1 is assigned during logic rule - # analysis. - rule_1.form_steps.set([step_1]) - rule_2 = FormLogicFactory.create( form=form, json_logic_trigger={"==": [{"var": "checkbox"}, True]}, @@ -80,10 +72,6 @@ def test_get_rules_to_evaluate(self): } ], ) - # We change the hidden property of the textfield component, so step 2 is - # assigned during logic rule analysis. - rule_2.form_steps.set([step_2]) - rule_3 = FormLogicFactory.create( form=form, json_logic_trigger={"==": [{"var": "checkbox"}, True]}, @@ -99,9 +87,7 @@ def test_get_rules_to_evaluate(self): } ], ) - # We set a value to a user-defined variable, so step 1 is assigned based on the - # input variables. - rule_3.form_steps.set([step_1]) + form.apply_logic_analysis() submission = SubmissionFactory.create(form=form) submission_step_1 = SubmissionStepFactory.create( @@ -111,31 +97,18 @@ def test_get_rules_to_evaluate(self): submission=submission, form_step=step_2 ) - with self.subTest("Rules old"): - rules = get_rules_to_evaluate(submission, submission_step_1) - # Rule 3 is not included because of the "trigger_from_step" - self.assertEqual(list(rules), [rule_1, rule_2]) - - rules = get_rules_to_evaluate(submission, submission_step_2) - self.assertEqual(list(rules), [rule_1, rule_2, rule_3]) - - with self.subTest("Rules new"): - form.new_logic_evaluation_enabled = True - form.save(update_fields=["new_logic_evaluation_enabled"]) - - rules = get_rules_to_evaluate(submission, submission_step_1) - # The "trigger_from_step" on rule 3 is ignored here - self.assertEqual(list(rules), [rule_1, rule_3]) + rules = get_rules_to_evaluate(submission, submission_step_1) + self.assertEqual(list(rules), [rule_1, rule_3]) - rules = get_rules_to_evaluate(submission, submission_step_2) - self.assertEqual(list(rules), [rule_2]) + rules = get_rules_to_evaluate(submission, submission_step_2) + self.assertEqual(list(rules), [rule_2]) def test_get_rules_to_evaluate_new_iterating_over_rules_does_not_exhaust_them(self): """ Ensure iterating over the rules once does not exhaust them, as we iterate evaluation we iterate over the rules multiple times. """ - form = FormFactory.create(new_logic_evaluation_enabled=True) + form = FormFactory.create() step = FormStepFactory.create( form=form, form_definition__configuration={ diff --git a/src/openforms/submissions/tests/form_logic/test_modify_variables.py b/src/openforms/submissions/tests/form_logic/test_modify_variables.py index 5d2fcd303d..f990b9654d 100644 --- a/src/openforms/submissions/tests/form_logic/test_modify_variables.py +++ b/src/openforms/submissions/tests/form_logic/test_modify_variables.py @@ -176,7 +176,7 @@ def test_modify_variable_related_to_another_step_than_the_one_being_edited(self) self.assertEqual(state.get_data(include_unsaved=True)["nTotalBoxes"], 7) def test_logic_with_repeating_groups(self): - form = FormFactory.create(new_logic_evaluation_enabled=True) + form = FormFactory.create() form_step = FormStepFactory.create( form=form, form_definition__configuration={ @@ -284,7 +284,6 @@ def test_logic_with_repeating_groups(self): def test_dates_and_timedeltas(self): form = FormFactory.create( generate_minimal_setup=True, - new_logic_evaluation_enabled=True, formstep__form_definition__configuration={ "components": [ { @@ -358,7 +357,6 @@ def test_evaluate_dmn_action(self, m): form = FormFactory.create( generate_minimal_setup=True, - new_logic_evaluation_enabled=True, formstep__form_definition__configuration={ "components": [ { @@ -457,7 +455,6 @@ def test_evaluate_dmn_with_nested_variables(self, m): form = FormFactory.create( generate_minimal_setup=True, - new_logic_evaluation_enabled=True, formstep__form_definition__configuration={ "components": [ { @@ -571,7 +568,6 @@ def test_evaluate_dmn_action_returns_empty_data(self, m): form = FormFactory.create( generate_minimal_setup=True, - new_logic_evaluation_enabled=True, formstep__form_definition__configuration={ "components": [ { @@ -648,7 +644,7 @@ def test_two_actions_on_the_same_variable(self): variable. It ensures that the returns from jsonLogic calls are converted to the Python-type domain (date object in this case). """ - form = FormFactory.create(new_logic_evaluation_enabled=True) + form = FormFactory.create() form_step = FormStepFactory.create( form=form, form_definition__configuration={ @@ -695,7 +691,7 @@ def test_two_actions_on_the_same_variable(self): ) def test_children_synchronization_not_allowing_selection(self): - form = FormFactory.create(new_logic_evaluation_enabled=True) + form = FormFactory.create() step1 = FormStepFactory.create( form=form, form_definition__configuration={ @@ -832,7 +828,7 @@ def test_children_synchronization_not_allowing_selection(self): ) def test_children_synchronization_with_no_destination_data(self): - form = FormFactory.create(new_logic_evaluation_enabled=True) + form = FormFactory.create() step1 = FormStepFactory.create( form=form, form_definition__configuration={ @@ -963,7 +959,7 @@ def test_children_synchronization_with_no_destination_data(self): ) def test_children_synchronization_with_destination_data_update(self): - form = FormFactory.create(new_logic_evaluation_enabled=True) + form = FormFactory.create() step1 = FormStepFactory.create( form=form, form_definition__configuration={ @@ -1106,7 +1102,7 @@ def test_children_synchronization_with_destination_data_update(self): ) def test_children_synchronization_with_no_destination_and_source_data(self): - form = FormFactory.create(new_logic_evaluation_enabled=True) + form = FormFactory.create() step1 = FormStepFactory.create( form=form, form_definition__configuration={ diff --git a/src/openforms/submissions/tests/form_logic/test_side_effects.py b/src/openforms/submissions/tests/form_logic/test_side_effects.py index 982a75fb73..a475df4500 100644 --- a/src/openforms/submissions/tests/form_logic/test_side_effects.py +++ b/src/openforms/submissions/tests/form_logic/test_side_effects.py @@ -126,7 +126,7 @@ def test_blocked_submission_is_reset(self): Assert that subsequent steps are reset when they become not-applicable. """ # set up the form with logic - form = FormFactory.create(new_logic_evaluation_enabled=False) + form = FormFactory.create() step1 = FormStepFactory.create( form=form, form_definition__configuration={ @@ -162,6 +162,7 @@ def test_blocked_submission_is_reset(self): ], trigger_from_step=step2, ) + form.apply_logic_analysis() # set up a submission submission = SubmissionFactory.create(form=form) diff --git a/src/openforms/submissions/tests/form_logic/test_submission_logic.py b/src/openforms/submissions/tests/form_logic/test_submission_logic.py index 0a5167c510..d3e2bb0b0c 100644 --- a/src/openforms/submissions/tests/form_logic/test_submission_logic.py +++ b/src/openforms/submissions/tests/form_logic/test_submission_logic.py @@ -97,7 +97,7 @@ def test_check_logic_on_whole_submission_with_variables(self): becomes relevant from step3 onwards * We include calculated variables that determine the availability of step3 """ - form = FormFactory.create(new_logic_evaluation_enabled=False) + form = FormFactory.create() step1 = FormStepFactory.create( form=form, form_definition__configuration={ @@ -205,6 +205,7 @@ def test_check_logic_on_whole_submission_with_variables(self): } ], ) + form.apply_logic_analysis() # create a submission where only step1 is submitted submission = SubmissionFactory.create(form=form) SubmissionStepFactory.create( @@ -232,7 +233,7 @@ def test_properly_determine_current_step(self): * In step two the logic rule triggers that prevents us from continuing * We go back to step 1 and it should be possible to continue to step 2 from there """ - form = FormFactory.create(new_logic_evaluation_enabled=False) + form = FormFactory.create() step1 = FormStepFactory.create( form=form, form_definition__configuration={ @@ -285,6 +286,7 @@ def test_properly_determine_current_step(self): } ], ) + form.apply_logic_analysis() submission = SubmissionFactory.create(form=form) self._add_submission_to_session(submission) @@ -335,7 +337,7 @@ def test_properly_determine_current_step(self): data = response.json() # It should be possible to go to the next step, because the logic rule - # to block going to the next step should only trigger from step2 + # to block going to the next step should only trigger on step2 self.assertTrue(data["step"]["canSubmit"]) def test_check_logic_with_full_datetime(self): @@ -1289,7 +1291,6 @@ def test_clear_on_hide_behaviour_when_hiding_a_parent_uses_intermediate_cleared_ ): form = FormFactory.create( generate_minimal_setup=True, - new_logic_evaluation_enabled=True, formstep__form_definition__configuration={ "components": [ { @@ -1386,7 +1387,6 @@ def test_clear_on_hide_behaviour_when_hiding_a_parent_uses_intermediate_cleared_ def test_clear_on_hide_behaviour_applied_during_evaluation(self): form = FormFactory.create( generate_minimal_setup=True, - new_logic_evaluation_enabled=True, formstep__form_definition__configuration={ "components": [ { @@ -1474,7 +1474,6 @@ def test_clear_on_hide_behaviour_hiding_a_parent_does_not_update_nested_field_da ): form = FormFactory.create( generate_minimal_setup=True, - new_logic_evaluation_enabled=True, formstep__form_definition__configuration={ "components": [ { diff --git a/src/openforms/submissions/tests/test_get_submission_step.py b/src/openforms/submissions/tests/test_get_submission_step.py index 679139f5b3..b1528cc857 100644 --- a/src/openforms/submissions/tests/test_get_submission_step.py +++ b/src/openforms/submissions/tests/test_get_submission_step.py @@ -845,7 +845,6 @@ def test_default_configuration_has_components_rewritten_for_request( self, m_get_solo ): step = FormStepFactory.create( - form__new_logic_evaluation_enabled=True, form_definition__configuration={ "components": [ { @@ -884,7 +883,6 @@ def test_default_configuration_has_components_rewritten_for_request( def test_without_logic_rules_and_without_dynamic_configuration(self): step = FormStepFactory.create( - form__new_logic_evaluation_enabled=True, form_definition__configuration={ "components": [ {"type": "textfield", "key": "textfield", "label": "Textfield"} @@ -924,7 +922,6 @@ def test_without_logic_rules_and_without_dynamic_configuration(self): def test_without_logic_rules_but_with_dynamic_configuration(self): step = FormStepFactory.create( - form__new_logic_evaluation_enabled=True, form_definition__configuration={ "components": [ {"type": "textfield", "key": "textfield", "label": "{{ foo }}"} @@ -968,7 +965,6 @@ def test_without_logic_rules_but_with_dynamic_configuration(self): @freeze_time("2026-03-18") def test_with_logic_rule_that_does_not_require_backend(self): step = FormStepFactory.create( - form__new_logic_evaluation_enabled=True, form_definition__configuration={ "components": [ {"type": "date", "key": "dateOfBirth", "label": "Date of birth"}, @@ -1056,7 +1052,6 @@ def test_with_logic_rule_that_does_not_require_backend_but_with_dynamic_configur self, ): step = FormStepFactory.create( - form__new_logic_evaluation_enabled=True, form_definition__configuration={ "components": [ {"type": "date", "key": "dateOfBirth", "label": "Date of birth"}, @@ -1121,7 +1116,6 @@ def test_with_logic_rule_that_does_not_require_backend_but_with_dynamic_configur def test_with_logic_rule_that_requires_backend(self): step = FormStepFactory.create( - form__new_logic_evaluation_enabled=True, form_definition__configuration={ "components": [ {"type": "date", "key": "dateOfBirth", "label": "Date of birth"}, @@ -1187,7 +1181,7 @@ def test_with_logic_rule_that_requires_backend(self): @freeze_time("2026-03-18") def test_logic_rule_is_serialized_properly(self): - form = FormFactory.create(new_logic_evaluation_enabled=True) + form = FormFactory.create() step_1 = FormStepFactory.create( form=form, form_definition__configuration={ @@ -1303,7 +1297,6 @@ def test_logic_rule_is_serialized_properly(self): @freeze_time("2026-03-18") def test_with_date_trigger_that_could_be_partially_resolved(self): step = FormStepFactory.create( - form__new_logic_evaluation_enabled=True, form_definition__configuration={ "components": [ {"type": "date", "key": "dateOfBirth", "label": "Date of birth"}, @@ -1350,7 +1343,7 @@ def test_with_date_trigger_that_could_be_partially_resolved(self): self.assertEqual(expected, response.json()["logicRules"][0]["jsonLogicTrigger"]) def test_variable_action_with_json_logic_expression_as_value(self): - form = FormFactory.create(new_logic_evaluation_enabled=True) + form = FormFactory.create() step_1 = FormStepFactory.create( form=form, form_definition__configuration={ diff --git a/src/openforms/submissions/tests/test_variables/test_update_with_logic.py b/src/openforms/submissions/tests/test_variables/test_update_with_logic.py index a1636ed553..c5e98f4b9e 100644 --- a/src/openforms/submissions/tests/test_variables/test_update_with_logic.py +++ b/src/openforms/submissions/tests/test_variables/test_update_with_logic.py @@ -214,7 +214,7 @@ def test_that_updated_data_is_used_by_subsequent_rules(self): @tag("gh-5862") def test_that_updated_data_is_used_by_subsequent_actions(self): - form = FormFactory.create(new_logic_evaluation_enabled=True) + form = FormFactory.create() step1 = FormStepFactory.create( form=form, form_definition__configuration={ From f229ba2d8947a6fffd10f53f667a7b36139db5bb Mon Sep 17 00:00:00 2001 From: Viktor van Wijk Date: Fri, 1 May 2026 16:04:28 +0200 Subject: [PATCH 02/10] :fire: [#6164] Remove newLogicEvaluationEnabled from the admin --- .../components/admin/form_design/Context.js | 1 - .../form_design/FormConfigurationFields.js | 24 ------- .../components/admin/form_design/FormLogic.js | 71 +++++-------------- .../admin/form_design/FormLogic.stories.js | 2 - .../admin/form_design/form-creation-form.js | 3 - .../admin/form_design/story-decorators.js | 1 - 6 files changed, 17 insertions(+), 85 deletions(-) diff --git a/src/openforms/js/components/admin/form_design/Context.js b/src/openforms/js/components/admin/form_design/Context.js index 8c1ff2cd98..8fed5c058d 100644 --- a/src/openforms/js/components/admin/form_design/Context.js +++ b/src/openforms/js/components/admin/form_design/Context.js @@ -21,7 +21,6 @@ const FormContext = React.createContext({ plugins: {}, languages: [], translationEnabled: false, - newLogicEvaluationEnabled: false, updateComponents: () => {}, }); FormContext.displayName = 'FormContext'; diff --git a/src/openforms/js/components/admin/form_design/FormConfigurationFields.js b/src/openforms/js/components/admin/form_design/FormConfigurationFields.js index fc39334cd1..30353aff8f 100644 --- a/src/openforms/js/components/admin/form_design/FormConfigurationFields.js +++ b/src/openforms/js/components/admin/form_design/FormConfigurationFields.js @@ -549,7 +549,6 @@ const FormConfigurationFields = ({ askPrivacyConsent, askStatementOfTruth, appointmentOptions, - newLogicEvaluationEnabled, } = form; const intl = useIntl(); @@ -788,29 +787,6 @@ const FormConfigurationFields = ({

)} - - - } - helpText={ - - } - checked={newLogicEvaluationEnabled} - disabled={hasTriggerFromStep} - onChange={event => onCheckboxChange(event, newLogicEvaluationEnabled)} - /> - ); diff --git a/src/openforms/js/components/admin/form_design/FormLogic.js b/src/openforms/js/components/admin/form_design/FormLogic.js index c870a290e3..b443297242 100644 --- a/src/openforms/js/components/admin/form_design/FormLogic.js +++ b/src/openforms/js/components/admin/form_design/FormLogic.js @@ -199,12 +199,11 @@ const RuleBody = ({ const [resetRequest, setResetRequest] = useState(0); const triggerFromStep = useFormStep(triggerFromStepIdentifier); const formSteps = useFormSteps(formStepIdentifiers); - const {newLogicEvaluationEnabled, formSteps: allFormSteps} = useContext(FormContext); + const {formSteps: allFormSteps} = useContext(FormContext); // Case in which there is an error: a trigger step was specified, but this step cannot be found in the form - if (!triggerFromStep && triggerFromStepIdentifier && !newLogicEvaluationEnabled) + if (!triggerFromStep && triggerFromStepIdentifier) setDisplayAdvancedOptions(true); - displayAdvancedOptions = displayAdvancedOptions && !newLogicEvaluationEnabled; const TriggerComponent = isAdvanced ? AdvancedTrigger : Trigger; @@ -283,22 +282,20 @@ const RuleBody = ({ )} - {newLogicEvaluationEnabled && ( -
- {allFormSteps.length === formSteps.length ? ( - - ) : ( - step.stepName).join(', ')}} - /> - )} -
- )} +
+ {allFormSteps.length === formSteps.length ? ( + + ) : ( + step.stepName).join(', ')}} + /> + )} +
{isAdvanced ? ( @@ -353,7 +350,7 @@ const RuleBody = ({ expandExpression={expandExpression} /> - {triggerFromStep && !newLogicEvaluationEnabled && ( + {triggerFromStep && (
{ const intl = useIntl(); const [displayAdvancedOptions, setDisplayAdvancedOptions] = useState(false); - const {newLogicEvaluationEnabled} = useContext(FormContext); const deleteConfirmMessage = intl.formatMessage({ description: 'Logic rule deletion confirm message', @@ -446,39 +442,6 @@ const Rule = ({ return (
- {!newLogicEvaluationEnabled && ( -
- onChange({target: {name: 'order', value: order - 1}})} - /> - onChange({target: {name: 'order', value: order + 1}})} - /> -
- )} - {!newLogicEvaluationEnabled && ( - setDisplayAdvancedOptions(!displayAdvancedOptions)} - /> - )} { @@ -392,7 +391,6 @@ export const WithLogicRuleAnalysis = { availableFormVariables: AVAILABLE_FORM_VARIABLES, availableFormSteps: AVAILABLE_FORM_STEPS, - newLogicEvaluationEnabled: true, }, play: async ({canvasElement}) => { const canvas = within(canvasElement); diff --git a/src/openforms/js/components/admin/form_design/form-creation-form.js b/src/openforms/js/components/admin/form_design/form-creation-form.js index 101b2f80dc..21192542fc 100644 --- a/src/openforms/js/components/admin/form_design/form-creation-form.js +++ b/src/openforms/js/components/admin/form_design/form-creation-form.js @@ -122,7 +122,6 @@ const initialFormState = { brpPersonenProcessingHeaderValue: '', }, authBackends: [], - newLogicEvaluationEnabled: true, }, newForm: true, formSteps: [], @@ -185,7 +184,6 @@ const FORM_FIELDS_TO_TAB_NAMES = { variables: 'variables', appointmentOptions: 'form', brpPersonenRequestOptions: 'advanced-configuration', - newLogicEvaluationEnabled: 'form', }; const TRANSLATION_FIELD_TO_TAB_NAMES = { @@ -1269,7 +1267,6 @@ const FormCreationForm = ({formUuid, formUrl, formHistoryUrl, outgoingRequestsUr translationEnabled: state.form.translationEnabled, registrationBackends: state.form.registrationBackends, selectedAuthPlugins: state.selectedAuthPlugins, - newLogicEvaluationEnabled: state.form.newLogicEvaluationEnabled, updateComponents: updateComponents, }} > diff --git a/src/openforms/js/components/admin/form_design/story-decorators.js b/src/openforms/js/components/admin/form_design/story-decorators.js index 9460ff0110..c8be8e5913 100644 --- a/src/openforms/js/components/admin/form_design/story-decorators.js +++ b/src/openforms/js/components/admin/form_design/story-decorators.js @@ -96,7 +96,6 @@ export const FormDecorator = (Story, {args}) => ( }, ], translationEnabled: false, - newLogicEvaluationEnabled: args.newLogicEvaluationEnabled || false, updateComponents: args.updateComponents ?? (() => {}), }} > From 810fa400998a6474e33ccbc894885f6a3b6cfd9e Mon Sep 17 00:00:00 2001 From: Viktor van Wijk Date: Fri, 1 May 2026 16:11:35 +0200 Subject: [PATCH 03/10] :white_check_mark: [#6164] Remove redundant tests test_change_component_to_hidden_with_new_logic_evaluation: The new logic evaluation is now the only logic evaluation, so this test basically becomes a duplicate of `test_change_component_is_hidden` in this same class. test_modify_variable_related_to_another_step_than_the_one_being_edited: As the comment already indicated, this test does not make sense in this context. We would need to add a PUT call to the submission step detail endpoint to persist the user-defined variable. However, this would basically mean testing the fact that logic is executed before persisting the variables, which is already tested in `FormStepSubmissionTests.test_user_defined_variables_are_persisted`. Note that `test_evaluate_rules_when_trigger_step_reached` (will be renamed to `test_evaluate_rules_on_all_available_steps` in a next commit) already tests the behavior when a different step is being evaluated than the relevant step of the logic rule. Therefore, I have opted to remove it instead. --- .../form_logic/test_modify_components.py | 50 ------------ .../tests/form_logic/test_modify_variables.py | 81 ------------------- 2 files changed, 131 deletions(-) diff --git a/src/openforms/submissions/tests/form_logic/test_modify_components.py b/src/openforms/submissions/tests/form_logic/test_modify_components.py index 4721956c5a..a21cf51184 100644 --- a/src/openforms/submissions/tests/form_logic/test_modify_components.py +++ b/src/openforms/submissions/tests/form_logic/test_modify_components.py @@ -1422,53 +1422,3 @@ def test_variable_action_with_non_triggered_and_property_action(self): state = submission.variables_state data = state.get_data(include_unsaved=True) self.assertEqual("foo", data["textfield"]) - - def test_change_component_to_hidden_with_new_logic_evaluation(self): - form = FormFactory.create(new_logic_evaluation_enabled=True) - step = FormStepFactory.create( - form=form, - form_definition__configuration={ - "components": [ - { - "type": "checkbox", - "key": "checkbox", - "label": "Checkbox", - }, - { - "type": "textfield", - "key": "textfield", - "label": "Textfield", - "hidden": False, - }, - ] - }, - ) - FormLogicFactory.create( - form=form, - json_logic_trigger={"==": [{"var": "checkbox"}, True]}, - actions=[ - { - "component": "textfield", - "action": { - "name": "Hide textfield", - "type": "property", - "property": { - "type": "bool", - "value": "hidden", - }, - "state": True, - }, - } - ], - ) - form.apply_logic_analysis() - submission = SubmissionFactory.create(form=form) - submission_step = SubmissionStepFactory.create( - submission=submission, form_step=step - ) - - configuration = evaluate_form_logic( - submission, submission_step, FormioData({"checkbox": True}) - ) - - self.assertTrue(configuration["components"][1]["hidden"]) diff --git a/src/openforms/submissions/tests/form_logic/test_modify_variables.py b/src/openforms/submissions/tests/form_logic/test_modify_variables.py index f990b9654d..40c0fdbab8 100644 --- a/src/openforms/submissions/tests/form_logic/test_modify_variables.py +++ b/src/openforms/submissions/tests/form_logic/test_modify_variables.py @@ -94,87 +94,6 @@ def test_modify_variable_related_to_step_being_edited(self): state = submission.variables_state self.assertEqual(state.get_data(include_unsaved=True)["nTotalBoxes"], 7) - def test_modify_variable_related_to_another_step_than_the_one_being_edited(self): - # Note that with new logic evaluation, this test doesn't make much sense in this - # context. Evaluating form logic for step 2 will not execute the logic rule - # at all, since it will be assigned to step 1, and the user-defined variable - # will be persisted upon step submission. - form = FormFactory.create(new_logic_evaluation_enabled=False) - step1 = FormStepFactory.create( - form=form, - form_definition__configuration={ - "components": [ - { - "type": "number", - "key": "nLargeBoxes", - }, - { - "type": "number", - "key": "nGiganticBoxes", - }, - ] - }, - ) - step2 = FormStepFactory.create( - form=form, form_definition__configuration={"components": []} - ) - FormVariableFactory.create( - form=form, - key="nTotalBoxes", - source=FormVariableSources.user_defined, - data_type=FormVariableDataTypes.float, - form_definition=step1.form_definition, - ) - - FormLogicFactory.create( - form=form, - json_logic_trigger={ - "and": [ - { - "!=": [ - {"var": "nLargeBoxes"}, - None, - ] - }, - { - "!=": [ - {"var": "nGiganticBoxes"}, - None, - ] - }, - ] - }, - actions=[ - { - "variable": "nTotalBoxes", - "action": { - "name": "Update variable", - "type": "variable", - "value": { - "+": [{"var": "nLargeBoxes"}, {"var": "nGiganticBoxes"}] - }, - }, - } - ], - ) - - submission = SubmissionFactory.create(form=form) - SubmissionStepFactory.create( - submission=submission, - form_step=step1, - data={"nLargeBoxes": 2, "nGiganticBoxes": 5}, - ) - # Step being edited - submission_step2 = SubmissionStepFactory.build( - submission=submission, - form_step=step2, - ) - - evaluate_form_logic(submission, submission_step2) - - state = submission.variables_state - self.assertEqual(state.get_data(include_unsaved=True)["nTotalBoxes"], 7) - def test_logic_with_repeating_groups(self): form = FormFactory.create() form_step = FormStepFactory.create( From 790d7b08e9a58dddb550df38731e9db2d7b4195c Mon Sep 17 00:00:00 2001 From: Viktor van Wijk Date: Fri, 1 May 2026 17:10:24 +0200 Subject: [PATCH 04/10] :fire: [#6164] Remove trigger_from_step from FormLogic model Kinda goes hand in hand with making the new logic evaluation the default one, as a trigger-from-step is not compatible with it. Note that we want to execute `report_invalid_form_logic.py` as an upgrade check, which means it has to work in both 3.5 and 4.0, so we cannot remove the model field completely yet. I have removed tests that tested trigger-from-step behavior, and modified them where applicable. Note on test_check_logic_on_whole_submission_with_variables: Tricky one, because it combined trigger-from-step behavior with `check_submission_logic`. We can force a certain step by using a component instead of a user-defined variable. Logic analysis will then assign it to the output step. I have made it pass for now, but I think this test also heavily depends on (the order of) the rules executed during `check_submission_logic` - see #6151. --- src/openforms/forms/admin/form_logic.py | 5 +- .../forms/api/serializers/logic/form_logic.py | 21 +-- src/openforms/forms/api/validators.py | 24 ---- src/openforms/forms/api/viewsets.py | 4 +- src/openforms/forms/models/form.py | 7 +- src/openforms/forms/models/logic.py | 17 +-- src/openforms/forms/tests/admin/test_form.py | 19 --- .../tests/e2e_tests/test_form_designer.py | 124 ------------------ src/openforms/forms/tests/factories.py | 7 - .../forms/tests/test_api_form_logic_bulk.py | 109 --------------- src/openforms/forms/tests/test_models.py | 26 ---- src/openforms/submissions/logic/rules.py | 2 +- .../form_logic/test_evaluation_determinism.py | 16 +-- .../form_logic/test_get_rules_to_evaluate.py | 1 - .../tests/form_logic/test_side_effects.py | 1 - .../tests/form_logic/test_submission_logic.py | 46 +++---- 16 files changed, 38 insertions(+), 391 deletions(-) diff --git a/src/openforms/forms/admin/form_logic.py b/src/openforms/forms/admin/form_logic.py index dd0f393b4f..065ed49c71 100644 --- a/src/openforms/forms/admin/form_logic.py +++ b/src/openforms/forms/admin/form_logic.py @@ -12,17 +12,16 @@ class FormLogicAdmin(OrderedModelAdmin): "uuid", "form_admin_name", "description", - "trigger_from_step", "is_advanced", "move_up_down_links", ) - list_select_related = ("form", "trigger_from_step") + list_select_related = ("form",) list_filter = ( "is_advanced", "form", ) search_fields = ("uuid", "description", "json_logic_trigger") - raw_id_fields = ("form", "trigger_from_step") + raw_id_fields = ("form",) readonly_fields = ("form_steps",) diff --git a/src/openforms/forms/api/serializers/logic/form_logic.py b/src/openforms/forms/api/serializers/logic/form_logic.py index 7e35150ab5..d637742c86 100644 --- a/src/openforms/forms/api/serializers/logic/form_logic.py +++ b/src/openforms/forms/api/serializers/logic/form_logic.py @@ -18,7 +18,6 @@ ) from ....models import Form, FormLogic, FormStep from ...validators import ( - FormLogicTriggerFromStepFormValidator, JsonLogicTriggerValidator, JsonLogicValidator, ) @@ -125,20 +124,6 @@ class FormLogicSerializer( required=True, context_name="forms", ) - trigger_from_step = NestedHyperlinkedRelatedField( - required=False, - allow_null=True, - queryset=FormStep.objects, - view_name="api:form-steps-detail", - lookup_field="uuid", - parent_lookup_kwargs={"form_uuid_or_slug": "form__uuid"}, - label=_("trigger from step"), - help_text=_( - "When set, the trigger will only be checked once the specified step is reached. " - "This means the rule will never trigger for steps before the specified trigger step. " - "If unset, the trigger will always be checked." - ), - ) actions = LogicComponentActionSerializer( many=True, label=_("Actions"), @@ -169,7 +154,6 @@ class Meta: "json_logic_trigger", "description", "order", - "trigger_from_step", "actions", "is_advanced", "form_steps", @@ -199,10 +183,7 @@ class Meta: ), }, } - validators = [ - JsonLogicTriggerValidator("json_logic_trigger"), - FormLogicTriggerFromStepFormValidator(), - ] + validators = [JsonLogicTriggerValidator("json_logic_trigger")] def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/src/openforms/forms/api/validators.py b/src/openforms/forms/api/validators.py index 836eaf313c..aab13f5329 100644 --- a/src/openforms/forms/api/validators.py +++ b/src/openforms/forms/api/validators.py @@ -170,30 +170,6 @@ def __call__(self, configuration: JSONObject) -> None: validator(component) -class FormLogicTriggerFromStepFormValidator: - requires_context = True - - def __call__(self, attrs: dict, serializer: serializers.Serializer): - trigger_from_step = get_from_serializer_data_or_instance( - "trigger_from_step", attrs, serializer - ) - form = get_from_serializer_data_or_instance("form", attrs, serializer) - if not form or not trigger_from_step: - return - - if trigger_from_step.form != form: - raise serializers.ValidationError( - { - "trigger_from_step": serializers.ErrorDetail( - _( - "You must specify a step that belongs to the same form as the logic rule itself." - ), - code="invalid", - ) - } - ) - - class FormStepIsApplicableIfFirstValidator: def __call__(self, attrs: dict): if not attrs.get("is_applicable", True) and attrs.get("order") == 0: diff --git a/src/openforms/forms/api/viewsets.py b/src/openforms/forms/api/viewsets.py index fbc941015f..d843693db9 100644 --- a/src/openforms/forms/api/viewsets.py +++ b/src/openforms/forms/api/viewsets.py @@ -624,9 +624,7 @@ def logic_rules_bulk_update(self, request, *args, **kwargs): def logic_rules_list(self, request, *args, **kwargs): form = self.get_object() - logic_rules = form.formlogic_set.prefetch_related( - "form", "trigger_from_step__form", "form_steps" - ) + logic_rules = form.formlogic_set.prefetch_related("form", "form_steps") serializer = FormLogicSerializer( instance=logic_rules, diff --git a/src/openforms/forms/models/form.py b/src/openforms/forms/models/form.py index 7527b434be..2332147cef 100644 --- a/src/openforms/forms/models/form.py +++ b/src/openforms/forms/models/form.py @@ -618,16 +618,11 @@ def copy(self): form_step.save() # logic rules - for logic in self.formlogic_set.all().select_related("trigger_from_step"): + for logic in self.formlogic_set.all(): logic.pk = None logic.uuid = _uuid.uuid4() logic.form = copy - if logic.trigger_from_step: - logic.trigger_from_step = logic.form.formstep_set.get( - order=logic.trigger_from_step.order - ) - # make sure we have the new uuids of the copied form steps for action in logic.actions: # form_step_uuid is not a required field and if provided can be None as well diff --git a/src/openforms/forms/models/logic.py b/src/openforms/forms/models/logic.py index 2c4ba384d6..a757574d6e 100644 --- a/src/openforms/forms/models/logic.py +++ b/src/openforms/forms/models/logic.py @@ -5,7 +5,6 @@ from collections.abc import Collection, Iterator, Mapping from typing import TYPE_CHECKING -from django.core.exceptions import ValidationError from django.db import models from django.utils.translation import gettext_lazy as _ @@ -41,6 +40,7 @@ class FormLogic(OrderedModel): blank=True, related_name="logic_rules", ) + # DeprecationWarning: remove in OF 4.1 trigger_from_step = models.ForeignKey( "FormStep", on_delete=models.SET_NULL, @@ -78,21 +78,6 @@ class Meta(OrderedModel.Meta): verbose_name = _("form logic") verbose_name_plural = _("form logic rules") - def clean(self): - super().clean() - - if ( - self.trigger_from_step - and self.form - and self.trigger_from_step.form != self.form - ): - raise ValidationError( - _( - "You must specify a step that belongs to the same form as the logic rule itself." - ), - code="invalid", - ) - @property def action_operations(self) -> Iterator[ActionOperation]: from openforms.submissions.logic.actions import compile_action_operation diff --git a/src/openforms/forms/tests/admin/test_form.py b/src/openforms/forms/tests/admin/test_form.py index 79c83bb65e..5b63e98b0f 100644 --- a/src/openforms/forms/tests/admin/test_form.py +++ b/src/openforms/forms/tests/admin/test_form.py @@ -533,25 +533,6 @@ def test_form_admin_copy(self): copied_form.confirmation_email_template.id, confirmation_email_template.id ) - @tag("gh-3378") - def test_copy_form_with_trigger_from_step_in_logic(self): - user = SuperUserFactory.create() - self.client.force_login(user) - - form_step = FormStepFactory.create() - FormLogicFactory.create(form=form_step.form, trigger_from_step=form_step) - - admin_url = reverse("admin:forms_form_change", args=(form_step.form.pk,)) - - # React UI renders this input, so simulate it in a raw POST call - self.client.post(admin_url, data={"_copy": "Copy"}) - - copied_form = Form.objects.exclude(pk=form_step.form.pk).get() - copied_step = FormStep.objects.exclude(pk=form_step.pk).get() - copied_logic = copied_form.formlogic_set.get() - - self.assertEqual(copied_logic.trigger_from_step, copied_step) - @disable_admin_mfa() class FormAdminActionsTests(FormListAjaxMixin, WebTest): diff --git a/src/openforms/forms/tests/e2e_tests/test_form_designer.py b/src/openforms/forms/tests/e2e_tests/test_form_designer.py index 0d220d1228..be3adeeff3 100644 --- a/src/openforms/forms/tests/e2e_tests/test_form_designer.py +++ b/src/openforms/forms/tests/e2e_tests/test_form_designer.py @@ -31,10 +31,8 @@ from .helpers import ( click_modal_button, close_modal, - enter_json_in_editor, open_component_options_modal, phase, - skip_on_webtest, ) @@ -862,76 +860,6 @@ def assertState(): await assertState() - @skip_on_webtest - async def test_logic_rule_trigger_from_step_show_saved_value_in_select(self): - """ - Regression test for https://github.com/open-formulieren/open-forms/issues/2636 - """ - - @sync_to_async - def setUpTestData(): - # set up a form - form = FormFactory.create( - name="Playwright test", - name_nl="Playwright test", - generate_minimal_setup=True, - formstep__form_definition__name_nl="Playwright test", - ) - return form - - await create_superuser() - form = await setUpTestData() - - @sync_to_async - def get_formstep_uuid(): - return str(form.formstep_set.first().uuid) - - formstep_uuid = await get_formstep_uuid() - - admin_url = str( - furl(self.live_server_url) - / reverse("admin:forms_form_change", args=(form.pk,)) - ) - - async with browser_page() as page: - await self._admin_login(page) - await page.goto(str(admin_url)) - await page.get_by_role("tab", name="Logic").click() - - with phase("Add logic rule with triggerFromStep"): - await page.get_by_text("Add rule").click() - await page.get_by_role("button", name="Advanced").click() - await page.get_by_title("Advanced options").click() - await page.locator("[name='triggerFromStep']").select_option( - label="Playwright test" - ) - editor = page.locator(".monaco-editor") - await enter_json_in_editor(page, editor, {"==": [1, 1]}) - - with phase("Save logic rule and check state"): - await page.get_by_text("Save and continue editing").click() - await page.get_by_role("tab", name="Logic").click() - await page.get_by_title("Advanced options").click() - - # Verify that the select still holds the correct value - await expect(page.locator("[name='triggerFromStep']")).to_have_value( - formstep_uuid - ) - - @sync_to_async - def assertState(): - form_logic = form.formlogic_set - - self.assertEqual(form_logic.count(), 1) - - created_form_logic = form_logic.first() - - self.assertEqual( - created_form_logic.trigger_from_step, form.formstep_set.first() - ) - - await assertState() - @tag("gh-2769") async def test_max_min_date_validation(self): @sync_to_async @@ -1229,58 +1157,6 @@ def setUpTestData(): error_node = page.locator("css=.error") await expect(error_node).not_to_be_visible() - @tag("gh-3422") - async def test_removing_step_doesnt_break_form(self): - @sync_to_async - def setUpTestData(): - # set up a form - form = FormFactory.create(name="Form with 2 steps") - FormStepFactory.create( - form=form, - form_definition__configuration={ - "components": [{"type": "textfield", "key": "textA"}] - }, - ) - form_step2 = FormStepFactory.create( - form=form, - form_definition__configuration={ - "components": [{"type": "textfield", "key": "textB"}] - }, - ) - FormLogicFactory.create(form=form, trigger_from_step=form_step2) - return form - - await create_superuser() - form = await setUpTestData() - admin_url = str( - furl(self.live_server_url) - / reverse("admin:forms_form_change", args=(form.pk,)) - ) - - async with browser_page() as page: - await self._admin_login(page) - await page.goto(str(admin_url)) - - await page.get_by_role("tab", name="Steps and fields").click() - - # Delete the second step - sidebar = page.locator("css=.edit-panel__nav").get_by_role("list") - bin_icon = sidebar.get_by_role("listitem").nth(1).get_by_title("Delete") - await bin_icon.click() - await page.get_by_role("button", name="Confirm").click() - - await page.get_by_role("tab", name="Logic").click() - - # Check that you can delete the logic rule - bin_icon = page.get_by_title("Delete").first - await expect(bin_icon).to_be_visible() - - # Check that a warning is present - warning = page.get_by_role("listitem").get_by_text( - "The selected trigger step could not be found in this form! Please change it!" - ) - await expect(warning).to_be_visible() - @tag("gh-3921") async def test_all_components_are_visible_in_component_select_dropdown(self): @sync_to_async diff --git a/src/openforms/forms/tests/factories.py b/src/openforms/forms/tests/factories.py index 913c265cf8..0177b10d5d 100644 --- a/src/openforms/forms/tests/factories.py +++ b/src/openforms/forms/tests/factories.py @@ -264,13 +264,6 @@ class Params: price_variable = "totalPrice" price_value = 5.00 # literal value or JSON logic expression - @classmethod - def _generate(cls, strategy, params): - instance = super()._generate(strategy, params) - if instance.trigger_from_step_id: - raise ValueError("Can't set trigger_from_step") - return instance - @factory.lazy_attribute def actions(self): if self.for_submission_price: # type: ignore diff --git a/src/openforms/forms/tests/test_api_form_logic_bulk.py b/src/openforms/forms/tests/test_api_form_logic_bulk.py index ce24fcd5df..4a1aae5f50 100644 --- a/src/openforms/forms/tests/test_api_form_logic_bulk.py +++ b/src/openforms/forms/tests/test_api_form_logic_bulk.py @@ -1121,115 +1121,6 @@ def test_mark_step_as_not_applicable_action_works_with_uuid(self): self.assertEqual(response.status_code, status.HTTP_200_OK) - def test_create_form_logic_with_trigger_from_step(self): - user = SuperUserFactory.create(username="test", password="test") - self.client.force_authenticate(user=user) - form1, form2 = FormFactory.create_batch(2) - step1 = FormStepFactory.create( - form=form1, - form_definition__configuration={ - "components": [{"type": "textfield", "key": "c1"}] - }, - ) - unrelated_step = FormStepFactory.create( - form=form2, - form_definition__configuration={ - "components": [{"type": "textfield", "key": "c2"}] - }, - ) - _form_logic_rule_data = { - "form": f"http://testserver{reverse('api:form-detail', kwargs={'uuid_or_slug': form1.uuid})}", - "order": 0, - "json_logic_trigger": { - "==": [ - {"var": "c1"}, - "hide step 1", - ] - }, - "actions": [], - } - - with self.subTest("Create logic rule with trigger_from_step set"): - step1_path = reverse( - "api:form-steps-detail", - kwargs={"form_uuid_or_slug": form1.uuid, "uuid": step1.uuid}, - ) - data = [ - { - **_form_logic_rule_data, - "triggerFromStep": f"http://testserver{step1_path}", - } - ] - - url = reverse("api:form-logic-rules", kwargs={"uuid_or_slug": form1.uuid}) - response = self.client.put(url, data=data) - - self.assertEqual(response.status_code, status.HTTP_200_OK) - - with self.subTest( - "Reject logic rule with trigger_from_step belonging to another form" - ): - step2_path = reverse( - "api:form-steps-detail", - kwargs={"form_uuid_or_slug": form2.uuid, "uuid": unrelated_step.uuid}, - ) - data = [ - { - **_form_logic_rule_data, - "triggerFromStep": f"http://testserver{step2_path}", - } - ] - - url = reverse("api:form-logic-rules", kwargs={"uuid_or_slug": form1.uuid}) - response = self.client.put(url, data=data) - - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual( - response.data["invalid_params"][0]["name"], "0.triggerFromStep" - ) - self.assertEqual(response.data["invalid_params"][0]["code"], "invalid") - - def test_create_form_logic_with_trigger_from_step_forbidden_with_new_logic_evaluation( - self, - ): - user = SuperUserFactory.create(username="test", password="test") - self.client.force_authenticate(user=user) - form = FormFactory.create( - new_logic_evaluation_enabled=True, - generate_minimal_setup=True, - formstep__form_definition__configuration={ - "components": [{"type": "textfield", "key": "textfield"}] - }, - ) - form_step = form.formstep_set.get() - step1_path = reverse( - "api:form-steps-detail", - kwargs={"form_uuid_or_slug": form.uuid, "uuid": form_step.uuid}, - ) - rule_data = [ - { - "form": f"http://testserver{reverse('api:form-detail', kwargs={'uuid_or_slug': form.uuid})}", - "order": 0, - "json_logic_trigger": { - "==": [ - {"var": "textfield"}, - "hide step 1", - ] - }, - "triggerFromStep": f"http://testserver{step1_path}", - "actions": [], - } - ] - url = reverse("api:form-logic-rules", kwargs={"uuid_or_slug": form.uuid}) - - response = self.client.put(url, data=rule_data) - - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - response_data = response.json() - error = response_data["invalidParams"][0] - self.assertEqual(error["name"], "0.triggerFromStep") - self.assertEqual(error["code"], "invalid") - def test_create_form_logic_doesnt_crash(self): user = SuperUserFactory.create(username="test", password="test") form = FormFactory.create() diff --git a/src/openforms/forms/tests/test_models.py b/src/openforms/forms/tests/test_models.py index f91ece352d..5939573ca9 100644 --- a/src/openforms/forms/tests/test_models.py +++ b/src/openforms/forms/tests/test_models.py @@ -623,32 +623,6 @@ def test_template_expression_multiple(self): class FormLogicTests(TestCase): - def test_block_form_logic_trigger_step_other_form(self): - form1, form2 = FormFactory.create_batch(2) - step = FormStepFactory.create(form=form1) - other_step = FormStepFactory.create(form=form2) - - with self.subTest("Invalid configuration"): - logic = FormLogicFactory.build(form=form1) - logic.trigger_from_step = other_step - with self.assertRaises(ValidationError): - logic.clean() - - with self.subTest("Valid configuration"): - logic = FormLogicFactory.build(form=form1) - logic.trigger_from_step = step - try: - logic.clean() - except ValidationError: - self.fail("Should be allowed") - - with self.subTest("No trigger from step configured"): - logic = FormLogicFactory.build(form=form1) - try: - logic.clean() - except ValidationError: - self.fail("Should be allowed") - def test_input_and_output_variable_keys(self): form = FormFactory.create( generate_minimal_setup=True, diff --git a/src/openforms/submissions/logic/rules.py b/src/openforms/submissions/logic/rules.py index bd42cddfe4..39be269b8c 100644 --- a/src/openforms/submissions/logic/rules.py +++ b/src/openforms/submissions/logic/rules.py @@ -10,7 +10,7 @@ FormioData, process_visibility, ) -from openforms.forms.models import FormLogic, FormStep +from openforms.forms.models import FormLogic from ..models import Submission, SubmissionStep from .actions import ActionOperation diff --git a/src/openforms/submissions/tests/form_logic/test_evaluation_determinism.py b/src/openforms/submissions/tests/form_logic/test_evaluation_determinism.py index 4727038c68..90f0940ae3 100644 --- a/src/openforms/submissions/tests/form_logic/test_evaluation_determinism.py +++ b/src/openforms/submissions/tests/form_logic/test_evaluation_determinism.py @@ -74,12 +74,12 @@ def test_logic_rule_order_respected(self): variable.value, 5 ) # rule with order 1 overwrites result of rule with order 0 - def test_evaluate_rules_when_trigger_step_reached(self): + def test_evaluate_rules_on_all_available_steps(self): """ - Test that only the rules are evaluated that reached the trigger step. + Test that only the rules are evaluated that are relevant for that step. - Set up creates a form with three steps, the logic rule may only kick in from - step2 onwards (i.e. - evaluate for step 2 and for step 3, but not step 1). + Set up creates a form with three steps, logic analysis will assign the rule to + the third step, so it should only evaluate for step 3. """ form = FormFactory.create() step1 = FormStepFactory.create( @@ -121,8 +121,7 @@ def test_evaluate_rules_when_trigger_step_reached(self): ) FormLogicFactory.create( form=form, - json_logic_trigger={"==": [1, 1]}, # trigger is always true - trigger_from_step=step2, + json_logic_trigger=True, actions=[ { "variable": "d", @@ -134,6 +133,7 @@ def test_evaluate_rules_when_trigger_step_reached(self): } ], ) + form.apply_logic_analysis() submission = SubmissionFactory.create(form=form) ss1 = SubmissionStepFactory.create( @@ -149,7 +149,7 @@ def test_evaluate_rules_when_trigger_step_reached(self): data = state.get_data(include_unsaved=True) self.assertEqual({"a": 2, "b": 4, "c": None, "d": None}, data) - with self.subTest("Evaluation not skipped on step 2"): + with self.subTest("Evaluation skipped on step 2"): ss2 = SubmissionStepFactory.create( submission=submission, form_step=step2, data={"c": 2} ) @@ -158,7 +158,7 @@ def test_evaluate_rules_when_trigger_step_reached(self): state = submission.variables_state data = state.get_data(include_unsaved=True) - self.assertEqual({"a": 2, "b": 4, "c": 2, "d": 6}, data) + self.assertEqual({"a": 2, "b": 4, "c": 2, "d": None}, data) with self.subTest("Evaluation not skipped on step 3"): ss3 = SubmissionStepFactory.create( diff --git a/src/openforms/submissions/tests/form_logic/test_get_rules_to_evaluate.py b/src/openforms/submissions/tests/form_logic/test_get_rules_to_evaluate.py index 1cfeb8c51a..63daf8e256 100644 --- a/src/openforms/submissions/tests/form_logic/test_get_rules_to_evaluate.py +++ b/src/openforms/submissions/tests/form_logic/test_get_rules_to_evaluate.py @@ -75,7 +75,6 @@ def test_get_rules_to_evaluate(self): rule_3 = FormLogicFactory.create( form=form, json_logic_trigger={"==": [{"var": "checkbox"}, True]}, - trigger_from_step=step_2, actions=[ { "action": { diff --git a/src/openforms/submissions/tests/form_logic/test_side_effects.py b/src/openforms/submissions/tests/form_logic/test_side_effects.py index a475df4500..87c99a03fa 100644 --- a/src/openforms/submissions/tests/form_logic/test_side_effects.py +++ b/src/openforms/submissions/tests/form_logic/test_side_effects.py @@ -160,7 +160,6 @@ def test_blocked_submission_is_reset(self): actions=[ {"form_step_uuid": str(step2.uuid), "action": {"type": "disable-next"}} ], - trigger_from_step=step2, ) form.apply_logic_analysis() diff --git a/src/openforms/submissions/tests/form_logic/test_submission_logic.py b/src/openforms/submissions/tests/form_logic/test_submission_logic.py index d3e2bb0b0c..b679c1bfc3 100644 --- a/src/openforms/submissions/tests/form_logic/test_submission_logic.py +++ b/src/openforms/submissions/tests/form_logic/test_submission_logic.py @@ -109,13 +109,23 @@ def test_check_logic_on_whole_submission_with_variables(self): ] }, ) - # we don't progress through these and set up user-defined variables, so it doesn't matter - # that there are no components. + # we don't progress through these and set up user-defined variables, so it + # doesn't matter that there are no components. step2 = FormStepFactory.create( form=form, form_definition__configuration={"components": []} ) - step3 = FormStepFactory.create( - form=form, form_definition__configuration={"components": []} + FormStepFactory.create( + form=form, + form_definition__configuration={ + "components": [ + { + "type": "number", + "key": "number", + "label": "Number", + "defaultValue": 0, + } + ] + }, ) # set up a number of variables to mutate with logic FormVariableFactory.create( @@ -124,13 +134,6 @@ def test_check_logic_on_whole_submission_with_variables(self): user_defined=True, data_type=FormVariableDataTypes.float, ) - FormVariableFactory.create( - form=form, - key="var2", - user_defined=True, - data_type=FormVariableDataTypes.float, - initial_value=0, - ) FormVariableFactory.create( form=form, key="var3", @@ -138,7 +141,7 @@ def test_check_logic_on_whole_submission_with_variables(self): data_type=FormVariableDataTypes.float, ) # set up logic rules: - # 1. set the variable var1 to the value 10, always + # 1. set the variable var1 to the value 10, step 1 FormLogicFactory.create( form=form, json_logic_trigger={"==": [{"var": "text1"}, "trigger-rule"]}, @@ -149,19 +152,18 @@ def test_check_logic_on_whole_submission_with_variables(self): } ], ) - # 2. set the variable var2 to the value 5, only from step3 + # 2. set number to the value 5, step 3 FormLogicFactory.create( form=form, - trigger_from_step=step3, json_logic_trigger={"==": [{"var": "text1"}, "trigger-rule"]}, actions=[ { - "variable": "var2", + "variable": "number", "action": {"type": "variable", "value": 5}, } ], ) - # 3. set the variable var3 to the sum of var1 and var2, always. + # 3. set the variable var3 to the sum of var1 and number, step 1 and 3. # In this test, the resulting value is 10 (10 + 0) because step3 has not been reached. FormLogicFactory.create( form=form, @@ -171,13 +173,13 @@ def test_check_logic_on_whole_submission_with_variables(self): "variable": "var3", "action": { "type": "variable", - "value": {"+": [{"var": "var1"}, {"var": "var2"}]}, + "value": {"+": [{"var": "var1"}, {"var": "number"}]}, }, } ], ) # 4. Disable step 2 if var1 is not equal to 10 - this should not happen because - # we always set it + # var1 is set in the first logic rule, so it must have been executed already. FormLogicFactory.create( form=form, json_logic_trigger={"!=": [{"var": "var1"}, 10]}, @@ -191,8 +193,8 @@ def test_check_logic_on_whole_submission_with_variables(self): } ], ) - # if the rule gets executed that sets var2 to 5, then the total is 15 (which - # should not happen). We can verify this by blocking submission. + # 5. if the rule gets executed that sets number to 5, then the total is 15 + # (which should not happen). We can verify this by blocking submission. FormLogicFactory.create( form=form, json_logic_trigger={"!=": [{"var": "var3"}, 15]}, @@ -226,8 +228,7 @@ def test_check_logic_on_whole_submission_with_variables(self): @tag("gh-4579") def test_properly_determine_current_step(self): """ - Assert that the submission logic check properly determines the current step for - rules that define `triggerFromStep` + Assert that the submission logic check properly determines the current step. * We fill out step 1 and continue to step 2 * In step two the logic rule triggers that prevents us from continuing @@ -277,7 +278,6 @@ def test_properly_determine_current_step(self): # 2. if `verdergaan` is `nee`, block going to step3 FormLogicFactory.create( form=form, - trigger_from_step=step2, json_logic_trigger={"==": [{"var": "verdergaan"}, "nee"]}, actions=[ { From eece1578119103dae575e0355e56ba56d03b3826 Mon Sep 17 00:00:00 2001 From: Viktor van Wijk Date: Thu, 7 May 2026 12:59:21 +0200 Subject: [PATCH 05/10] :fire: [#6164] Remove check script for disable-next action This script must be run before upgrading to 3.5, and I think 3.5 will be a prerequisite before upgrading to 4.0, so it is no longer relevant --- Dockerfile | 1 - bin/check_disable_next_logic_action.py | 240 ------------------------- 2 files changed, 241 deletions(-) delete mode 100755 bin/check_disable_next_logic_action.py diff --git a/Dockerfile b/Dockerfile index c7d43c60c3..94cf5c01fb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -115,7 +115,6 @@ COPY \ ./bin/report_component_problems.py \ ./bin/fix_submission_value_variable_missing_fields.py \ ./bin/fix_payment_status.py \ - ./bin/check_disable_next_logic_action.py \ ./bin/report_completed_submissions_access.py \ ./bin/report_invalid_form_logic.py \ ./bin/report_logic_with_deprecated_clear_on_hide_behavior.py \ diff --git a/bin/check_disable_next_logic_action.py b/bin/check_disable_next_logic_action.py deleted file mode 100755 index c8e6a7da5c..0000000000 --- a/bin/check_disable_next_logic_action.py +++ /dev/null @@ -1,240 +0,0 @@ -#!/usr/bin/env python -# -# Show which rules need a manual check after adding a step to the "disable next" action, -# to make sure it is the correct one. Note that the actual addition of a step happens in -# a migration. The output of the script is the same, independent of whether it was -# executed before or after the migration. Use the flag '--show-all' to list all rules -# which will be affected by the migration. -# -import sys -from collections.abc import Collection -from pathlib import Path - -import django - -import click -from tabulate import tabulate -from tqdm import tqdm - -SRC_DIR = Path(__file__).parent.parent / "src" -sys.path.insert(0, str(SRC_DIR.resolve())) - - -def resolve_key(input_key: str, all_form_variable_keys: Collection[str]) -> str | None: - """ - Resolve a (nested) key to its corresponding form variable key. - - Submission data of container-type components (e.g. editgrid, selectboxes, etc.) - can be accessed with dot notation in a ``FormioData`` instance, but also in JSON - logic. For example, key "selectboxes.a" with data ``{"a": "A", "b": "B"}``. This - routine resolves the form variable key that corresponds to this data access key. - - :param input_key: The key to resolve. - :param all_form_variable_keys: Collection of form variable keys to resolve for. - :return: The resolved form variable key, or ``None`` if not resolved. - """ - # There is a variable with this exact key, it is a valid reference. - if input_key in all_form_variable_keys: - return input_key - - # Process nested paths (editgrid, selectboxes, partners, children). Note that this - # doesn't include other nested fields anymore, e.g. a textfield component with key - # "foo.bar" will have already been resolved. We process all slices, as these keys - # could also include dots. - parts = input_key.split(".") - for i in range(1, len(parts)): - if (key := ".".join(parts[:i])) in all_form_variable_keys: - return key - - # If none of the slices exist, we cannot resolve the complete key, so we just - # return `None`. Note that the digest email should notify the user of invalid - # logic rules. - return None - - -def report_disable_next_logic_action_manual_check(show_all=False): - from openforms.formio.service import iter_components - from openforms.forms.models import Form, FormLogic, FormStep, FormVariable - from openforms.utils.json_logic import introspect_json_logic - - queryset = Form.objects.prefetch_related( - "formlogic_set", "formvariable_set", "formstep_set" - ) - rules_that_need_check = [] - rules_with_disable_action = [] - for form in tqdm( - queryset.iterator(chunk_size=10), - desc="Forms processed", - total=queryset.count(), - dynamic_ncols=True, - mininterval=0.5, - unit="form", - ): - # We don't care about deleted forms - if form._is_deleted: - continue - - # Set of logic rules which contain a "disable next" action. - logic_rules: set[FormLogic] = { - rule - for rule in form.formlogic_set.all() - for action in rule.actions - if action["action"]["type"] == "disable-next" - } - # Exit early if none of the rules contain a disable next action - if not logic_rules: - continue - - # Mapping from component to step for quick access - form_steps = form.formstep_set.select_related("form_definition") - component_to_step: dict[str, FormStep] = { - component["key"]: step - for step in form_steps.iterator() - for component in iter_components( - step.form_definition.configuration, - recursive=True, - recurse_into_editgrid=False, - ) - } - form_variables: dict[str, FormVariable] = { - var.key: var for var in form.formvariable_set.iterator() - } - - # Process logic rules - for rule in sorted(logic_rules, key=lambda rule: rule.order): - rules_with_disable_action.append( - [form.admin_name, rule.order + 1, rule.description] - ) - if show_all: - # Do not process anything if the user only wants to view all logic rules - # with a "disable next" action. - continue - - # Create a set of input variable steps by analyzing the logic trigger. - input_variable_steps = set() - for input_var in introspect_json_logic( - rule.json_logic_trigger - ).get_input_keys(): - form_variable_key = resolve_key(input_var.key, form_variables) - if (form_variable := form_variables.get(form_variable_key)) is None: - continue - - if form_variable.prefill_plugin: - # If the variable has prefill configured -> add the first step. - # This is because prefilled data will be available upon submission - # creation, so the rule _could_ be triggered on the first step. If - # prefill did not succeed, we still need to execute it on step of - # the input variable as well, because the user might be asked to - # fill in the data manually. - input_variable_steps.add(form_steps.first()) - - # Note that we also add `None` if we cannot resolve a step (form - # variable is user defined). This is different from the migration - - # where we don't add anything and assign the first step later - because - # we want to notify the form designers to check these rules. - step = component_to_step.get(form_variable.key, None) - input_variable_steps.add(step) - - for action in rule.actions: - if action["action"]["type"] != "disable-next": - continue - - if len(input_variable_steps) == 0: - # There are no input variables from the logic trigger, so we assign - # the first step as a best guess in the migration - # ("trigger_from_step" overrides it). - if rule.trigger_from_step: - continue - - rules_that_need_check.append( - [ - form.admin_name, - rule.order + 1, - rule.description, - "First step assigned as best guess", - ] - ) - - elif None in input_variable_steps: - # If one of the variables is user defined -> mark this rule for a - # manual check. User-defined variables can be set with prefilled - # data, but also with user input through other logic rules, so we - # cannot be sure of the step. The only way to do this properly would - # be to have the full logic rule graph available. - rules_that_need_check.append( - [ - form.admin_name, - rule.order + 1, - rule.description, - "JSON logic trigger contains user-defined variable", - ] - ) - - elif len(input_variable_steps) == 1: - # If there is only one step, we assign it to the action in the - # migration ("trigger_from_step" overrides it). Not necessary to - # perform a manual check. - pass - else: - # If we have multiple input steps, we add new actions for each step. - # If "trigger_from_step" is defined, any steps before this step are - # removed. Not necessary to perform a manual check. - pass - - if show_all: - click.echo( - click.style( - "The following rules contain a 'disable next' action", fg="yellow" - ) - ) - click.echo( - tabulate( - rules_with_disable_action, - headers=("Form", "Rule number", "Rule description"), - ) - ) - return - - if not rules_that_need_check: - click.echo(click.style("No rules need a manual check", fg="green")) - return - - click.echo( - click.style( - "Found 'disable next' actions which need a manual check.\n" - "For the following rules, please make sure that the assigned step to the " - "'disable next' action is the correct one.\n" - "Note that it is possible to create more 'disable next' actions to assign " - "additional steps if necessary.", - fg="red", - ), - ) - click.echo( - tabulate( - rules_that_need_check, - headers=("Form", "Rule number", "Rule description", "Reason"), - ) - ) - - -@click.command() -@click.option( - "--show-all", - is_flag=True, - default=False, - help=( - "Show all logic rules that contain a 'disable next' action, and will therefore " - "be affected by the data migration" - ), -) -def cli(show_all: bool): - from openforms.setup import setup_env - - setup_env() - django.setup() - - report_disable_next_logic_action_manual_check(show_all) - - -if __name__ == "__main__": - cli() From f7447c42f7e064b2891ab09b97737369c0b20b69 Mon Sep 17 00:00:00 2001 From: Viktor van Wijk Date: Thu, 7 May 2026 15:49:49 +0200 Subject: [PATCH 06/10] :fire: [#6164] Remove triggerFromStep from the admin This includes displaying the advanced options as well, because it was the only one :) --- .../form_design/FormConfigurationFields.js | 24 ------ .../components/admin/form_design/FormLogic.js | 75 ------------------- .../admin/form_design/FormLogic.stories.js | 9 --- .../admin/form_design/data/complete-form.js | 2 - .../admin/form_design/form-creation-form.js | 1 - 5 files changed, 111 deletions(-) diff --git a/src/openforms/js/components/admin/form_design/FormConfigurationFields.js b/src/openforms/js/components/admin/form_design/FormConfigurationFields.js index 30353aff8f..368c9af6c6 100644 --- a/src/openforms/js/components/admin/form_design/FormConfigurationFields.js +++ b/src/openforms/js/components/admin/form_design/FormConfigurationFields.js @@ -526,7 +526,6 @@ const FormConfigurationFields = ({ onAuthPluginChange, availableCategories, formStepsAmount, - hasTriggerFromStep = false, }) => { const { uuid, @@ -766,28 +765,6 @@ const FormConfigurationFields = ({ askStatementOfTruth={askStatementOfTruth} onChange={onChange} /> - -
- } - collapsible - initialCollapsed - > - {hasTriggerFromStep && ( - -

- -

-
- )} -
); }; @@ -827,7 +804,6 @@ FormConfigurationFields.propTypes = { selectedAuthPlugins: PropTypes.array.isRequired, onAuthPluginChange: PropTypes.func.isRequired, formStepsAmount: PropTypes.number.isRequired, - hasTriggerFromStep: PropTypes.bool, }; export default FormConfigurationFields; diff --git a/src/openforms/js/components/admin/form_design/FormLogic.js b/src/openforms/js/components/admin/form_design/FormLogic.js index b443297242..aa218f09f0 100644 --- a/src/openforms/js/components/admin/form_design/FormLogic.js +++ b/src/openforms/js/components/admin/form_design/FormLogic.js @@ -185,10 +185,7 @@ const RuleBody = ({ isAdvanced, jsonLogicTrigger, actions, - displayAdvancedOptions, - setDisplayAdvancedOptions, description, - triggerFromStepIdentifier, formStepIdentifiers, onChange, errors, @@ -197,14 +194,9 @@ const RuleBody = ({ const intl = useIntl(); const [expandExpression, setExpandExpression] = useState(isCreate); const [resetRequest, setResetRequest] = useState(0); - const triggerFromStep = useFormStep(triggerFromStepIdentifier); const formSteps = useFormSteps(formStepIdentifiers); const {formSteps: allFormSteps} = useContext(FormContext); - // Case in which there is an error: a trigger step was specified, but this step cannot be found in the form - if (!triggerFromStep && triggerFromStepIdentifier) - setDisplayAdvancedOptions(true); - const TriggerComponent = isAdvanced ? AdvancedTrigger : Trigger; const onDescriptionGenerated = generatedDescription => { @@ -235,53 +227,6 @@ const RuleBody = ({
- {displayAdvancedOptions && ( - <> -
-
- - - - - - - {!triggerFromStepIdentifier && ( - <> -   - - - )} - -
-
- {triggerFromStepIdentifier && !triggerFromStep && ( - - ), - level: 'warning', - }, - ]} - /> - )} - - )} -
{allFormSteps.length === formSteps.length ? ( - - {triggerFromStep && ( -
- -
- )}
@@ -372,10 +307,7 @@ RuleBody.propTypes = { isAdvanced: PropTypes.bool.isRequired, jsonLogicTrigger: jsonPropTypeValidator, actions: PropTypes.arrayOf(PropTypes.object), - displayAdvancedOptions: PropTypes.bool, - setDisplayAdvancedOptions: PropTypes.func.isRequired, description: PropTypes.string, - triggerFromStepIdentifier: PropTypes.string, formStepIdentifiers: PropTypes.arrayOf(PropTypes.string), onChange: PropTypes.func.isRequired, errors: PropTypes.object, @@ -389,7 +321,6 @@ const Rule = ({ _mayGenerateDescription, order, jsonLogicTrigger, - triggerFromStep: triggerFromStepIdentifier, actions, isAdvanced, formSteps: formStepIdentifiers, @@ -398,7 +329,6 @@ const Rule = ({ errors = {}, }) => { const intl = useIntl(); - const [displayAdvancedOptions, setDisplayAdvancedOptions] = useState(false); const deleteConfirmMessage = intl.formatMessage({ description: 'Logic rule deletion confirm message', @@ -429,7 +359,6 @@ const Rule = ({ {JSON.stringify({ jsonLogicTrigger, - triggerFromStepIdentifier, order, actions, isAdvanced, @@ -462,9 +391,6 @@ const Rule = ({ isAdvanced={isAdvanced} jsonLogicTrigger={jsonLogicTrigger} actions={actions} - triggerFromStepIdentifier={triggerFromStepIdentifier} - displayAdvancedOptions={displayAdvancedOptions} - setDisplayAdvancedOptions={setDisplayAdvancedOptions} description={description} formStepIdentifiers={formStepIdentifiers} onChange={onChange} @@ -483,7 +409,6 @@ Rule.propTypes = { _mayGenerateDescription: PropTypes.bool.isRequired, order: PropTypes.number.isRequired, jsonLogicTrigger: jsonPropTypeValidator, - triggerFromStep: PropTypes.string, actions: PropTypes.arrayOf(PropTypes.object), isAdvanced: PropTypes.bool.isRequired, formSteps: PropTypes.arrayOf(PropTypes.string), diff --git a/src/openforms/js/components/admin/form_design/FormLogic.stories.js b/src/openforms/js/components/admin/form_design/FormLogic.stories.js index 9602e579e0..03a782e275 100644 --- a/src/openforms/js/components/admin/form_design/FormLogic.stories.js +++ b/src/openforms/js/components/admin/form_design/FormLogic.stories.js @@ -224,15 +224,6 @@ export const FullFunctionality = { availableFormVariables: AVAILABLE_FORM_VARIABLES, availableFormSteps: AVAILABLE_FORM_STEPS, }, - - play: async ({canvasElement}) => { - const canvas = within(canvasElement); - - const icons = await canvas.findAllByTitle('Geavanceerde opties'); - expect(icons[0]).toBeVisible(); - await userEvent.click(icons[0]); - expect(canvas.getByText('Inschakelen vanaf stap:')).toBeVisible(); - }, }; export const DeletingOneOfMultipleActionsInSameTrigger = { diff --git a/src/openforms/js/components/admin/form_design/data/complete-form.js b/src/openforms/js/components/admin/form_design/data/complete-form.js index 9c302cf48b..9ee4420856 100644 --- a/src/openforms/js/components/admin/form_design/data/complete-form.js +++ b/src/openforms/js/components/admin/form_design/data/complete-form.js @@ -245,8 +245,6 @@ const saveLogic = async (state, csrftoken) => { let newState = produce(state, draft => { for (const rule of draft.logicRules) { rule.form = formUrl; - // fix the trigger from step reference - rule.triggerFromStep = getStepReference(stepsByGeneratedId, rule.triggerFromStep); // fix the actions that do something with the step(s) for (const action of rule.actions) { action.formStep = getStepReference(stepsByGeneratedId, action.formStep); diff --git a/src/openforms/js/components/admin/form_design/form-creation-form.js b/src/openforms/js/components/admin/form_design/form-creation-form.js index 21192542fc..e43d9e71f3 100644 --- a/src/openforms/js/components/admin/form_design/form-creation-form.js +++ b/src/openforms/js/components/admin/form_design/form-creation-form.js @@ -1356,7 +1356,6 @@ const FormCreationForm = ({formUuid, formUrl, formHistoryUrl, outgoingRequestsUr availableCategories={state.availableCategories} availableThemes={state.availableThemes} onAuthPluginChange={onAuthPluginChange} - hasTriggerFromStep={state.logicRules.some(rule => !!rule.triggerFromStep)} formStepsAmount={state.formSteps.length} /> From afbe5284c98093c3c03e9f86198f10e0e3406909 Mon Sep 17 00:00:00 2001 From: Viktor van Wijk Date: Thu, 7 May 2026 16:08:48 +0200 Subject: [PATCH 07/10] :globe_with_meridians: [#6164] Update translations --- src/openforms/js/compiled-lang/en.json | 62 -------------------------- src/openforms/js/compiled-lang/nl.json | 62 -------------------------- src/openforms/js/lang/en.json | 45 ------------------- src/openforms/js/lang/nl.json | 45 ------------------- 4 files changed, 214 deletions(-) diff --git a/src/openforms/js/compiled-lang/en.json b/src/openforms/js/compiled-lang/en.json index cc807d7526..d5558307a5 100644 --- a/src/openforms/js/compiled-lang/en.json +++ b/src/openforms/js/compiled-lang/en.json @@ -1961,12 +1961,6 @@ "value": "This field is required." } ], - "DqG0YS": [ - { - "type": 0, - "value": "Enable from step:" - } - ], "DxGKNm": [ { "type": 0, @@ -2491,20 +2485,6 @@ "value": "You are about to reset the submissions counter and this action is irreversible. Are you sure that you want to do this?" } ], - "HuAm1K": [ - { - "type": 0, - "value": "and the step \"" - }, - { - "type": 1, - "value": "step" - }, - { - "type": 0, - "value": "\" has been reached" - } - ], "I0bgsg": [ { "type": 0, @@ -2799,12 +2779,6 @@ "value": " configuration?" } ], - "KYTAJZ": [ - { - "type": 0, - "value": "(checked for every step)" - } - ], "Kdgpxf": [ { "type": 0, @@ -2955,12 +2929,6 @@ "value": "Plugin configuration: Email" } ], - "LiXrER": [ - { - "type": 0, - "value": "The selected trigger step could not be found in this form! Please change it!" - } - ], "LkrFow": [ { "type": 0, @@ -3125,12 +3093,6 @@ "value": "Case type API resource URL in the Catalogi API." } ], - "NqShTs": [ - { - "type": 0, - "value": "Experimental features" - } - ], "NrF5vy": [ { "type": 0, @@ -3793,12 +3755,6 @@ "value": "the path should not have a leading slash" } ], - "TPy506": [ - { - "type": 0, - "value": "Enable new logic rule evaluation" - } - ], "TQv+8T": [ { "type": 0, @@ -4683,12 +4639,6 @@ "value": "There are errors in the DMN configuration." } ], - "aoNZfg": [ - { - "type": 0, - "value": "Before you can enable the new logic evaluation, remove the 'trigger from step' configuration from your logic rules." - } - ], "aqYeqv": [ { "type": 0, @@ -5221,12 +5171,6 @@ "value": "Attachments" } ], - "eLDxD5": [ - { - "type": 0, - "value": "Advanced options" - } - ], "eLIclD": [ { "type": 0, @@ -6283,12 +6227,6 @@ "value": "Partner 1" } ], - "lyU7fP": [ - { - "type": 0, - "value": "Enabling this will analyze logic rules and re-order them according to their dependency on other logic rules (happens when the form is saved). Each rule will be automatically assigned to one or more steps on which it will be executed." - } - ], "m/NVtu": [ { "type": 0, diff --git a/src/openforms/js/compiled-lang/nl.json b/src/openforms/js/compiled-lang/nl.json index c7ff1761d8..659ae088dd 100644 --- a/src/openforms/js/compiled-lang/nl.json +++ b/src/openforms/js/compiled-lang/nl.json @@ -1948,12 +1948,6 @@ "value": "Dit veld is verplicht." } ], - "DqG0YS": [ - { - "type": 0, - "value": "Inschakelen vanaf stap:" - } - ], "DxGKNm": [ { "type": 0, @@ -2478,20 +2472,6 @@ "value": "Je staat op het punt om het aantal gemaakte inzendingen terug op nul te zetten. Dit kan niet ongedaan gemaakt worden. Ben je zeker dat je door wil gaan?" } ], - "HuAm1K": [ - { - "type": 0, - "value": "en de stap \"" - }, - { - "type": 1, - "value": "step" - }, - { - "type": 0, - "value": "\" is bereikt" - } - ], "I0bgsg": [ { "type": 0, @@ -2782,12 +2762,6 @@ "value": " instellingen bijwerken?" } ], - "KYTAJZ": [ - { - "type": 0, - "value": "(actief binnen elke stap)" - } - ], "Kdgpxf": [ { "type": 0, @@ -2938,12 +2912,6 @@ "value": "Plugin-instellingen: e-mail" } ], - "LiXrER": [ - { - "type": 0, - "value": "De gekozen trigger-stap bestaat niet (meer) in het formulier. Gelieve deze te wijzigen." - } - ], "LkrFow": [ { "type": 0, @@ -3108,12 +3076,6 @@ "value": "Zaaktype-API-resource URL in de Catalogi-API." } ], - "NqShTs": [ - { - "type": 0, - "value": "Experimentele functionaliteit" - } - ], "NrF5vy": [ { "type": 0, @@ -3772,12 +3734,6 @@ "value": "Zorg ervoor dat het pad niet met een schuine streep ('/') begint." } ], - "TPy506": [ - { - "type": 0, - "value": "Gebruik nieuwe logica-evaluatie" - } - ], "TQv+8T": [ { "type": 0, @@ -4663,12 +4619,6 @@ "value": "De DMN-instellingen zijn niet geldig." } ], - "aoNZfg": [ - { - "type": 0, - "value": "Voordat je de nieuwe logica-evaluatie kunt inschakelen, moet je de configuratie ‘Inschakelen vanaf stap’ uit je logicaregels verwijderen." - } - ], "aqYeqv": [ { "type": 0, @@ -5205,12 +5155,6 @@ "value": "Bijlagen" } ], - "eLDxD5": [ - { - "type": 0, - "value": "Geavanceerde opties" - } - ], "eLIclD": [ { "type": 0, @@ -6267,12 +6211,6 @@ "value": "Partner 1" } ], - "lyU7fP": [ - { - "type": 0, - "value": "Als je dit inschakelt, dan wordt de formulierlogica geanalyseerd en automatisch gesorteerd op basis van hun onderlinge afhankelijkheden (dit gebeurt wanneer je het formulier opslaat). Elke regel wordt automatisch toegewezen aan een of meer stappen waarop deze zal worden uitgevoerd. Daarnaast zullen ook andere snelheidsverbeteringen toegepast worden waar mogelijk." - } - ], "m/NVtu": [ { "type": 0, diff --git a/src/openforms/js/lang/en.json b/src/openforms/js/lang/en.json index 8e4a8f0815..0f44c8fb38 100644 --- a/src/openforms/js/lang/en.json +++ b/src/openforms/js/lang/en.json @@ -854,11 +854,6 @@ "description": "Required error message", "originalDefault": "This field is required." }, - "DqG0YS": { - "defaultMessage": "Enable from step:", - "description": "'Trigger from step' label", - "originalDefault": "Enable from step:" - }, "DxGKNm": { "defaultMessage": "Logic", "description": "Logic fieldset title", @@ -1139,11 +1134,6 @@ "description": "Reset the submissions counter confirmation message", "originalDefault": "You are about to reset the submissions counter and this action is irreversible. Are you sure that you want to do this?" }, - "HuAm1K": { - "defaultMessage": "and the step \"{step}\" has been reached", - "description": "Additional 'trigger from step' condition", - "originalDefault": "and the step \"{step}\" has been reached" - }, "I0bgsg": { "defaultMessage": "Successful Submissions Removal Method field label", "description": "Successful Submissions Removal Method", @@ -1309,11 +1299,6 @@ "description": "Accessible label for check/uncheck single file component", "originalDefault": "Update {label} configuration?" }, - "KYTAJZ": { - "defaultMessage": "(checked for every step)", - "description": "'Trigger from step' information for when unset", - "originalDefault": "(checked for every step)" - }, "KjDolH": { "defaultMessage": "Manually defined", "description": "JSON editor: \"manual\" variable source label", @@ -1404,11 +1389,6 @@ "description": "Email registration options modal title", "originalDefault": "Plugin configuration: Email" }, - "LiXrER": { - "defaultMessage": "The selected trigger step could not be found in this form! Please change it!", - "description": "Warning missing trigger step", - "originalDefault": "The selected trigger step could not be found in this form! Please change it!" - }, "LkrFow": { "defaultMessage": "Something went wrong while retrieving the available catalogues or case/document types defined in the selected catalogue. Please check that the services in the selected API group are configured correctly.", "description": "ZGW APIs registrations options: generic error", @@ -1509,11 +1489,6 @@ "description": "ZGW APIs registration options 'CaseType' help text", "originalDefault": "Case type API resource URL in the Catalogi API." }, - "NqShTs": { - "defaultMessage": "Experimental features", - "description": "Form feature flags fieldset title", - "originalDefault": "Experimental features" - }, "NrF5vy": { "defaultMessage": "Document type", "description": "ZGW APIs registration options 'document type' label", @@ -1839,11 +1814,6 @@ "description": "Service fetch configuration modal form Path field help text", "originalDefault": "the path should not have a leading slash" }, - "TPy506": { - "defaultMessage": "Enable new logic rule evaluation", - "description": "New logic rule evaluation feature flag label", - "originalDefault": "Enable new logic rule evaluation" - }, "TQv+8T": { "defaultMessage": "Action property selection", "description": "Accessible label for action property selection", @@ -2229,11 +2199,6 @@ "description": "DMN evaluation configuration errors message", "originalDefault": "There are errors in the DMN configuration." }, - "aoNZfg": { - "defaultMessage": "Before you can enable the new logic evaluation, remove the 'trigger from step' configuration from your logic rules.", - "description": "New logic disabled reason explanation", - "originalDefault": "Before you can enable the new logic evaluation, remove the 'trigger from step' configuration from your logic rules." - }, "auZOcD": { "defaultMessage": "Mapping expression", "description": "Service fetch configuration modal form mapping expression field label", @@ -2439,11 +2404,6 @@ "description": "Email registration: attachments fieldset title", "originalDefault": "Attachments" }, - "eLDxD5": { - "defaultMessage": "Advanced options", - "description": "Logic rule advanced options icon title", - "originalDefault": "Advanced options" - }, "eLIclD": { "defaultMessage": "Optional custom template for the description, included in the payment overviews for the backoffice. Use this to link the payment back to a particular process or form.

You can include form variables (using their keys) and the public-reference variable (using expression '{{' public_reference '}}'). If unspecified, a default description is used.

Note: the length of the result is capped to 100 characters.", "description": "Ogone legacy payment options 'comTemplate' help text", @@ -2914,11 +2874,6 @@ "description": "ZGW APIs registration options 'objecttype' help text", "originalDefault": "URL to the object type in the objecttypes API. If provided, an object will be created and a case object relation will be added to the case." }, - "lyU7fP": { - "defaultMessage": "Enabling this will analyze logic rules and re-order them according to their dependency on other logic rules (happens when the form is saved). Each rule will be automatically assigned to one or more steps on which it will be executed.", - "description": "New logic rule evaluation feature flag help text", - "originalDefault": "Enabling this will analyze logic rules and re-order them according to their dependency on other logic rules (happens when the form is saved). Each rule will be automatically assigned to one or more steps on which it will be executed." - }, "m/NVtu": { "defaultMessage": "Transform to list", "description": "'Transform to list' checkbox label", diff --git a/src/openforms/js/lang/nl.json b/src/openforms/js/lang/nl.json index 962ea72d2b..28dd9076f2 100644 --- a/src/openforms/js/lang/nl.json +++ b/src/openforms/js/lang/nl.json @@ -864,11 +864,6 @@ "description": "Required error message", "originalDefault": "This field is required." }, - "DqG0YS": { - "defaultMessage": "Inschakelen vanaf stap:", - "description": "'Trigger from step' label", - "originalDefault": "Enable from step:" - }, "DxGKNm": { "defaultMessage": "Logica", "description": "Logic fieldset title", @@ -1151,11 +1146,6 @@ "description": "Reset the submissions counter confirmation message", "originalDefault": "You are about to reset the submissions counter and this action is irreversible. Are you sure that you want to do this?" }, - "HuAm1K": { - "defaultMessage": "en de stap \"{step}\" is bereikt", - "description": "Additional 'trigger from step' condition", - "originalDefault": "and the step \"{step}\" has been reached" - }, "I0bgsg": { "defaultMessage": "Opschoonmethode voor voltooide inzendingen", "description": "Successful Submissions Removal Method", @@ -1321,11 +1311,6 @@ "description": "Accessible label for check/uncheck single file component", "originalDefault": "Update {label} configuration?" }, - "KYTAJZ": { - "defaultMessage": "(actief binnen elke stap)", - "description": "'Trigger from step' information for when unset", - "originalDefault": "(checked for every step)" - }, "KjDolH": { "defaultMessage": "Handmatig ingesteld", "description": "JSON editor: \"manual\" variable source label", @@ -1416,11 +1401,6 @@ "description": "Email registration options modal title", "originalDefault": "Plugin configuration: Email" }, - "LiXrER": { - "defaultMessage": "De gekozen trigger-stap bestaat niet (meer) in het formulier. Gelieve deze te wijzigen.", - "description": "Warning missing trigger step", - "originalDefault": "The selected trigger step could not be found in this form! Please change it!" - }, "LkrFow": { "defaultMessage": "Er ging iets fout bij het ophalen van de beschikbare catalogi, zaak- of documenttypen. Controleer of de services in de geselecteerde API-groep goed ingesteld zijn.", "description": "ZGW APIs registrations options: generic error", @@ -1522,11 +1502,6 @@ "description": "ZGW APIs registration options 'CaseType' help text", "originalDefault": "Case type API resource URL in the Catalogi API." }, - "NqShTs": { - "defaultMessage": "Experimentele functionaliteit", - "description": "Form feature flags fieldset title", - "originalDefault": "Experimental features" - }, "NrF5vy": { "defaultMessage": "Documenttype", "description": "ZGW APIs registration options 'document type' label", @@ -1856,11 +1831,6 @@ "description": "Service fetch configuration modal form Path field help text", "originalDefault": "the path should not have a leading slash" }, - "TPy506": { - "defaultMessage": "Gebruik nieuwe logica-evaluatie", - "description": "New logic rule evaluation feature flag label", - "originalDefault": "Enable new logic rule evaluation" - }, "TQv+8T": { "defaultMessage": "Selecteer actie-eigenschap", "description": "Accessible label for action property selection", @@ -2249,11 +2219,6 @@ "description": "DMN evaluation configuration errors message", "originalDefault": "There are errors in the DMN configuration." }, - "aoNZfg": { - "defaultMessage": "Voordat je de nieuwe logica-evaluatie kunt inschakelen, moet je de configuratie ‘Inschakelen vanaf stap’ uit je logicaregels verwijderen.", - "description": "New logic disabled reason explanation", - "originalDefault": "Before you can enable the new logic evaluation, remove the 'trigger from step' configuration from your logic rules." - }, "auZOcD": { "defaultMessage": "Mappingexpressie", "description": "Service fetch configuration modal form mapping expression field label", @@ -2461,11 +2426,6 @@ "description": "Email registration: attachments fieldset title", "originalDefault": "Attachments" }, - "eLDxD5": { - "defaultMessage": "Geavanceerde opties", - "description": "Logic rule advanced options icon title", - "originalDefault": "Advanced options" - }, "eLIclD": { "defaultMessage": "Sjabloon voor de omschrijving van de betaling in de afschriften die je bij Ogone kan ophalen. Gebruik dit om betalingen aan processen/afdelingen te koppelen.

Je kan alle formuliervariabelen gebruiken (gebruik de variabelesleutel als naam) en de `public_reference` sjabloonvariabele (gebruik de expressie '{{' public_reference '}}'). Indien geen sjabloon opgegeven is, dan wordt een standaardomschrijving gebruikt.

Belangrijk: het resultaat wordt op 100 karakters afgekapt en enkel alfanumerieke tekens zijn toegestaan.", "description": "Ogone legacy payment options 'comTemplate' help text", @@ -2937,11 +2897,6 @@ "description": "ZGW APIs registration options 'objecttype' help text", "originalDefault": "URL to the object type in the objecttypes API. If provided, an object will be created and a case object relation will be added to the case." }, - "lyU7fP": { - "defaultMessage": "Als je dit inschakelt, dan wordt de formulierlogica geanalyseerd en automatisch gesorteerd op basis van hun onderlinge afhankelijkheden (dit gebeurt wanneer je het formulier opslaat). Elke regel wordt automatisch toegewezen aan een of meer stappen waarop deze zal worden uitgevoerd. Daarnaast zullen ook andere snelheidsverbeteringen toegepast worden waar mogelijk.", - "description": "New logic rule evaluation feature flag help text", - "originalDefault": "Enabling this will analyze logic rules and re-order them according to their dependency on other logic rules (happens when the form is saved). Each rule will be automatically assigned to one or more steps on which it will be executed." - }, "m/NVtu": { "defaultMessage": "Verstuur als lijst", "description": "'Transform to list' checkbox label", From c9df6c50fd6e1a1ba84f4cd216823af24b1d53be Mon Sep 17 00:00:00 2001 From: Viktor van Wijk Date: Thu, 7 May 2026 16:10:10 +0200 Subject: [PATCH 08/10] :bento: [#6164] Update API schema --- src/openapi.yaml | 31 +------------------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/src/openapi.yaml b/src/openapi.yaml index cb5941e62d..fec052e71f 100644 --- a/src/openapi.yaml +++ b/src/openapi.yaml @@ -8124,13 +8124,6 @@ components: allOf: - $ref: '#/components/schemas/BRPPersonenRequestOptions' nullable: true - newLogicEvaluationEnabled: - type: boolean - title: Enable new logic rule evaluation - description: Enabling this will analyze logic rules and re-order them according - to their dependency on other logic rules (happens when the form is saved). - Each rule will be automatically assigned to one or more steps on which - it will be executed. required: - communicationPreferencesPortalUrl - cosignHasLinkInEmail @@ -8554,13 +8547,6 @@ components: description: Order of the rule relative to the form it belongs to. Logic rules are evaluated in this order. Note that specifying a value for the order here will cause the rules before/after this rule to be re-calculated. - triggerFromStep: - type: string - format: uri - nullable: true - description: When set, the trigger will only be checked once the specified - step is reached. This means the rule will never trigger for steps before - the specified trigger step. If unset, the trigger will always be checked. actions: type: array items: @@ -8579,8 +8565,7 @@ components: title: form steps readOnly: true description: Form steps on which the rule will be executed, determined by - logic rule analysis. Note that this is only relevant when the `new_logic_evaluation_enabled` - feature flag on the form is set to `True`. + logic rule analysis. required: - actions - form @@ -9106,13 +9091,6 @@ components: description: Display the instruction from the confirmation page in the PDF. translations: $ref: '#/components/schemas/FormModelTranslations' - newLogicEvaluationEnabled: - type: boolean - title: Enable new logic rule evaluation - description: Enabling this will analyze logic rules and re-order them according - to their dependency on other logic rules (happens when the form is saved). - Each rule will be automatically assigned to one or more steps on which - it will be executed. required: - loginRequired - name @@ -10214,13 +10192,6 @@ components: allOf: - $ref: '#/components/schemas/BRPPersonenRequestOptions' nullable: true - newLogicEvaluationEnabled: - type: boolean - title: Enable new logic rule evaluation - description: Enabling this will analyze logic rules and re-order them according - to their dependency on other logic rules (happens when the form is saved). - Each rule will be automatically assigned to one or more steps on which - it will be executed. PatchedFormDefinition: type: object properties: From db66087eef427cecc6145ec36778ce6d3c8ff9c9 Mon Sep 17 00:00:00 2001 From: Viktor van Wijk Date: Thu, 7 May 2026 16:50:54 +0200 Subject: [PATCH 09/10] :memo: [#6164] Add logic engine rework documentation to 4.0 upgrade details --- docs/installation/upgrade-400.rst | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/docs/installation/upgrade-400.rst b/docs/installation/upgrade-400.rst index a7229507d8..c2fe9e204f 100644 --- a/docs/installation/upgrade-400.rst +++ b/docs/installation/upgrade-400.rst @@ -234,3 +234,31 @@ The default way to generate public references for the ZGW APIs registration plug Previously, Open Forms always used case numbers as the submission public references. Since Open Forms 3.5, both options have been available, but the default remained the ZGW API-generated case numbers. The default is now switched to Open Forms-generated public references. + +Logic engine rework +=================== + +.. note:: Relevant for: devops, form designers + +The support for legacy logic evaluation has been removed, which means all forms will use the new logic evaluation. +Since Open Forms will now automatically assign all logic rules to the relevant form steps, it is also no longer +possible to specify a "trigger from step" for a logic rule. + +TODO: this should probably a separate page describing the logic engine in more detail +For more information about the new logic evaluation, please refer to the +:ref:`detailed release notes of 3.5.0 `. + +.. warning:: + + Before upgrading, all existing forms should be converted to the new logic evaluation. This can be done on a + per-form basis, or in bulk using the following management command. Any forms which contain cycles in their + logic rules need to be resolved manually. The output of the management command will include relevant form + details if this is the case. + + .. code-block:: bash + + python /app/src/manage.py enable_new_logic_evaluation_for_all_forms + +Clearing of values +------------------ +TODO From 22d2f45f94396ac4c5c25027e5698e029d81c79f Mon Sep 17 00:00:00 2001 From: Viktor van Wijk Date: Fri, 15 May 2026 14:09:49 +0200 Subject: [PATCH 10/10] :wrench: [#6164] Add upgrade check for 4.0 We also want to include the invalid form logic script as an additional constraint. I have revised the script to block the upgrade if there are forms (with logic rules) that have not been converted to the new logic evaluation. Note that these will also include forms with cycles, as a conversion is illegal if cycles are present 4.0.0 has not been released officially, but we are tricking it to run the upgrade check already :) --- bin/report_invalid_form_logic.py | 53 +++++-------------- src/openforms/conf/base.py | 2 + .../upgrades/tests/test_upgrade_paths.py | 53 +++++++++++++++++++ 3 files changed, 69 insertions(+), 39 deletions(-) diff --git a/bin/report_invalid_form_logic.py b/bin/report_invalid_form_logic.py index b74a8d0f13..962c00275f 100755 --- a/bin/report_invalid_form_logic.py +++ b/bin/report_invalid_form_logic.py @@ -2,12 +2,9 @@ from __future__ import annotations import sys -from collections.abc import Iterator from pathlib import Path -from traceback import format_exc import django -from django.db import transaction from tabulate import tabulate @@ -21,8 +18,10 @@ def report_invalid_forms() -> bool: """ Query forms that have new logic evaluation enabled but inconsistent logic rule configuration. + + In 4.0, this script will be used as an upgrade check, which means we can also + report forms that are not yet converted to new logic evalution at all. """ - from openforms.forms.logic_analysis import CyclesDetected from openforms.forms.models import Form invalid_rules_detected = False @@ -45,52 +44,28 @@ def report_invalid_forms() -> bool: ) ) - # check each individual form for logic rule cycles. Run this in a transaction and - # roll back, because apply_logic_analysis() saves the result to the DB. + # check if any forms still have new logic evaluation disabled forms_to_check = Form.objects.filter( _is_deleted=False, new_logic_evaluation_enabled=False, formlogic__isnull=False, ).distinct() - transaction.set_autocommit(False) - cycles_detected = False - - def _detect_cycles() -> Iterator[tuple[int, str, bool, str]]: - nonlocal cycles_detected - - for form in forms_to_check.iterator(): - form.new_logic_evaluation_enabled = True - try: - form.apply_logic_analysis() - except CyclesDetected as exc: - variables = {var for cycle in exc.cycles for var in cycle.variables} - cycles_detected = True - yield (form.pk, form.admin_name, form.active, ", ".join(variables)) - except Exception: - print( - f"Unexpected error in form {form.admin_name} ({form.pk}): " - f"{format_exc()}" - ) - - try: + non_converted_forms_detected = False + if forms_to_check.exists(): + non_converted_forms_detected = True print( - tabulate( - _detect_cycles(), - headers=("Form ID", "Form name", "Active", "Variables in cycles"), - ) + "The following forms have have not been converted to the new logic " + "evaluation. Please review and change them to upgrade to Open Forms 4.0." ) - finally: - transaction.rollback() - - if cycles_detected: print( - "\nThe table above lists all the forms where cycles in logic rules were " - "detected. Please review and fix them as soon as possible. Open Forms 4.0 " - "will require this to be resolved before you can upgrade." + tabulate( + ((form.pk, form.admin_name) for form in forms_to_check.iterator()), + headers=("Form ID", "Form name"), + ) ) - return not invalid_rules_detected and not cycles_detected + return not invalid_rules_detected and not non_converted_forms_detected def main(skip_setup=False) -> bool: diff --git a/src/openforms/conf/base.py b/src/openforms/conf/base.py index c2198b79fb..8ef29c9729 100644 --- a/src/openforms/conf/base.py +++ b/src/openforms/conf/base.py @@ -25,6 +25,7 @@ add_open_telemetry_spans, drop_user_agent_in_dev, ) +from openforms.upgrades.script_checks import BinScriptCheck from .utils import Filesize, get_sentry_integrations, sentry_before_send @@ -1324,6 +1325,7 @@ # 3.5.3 will provide the necessary migration tooling, update when it's released VersionRange(minimum="3.5.2"), code_checks=[ + BinScriptCheck("report_invalid_form_logic"), CommandCheck("check_legacy_catalogi_api_urls"), ], ), diff --git a/src/openforms/upgrades/tests/test_upgrade_paths.py b/src/openforms/upgrades/tests/test_upgrade_paths.py index d99d1993fa..22890ac3c2 100644 --- a/src/openforms/upgrades/tests/test_upgrade_paths.py +++ b/src/openforms/upgrades/tests/test_upgrade_paths.py @@ -188,3 +188,56 @@ def test_with_selectboxes_inside_editgrid(self): ) self.assertTrue(self.script.execute()) + + +class ReportInvalidFormLogicTests(TestCase): + script = BinScriptCheck("report_invalid_form_logic") + + def test_no_problems(self): + FormFactory.create( + generate_minimal_setup=True, new_logic_evaluation_enabled=True + ) + + self.assertTrue(self.script.execute()) + + def test_form_not_converted(self): + form = FormFactory.create( + generate_minimal_setup=True, new_logic_evaluation_enabled=False + ) + FormLogicFactory.create( + form=form, + trigger_from_step=form.formstep_set.get(), + json_logic_trigger=True, + actions=[ + { + "form_step_uuid": str(form.formstep_set.get().uuid), + "action": { + "name": "Step is not applicable", + "type": "disable-next", + }, + } + ], + ) + + self.assertFalse(self.script.execute()) + + def test_new_logic_evaluation_enabled_with_trigger_from_step(self): + form = FormFactory.create( + generate_minimal_setup=True, new_logic_evaluation_enabled=True + ) + FormLogicFactory.create( + form=form, + trigger_from_step=form.formstep_set.get(), + json_logic_trigger=True, + actions=[ + { + "form_step_uuid": str(form.formstep_set.get().uuid), + "action": { + "name": "Step is not applicable", + "type": "disable-next", + }, + } + ], + ) + + self.assertFalse(self.script.execute())