Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix validation issue with (dynamically) optional field #4114

Merged
merged 2 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions src/openforms/submissions/form_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,9 @@
from openforms.formio.utils import get_component_empty_value
from openforms.typing import DataMapping

from .logic.actions import PropertyAction
from .logic.actions import ActionOperation
from .logic.datastructures import DataContainer
from .logic.rules import (
EvaluatedRule,
get_current_step,
get_rules_to_evaluate,
iter_evaluate_rules,
)
from .logic.rules import EvaluatedRule, get_rules_to_evaluate, iter_evaluate_rules
from .models.submission_step import DirtyData

if TYPE_CHECKING:
Expand Down Expand Up @@ -205,7 +200,6 @@ def check_submission_logic(
if not submission_state.form_steps:
return

step = get_current_step(submission)
rules = get_rules_to_evaluate(submission)

# load the data state and all variables
Expand All @@ -214,15 +208,19 @@ def check_submission_logic(
submission_variables_state.set_values(unsaved_data)
data_container = DataContainer(state=submission_variables_state)

mutation_operations = []
mutation_operations: list[ActionOperation] = []
for operation in iter_evaluate_rules(rules, data_container, submission):
Comment on lines +211 to 212
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably this can be built using a list comprehension now

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a list comprehension makes the code look pure, but it isn't, data mutations are applied as the logic rules are evaluated by looping through them.

# component mutations can be skipped as we're not in the context of a single
# form step.
if isinstance(operation, PropertyAction):
continue
mutation_operations.append(operation)

# we loop over all steps because we have validations that ensure unique component
# keys across multiple steps for the whole form.
#
# We need to apply all component mutations at this stage too, because for validation
# reasons, a serializer is built from them.
for mutation in mutation_operations:
mutation.apply(step, {})
for step in submission_state.submission_steps:
assert step.form_step
configuration = step.form_step.form_definition.configuration_wrapper
mutation.apply(step, configuration)

submission._form_logic_evaluated = True
87 changes: 86 additions & 1 deletion src/openforms/tests/e2e/test_input_validation_regressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
from furl import furl
from playwright.async_api import expect

from openforms.forms.tests.factories import FormFactory, FormStepFactory
from openforms.forms.tests.factories import (
FormFactory,
FormLogicFactory,
FormStepFactory,
)

from .base import E2ETestCase, browser_page

Expand Down Expand Up @@ -226,3 +230,84 @@ def setUpTestData():
await expect(
page.get_by_text("Een moment geduld", exact=False)
).to_be_visible()

@tag("dh-671")
async def test_component_not_required_via_logic(self):
@sync_to_async
def setUpTestData():
form = FormFactory.create(
slug="validation",
generate_minimal_setup=True,
registration_backend=None,
translation_enabled=False, # force Dutch
ask_privacy_consent=False,
ask_statement_of_truth=False,
formstep__next_text="Volgende",
formstep__form_definition__configuration={
"components": [
{
"type": "textfield",
"key": "textfield",
"label": "Visible text field",
"validate": {"required": True},
},
]
},
)
# add a second step
FormStepFactory.create(
form=form,
next_text="Volgende",
is_applicable=True, # should not be validated
form_definition__configuration={
"components": [
{
"type": "textfield",
"key": "textfield2",
"label": "Textfield 2",
"validate": {"required": False},
},
]
},
)
FormLogicFactory.create(
form=form,
json_logic_trigger=True,
actions=[
{
"component": "textfield",
"formStepUuid": None,
"action": {
"type": "property",
"property": {"value": "validate.required", "type": "bool"},
"state": False,
},
}
],
)
return form

form = await setUpTestData()
form_url = str(
furl(self.live_server_url)
/ reverse("forms:form-detail", kwargs={"slug": form.slug})
)

async with browser_page() as page:
await page.goto(form_url)
# Start the form
await page.get_by_role("button", name="Formulier starten").click()

# Confirm first step is visible
await expect(page.get_by_label("Visible text field")).to_be_visible()
await page.get_by_role("button", name="Volgende").click()

# Handle second step
await expect(page.get_by_label("Textfield 2")).to_be_visible()
await page.get_by_role("button", name="Volgende").click()

# Confirm and finish the form
await page.get_by_role("button", name="Verzenden").click()
await expect(
page.get_by_text("Een moment geduld", exact=False)
).to_be_visible()
Loading