Components keeping original defaultValue when editing

[robinmolen]

Closes #5104

Changes

When creating radio component the configuration defined in the component schema will be used. Providing an empty string as the defaultValue. If you leave the defaultValue as an empty string, save this component and edit it again, the defaultValue will change to null. This defaultValue change happens because of how we construct the component data for the edit modal.

In the WebformBuilder.editComponent we receive the component property, which is a simplified representation of the component (a sub-set of all the component information). If the defaultValue is a falsey value, like in this case an empty string, then defaultValue won't be present in this component prop. When reconstructing the component to its full representation, the defaultValue defined in the Formio component schema will be used (null).

To make sure we retain the actual component defaultValue, we combine the original prop (which is the original component data) and the reconstructed component data.

Checklist

Check off the items that are completed or not relevant.

• Impact on features

  • Checked copying a form
  • Checked import/export of a form
  • Config checks in the configuration overview admin page
  • Problem detection in the admin email digest is handled

• Release management

  • I have labelled the PR as "needs-backport" accordingly

• I have updated the translations assets (you do NOT need to provide translations)

  • Ran ./bin/makemessages_js.sh
  • Ran ./bin/compilemessages_js.sh

• Dockerfile/scripts

  • Updated the Dockerfile with the necessary scripts from the ./bin folder

• Commit hygiene

  • Commit messages refer to the relevant Github issue
  • Commit messages explain the "why" of change, not the how

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.75%. Comparing base (f8ac4ef) to head (57a51ea).
Report is 5 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5123   +/-   ##
=======================================
  Coverage   96.75%   96.75%           
=======================================
  Files         778      778           
  Lines       26741    26741           
  Branches     3474     3474           
=======================================
  Hits        25873    25873           
  Misses        606      606           
  Partials      262      262           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sergei-maertens]

I'm terrified of this change because of the earlier issues that happened with it, and on how Formio itself figures out the default value for a component: https://github.com/formio/formio.js/blob/bebc2ad73cad138a6de0a8247df47f0085a314cc/src/components/_classes/component/Component.js#L2302

Can you please make sure that all the old reported bugs are not re-introduced and test them manually? You should be able to figure out which ones that are from the commit history/git blame.

[sergei-maertens]

We should also document the manual JSON editing requirement when backporting the fix. For the master branch, we can do a data migration.

[robinmolen]

I'm terrified of this change because of the earlier issues that happened with it, and on how Formio itself figures out the default value for a component: https://github.com/formio/formio.js/blob/bebc2ad73cad138a6de0a8247df47f0085a314cc/src/components/_classes/component/Component.js#L2302

Your comment was correct, the change of merging the original into the generated component isn't needed. (the change in the WebformBuilder.js) The problem comes from the default formio component schema's. Formio doesn't define a defaultValue in the radio component schema, and our radio component schema wasn't seen as the schema that formio should use.

So the bug was as follows: you create a form with a radio component. This component has a default value of empty string, so when you save the form the defaultValue for the component is set to empty string. If you then edit the component thedefaultValue is set to null. This is done by how formio calculates the default value (the link in your comment). It uses the schema defaultValue as base, and if the component defaultValue is truety this then will be uses as the real defaultValue. Because the component defaultValue was set to empty string, which is falsey, the defaultValue of the formio schema was used (i.e. a undefined value, resulting in null).

By telling formio to use our schema's (which is the formio schema + some custom stuff) we can ensure that a valid defaultValue is used.

[sergei-maertens]

I have... strong feelings... about Formio

[robinmolen]

For the backport: To ease the pain of having to fix all radio components defaultValues by hand, i've added a fix script /bin/fix_radio_component_default_values.py.
This looks at all radio components in all form definitions. Any radio component that has a defaultValue of None (this will present in the admin view as a defaultValue of null), will have its defaultValue changed to an empty string.

[robinmolen]

Backports:

stable/2.8.x: 870fc20
stable/3.0.x: a96d095, 1e2d105