From 0ad3170452e64517f36a59ab5c154544141a80b8 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Tue, 24 Dec 2024 09:29:01 +0100 Subject: [PATCH] :bug: [#4579] Ensure triggerFromStep uses the correct current_step previously the current step was inferred by looking at the last completed step and considering the step after that the current step. This does not work however if you complete a step and go back to the previous step, so instead we explicitly pass the current form step via the serializer context Backport-of: #4967 --- src/openforms/submissions/api/serializers.py | 6 +- src/openforms/submissions/api/viewsets.py | 6 +- src/openforms/submissions/form_logic.py | 6 +- .../tests/form_logic/test_submission_logic.py | 120 ++++++++++++++++++ 4 files changed, 134 insertions(+), 4 deletions(-) diff --git a/src/openforms/submissions/api/serializers.py b/src/openforms/submissions/api/serializers.py index 2c081bcb41..93cdb7dc34 100644 --- a/src/openforms/submissions/api/serializers.py +++ b/src/openforms/submissions/api/serializers.py @@ -200,7 +200,11 @@ def create(self, validated_data): return super().create(validated_data) def to_representation(self, instance): - check_submission_logic(instance, unsaved_data=self.context.get("unsaved_data")) + check_submission_logic( + instance, + unsaved_data=self.context.get("unsaved_data"), + current_step=self.context.get("current_step"), + ) return super().to_representation(instance) diff --git a/src/openforms/submissions/api/viewsets.py b/src/openforms/submissions/api/viewsets.py index b40acd9cfc..58ed84bef7 100644 --- a/src/openforms/submissions/api/viewsets.py +++ b/src/openforms/submissions/api/viewsets.py @@ -639,6 +639,10 @@ def logic_check(self, request, *args, **kwargs): submission_state_logic_serializer = SubmissionStateLogicSerializer( instance=SubmissionStateLogic(submission=submission, step=submission_step), - context={"request": request, "unsaved_data": data}, + context={ + "request": request, + "unsaved_data": data, + "current_step": submission_step, + }, ) return Response(submission_state_logic_serializer.data) diff --git a/src/openforms/submissions/form_logic.py b/src/openforms/submissions/form_logic.py index 8691dfc8d4..f48c999c9f 100644 --- a/src/openforms/submissions/form_logic.py +++ b/src/openforms/submissions/form_logic.py @@ -188,7 +188,9 @@ def evaluate_form_logic( def check_submission_logic( - submission: "Submission", unsaved_data: dict | None = None + submission: "Submission", + unsaved_data: dict | None = None, + current_step: "SubmissionStep | None" = None, ) -> None: if getattr(submission, "_form_logic_evaluated", False): return @@ -198,7 +200,7 @@ def check_submission_logic( if not submission_state.form_steps: return - rules = get_rules_to_evaluate(submission) + rules = get_rules_to_evaluate(submission, current_step) # load the data state and all variables submission_variables_state = submission.load_submission_value_variables_state() 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 567630e5db..dfc57bb740 100644 --- a/src/openforms/submissions/tests/form_logic/test_submission_logic.py +++ b/src/openforms/submissions/tests/form_logic/test_submission_logic.py @@ -225,6 +225,126 @@ def test_check_logic_on_whole_submission_with_variables(self): self.assertTrue(data["steps"][1]["isApplicable"]) self.assertFalse(data["steps"][1]["canSubmit"]) + @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` + + * We fill out step 1 and continue to step 2 + * 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() + step1 = FormStepFactory.create( + form=form, + form_definition__configuration={ + "components": [ + { + "type": "textfield", + "key": "text1", + } + ] + }, + ) + step2 = FormStepFactory.create( + form=form, + form_definition__configuration={ + "components": [ + { + "type": "textfield", + "key": "text2", + } + ] + }, + ) + FormVariableFactory.create( + form=form, + key="verdergaan", + user_defined=True, + data_type=FormVariableDataTypes.string, + ) + # set up logic rules: + # 1. setting `text1` to `trigger-value` should set our user defined variable to `nee` + FormLogicFactory.create( + form=form, + json_logic_trigger={"==": [{"var": "text1"}, "trigger-rule"]}, + actions=[ + { + "component": "", + "variable": "verdergaan", + "formStepUuid": None, + "action": {"type": "variable", "value": "nee"}, + } + ], + ) + # 2. if `verdergaan` is `nee`, block going to step3 + FormLogicFactory.create( + form=form, + trigger_from_step=step2, + json_logic_trigger={"==": [{"var": "verdergaan"}, "nee"]}, + actions=[ + { + "component": "", + "formStepUuid": None, + "action": {"type": "disable-next"}, + } + ], + ) + submission = SubmissionFactory.create(form=form) + self._add_submission_to_session(submission) + + with self.subTest("fill in step1"): + endpoint = reverse( + "api:submission-steps-detail", + kwargs={ + "submission_uuid": submission.uuid, + "step_uuid": step1.uuid, + }, + ) + + response = self.client.put( + endpoint, data={"data": {"text1": "trigger-rule"}} + ) + + self.assertEqual(status.HTTP_201_CREATED, response.status_code) + + data = response.json() + + self.assertTrue(data["completed"]) + self.assertTrue(data["canSubmit"]) + + with self.subTest("logic step for step 2 should not allow submitting"): + endpoint = reverse( + "api:submission-steps-logic-check", + kwargs={"submission_uuid": submission.uuid, "step_uuid": step2.uuid}, + ) + response = self.client.post(endpoint, data={"data": {"text2": "foo"}}) + + self.assertEqual(status.HTTP_200_OK, response.status_code) + + data = response.json() + + self.assertFalse(data["step"]["canSubmit"]) + + with self.subTest("logic check for step 1 should still allow submitting"): + endpoint = reverse( + "api:submission-steps-logic-check", + kwargs={"submission_uuid": submission.uuid, "step_uuid": step1.uuid}, + ) + + response = self.client.post( + endpoint, data={"data": {"text1": "trigger-rule"}} + ) + + self.assertEqual(status.HTTP_200_OK, response.status_code) + + 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 + self.assertTrue(data["step"]["canSubmit"]) + def test_check_logic_with_full_datetime(self): form = FormFactory.create() step1 = FormStepFactory.create(