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

Hidden components with "Multiple values" trigger validation #4659

Closed
LaurensBurger opened this issue Sep 16, 2024 · 7 comments · Fixed by #4738
Closed

Hidden components with "Multiple values" trigger validation #4659

LaurensBurger opened this issue Sep 16, 2024 · 7 comments · Fixed by #4738
Assignees
Labels
bug Something isn't working formio Limitation in Formio.js owner: maykin topic: form builder
Milestone

Comments

@LaurensBurger
Copy link
Collaborator

LaurensBurger commented Sep 16, 2024

Product versie / Product version

latest

Customer reference

no particular but it get's reported

Omschrijf het probleem / Describe the bug

Hidden component with "Multiple values" set get their defaultValue changed from:

  "defaultValue": null,

to:

  "defaultValue": [
    null
  ],

This causes validation to trigger right after loading since their value is now [] instead of [ null ]

  "telefoonnummer": []

This can be resolved by changing the defaultValue to via the json of the components:

  "defaultValue": [],
@LaurensBurger LaurensBurger added bug Something isn't working triage Issue needs to be validated. Remove this label if the issue considered valid. labels Sep 16, 2024
@joeribekker joeribekker added formio Limitation in Formio.js and removed triage Issue needs to be validated. Remove this label if the issue considered valid. labels Sep 23, 2024
@joeribekker
Copy link
Contributor

Refinement: Task estimated here is changing the default value to an array ([]) without the null.

Robin can do this in an hour. So, let's timebox this to 2 hours. After that, the proper estimate is: (whatever Robin thinks)

@robinmolen
Copy link
Contributor

I cannot reproduce this issue. I've followed these steps:

  • Create a new form
  • Add a radio button field, that isn't required, with a option
  • Add a textfield, with the option multiple values and without defining a default value
    • (in the json this now indeed has the default value of [null])
  • Add advanced logic to the textfield to only show if the previously defined radio has its value checked
  • Save form and open it
  • Go to the form step, keep the radio button unchecked, and go to the next step

robinmolen added a commit to open-formulieren/formio-builder that referenced this issue Sep 26, 2024
@robinmolen
Copy link
Contributor

I've made a PR that ensures that textfields always have a valid value (so null will be turned into '').

When a textfield (with the default value null) gets the multiple values property, the default value will change to [''].

robinmolen added a commit to open-formulieren/formio-builder that referenced this issue Oct 2, 2024
robinmolen added a commit to open-formulieren/formio-builder that referenced this issue Oct 2, 2024
robinmolen added a commit to open-formulieren/formio-builder that referenced this issue Oct 2, 2024
robinmolen added a commit to open-formulieren/formio-builder that referenced this issue Oct 2, 2024
robinmolen added a commit to open-formulieren/formio-builder that referenced this issue Oct 2, 2024
@sergei-maertens
Copy link
Member

Looking at open-formulieren/formio-builder#184 I fail to see how this relates to the reported bug. Combined with the comment saying it can't be reproduced, I suspect we're looking in the wrong place.

For any fix to be attempted, the issue must first be reproduced, then a failing regression test needs to be provided and finally a patch. The failing regression test now passing then proves the patch works, the regression test proves the problem is understood and will be prevented in the future.

@sergei-maertens
Copy link
Member

This seems to be some sort of regression of #2213 - that one was limited to copying, but here it seems to apply even for new text fields dragged into the editor.

When fixing this, make sure to check all related issues (and issues related to #3128) for possible regressions.

@sergei-maertens
Copy link
Member

I quickly checked email component, and it has the same issue - defaultValue is null.

@sergei-maertens
Copy link
Member

sergei-maertens commented Oct 2, 2024

Discussed in the office:

  1. Let's write an e2e regression test with playwright, for reference: https://github.com/open-formulieren/open-forms/blob/master/src/openforms/forms/tests/e2e_tests/test_form_designer.py
  2. the test should: drag and drop the textfield (and other affected types) component, click the save button (without additional changes), and save the form definition
  3. then query the created FD and assert that the component defaultValue is equal to the empty string (as opposed to None)
  4. for the patch, you can probably apply the same fixup like the cosign.js, see 1e8d6af
  5. we'll need a data migration to update existing components - CTRL + F on ConvertComponentsOperation for examples

robinmolen added a commit that referenced this issue Oct 10, 2024
robinmolen added a commit that referenced this issue Oct 10, 2024
robinmolen added a commit that referenced this issue Oct 10, 2024
robinmolen added a commit that referenced this issue Oct 14, 2024
sergei-maertens added a commit that referenced this issue Oct 16, 2024
…ue-null-for-textfield

[#4659] Fix default value for textfield
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment